From 047ee545e5c5625a4ab9071cbb383d11cbdf8507 Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Wed, 2 Sep 2026 11:31:16 +0200 Subject: [PATCH] aitools: add --output json to install 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 --- .../cli/aitools-install-output-json.md | 1 + .../skills/install-output-json/out.test.toml | 2 + .../skills/install-output-json/output.txt | 17 ++ .../aitools/skills/install-output-json/script | 9 + .../skills/install-output-json/test.toml | 35 +++ cmd/aitools/install.go | 204 +++++++++++++-- cmd/aitools/install_test.go | 233 ++++++++++++++++-- cmd/aitools/json.go | 14 ++ cmd/aitools/list.go | 10 +- cmd/aitools/list_test.go | 6 +- 10 files changed, 484 insertions(+), 47 deletions(-) create mode 100644 .nextchanges/cli/aitools-install-output-json.md create mode 100644 acceptance/experimental/aitools/skills/install-output-json/out.test.toml create mode 100644 acceptance/experimental/aitools/skills/install-output-json/output.txt create mode 100644 acceptance/experimental/aitools/skills/install-output-json/script create mode 100644 acceptance/experimental/aitools/skills/install-output-json/test.toml create mode 100644 cmd/aitools/json.go diff --git a/.nextchanges/cli/aitools-install-output-json.md b/.nextchanges/cli/aitools-install-output-json.md new file mode 100644 index 00000000000..76c5e60c05f --- /dev/null +++ b/.nextchanges/cli/aitools-install-output-json.md @@ -0,0 +1 @@ +`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](https://github.com/databricks/cli/pull/6481)). diff --git a/acceptance/experimental/aitools/skills/install-output-json/out.test.toml b/acceptance/experimental/aitools/skills/install-output-json/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/experimental/aitools/skills/install-output-json/output.txt b/acceptance/experimental/aitools/skills/install-output-json/output.txt new file mode 100644 index 00000000000..f48e5f84bd7 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json/output.txt @@ -0,0 +1,17 @@ + +=== install --output json emits parseable JSON on stdout; progress stays on stderr +>>> [CLI] experimental aitools install --skills-only --scope=global --agents=claude-code --output json +Command "install" is deprecated, use "databricks aitools install" instead. +Using skills version test-ref +Fetching skills manifest... +Installed 1 skill. +{ + "scope": "global", + "agents": [ + { + "name": "claude-code", + "delivery": "skills", + "status": "installed" + } + ] +} diff --git a/acceptance/experimental/aitools/skills/install-output-json/script b/acceptance/experimental/aitools/skills/install-output-json/script new file mode 100644 index 00000000000..abc1e627289 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json/script @@ -0,0 +1,9 @@ +# Isolate HOME so parallel aitools tests don't race on a shared ~/.databricks. +sethome home + +title "install --output json emits parseable JSON on stdout; progress stays on stderr" +# --agents makes the run fully non-interactive (no picker, no scope prompt), which +# --output json requires. Piping stdout through jq proves the JSON payload is the +# only thing on stdout; the human-readable progress lines go to stderr and still +# show in the merged capture below. +trace $CLI experimental aitools install --skills-only --scope=global --agents=claude-code --output json | jq . diff --git a/acceptance/experimental/aitools/skills/install-output-json/test.toml b/acceptance/experimental/aitools/skills/install-output-json/test.toml new file mode 100644 index 00000000000..d5267e964ac --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json/test.toml @@ -0,0 +1,35 @@ +# Mock server replaces raw.githubusercontent.com for manifest + skill files. +Env.DATABRICKS_SKILLS_BASE_URL = "$DATABRICKS_HOST" +Env.DATABRICKS_SKILLS_REF = "test-ref" + +Ignore = [ + "home", +] + +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +[[Server]] +Pattern = "GET /test-ref/manifest.json" +Response.Body = ''' +{ + "version": "2", + "updated_at": "2026-01-01T00:00:00Z", + "skills": { + "test-stable": { + "version": "1.0.0", + "description": "Stable test skill", + "files": ["SKILL.md"], + "repo_dir": "skills" + } + } +} +''' + +[[Server]] +Pattern = "GET /test-ref/skills/test-stable/SKILL.md" +Response.Body = '''--- +name: test-stable +--- + +# Test stable skill +''' diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index 3543ad4de1b..42b9ee9682c 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -7,9 +7,11 @@ import ( "strings" "github.com/charmbracelet/huh" + "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/aitools/agents" "github.com/databricks/cli/libs/aitools/installer" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" ) @@ -95,15 +97,27 @@ Agent selection: (unset, interactive) A picker over all known agents, detected ones pre-checked. (unset, non-interactive) Act on every detected agent. +Output: + --output json Emit a structured result instead of text. Requires --scope + and --agents so the command runs without interactive prompts. + Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() + jsonMode := installOutputIsJSON(cmd) if skillsOnly && pathFlag != "" { return errors.New("cannot use --skills-only with --path; --path always writes raw skill files") } + // --path is a plain file dump with no agents or install state, so there + // is no per-agent result to report. Reject --output json here rather than + // letting the dump run and silently emit no JSON. + if jsonMode && pathFlag != "" { + return errors.New("cannot use --output json with --path; --path writes raw skill files and produces no JSON result") + } + opts := installer.InstallOptions{ IncludeExperimental: includeExperimental, SpecificSkills: splitAndTrim(skillsFlag), @@ -128,6 +142,24 @@ Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), if err != nil { return err } + + // JSON output must be fully non-interactive: every choice has to come + // from flags so no scope prompt, agent picker, or confirm is shown. + // Require the flags those prompts would otherwise resolve, and fail + // fast naming them. + if jsonMode { + var missing []string + if !projectFlag && !globalFlag { + missing = append(missing, "--scope") + } + if agentsFlag == "" { + missing = append(missing, "--agents") + } + if len(missing) > 0 { + return fmt.Errorf("--output json requires %s so the command runs without interactive prompts", strings.Join(missing, " and ")) + } + } + scope, err := resolveScopeWithPrompt(ctx, projectFlag, globalFlag) if err != nil { return err @@ -173,7 +205,28 @@ Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), Experimental: opts.IncludeExperimental, }) - return executePlan(ctx, src, plan, opts) + outcomes, runErr := executePlan(ctx, src, plan, opts, jsonMode) + if jsonMode { + if jerr := renderJSON(cmd.OutOrStdout(), buildInstallOutput(opts.Scope, outcomes, runErr)); jerr != nil { + // Rendering failed, so the JSON the caller parses is broken. + // Report the render error unless the run already failed for + // another reason. + if runErr == nil { + runErr = jerr + } + return runErr + } + // The JSON payload is the only thing on stdout and already reports + // the outcome. On failure, exit non-zero without root printing a + // duplicate "Error: ..." line to stderr; root prints errors itself + // (see cmd/root/root.go), so ErrAlreadyPrinted is how a command + // opts out of that, not cmd.SilenceErrors. + if runErr != nil { + return root.ErrAlreadyPrinted + } + return nil + } + return runErr }, } @@ -189,6 +242,20 @@ Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), return cmd } +// installOutputIsJSON reports whether --output json was requested. Unlike list, +// install can run detached from root: the legacy `skills install` alias builds a +// NewInstallCmd and executes it directly (see newLegacySkillsInstallCmd), so the +// root-supplied --output flag may be absent. Treat a missing flag as text rather +// than panicking the way root.OutputType would. +func installOutputIsJSON(cmd *cobra.Command) bool { + f := cmd.Flag("output") + if f == nil { + return false + } + out, ok := f.Value.(*flags.Output) + return ok && *out == flags.OutputJSON +} + // selectAgents returns the agents to act on when --agents is not given. The // interactive path shows a picker over all known agents; the non-interactive // path acts on detected agents, matching today's default. Skills delivery only @@ -368,11 +435,48 @@ func printPlanSummary(ctx context.Context, plan []agentPlanItem, scope string) { cmdio.LogString(ctx, "") } -// executePlan carries out the plan. Skills installs go through the existing -// skills path (preserving its output). Plugin installs are reported but never -// silently fall back to skills: a blocked install is a warning (exit 0), unless -// the agent was explicitly named via --agents, which is an error. -func executePlan(ctx context.Context, src installer.ManifestSource, plan []agentPlanItem, opts installer.InstallOptions) error { +// agentOutcome is one agent's result after executePlan: how the databricks +// tools were delivered (or attempted), and, when the agent did not succeed, a +// human-readable message for --output json. +type agentOutcome struct { + agent *agents.Agent + delivery delivery + status outcomeStatus + message string // set when skipped or failed +} + +type outcomeStatus string + +const ( + outcomeInstalled outcomeStatus = "installed" + outcomeSkipped outcomeStatus = "skipped" + outcomeFailed outcomeStatus = "failed" +) + +// agentErrors wraps the failures of explicitly named agents, which are already +// reported in their per-agent outcomes. Wrapping lets the JSON layer tell a +// per-agent failure apart from a top-level failure that has no per-agent entry, +// so each is surfaced exactly once. +type agentErrors struct{ err error } + +func (e *agentErrors) Error() string { return e.err.Error() } +func (e *agentErrors) Unwrap() error { return e.err } + +// topLevelFailure returns the run error when it has no per-agent entry, or nil +// when the failure is already reported per agent — so a per-agent failure is not +// duplicated in the top-level error field. +func topLevelFailure(runErr error) error { + if _, ok := errors.AsType[*agentErrors](runErr); ok { + return nil + } + return runErr +} + +// executePlan carries out the plan and returns each agent's outcome. Skills +// installs go through the existing skills path. Plugin installs are reported but +// never silently fall back to skills: a blocked install is a warning (exit 0), +// unless the agent was explicitly named via --agents, which is an error. +func executePlan(ctx context.Context, src installer.ManifestSource, plan []agentPlanItem, opts installer.InstallOptions, quiet bool) ([]agentOutcome, error) { var skillsAgents []*agents.Agent var pluginItems, skipItems []agentPlanItem for _, it := range plan { @@ -386,12 +490,19 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent } } + var outcomes []agentOutcome var explicitErrs []error if len(skillsAgents) > 0 { - installer.PrintInstallingFor(ctx, skillsAgents) + if !quiet { + installer.PrintInstallingFor(ctx, skillsAgents) + } + // A skills install runs as a group; on failure the whole command fails. if err := installSkillsForAgentsFn(ctx, src, skillsAgents, opts); err != nil { - return err + return outcomes, err + } + for _, a := range skillsAgents { + outcomes = append(outcomes, agentOutcome{agent: a, delivery: deliverySkills, status: outcomeInstalled}) } } @@ -399,14 +510,24 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent if len(pluginItems) > 0 { ref, _, err := installer.GetSkillsRef(ctx) if err != nil { - return err + return outcomes, err } records := map[string]installer.PluginRecord{} for _, it := range pluginItems { - cmdio.LogString(ctx, fmt.Sprintf("Installing databricks plugin for %s...", it.agent.DisplayName)) + if !quiet { + cmdio.LogString(ctx, fmt.Sprintf("Installing databricks plugin for %s...", it.agent.DisplayName)) + } rec, err := installPluginForAgentFn(ctx, it.agent, it.scope, ref) if err != nil { - cmdio.LogString(ctx, cmdio.Yellow(ctx, fmt.Sprintf("Skipped %s: %v", it.agent.DisplayName, err))) + if !quiet { + cmdio.LogString(ctx, cmdio.Yellow(ctx, fmt.Sprintf("Skipped %s: %v", it.agent.DisplayName, err))) + } + outcomes = append(outcomes, agentOutcome{ + agent: it.agent, + delivery: deliveryPlugin, + status: outcomeFailed, + message: err.Error(), + }) if it.explicit { explicitErrs = append(explicitErrs, err) } @@ -414,28 +535,39 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent } records[it.agent.Name] = rec pluginCount++ + outcomes = append(outcomes, agentOutcome{agent: it.agent, delivery: deliveryPlugin, status: outcomeInstalled}) // Remove any raw skills we previously dropped on this agent so the // plugin and leftover files don't surface the same skills twice. if err := cleanupLegacyFn(ctx, it.agent, opts.Scope); err != nil { log.Debugf(ctx, "Legacy skill cleanup for %s failed: %v", it.agent.DisplayName, err) } - cmdio.LogString(ctx, fmt.Sprintf(" %s databricks plugin %s", it.agent.DisplayName, versionToken(rec.Version))) + if !quiet { + cmdio.LogString(ctx, fmt.Sprintf(" %s databricks plugin %s", it.agent.DisplayName, versionToken(rec.Version))) + } } if len(records) > 0 { if err := recordPluginInstallsFn(ctx, opts.Scope, records, ref); err != nil { - return err + return outcomes, err } } } for _, it := range skipItems { - cmdio.LogString(ctx, cmdio.Yellow(ctx, "Skipped "+it.agent.DisplayName+": "+it.reason)) + if !quiet { + cmdio.LogString(ctx, cmdio.Yellow(ctx, "Skipped "+it.agent.DisplayName+": "+it.reason)) + } + outcomes = append(outcomes, agentOutcome{ + agent: it.agent, + delivery: deliverySkip, + status: outcomeSkipped, + message: it.reason, + }) if it.explicit { explicitErrs = append(explicitErrs, fmt.Errorf("%s: %s", it.agent.DisplayName, it.reason)) } } - if pluginCount > 0 { + if pluginCount > 0 && !quiet { noun := "agent" if pluginCount != 1 { noun = "agents" @@ -444,9 +576,47 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent } if len(explicitErrs) > 0 { - return errors.Join(explicitErrs...) + return outcomes, &agentErrors{err: errors.Join(explicitErrs...)} + } + return outcomes, nil +} + +type installOutput struct { + Scope string `json:"scope"` + Agents []agentResultJSON `json:"agents"` + + // Error is a top-level failure message with no per-agent entry (e.g. a + // skills-group install failure); empty on success. It is local-only and never + // sent to telemetry. + Error string `json:"error,omitempty"` +} + +type agentResultJSON struct { + Name string `json:"name"` + Delivery string `json:"delivery"` + Status string `json:"status"` + Message string `json:"message,omitempty"` +} + +func buildInstallOutput(scope string, outcomes []agentOutcome, runErr error) installOutput { + out := installOutput{Scope: scope, Agents: make([]agentResultJSON, 0, len(outcomes))} + for _, o := range outcomes { + entry := agentResultJSON{ + Name: o.agent.Name, + Delivery: o.delivery.String(), + Status: string(o.status), + Message: o.message, + } + out.Agents = append(out.Agents, entry) + } + // A top-level failure (skills-group install, ref lookup, plugin recording) + // has no per-agent entry, so surface it here too; otherwise the consumer sees + // a non-zero exit with an empty agents array and no reason. Per-agent failures + // stay in the agents entries above and are not repeated here. + if e := topLevelFailure(runErr); e != nil { + out.Error = e.Error() } - return nil + return out } // resolveAgentNames parses a comma-separated list of agent names and validates diff --git a/cmd/aitools/install_test.go b/cmd/aitools/install_test.go index 0ce6452ee01..1972262c066 100644 --- a/cmd/aitools/install_test.go +++ b/cmd/aitools/install_test.go @@ -2,17 +2,22 @@ package aitools import ( "bufio" + "bytes" "context" + "encoding/json" "errors" "os" "path/filepath" "runtime" "testing" + "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/aitools/agents" "github.com/databricks/cli/libs/aitools/installer" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/telemetry" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -28,6 +33,21 @@ func drainReader(r *bufio.Reader) { // --- Test helpers --- +// newTestInstallCmd builds the install command with the pieces the root command +// supplies in production: the persistent --output flag (so cobra can parse +// `--output` on this detached command; install reads it via installOutputIsJSON) +// and silenced cobra error/usage output (root sets SilenceErrors and prints +// errors itself). Without the latter, a detached command prints cobra's own +// "Error:"/usage to the captured buffers. +func newTestInstallCmd() *cobra.Command { + cmd := NewInstallCmd() + output := flags.OutputText + cmd.PersistentFlags().VarP(&output, "output", "o", "output type: text or json") + cmd.SilenceErrors = true + cmd.SilenceUsage = true + return cmd +} + type installCall struct { agents []string opts installer.InstallOptions @@ -228,13 +248,18 @@ func TestExecutePlanSkipBlockedPluginExit0(t *testing.T) { claude := testPluginAgent(agents.NameClaudeCode, "Claude Code", "claude") ctx := cmdio.MockDiscard(t.Context()) - // Non-explicit blocked install is a warning, not an error. + // Non-explicit blocked install is a warning, not an error, but the agent's + // failure is still recorded in its outcome. plan := buildPlan([]*agents.Agent{claude}, installer.ScopeGlobal, false, false) - require.NoError(t, executePlan(ctx, nil, plan, installer.InstallOptions{Scope: installer.ScopeGlobal})) + outcomes, err := executePlan(ctx, nil, plan, installer.InstallOptions{Scope: installer.ScopeGlobal}, false) + require.NoError(t, err) + require.Len(t, outcomes, 1) + assert.Equal(t, outcomeFailed, outcomes[0].status) // Explicit (--agents) blocked install is an error. planExplicit := buildPlan([]*agents.Agent{claude}, installer.ScopeGlobal, false, true) - require.Error(t, executePlan(ctx, nil, planExplicit, installer.InstallOptions{Scope: installer.ScopeGlobal})) + _, err = executePlan(ctx, nil, planExplicit, installer.InstallOptions{Scope: installer.ScopeGlobal}, false) + require.Error(t, err) } // --- RunE: skills-only path (config-dir detection, no plugin) --- @@ -244,7 +269,7 @@ func TestInstallSkillsOnlyAllAgents(t *testing.T) { calls := setupInstallMock(t) ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--skills-only"}) @@ -259,7 +284,7 @@ func TestInstallSkillsOnlySpecificSkills(t *testing.T) { calls := setupInstallMock(t) ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--skills-only", "--skills", "databricks,databricks-apps"}) @@ -273,7 +298,7 @@ func TestInstallSkillsOnlyExperimental(t *testing.T) { calls := setupInstallMock(t) ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--skills-only", "--experimental"}) @@ -292,7 +317,7 @@ func TestInstallPluginFirstDefault(t *testing.T) { skills := setupInstallMock(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(telemetry.WithNewLogger(ctx)) require.NoError(t, cmd.Execute()) @@ -341,7 +366,7 @@ func TestInstallInteractivePickerAndConfirm(t *testing.T) { go drainReader(test.Stdout) go drainReader(test.Stderr) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(telemetry.WithNewLogger(ctx)) require.NoError(t, cmd.RunE(cmd, nil)) @@ -360,7 +385,7 @@ func TestInstallExplicitAgentWorksUndetected(t *testing.T) { plugins := setupPluginMock(t) ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--agents", "codex"}) @@ -369,10 +394,165 @@ func TestInstallExplicitAgentWorksUndetected(t *testing.T) { assert.Equal(t, agents.NameCodex, (*plugins)[0].agent) } +func TestInstallOutputJSON(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("USERPROFILE", tmp) + fakeBinsOnPath(t, "codex") + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + + origInstall := installPluginForAgentFn + origRecord := recordPluginInstallsFn + t.Cleanup(func() { installPluginForAgentFn = origInstall; recordPluginInstallsFn = origRecord }) + installPluginForAgentFn = func(_ context.Context, a *agents.Agent, _, _ string) (installer.PluginRecord, error) { + return installer.PluginRecord{}, &installer.BlockedError{Agent: a.Name, Reason: installer.ReasonInstallFailed, Detail: "boom"} + } + recordPluginInstallsFn = func(context.Context, string, map[string]installer.PluginRecord, string) error { return nil } + + var out bytes.Buffer + ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) + cmd := newTestInstallCmd() + cmd.SetContext(ctx) + cmd.SetOut(&out) + cmd.SetArgs([]string{"--agents", "codex", "--scope", "global", "--output", "json"}) + + // Explicit --agents makes a blocked install a hard error, but the JSON result + // is still emitted for the extension to consume. The command returns + // ErrAlreadyPrinted so root exits non-zero without printing a duplicate + // "Error:" line over the JSON (exercised end-to-end in + // TestInstallOutputJSONThroughRoot). + err := cmd.Execute() + require.ErrorIs(t, err, root.ErrAlreadyPrinted) + + var got installOutput + require.NoError(t, json.Unmarshal(out.Bytes(), &got)) + require.Len(t, got.Agents, 1) + assert.Equal(t, agents.NameCodex, got.Agents[0].Name) + assert.Equal(t, deliveryPlugin.String(), got.Agents[0].Delivery) + assert.Equal(t, string(outcomeFailed), got.Agents[0].Status) + // A per-agent failure stays in the agent entry; it is not repeated in the + // top-level error field. + assert.Empty(t, got.Error) +} + +// TestInstallOutputJSONThroughRoot runs a failing `install --output json` through +// the real root command, where the "Error:" line is actually printed (root does +// it, not cobra). It guards the contract that a failed JSON run writes only the +// JSON to stdout and no text error to stderr. +func TestInstallOutputJSONThroughRoot(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("USERPROFILE", tmp) + fakeBinsOnPath(t, "codex") + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + + origInstall := installPluginForAgentFn + origRecord := recordPluginInstallsFn + t.Cleanup(func() { installPluginForAgentFn = origInstall; recordPluginInstallsFn = origRecord }) + installPluginForAgentFn = func(_ context.Context, a *agents.Agent, _, _ string) (installer.PluginRecord, error) { + return installer.PluginRecord{}, &installer.BlockedError{Agent: a.Name, Reason: installer.ReasonInstallFailed, Detail: "boom"} + } + recordPluginInstallsFn = func(context.Context, string, map[string]installer.PluginRecord, string) error { return nil } + + ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) + cli := root.New(ctx) + cli.AddCommand(NewInstallCmd()) + var out, errOut bytes.Buffer + cli.SetOut(&out) + cli.SetErr(&errOut) + cli.SetArgs([]string{"install", "--agents", "codex", "--scope", "global", "--output", "json"}) + + err := root.Execute(ctx, cli) + require.ErrorIs(t, err, root.ErrAlreadyPrinted) + assert.NotContains(t, errOut.String(), "Error:") + + var got installOutput + require.NoError(t, json.Unmarshal(out.Bytes(), &got)) + require.Len(t, got.Agents, 1) + assert.Equal(t, string(outcomeFailed), got.Agents[0].Status) +} + +func TestInstallOutputJSONTopLevelFailure(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("USERPROFILE", tmp) + + orig := installSkillsForAgentsFn + t.Cleanup(func() { installSkillsForAgentsFn = orig }) + installSkillsForAgentsFn = func(context.Context, installer.ManifestSource, []*agents.Agent, installer.InstallOptions) error { + return errors.New(`skill "databricks" not found`) + } + + var out bytes.Buffer + ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) + cmd := newTestInstallCmd() + cmd.SetContext(ctx) + cmd.SetOut(&out) + // Cursor is skills-only, so this fails in the skills-group path, which returns + // before appending any per-agent outcome. + cmd.SetArgs([]string{"--agents", "cursor", "--scope", "global", "--output", "json"}) + + // A top-level failure has no per-agent entry, so it must still be represented + // in the JSON (not just a bare non-zero exit with an empty agents array). + err := cmd.Execute() + require.ErrorIs(t, err, root.ErrAlreadyPrinted) + + var got installOutput + require.NoError(t, json.Unmarshal(out.Bytes(), &got)) + assert.Empty(t, got.Agents) + assert.Contains(t, got.Error, "databricks") +} + +func TestInstallOutputJSONRequiresNonInteractiveFlags(t *testing.T) { + setupTestAgents(t) + + cases := []struct { + name string + args []string + want []string // substrings the error must name + }{ + { + name: "no scope or agents", + args: []string{"--output", "json"}, + want: []string{"--scope", "--agents"}, + }, + { + name: "agents without scope", + args: []string{"--agents", "claude-code", "--output", "json"}, + want: []string{"--scope"}, + }, + { + name: "scope without agents", + args: []string{"--scope", "global", "--output", "json"}, + want: []string{"--agents"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var out bytes.Buffer + ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) + cmd := newTestInstallCmd() + cmd.SetContext(ctx) + cmd.SetOut(&out) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + cmd.SetArgs(tc.args) + + err := cmd.Execute() + require.Error(t, err) + for _, w := range tc.want { + assert.Contains(t, err.Error(), w) + } + // The command errors before rendering, so no JSON is emitted. + assert.Empty(t, out.String()) + }) + } +} + func TestInstallUnknownAgentErrors(t *testing.T) { setupTestAgents(t) ctx := cmdio.MockDiscard(t.Context()) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--agents", "invalid-agent"}) cmd.SilenceErrors = true @@ -393,7 +573,7 @@ func TestInstallNoAgentsDetected(t *testing.T) { skills := setupInstallMock(t) ctx := cmdio.MockDiscard(t.Context()) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) require.NoError(t, cmd.Execute()) @@ -404,7 +584,7 @@ func TestInstallNoAgentsDetected(t *testing.T) { func TestInstallSkillsRequiresSkillsOnlyOrPath(t *testing.T) { setupTestAgents(t) ctx := cmdio.MockDiscard(t.Context()) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--skills", "databricks"}) cmd.SilenceErrors = true @@ -430,7 +610,7 @@ func TestInstallInteractivePickerErrorPropagates(t *testing.T) { go drainReader(test.Stdout) go drainReader(test.Stderr) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) err := cmd.RunE(cmd, nil) @@ -440,7 +620,7 @@ func TestInstallInteractivePickerErrorPropagates(t *testing.T) { func TestInstallPathConflictsWithSkillsOnly(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--skills-only", "--path", "./out"}) cmd.SilenceErrors = true @@ -451,6 +631,23 @@ func TestInstallPathConflictsWithSkillsOnly(t *testing.T) { assert.Contains(t, err.Error(), "cannot use --skills-only with --path") } +func TestInstallOutputJSONConflictsWithPath(t *testing.T) { + var out bytes.Buffer + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestInstallCmd() + cmd.SetContext(ctx) + cmd.SetOut(&out) + cmd.SetArgs([]string{"--path", "./out", "--output", "json"}) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot use --output json with --path") + // The command errors before dumping, so nothing is emitted. + assert.Empty(t, out.String()) +} + // --- Scope flag parsing (exercised via the skills path so opts.Scope is observable) --- func TestInstallScopeFlag(t *testing.T) { @@ -473,7 +670,7 @@ func TestInstallScopeFlag(t *testing.T) { calls := setupInstallMock(t) ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs(tt.args) cmd.SilenceErrors = true @@ -497,7 +694,7 @@ func TestInstallGlobalAndProjectErrors(t *testing.T) { setupInstallMock(t) ctx := cmdio.MockDiscard(t.Context()) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--global", "--project"}) cmd.SilenceErrors = true @@ -513,7 +710,7 @@ func TestInstallNoFlagNonInteractiveUsesGlobal(t *testing.T) { calls := setupInstallMock(t) ctx := telemetry.WithNewLogger(cmdio.MockDiscard(t.Context())) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"--skills-only"}) @@ -526,7 +723,7 @@ func TestInstallNoFlagNonInteractiveUsesGlobal(t *testing.T) { func TestInstallRejectsPositionalArgs(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) - cmd := NewInstallCmd() + cmd := newTestInstallCmd() cmd.SetContext(ctx) cmd.SetArgs([]string{"databricks-jobs"}) cmd.SilenceErrors = true diff --git a/cmd/aitools/json.go b/cmd/aitools/json.go new file mode 100644 index 00000000000..15f4080ee2c --- /dev/null +++ b/cmd/aitools/json.go @@ -0,0 +1,14 @@ +package aitools + +import ( + "encoding/json" + "io" +) + +// renderJSON writes v as indented JSON. Shared by the install and list commands +// so their --output json payloads are formatted identically. +func renderJSON(w io.Writer, v any) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(v) +} diff --git a/cmd/aitools/list.go b/cmd/aitools/list.go index e26a7116719..e6decb6a120 100644 --- a/cmd/aitools/list.go +++ b/cmd/aitools/list.go @@ -2,10 +2,8 @@ package aitools import ( "context" - "encoding/json" "errors" "fmt" - "io" "maps" "os" "slices" @@ -142,7 +140,7 @@ func defaultListSkills(cmd *cobra.Command, scope string) error { switch root.OutputType(cmd) { case flags.OutputJSON: - return renderListJSON(cmd.OutOrStdout(), out) + return renderJSON(cmd.OutOrStdout(), out) default: renderListText(ctx, out, scope) return nil @@ -311,12 +309,6 @@ func loadStateForScope(ctx context.Context, scopeFilter, excludeScope string, di return state } -func renderListJSON(w io.Writer, out listOutput) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - return enc.Encode(out) -} - func renderListText(ctx context.Context, out listOutput, scope string) { bothScopes := scope == "" && out.Summary[installer.ScopeGlobal].loaded && diff --git a/cmd/aitools/list_test.go b/cmd/aitools/list_test.go index 9b07e0afb92..72e236933b4 100644 --- a/cmd/aitools/list_test.go +++ b/cmd/aitools/list_test.go @@ -80,7 +80,7 @@ func TestRenderListJSON(t *testing.T) { } var buf bytes.Buffer - require.NoError(t, renderListJSON(&buf, out)) + require.NoError(t, renderJSON(&buf, out)) var got listOutput require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) @@ -131,7 +131,7 @@ func TestRenderListJSONWithAgents(t *testing.T) { } var buf bytes.Buffer - require.NoError(t, renderListJSON(&buf, out)) + require.NoError(t, renderJSON(&buf, out)) var raw map[string]any require.NoError(t, json.Unmarshal(buf.Bytes(), &raw)) @@ -358,7 +358,7 @@ func TestRenderListJSONScopeFiltersSummary(t *testing.T) { } var buf bytes.Buffer - require.NoError(t, renderListJSON(&buf, out)) + require.NoError(t, renderJSON(&buf, out)) var raw map[string]any require.NoError(t, json.Unmarshal(buf.Bytes(), &raw))