aitools: add --output json to install - #6481
Conversation
583618b to
5dbbee3
Compare
rugpanov
left a comment
There was a problem hiding this comment.
Automated multi-reviewer pass (Isaac, Codex, and Claude reviewers, cross-checked against the code). The feature itself is sound and mirrors aitools list --output json. Inline comments below; a few findings that don't anchor to a changed line:
[nice to have] --path + --output json silently ignores JSON (cmd/aitools/install.go:124) — the --path block returns before the jsonMode check (:138) and render (:197), so install --path DIR --output json does a text dump with exit 0 and no JSON. --skills-only+--path is already rejected at :105; per CLAUDE.md's "reject incompatible inputs early" rule, either reject this combination or honor JSON in the dump path.
[nice to have] Coverage / convention gaps — (1) no .nextchanges/cli/ changelog fragment for this user-visible output mode (the analogous aitools list --output json, #5233, shipped one; not CI-blocking but expected by .agents/rules/changelog.md); (2) only unit tests were added — .agents/rules/testing.md prefers acceptance coverage for user-visible output, and the suite already exists at acceptance/experimental/aitools/skills/install/. An acceptance case would also have caught the stderr finding below, since it runs the real binary through root.Execute.
[nit] Help text (cmd/aitools/install.go:83) — the Long help / "Escape hatches" list doesn't mention --output json or its --scope+--agents requirement, so users hit the error only at runtime.
0f9f04a to
af4bb2a
Compare
Add JSON output to `aitools install`, driven entirely by flags so the run is fully non-interactive: require --scope and --agents (erroring and naming the missing flags otherwise) so no scope prompt, agent picker, or confirm is shown. executePlan now returns a per-agent outcome (name, delivery, status, message) that the JSON payload lists. A top-level failure with no per-agent entry (e.g. a skills-group install failure) is surfaced in a top-level "error" field; per-agent failures stay in their agent entry and are not duplicated there (executePlan wraps them so the two are distinguishable). Once the JSON result is rendered, silence cobra's text "Error:"/usage output so a failure is not reported twice; the non-zero exit still comes from returning the run error. Share the indented-JSON encoder between install and list as renderJSON. Co-authored-by: Isaac <no-reply@databricks.com>
af4bb2a to
047ee54
Compare
The changelog validator now requires each fragment to be a single line starting with a `* ` bullet and ending with a period before the trailing PR link group. Reformat the install --output json fragment accordingly. Co-authored-by: Isaac <no-reply@databricks.com>
Integration test reportCommit: 28dd936
Top 6 slowest tests (at least 2 minutes):
|
--output json is meant to emit only the structured JSON document, but the
installer still wrote progress lines ("Using skills version", "Fetching
skills manifest...", "Installed N skills.") to stderr, so a consumer saw
non-JSON interleaved with the result. Mark the context quiet in JSON mode
and route those library messages through cmdio.LogProgress, which respects
it. Text mode is unaffected. Update the acceptance test to use the
non-deprecated `aitools install` and assert only JSON is emitted.
Co-authored-by: Isaac <no-reply@databricks.com>
rugpanov
left a comment
There was a problem hiding this comment.
Approving. The blocking issue from the earlier pass (a failed --output json run printing a duplicate Error: line — SilenceErrors is a no-op since root prints errors itself) is fixed correctly via root.ErrAlreadyPrinted, and it's now covered end-to-end through root.Execute (TestInstallOutputJSONThroughRoot). --path + --output json is rejected, the changelog fragment and acceptance test are in, and a fresh multi-reviewer round found nothing blocking. installOutputIsJSON keeping its own nil-safe check (rather than root.OutputType) is justified by the detached legacy skills install alias — confirmed.
One non-blocking cleanup you can take or defer: executePlan's quiet bool duplicates the cmdio.WithQuiet/LogProgress mechanism this PR already uses in the installer — converting its progress LogString calls to LogProgress would drop the param and its guards with no behavior change. LGTM either way.
Integration test reportCommit: dc00178
697 interesting tests: 631 MISS, 65 FAIL, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
Stacked on databricks#6481 ## Changes Categorize `aitools install` errors, and emit those in telemetry and JSON output ## Why To better understand why installations failed ## Tests Added unit tests <!-- If your PR needs to be included in the release notes for next release, add a changelog fragment: create .nextchanges/<section>/<name>.md with a one-line description (e.g. .nextchanges/cli/quickstart.md). See .nextchanges/README.md. --> --------- Co-authored-by: Isaac <no-reply@databricks.com>
## Release v1.16.0 ### CLI * `aitools install` now registers the official Claude marketplace if it is missing before installing the Databricks Claude plugin. ([#6485](#6485)) * `databricks aitools install --output json` now reports an `error_category` for a failed or skipped install (per agent, and at the top level for a failure with no per-agent entry), giving coding agents and CI a stable classification of why an install did not complete. ([#6482](#6482)) * `databricks aitools install` honors `--output json`, emitting a structured `{scope, agents[...]}` document that reports each agent's delivery and install status so coding agents and CI can consume the result without scraping the text output. JSON mode requires `--scope` and `--agents` so the command runs without interactive prompts. ([#6481](#6481)) * `databricks bundle sync` now prints sync progress (`Action: PUT`, `Uploaded ...`) by default, matching `databricks sync`. Previously it was silent unless `--output` was passed. Use `--output json` for machine-readable output. ([#6568](#6568)) * Support major-only DBR runtime versions such as `19.x-scala2.13` in the cluster picker used by `databricks auth login --configure-cluster` and `databricks labs`. ([#6574](#6574)) * Deprecated the `databricks environments setup-local --constraints-only` flag in favour of the orthogonal `--no-dbconnect`; the flag still works as a hidden alias but is hidden from `--help` and prints a one-line deprecation notice, and will be removed in a later release. ([#6470](#6470)) * Add orthogonal `--no-constraints` and `--no-dbconnect` flags to `databricks environments setup-local`: `--no-constraints` skips writing the remote Python-version and dependency pins, and `--no-dbconnect` skips the databricks-connect dependency. ([#6464](#6464)) * `databricks environments setup-local` now reports a distinct `E_PROVISION_CONFLICT` error code in `--output json` when the project's dependencies conflict with the pins written for the target environment, making the requirements unsatisfiable (the same conflict surfaced as a `W_USER_CONSTRAINT_CONFLICT` warning); it is reported after the project files are written, without attempting the doomed provisioning, while other provisioning failures continue to report `E_PROVISION`. ([#6479](#6479)) * `databricks ssh connect` and `ssh setup` now verify the tunnel's SSH host key against the key the workspace published for the connection, recorded in `~/.databricks/ssh-tunnel-known-hosts/<name>` instead of `~/.ssh/known_hosts`. Reconnecting with a name used before no longer fails with `Host key verification failed` when the compute behind that name changed, and no longer needs a manual `ssh-keygen -R`; host blocks written by an earlier `databricks ssh setup` pick this up once you re-run it. ([#6557](#6557)) * Stop `databricks ssh connect --ide` from adding a duplicate entry to the IDE's Remote Explorer on every connect: the remote authority is now the SSH host alias alone, instead of embedding the per-instance remote OS user. ([#6550](#6550)) * Add `--max-clients` and `--server-timeout` flags to `databricks ssh setup`, and `--server-timeout` to `databricks ssh connect`. Both are fixed when the SSH tunnel server job is submitted, so `ssh setup` now serializes them into the generated `ProxyCommand` instead of falling back to the built-in defaults. ([#6547](#6547)) * `ssh connect` sessions no longer end when the tunnel's websocket connection is lost. The CLI reattaches to the running session and replays the bytes that were missed, so the shell and everything running in it stay intact, and a transient failure to open a replacement connection for the periodic auth refresh is retried rather than ending the session. Reattaching requires an SSH server started by a CLI that supports it; against an older server the connection behaves as before. ([#6558](#6558)) ### Bundles * Added PyDABs (Python) support for secrets: `Resources.add_secret` and the `secret_mutator` decorator. ([#6553](#6553)) * Fix job and pipeline environment dependencies with a `*` version wildcard (e.g. `numpy==2.5.*`) being treated as local file paths. ([#6555](#6555)) * Add the `postgres_snapshot_schedules` bundle resource for managing a Lakebase Postgres branch's automatic-snapshot schedule (direct deployment engine only). ([#6449](#6449)) ### Dependency Updates * Bump `github.com/databricks/databricks-sdk-go` from v0.175.0 to v0.177.0. ([#6448](#6448)) * Bump Terraform provider from v1.128.0 to v1.131.0. ([#6544](#6544))
## Changes Add `--output json` to `aitools install`. This requires `--scope` and `--agents` to avoid needing to prompt users on stdout ## Why Needed for #6482 ## Tests Added unit tests <!-- If your PR needs to be included in the release notes for next release, add a changelog fragment: create .nextchanges/<section>/<name>.md with a one-line description (e.g. .nextchanges/cli/quickstart.md). See .nextchanges/README.md. --> --------- Co-authored-by: Isaac <no-reply@databricks.com>
Stacked on #6481 ## Changes Categorize `aitools install` errors, and emit those in telemetry and JSON output ## Why To better understand why installations failed ## Tests Added unit tests <!-- If your PR needs to be included in the release notes for next release, add a changelog fragment: create .nextchanges/<section>/<name>.md with a one-line description (e.g. .nextchanges/cli/quickstart.md). See .nextchanges/README.md. --> --------- Co-authored-by: Isaac <no-reply@databricks.com>
## Release v1.16.0 ### CLI * `aitools install` now registers the official Claude marketplace if it is missing before installing the Databricks Claude plugin. ([#6485](#6485)) * `databricks aitools install --output json` now reports an `error_category` for a failed or skipped install (per agent, and at the top level for a failure with no per-agent entry), giving coding agents and CI a stable classification of why an install did not complete. ([#6482](#6482)) * `databricks aitools install` honors `--output json`, emitting a structured `{scope, agents[...]}` document that reports each agent's delivery and install status so coding agents and CI can consume the result without scraping the text output. JSON mode requires `--scope` and `--agents` so the command runs without interactive prompts. ([#6481](#6481)) * `databricks bundle sync` now prints sync progress (`Action: PUT`, `Uploaded ...`) by default, matching `databricks sync`. Previously it was silent unless `--output` was passed. Use `--output json` for machine-readable output. ([#6568](#6568)) * Support major-only DBR runtime versions such as `19.x-scala2.13` in the cluster picker used by `databricks auth login --configure-cluster` and `databricks labs`. ([#6574](#6574)) * Deprecated the `databricks environments setup-local --constraints-only` flag in favour of the orthogonal `--no-dbconnect`; the flag still works as a hidden alias but is hidden from `--help` and prints a one-line deprecation notice, and will be removed in a later release. ([#6470](#6470)) * Add orthogonal `--no-constraints` and `--no-dbconnect` flags to `databricks environments setup-local`: `--no-constraints` skips writing the remote Python-version and dependency pins, and `--no-dbconnect` skips the databricks-connect dependency. ([#6464](#6464)) * `databricks environments setup-local` now reports a distinct `E_PROVISION_CONFLICT` error code in `--output json` when the project's dependencies conflict with the pins written for the target environment, making the requirements unsatisfiable (the same conflict surfaced as a `W_USER_CONSTRAINT_CONFLICT` warning); it is reported after the project files are written, without attempting the doomed provisioning, while other provisioning failures continue to report `E_PROVISION`. ([#6479](#6479)) * `databricks ssh connect` and `ssh setup` now verify the tunnel's SSH host key against the key the workspace published for the connection, recorded in `~/.databricks/ssh-tunnel-known-hosts/<name>` instead of `~/.ssh/known_hosts`. Reconnecting with a name used before no longer fails with `Host key verification failed` when the compute behind that name changed, and no longer needs a manual `ssh-keygen -R`; host blocks written by an earlier `databricks ssh setup` pick this up once you re-run it. ([#6557](#6557)) * Stop `databricks ssh connect --ide` from adding a duplicate entry to the IDE's Remote Explorer on every connect: the remote authority is now the SSH host alias alone, instead of embedding the per-instance remote OS user. ([#6550](#6550)) * Add `--max-clients` and `--server-timeout` flags to `databricks ssh setup`, and `--server-timeout` to `databricks ssh connect`. Both are fixed when the SSH tunnel server job is submitted, so `ssh setup` now serializes them into the generated `ProxyCommand` instead of falling back to the built-in defaults. ([#6547](#6547)) * `ssh connect` sessions no longer end when the tunnel's websocket connection is lost. The CLI reattaches to the running session and replays the bytes that were missed, so the shell and everything running in it stay intact, and a transient failure to open a replacement connection for the periodic auth refresh is retried rather than ending the session. Reattaching requires an SSH server started by a CLI that supports it; against an older server the connection behaves as before. ([#6558](#6558)) ### Bundles * Added PyDABs (Python) support for secrets: `Resources.add_secret` and the `secret_mutator` decorator. ([#6553](#6553)) * Fix job and pipeline environment dependencies with a `*` version wildcard (e.g. `numpy==2.5.*`) being treated as local file paths. ([#6555](#6555)) * Add the `postgres_snapshot_schedules` bundle resource for managing a Lakebase Postgres branch's automatic-snapshot schedule (direct deployment engine only). ([#6449](#6449)) ### Dependency Updates * Bump `github.com/databricks/databricks-sdk-go` from v0.175.0 to v0.177.0. ([#6448](#6448)) * Bump Terraform provider from v1.128.0 to v1.131.0. ([#6544](#6544))
Changes
Add
--output jsontoaitools install. This requires--scopeand--agentsto avoid needing to prompt users on stdoutWhy
Needed for #6482
Tests
Added unit tests