From 047ee545e5c5625a4ab9071cbb383d11cbdf8507 Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Wed, 2 Sep 2026 11:31:16 +0200 Subject: [PATCH 1/9] 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)) From d3baa4bfb25deec7c73c212e7ccb4f4256f0e6dc Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Fri, 4 Sep 2026 10:47:21 +0200 Subject: [PATCH 2/9] aitools: fix changelog fragment format 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 --- .nextchanges/cli/aitools-install-output-json.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/cli/aitools-install-output-json.md b/.nextchanges/cli/aitools-install-output-json.md index 76c5e60c05f..f83e9377566 100644 --- a/.nextchanges/cli/aitools-install-output-json.md +++ b/.nextchanges/cli/aitools-install-output-json.md @@ -1 +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)). +* `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)) From f05afd8110b43d78a306be04d4caf5a0fdce964d Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Fri, 4 Sep 2026 16:31:45 +0200 Subject: [PATCH 3/9] aitools: silence human-readable progress in --output json mode --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 --- .../aitools/skills/install-output-json/output.txt | 8 ++------ .../aitools/skills/install-output-json/script | 9 ++++----- cmd/aitools/install.go | 6 ++++++ libs/aitools/installer/installer.go | 8 ++++---- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/acceptance/experimental/aitools/skills/install-output-json/output.txt b/acceptance/experimental/aitools/skills/install-output-json/output.txt index f48e5f84bd7..5caa3daf14b 100644 --- a/acceptance/experimental/aitools/skills/install-output-json/output.txt +++ b/acceptance/experimental/aitools/skills/install-output-json/output.txt @@ -1,10 +1,6 @@ -=== 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. +=== install --output json emits only the JSON document; progress is silenced +>>> [CLI] aitools install --skills-only --scope=global --agents=claude-code --output json { "scope": "global", "agents": [ diff --git a/acceptance/experimental/aitools/skills/install-output-json/script b/acceptance/experimental/aitools/skills/install-output-json/script index abc1e627289..93c6637462c 100644 --- a/acceptance/experimental/aitools/skills/install-output-json/script +++ b/acceptance/experimental/aitools/skills/install-output-json/script @@ -1,9 +1,8 @@ # 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" +title "install --output json emits only the JSON document; progress is silenced" # --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 . +# --output json requires. In JSON mode the command silences its human-readable +# progress, so the only thing written is the JSON payload on stdout. +trace $CLI aitools install --skills-only --scope=global --agents=claude-code --output json diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index 42b9ee9682c..1e360bb08bb 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -106,6 +106,12 @@ Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() jsonMode := installOutputIsJSON(cmd) + if jsonMode { + // JSON mode emits only the structured result on stdout; silence all + // human-readable progress (via cmdio.LogProgress) so nothing but the + // JSON document is written. + ctx = cmdio.WithQuiet(ctx) + } if skillsOnly && pathFlag != "" { return errors.New("cannot use --skills-only with --path; --path always writes raw skill files") diff --git a/libs/aitools/installer/installer.go b/libs/aitools/installer/installer.go index ae5b8e91acc..168c849f285 100644 --- a/libs/aitools/installer/installer.go +++ b/libs/aitools/installer/installer.go @@ -275,8 +275,8 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent if err != nil { return err } - cmdio.LogString(ctx, "Using skills version "+DisplaySkillsVersion(ref)) - cmdio.LogString(ctx, "Fetching skills manifest...") + cmdio.LogProgress(ctx, "Using skills version "+DisplaySkillsVersion(ref)) + cmdio.LogProgress(ctx, "Fetching skills manifest...") manifest, ref, err := FetchSkillsManifestWithFallback(ctx, src, ref, !explicit) if err != nil { return err @@ -422,7 +422,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent if len(targetSkills) == 1 { noun = "skill" } - cmdio.LogString(ctx, fmt.Sprintf("Installed %d %s.", len(targetSkills), noun)) + cmdio.LogProgress(ctx, fmt.Sprintf("Installed %d %s.", len(targetSkills), noun)) return nil } @@ -441,7 +441,7 @@ func filterProjectAgents(ctx context.Context, targetAgents []*agents.Agent) []*a if a.SupportsProjectScope { compatible = append(compatible, a) } else { - cmdio.LogString(ctx, "Skipped "+a.DisplayName+": does not support project-scoped skills.") + cmdio.LogProgress(ctx, "Skipped "+a.DisplayName+": does not support project-scoped skills.") } } return compatible From 28dd9366a4ee7654dd0a65b009fe653e5d8cb555 Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Mon, 7 Sep 2026 10:48:14 +0200 Subject: [PATCH 4/9] trim comment --- cmd/aitools/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index 1e360bb08bb..f478d219c70 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -226,7 +226,7 @@ Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), // 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. + // opts out of that if runErr != nil { return root.ErrAlreadyPrinted } From 4d8567cdd154e8dbf46ae850c92c420e00b4aa9e Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Wed, 2 Sep 2026 12:04:05 +0200 Subject: [PATCH 5/9] categorize aitools install errors Classify why an `aitools install` run, or one agent within it, failed into a stable AitoolsErrorCategory, so install failures can be aggregated in telemetry without sending any user-authored error text. Introduce SkillError alongside the existing BlockedError, map both to categories via classifyInstallError, and record the per-agent categories on the install event. Surface the per-agent category in the `--output json` result too. The top-level errorCategory (telemetry and JSON) is set only for a failure with no per-agent entry; a per-agent failure keeps its category in its own entry and leaves the top-level category Unspecified, so it is never counted twice. Co-authored-by: Isaac --- .../cli/aitools-install-error-category.md | 1 + .../install-output-json-error/out.test.toml | 2 + .../install-output-json-error/output.txt | 14 +++ .../skills/install-output-json-error/script | 9 ++ .../install-output-json-error/test.toml | 28 ++++++ cmd/aitools/categorize.go | 44 +++++++++ cmd/aitools/categorize_test.go | 70 ++++++++++++++ cmd/aitools/install.go | 94 +++++++++++-------- cmd/aitools/install_test.go | 11 ++- cmd/aitools/telemetry.go | 37 +++++++- cmd/aitools/telemetry_test.go | 24 +++++ libs/aitools/installer/errors.go | 26 +++++ libs/aitools/installer/installer.go | 4 +- libs/telemetry/protos/aitools_install.go | 34 +++++++ 14 files changed, 352 insertions(+), 46 deletions(-) create mode 100644 .nextchanges/cli/aitools-install-error-category.md create mode 100644 acceptance/experimental/aitools/skills/install-output-json-error/out.test.toml create mode 100644 acceptance/experimental/aitools/skills/install-output-json-error/output.txt create mode 100644 acceptance/experimental/aitools/skills/install-output-json-error/script create mode 100644 acceptance/experimental/aitools/skills/install-output-json-error/test.toml create mode 100644 cmd/aitools/categorize.go create mode 100644 cmd/aitools/categorize_test.go create mode 100644 libs/aitools/installer/errors.go diff --git a/.nextchanges/cli/aitools-install-error-category.md b/.nextchanges/cli/aitools-install-error-category.md new file mode 100644 index 00000000000..5b7ac8487d8 --- /dev/null +++ b/.nextchanges/cli/aitools-install-error-category.md @@ -0,0 +1 @@ +`databricks aitools install --output json` now reports an `errorCategory` 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](https://github.com/databricks/cli/pull/6482)). diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/out.test.toml b/acceptance/experimental/aitools/skills/install-output-json-error/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-error/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/output.txt b/acceptance/experimental/aitools/skills/install-output-json-error/output.txt new file mode 100644 index 00000000000..7709477d226 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-error/output.txt @@ -0,0 +1,14 @@ + +=== install --output json reports a top-level error category and exits non-zero +>>> [CLI] experimental aitools install --skills-only --scope=global --agents=claude-code --skills=nonexistent --output json +Command "install" is deprecated, use "databricks aitools install" instead. +Using skills version test-ref +Fetching skills manifest... +{ + "scope": "global", + "agents": [], + "error": "skill \"nonexistent\" not found", + "errorCategory": "SKILL_NOT_FOUND" +} + +Exit code: 1 diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/script b/acceptance/experimental/aitools/skills/install-output-json-error/script new file mode 100644 index 00000000000..71ed2836a73 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-error/script @@ -0,0 +1,9 @@ +# Isolate HOME so parallel aitools tests don't race on a shared ~/.databricks. +sethome home + +title "install --output json reports a top-level error category and exits non-zero" +# A --skills entry absent from the manifest fails before any agent is touched, so the +# failure has no per-agent entry and surfaces in the top-level error/errorCategory +# fields (agents stays empty). The command exits non-zero, but root prints no +# duplicate "Error:" line to stderr because the JSON already reported the failure. +trace $CLI experimental aitools install --skills-only --scope=global --agents=claude-code --skills=nonexistent --output json diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/test.toml b/acceptance/experimental/aitools/skills/install-output-json-error/test.toml new file mode 100644 index 00000000000..edf1106ca21 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-error/test.toml @@ -0,0 +1,28 @@ +# 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"] + +# The manifest has one skill; the script asks for a different one so the resolve +# fails with a skill-not-found error. +[[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" + } + } +} +''' diff --git a/cmd/aitools/categorize.go b/cmd/aitools/categorize.go new file mode 100644 index 00000000000..33bbf297fb5 --- /dev/null +++ b/cmd/aitools/categorize.go @@ -0,0 +1,44 @@ +package aitools + +import ( + "errors" + + "github.com/databricks/cli/libs/aitools/installer" + "github.com/databricks/cli/libs/telemetry/protos" +) + +func classifyInstallError(err error) protos.AitoolsErrorCategory { + if err == nil { + return protos.AitoolsErrorCategoryUnspecified + } + + if blocked, ok := errors.AsType[*installer.BlockedError](err); ok { + return blockedErrorCategory(blocked) + } + if skill, ok := errors.AsType[*installer.SkillError](err); ok { + return skillErrorCategory(skill) + } + return protos.AitoolsErrorCategoryUncategorized +} + +func skillErrorCategory(e *installer.SkillError) protos.AitoolsErrorCategory { + switch e.Reason { + case installer.ReasonSkillNotFound: + return protos.AitoolsErrorCategorySkillNotFound + case installer.ReasonVersionIncompatible: + return protos.AitoolsErrorCategoryVersionIncompatible + default: + return protos.AitoolsErrorCategoryUncategorized + } +} + +func blockedErrorCategory(e *installer.BlockedError) protos.AitoolsErrorCategory { + switch e.Reason { + case installer.ReasonCLINotOnPath: + return protos.AitoolsErrorCategoryCLINotOnPath + case installer.ReasonInstallFailed: + return protos.AitoolsErrorCategoryPluginInstallFailed + default: + return protos.AitoolsErrorCategoryUncategorized + } +} diff --git a/cmd/aitools/categorize_test.go b/cmd/aitools/categorize_test.go new file mode 100644 index 00000000000..2b6e337fea6 --- /dev/null +++ b/cmd/aitools/categorize_test.go @@ -0,0 +1,70 @@ +package aitools + +import ( + "errors" + "fmt" + "testing" + + "github.com/databricks/cli/libs/aitools/installer" + "github.com/databricks/cli/libs/telemetry/protos" + "github.com/stretchr/testify/assert" +) + +func TestClassifyInstallError(t *testing.T) { + cases := []struct { + name string + err error + want protos.AitoolsErrorCategory + }{ + { + name: "nil is success", + err: nil, + want: protos.AitoolsErrorCategoryUnspecified, + }, + { + name: "blocked cli not on path", + err: &installer.BlockedError{Agent: "claude-code", Reason: installer.ReasonCLINotOnPath}, + want: protos.AitoolsErrorCategoryCLINotOnPath, + }, + { + name: "blocked install failed", + err: &installer.BlockedError{Agent: "codex", Reason: installer.ReasonInstallFailed}, + want: protos.AitoolsErrorCategoryPluginInstallFailed, + }, + { + name: "blocked no plugin is uncategorized", + err: &installer.BlockedError{Agent: "codex", Reason: installer.ReasonNoPlugin}, + want: protos.AitoolsErrorCategoryUncategorized, + }, + { + name: "wrapped skill not found", + err: fmt.Errorf("resolve failed: %w", &installer.SkillError{Skill: "databricks", Reason: installer.ReasonSkillNotFound, Detail: "not found"}), + want: protos.AitoolsErrorCategorySkillNotFound, + }, + { + name: "version incompatible", + err: &installer.SkillError{Skill: "databricks", Reason: installer.ReasonVersionIncompatible, Detail: "requires CLI version 0.5 (running 0.4)"}, + want: protos.AitoolsErrorCategoryVersionIncompatible, + }, + { + name: "skill error with unknown reason is uncategorized", + err: &installer.SkillError{Skill: "databricks", Reason: "some-future-reason"}, + want: protos.AitoolsErrorCategoryUncategorized, + }, + { + name: "blocked error joined with another error is still classified", + err: errors.Join(&installer.BlockedError{Agent: "codex", Reason: installer.ReasonInstallFailed}, errors.New("other")), + want: protos.AitoolsErrorCategoryPluginInstallFailed, + }, + { + name: "unrecognized error is uncategorized", + err: errors.New("boom"), + want: protos.AitoolsErrorCategoryUncategorized, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, classifyInstallError(tc.err)) + }) + } +} diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index f478d219c70..59657ac7412 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -13,6 +13,7 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/telemetry/protos" "github.com/spf13/cobra" ) @@ -57,11 +58,12 @@ func (d delivery) String() string { // agentPlanItem is the resolved plan for one agent: what we'll do and why. type agentPlanItem struct { - agent *agents.Agent - delivery delivery - scope string // agent-native plugin scope (deliveryPlugin only) - reason string // why the agent is skipped (deliverySkip only) - explicit bool // named via --agents (blocking it is an error) + agent *agents.Agent + delivery delivery + scope string // agent-native plugin scope (deliveryPlugin only) + reason string // why the agent is skipped (deliverySkip only) + skipError protos.AitoolsErrorCategory // error category for the skip (deliverySkip only) + explicit bool // named via --agents (blocking it is an error) } // agentChoice is one row in the interactive agent picker. @@ -206,12 +208,16 @@ Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), } } - defer logInstallEvent(ctx, plan, installOpts{ - Scope: opts.Scope, - Experimental: opts.IncludeExperimental, - }) + var outcomes []agentOutcome + var runErr error + defer func() { + logInstallEvent(ctx, plan, installOpts{ + Scope: opts.Scope, + Experimental: opts.IncludeExperimental, + }, classifyInstallError(topLevelFailure(runErr)), outcomes) + }() - outcomes, runErr := executePlan(ctx, src, plan, opts, jsonMode) + 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. @@ -408,6 +414,7 @@ func planItemFor(a *agents.Agent, scope string, skillsOnly, explicit bool) agent if scope == installer.ScopeProject && !a.SupportsProjectScope { item.delivery = deliverySkip item.reason = "does not support project-scoped skills" + item.skipError = protos.AitoolsErrorCategoryUnsupportedScope } else { item.delivery = deliverySkills } @@ -416,6 +423,7 @@ func planItemFor(a *agents.Agent, scope string, skillsOnly, explicit bool) agent if !ok { item.delivery = deliverySkip item.reason = reason + item.skipError = protos.AitoolsErrorCategoryUnsupportedScope } else { item.delivery = deliveryPlugin item.scope = nativeScope @@ -442,13 +450,15 @@ func printPlanSummary(ctx context.Context, plan []agentPlanItem, scope string) { } // 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. +// tools were delivered (or attempted), and, when the agent did not succeed, the +// failure category and a human-readable message for --output json. The category +// is what telemetry records; the message is local-only and never sent. type agentOutcome struct { - agent *agents.Agent - delivery delivery - status outcomeStatus - message string // set when skipped or failed + agent *agents.Agent + delivery delivery + status outcomeStatus + errorCategory protos.AitoolsErrorCategory // Unspecified when status == outcomeInstalled + message string // set when skipped or failed } type outcomeStatus string @@ -460,9 +470,9 @@ const ( ) // 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. +// reported in their per-agent outcomes. Wrapping lets the JSON and telemetry +// layers 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() } @@ -470,7 +480,7 @@ 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. +// duplicated in the top-level error/errorCategory fields. func topLevelFailure(runErr error) error { if _, ok := errors.AsType[*agentErrors](runErr); ok { return nil @@ -503,7 +513,8 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent if !quiet { installer.PrintInstallingFor(ctx, skillsAgents) } - // A skills install runs as a group; on failure the whole command fails. + // A skills install runs as a group; on failure the whole command fails and + // the top-level error category classifies it. if err := installSkillsForAgentsFn(ctx, src, skillsAgents, opts); err != nil { return outcomes, err } @@ -529,10 +540,11 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent 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(), + agent: it.agent, + delivery: deliveryPlugin, + status: outcomeFailed, + errorCategory: classifyInstallError(err), + message: err.Error(), }) if it.explicit { explicitErrs = append(explicitErrs, err) @@ -563,10 +575,11 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent 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, + agent: it.agent, + delivery: deliverySkip, + status: outcomeSkipped, + errorCategory: it.skipError, + message: it.reason, }) if it.explicit { explicitErrs = append(explicitErrs, fmt.Errorf("%s: %s", it.agent.DisplayName, it.reason)) @@ -591,17 +604,20 @@ 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"` + // Error and ErrorCategory describe a top-level failure with no per-agent + // entry (e.g. a skills-group install failure); both empty on success. Error + // is the local-only message; ErrorCategory is the classification telemetry + // also records. + Error string `json:"error,omitempty"` + ErrorCategory string `json:"errorCategory,omitempty"` } type agentResultJSON struct { - Name string `json:"name"` - Delivery string `json:"delivery"` - Status string `json:"status"` - Message string `json:"message,omitempty"` + Name string `json:"name"` + Delivery string `json:"delivery"` + Status string `json:"status"` + ErrorCategory string `json:"errorCategory,omitempty"` + Message string `json:"message,omitempty"` } func buildInstallOutput(scope string, outcomes []agentOutcome, runErr error) installOutput { @@ -613,6 +629,9 @@ func buildInstallOutput(scope string, outcomes []agentOutcome, runErr error) ins Status: string(o.status), Message: o.message, } + if o.errorCategory != "" { + entry.ErrorCategory = string(o.errorCategory) + } out.Agents = append(out.Agents, entry) } // A top-level failure (skills-group install, ref lookup, plugin recording) @@ -621,6 +640,7 @@ func buildInstallOutput(scope string, outcomes []agentOutcome, runErr error) ins // stay in the agents entries above and are not repeated here. if e := topLevelFailure(runErr); e != nil { out.Error = e.Error() + out.ErrorCategory = string(classifyInstallError(e)) } return out } diff --git a/cmd/aitools/install_test.go b/cmd/aitools/install_test.go index 1972262c066..d3f0b38fb25 100644 --- a/cmd/aitools/install_test.go +++ b/cmd/aitools/install_test.go @@ -17,6 +17,7 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/telemetry" + "github.com/databricks/cli/libs/telemetry/protos" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -255,6 +256,7 @@ func TestExecutePlanSkipBlockedPluginExit0(t *testing.T) { require.NoError(t, err) require.Len(t, outcomes, 1) assert.Equal(t, outcomeFailed, outcomes[0].status) + assert.Equal(t, protos.AitoolsErrorCategoryCLINotOnPath, outcomes[0].errorCategory) // Explicit (--agents) blocked install is an error. planExplicit := buildPlan([]*agents.Agent{claude}, installer.ScopeGlobal, false, true) @@ -394,7 +396,7 @@ func TestInstallExplicitAgentWorksUndetected(t *testing.T) { assert.Equal(t, agents.NameCodex, (*plugins)[0].agent) } -func TestInstallOutputJSON(t *testing.T) { +func TestInstallOutputJSONReportsErrorCategories(t *testing.T) { tmp := t.TempDir() t.Setenv("HOME", tmp) t.Setenv("USERPROFILE", tmp) @@ -430,9 +432,11 @@ func TestInstallOutputJSON(t *testing.T) { 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) + assert.Equal(t, string(protos.AitoolsErrorCategoryPluginInstallFailed), got.Agents[0].ErrorCategory) // A per-agent failure stays in the agent entry; it is not repeated in the - // top-level error field. + // top-level error fields. assert.Empty(t, got.Error) + assert.Empty(t, got.ErrorCategory) } // TestInstallOutputJSONThroughRoot runs a failing `install --output json` through @@ -480,7 +484,7 @@ func TestInstallOutputJSONTopLevelFailure(t *testing.T) { 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`) + return &installer.SkillError{Skill: "databricks", Reason: installer.ReasonSkillNotFound, Detail: "not found"} } var out bytes.Buffer @@ -501,6 +505,7 @@ func TestInstallOutputJSONTopLevelFailure(t *testing.T) { require.NoError(t, json.Unmarshal(out.Bytes(), &got)) assert.Empty(t, got.Agents) assert.Contains(t, got.Error, "databricks") + assert.Equal(t, string(protos.AitoolsErrorCategorySkillNotFound), got.ErrorCategory) } func TestInstallOutputJSONRequiresNonInteractiveFlags(t *testing.T) { diff --git a/cmd/aitools/telemetry.go b/cmd/aitools/telemetry.go index 1e85d5dfbf7..c8174b4a883 100644 --- a/cmd/aitools/telemetry.go +++ b/cmd/aitools/telemetry.go @@ -1,6 +1,7 @@ package aitools import ( + "cmp" "context" "slices" @@ -30,16 +31,44 @@ type installOpts struct { } // logInstallEvent buffers an install event; cmd/root uploads it at exit. -func logInstallEvent(ctx context.Context, plan []agentPlanItem, opts installOpts) { +// errCategory is the top-level command outcome (Unspecified on success or when +// the failure is reported per agent), and outcomes carries the per-agent results +// so a skipped-with-warning failure is still recorded even when the command +// exits 0. +func logInstallEvent(ctx context.Context, plan []agentPlanItem, opts installOpts, errCategory protos.AitoolsErrorCategory, outcomes []agentOutcome) { telemetry.Log(ctx, protos.DatabricksCliLog{ AitoolsInstallEvent: &protos.AitoolsInstallEvent{ - Agents: agentsField(plan), - Scope: scopeType(opts.Scope), - Experimental: opts.Experimental, + Agents: agentsField(plan), + Scope: scopeType(opts.Scope), + Experimental: opts.Experimental, + ErrorCategory: errCategory, + AgentResults: agentResultsField(outcomes), }, }) } +// agentResultsField returns the per-agent failure/skip categories, one entry per +// non-successful agent, sorted by agent enum for stable output. Successful +// agents produce no entry. +func agentResultsField(outcomes []agentOutcome) []protos.AitoolsAgentResult { + var out []protos.AitoolsAgentResult + for _, o := range outcomes { + // A successful agent leaves errorCategory at its zero value ("", not the + // TYPE_UNSPECIFIED sentinel), so key on emptiness to drop it here. + if o.agent == nil || o.errorCategory == "" { + continue + } + out = append(out, protos.AitoolsAgentResult{ + Agent: agentType(o.agent.Name), + ErrorCategory: o.errorCategory, + }) + } + slices.SortFunc(out, func(a, b protos.AitoolsAgentResult) int { + return cmp.Compare(a.Agent, b.Agent) + }) + return out +} + // agentsField returns the deduped agent enums from the plan, sorted so the // same set of agents always produces the same array on the analytics side. func agentsField(plan []agentPlanItem) []protos.AitoolsAgentType { diff --git a/cmd/aitools/telemetry_test.go b/cmd/aitools/telemetry_test.go index 85705161955..0b5cb5be05b 100644 --- a/cmd/aitools/telemetry_test.go +++ b/cmd/aitools/telemetry_test.go @@ -84,3 +84,27 @@ func TestScopeType(t *testing.T) { assert.Equal(t, protos.AitoolsInstallScopeProject, scopeType(installer.ScopeProject)) assert.Equal(t, protos.AitoolsInstallScopeUnspecified, scopeType("")) } + +func TestAgentResultsField(t *testing.T) { + claude := &agents.Agent{Name: agents.NameClaudeCode} + codex := &agents.Agent{Name: agents.NameCodex} + cursor := &agents.Agent{Name: agents.NameCursor} + + outcomes := []agentOutcome{ + // Successful agents produce no entry; production leaves errorCategory unset. + {agent: cursor, status: outcomeInstalled}, + {agent: codex, status: outcomeFailed, errorCategory: protos.AitoolsErrorCategoryPluginInstallFailed}, + {agent: claude, status: outcomeSkipped, errorCategory: protos.AitoolsErrorCategoryUnsupportedScope}, + // A nil agent is skipped defensively. + {agent: nil, status: outcomeFailed, errorCategory: protos.AitoolsErrorCategoryPluginInstallFailed}, + } + + // Sorted by agent enum, only non-successful agents included. + want := []protos.AitoolsAgentResult{ + {Agent: protos.AitoolsAgentTypeClaudeCode, ErrorCategory: protos.AitoolsErrorCategoryUnsupportedScope}, + {Agent: protos.AitoolsAgentTypeCodex, ErrorCategory: protos.AitoolsErrorCategoryPluginInstallFailed}, + } + assert.Equal(t, want, agentResultsField(outcomes)) + + assert.Nil(t, agentResultsField(nil)) +} diff --git a/libs/aitools/installer/errors.go b/libs/aitools/installer/errors.go new file mode 100644 index 00000000000..dc2bee212c4 --- /dev/null +++ b/libs/aitools/installer/errors.go @@ -0,0 +1,26 @@ +package installer + +import "fmt" + +// SkillError reports that a skill named via --skills could not be resolved from +// the manifest. Reason drives telemetry categorization (the command layer maps +// it with errors.AsType); Detail is the human-readable remainder of the message. +// Building the message from fields keeps a classification tag out of the +// user-facing string, so the error is stated exactly once. +type SkillError struct { + Skill string + Reason string + Detail string +} + +// Reasons a --skills entry can fail to resolve. +const ( + // ReasonSkillNotFound: the named skill is absent from the resolved manifest. + ReasonSkillNotFound = "skill-not-found" + // ReasonVersionIncompatible: the skill requires a newer CLI than the one running. + ReasonVersionIncompatible = "version-incompatible" +) + +func (e *SkillError) Error() string { + return fmt.Sprintf("skill %q %s", e.Skill, e.Detail) +} diff --git a/libs/aitools/installer/installer.go b/libs/aitools/installer/installer.go index 168c849f285..accaa086269 100644 --- a/libs/aitools/installer/installer.go +++ b/libs/aitools/installer/installer.go @@ -472,7 +472,7 @@ func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts Instal for _, name := range opts.SpecificSkills { meta, ok := skills[name] if !ok { - return nil, fmt.Errorf("skill %q not found", name) + return nil, &SkillError{Skill: name, Reason: ReasonSkillNotFound, Detail: "not found"} } candidates[name] = meta } @@ -492,7 +492,7 @@ func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts Instal if meta.MinCLIVer != "" && !isDev && semver.Compare("v"+cliVersion, "v"+meta.MinCLIVer) < 0 { if isSpecific { - return nil, fmt.Errorf("skill %q requires CLI version %s (running %s)", name, meta.MinCLIVer, cliVersion) + return nil, &SkillError{Skill: name, Reason: ReasonVersionIncompatible, Detail: fmt.Sprintf("requires CLI version %s (running %s)", meta.MinCLIVer, cliVersion)} } log.Warnf(ctx, "Skipping %s: requires CLI version %s (running %s)", name, meta.MinCLIVer, cliVersion) continue diff --git a/libs/telemetry/protos/aitools_install.go b/libs/telemetry/protos/aitools_install.go index 42d2b912490..d5795c3b4b3 100644 --- a/libs/telemetry/protos/aitools_install.go +++ b/libs/telemetry/protos/aitools_install.go @@ -28,6 +28,31 @@ const ( AitoolsInstallScopeProject AitoolsInstallScope = "PROJECT" ) +// AitoolsErrorCategory classifies why an `aitools install` run, or one agent +// within it, failed. It mirrors AitoolsErrorCategory.Type in the databricks_cli +// lumberjack proto and lets us aggregate install failures without sending any +// user-authored error text. AitoolsErrorCategoryUncategorized absorbs failures +// a newer CLI has not classified yet. +type AitoolsErrorCategory string + +const ( + AitoolsErrorCategoryUnspecified AitoolsErrorCategory = "TYPE_UNSPECIFIED" + AitoolsErrorCategoryVersionIncompatible AitoolsErrorCategory = "VERSION_INCOMPATIBLE" + AitoolsErrorCategorySkillNotFound AitoolsErrorCategory = "SKILL_NOT_FOUND" + AitoolsErrorCategoryCLINotOnPath AitoolsErrorCategory = "CLI_NOT_ON_PATH" + AitoolsErrorCategoryPluginInstallFailed AitoolsErrorCategory = "PLUGIN_INSTALL_FAILED" + AitoolsErrorCategoryUnsupportedScope AitoolsErrorCategory = "UNSUPPORTED_SCOPE" + AitoolsErrorCategoryUncategorized AitoolsErrorCategory = "UNCATEGORIZED" +) + +// AitoolsAgentResult records one agent's failed or skipped outcome within an +// install run. Successful agents produce no entry, and Category never carries +// user-authored text. +type AitoolsAgentResult struct { + Agent AitoolsAgentType `json:"agent"` + ErrorCategory AitoolsErrorCategory `json:"errorCategory"` +} + // AitoolsInstallEvent is emitted on every execution of the `databricks aitools // install` command. type AitoolsInstallEvent struct { @@ -39,4 +64,13 @@ type AitoolsInstallEvent struct { // Whether the user passed --experimental to include experimental skills. Experimental bool `json:"experimental,omitempty"` + + // ErrorCategory is the top-level command outcome: the category of the error + // that failed the run, or Unspecified when the command succeeded. It captures + // failures that have no per-agent entry (e.g. a skills-group install failure). + ErrorCategory AitoolsErrorCategory `json:"error_category,omitempty"` + + // AgentResults records the per-agent failure/skip categories, one entry per + // non-successful agent. Empty when every targeted agent succeeded. + AgentResults []AitoolsAgentResult `json:"agent_results,omitempty"` } From 06f3bf52d807cee6addab9474c24f87daba1f3e5 Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Fri, 4 Sep 2026 12:26:55 +0200 Subject: [PATCH 6/9] aitools: address review feedback on error categories - Rename the JSON/telemetry field errorCategory -> error_category (snake_case), matching the CLI's --output json convention and the rest of libs/telemetry/protos. Update the acceptance golden accordingly. - Drop omitempty on AitoolsInstallEvent.ErrorCategory: it is always populated (Unspecified on success), so the tag never fired. - agentResultsField: key on status == outcomeInstalled instead of errorCategory == "", and drop the dead o.agent == nil guard (agents always come from the validated registry, matching buildInstallOutput). - SkillError.Error() falls back to Reason when Detail is empty so the message stays self-describing. Co-authored-by: Isaac --- .nextchanges/cli/aitools-install-error-category.md | 2 +- .../aitools/skills/install-output-json-error/output.txt | 2 +- .../aitools/skills/install-output-json-error/script | 2 +- cmd/aitools/install.go | 4 ++-- cmd/aitools/telemetry.go | 7 ++++--- cmd/aitools/telemetry_test.go | 2 -- libs/aitools/installer/errors.go | 7 ++++++- libs/telemetry/protos/aitools_install.go | 5 +++-- 8 files changed, 18 insertions(+), 13 deletions(-) diff --git a/.nextchanges/cli/aitools-install-error-category.md b/.nextchanges/cli/aitools-install-error-category.md index 5b7ac8487d8..ce3594049fa 100644 --- a/.nextchanges/cli/aitools-install-error-category.md +++ b/.nextchanges/cli/aitools-install-error-category.md @@ -1 +1 @@ -`databricks aitools install --output json` now reports an `errorCategory` 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](https://github.com/databricks/cli/pull/6482)). +* `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](https://github.com/databricks/cli/pull/6482)) diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/output.txt b/acceptance/experimental/aitools/skills/install-output-json-error/output.txt index 7709477d226..f4608d33df5 100644 --- a/acceptance/experimental/aitools/skills/install-output-json-error/output.txt +++ b/acceptance/experimental/aitools/skills/install-output-json-error/output.txt @@ -8,7 +8,7 @@ Fetching skills manifest... "scope": "global", "agents": [], "error": "skill \"nonexistent\" not found", - "errorCategory": "SKILL_NOT_FOUND" + "error_category": "SKILL_NOT_FOUND" } Exit code: 1 diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/script b/acceptance/experimental/aitools/skills/install-output-json-error/script index 71ed2836a73..7857967ba36 100644 --- a/acceptance/experimental/aitools/skills/install-output-json-error/script +++ b/acceptance/experimental/aitools/skills/install-output-json-error/script @@ -3,7 +3,7 @@ sethome home title "install --output json reports a top-level error category and exits non-zero" # A --skills entry absent from the manifest fails before any agent is touched, so the -# failure has no per-agent entry and surfaces in the top-level error/errorCategory +# failure has no per-agent entry and surfaces in the top-level error/error_category # fields (agents stays empty). The command exits non-zero, but root prints no # duplicate "Error:" line to stderr because the JSON already reported the failure. trace $CLI experimental aitools install --skills-only --scope=global --agents=claude-code --skills=nonexistent --output json diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index 59657ac7412..5f2f2cf9ccc 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -609,14 +609,14 @@ type installOutput struct { // is the local-only message; ErrorCategory is the classification telemetry // also records. Error string `json:"error,omitempty"` - ErrorCategory string `json:"errorCategory,omitempty"` + ErrorCategory string `json:"error_category,omitempty"` } type agentResultJSON struct { Name string `json:"name"` Delivery string `json:"delivery"` Status string `json:"status"` - ErrorCategory string `json:"errorCategory,omitempty"` + ErrorCategory string `json:"error_category,omitempty"` Message string `json:"message,omitempty"` } diff --git a/cmd/aitools/telemetry.go b/cmd/aitools/telemetry.go index c8174b4a883..09eed34f3b2 100644 --- a/cmd/aitools/telemetry.go +++ b/cmd/aitools/telemetry.go @@ -53,9 +53,10 @@ func logInstallEvent(ctx context.Context, plan []agentPlanItem, opts installOpts func agentResultsField(outcomes []agentOutcome) []protos.AitoolsAgentResult { var out []protos.AitoolsAgentResult for _, o := range outcomes { - // A successful agent leaves errorCategory at its zero value ("", not the - // TYPE_UNSPECIFIED sentinel), so key on emptiness to drop it here. - if o.agent == nil || o.errorCategory == "" { + // Successful agents produce no entry; only failed/skipped outcomes carry a + // category. Keying on status (rather than errorCategory == "") avoids a trap + // where a future Unspecified category on a success would emit a bogus entry. + if o.status == outcomeInstalled { continue } out = append(out, protos.AitoolsAgentResult{ diff --git a/cmd/aitools/telemetry_test.go b/cmd/aitools/telemetry_test.go index 0b5cb5be05b..160bb4fe98e 100644 --- a/cmd/aitools/telemetry_test.go +++ b/cmd/aitools/telemetry_test.go @@ -95,8 +95,6 @@ func TestAgentResultsField(t *testing.T) { {agent: cursor, status: outcomeInstalled}, {agent: codex, status: outcomeFailed, errorCategory: protos.AitoolsErrorCategoryPluginInstallFailed}, {agent: claude, status: outcomeSkipped, errorCategory: protos.AitoolsErrorCategoryUnsupportedScope}, - // A nil agent is skipped defensively. - {agent: nil, status: outcomeFailed, errorCategory: protos.AitoolsErrorCategoryPluginInstallFailed}, } // Sorted by agent enum, only non-successful agents included. diff --git a/libs/aitools/installer/errors.go b/libs/aitools/installer/errors.go index dc2bee212c4..774b327df1b 100644 --- a/libs/aitools/installer/errors.go +++ b/libs/aitools/installer/errors.go @@ -22,5 +22,10 @@ const ( ) func (e *SkillError) Error() string { - return fmt.Sprintf("skill %q %s", e.Skill, e.Detail) + // Detail is the human-readable remainder; fall back to Reason when it is empty + // so the message stays self-describing (mirrors BlockedError.Error()). + if e.Detail != "" { + return fmt.Sprintf("skill %q %s", e.Skill, e.Detail) + } + return fmt.Sprintf("skill %q %s", e.Skill, e.Reason) } diff --git a/libs/telemetry/protos/aitools_install.go b/libs/telemetry/protos/aitools_install.go index d5795c3b4b3..7bd034b3665 100644 --- a/libs/telemetry/protos/aitools_install.go +++ b/libs/telemetry/protos/aitools_install.go @@ -50,7 +50,7 @@ const ( // user-authored text. type AitoolsAgentResult struct { Agent AitoolsAgentType `json:"agent"` - ErrorCategory AitoolsErrorCategory `json:"errorCategory"` + ErrorCategory AitoolsErrorCategory `json:"error_category"` } // AitoolsInstallEvent is emitted on every execution of the `databricks aitools @@ -68,7 +68,8 @@ type AitoolsInstallEvent struct { // ErrorCategory is the top-level command outcome: the category of the error // that failed the run, or Unspecified when the command succeeded. It captures // failures that have no per-agent entry (e.g. a skills-group install failure). - ErrorCategory AitoolsErrorCategory `json:"error_category,omitempty"` + // Always populated (Unspecified on success), so no omitempty. + ErrorCategory AitoolsErrorCategory `json:"error_category"` // AgentResults records the per-agent failure/skip categories, one entry per // non-successful agent. Empty when every targeted agent succeeded. From 5c989f057e630ef2599665aef16bc03dfcd0aac1 Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Fri, 4 Sep 2026 16:34:06 +0200 Subject: [PATCH 7/9] aitools: clean up install-output-json-error acceptance test Use the non-deprecated `aitools install` command. With progress now silenced in JSON mode (see the --output json branch), stdout carries only the JSON document, so the golden no longer has non-JSON text before it. Co-authored-by: Isaac --- .../aitools/skills/install-output-json-error/output.txt | 5 +---- .../aitools/skills/install-output-json-error/script | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/output.txt b/acceptance/experimental/aitools/skills/install-output-json-error/output.txt index f4608d33df5..db5a78e81e4 100644 --- a/acceptance/experimental/aitools/skills/install-output-json-error/output.txt +++ b/acceptance/experimental/aitools/skills/install-output-json-error/output.txt @@ -1,9 +1,6 @@ === install --output json reports a top-level error category and exits non-zero ->>> [CLI] experimental aitools install --skills-only --scope=global --agents=claude-code --skills=nonexistent --output json -Command "install" is deprecated, use "databricks aitools install" instead. -Using skills version test-ref -Fetching skills manifest... +>>> [CLI] aitools install --skills-only --scope=global --agents=claude-code --skills=nonexistent --output json { "scope": "global", "agents": [], diff --git a/acceptance/experimental/aitools/skills/install-output-json-error/script b/acceptance/experimental/aitools/skills/install-output-json-error/script index 7857967ba36..e4b34a80ff3 100644 --- a/acceptance/experimental/aitools/skills/install-output-json-error/script +++ b/acceptance/experimental/aitools/skills/install-output-json-error/script @@ -4,6 +4,7 @@ sethome home title "install --output json reports a top-level error category and exits non-zero" # A --skills entry absent from the manifest fails before any agent is touched, so the # failure has no per-agent entry and surfaces in the top-level error/error_category -# fields (agents stays empty). The command exits non-zero, but root prints no -# duplicate "Error:" line to stderr because the JSON already reported the failure. -trace $CLI experimental aitools install --skills-only --scope=global --agents=claude-code --skills=nonexistent --output json +# fields (agents stays empty). In JSON mode progress is silenced and root prints no +# duplicate "Error:" line, so stdout carries only the JSON document; the command +# still exits non-zero. +trace $CLI aitools install --skills-only --scope=global --agents=claude-code --skills=nonexistent --output json From c0840bd58c39fe72985cc97505c66d1d5fc5eede Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Mon, 7 Sep 2026 10:39:23 +0200 Subject: [PATCH 8/9] aitools: add acceptance tests for per-agent error categories Cover the --output json path where a failure is reported per agent rather than at the top level: - install-output-json-agent-error: claude-code installs while cursor is skipped with an UNSUPPORTED_SCOPE category in its own agents[] entry, so the array mixes a success with a per-agent error and the top-level error_category stays unset. - install-output-json-agents-skipped: every named agent (cursor, codex) is skipped for scope, giving multiple per-agent categories with no successful install and no top-level error. Both exit non-zero because the agents were named explicitly. Co-authored-by: Isaac --- .../out.test.toml | 2 + .../output.txt | 22 +++++++++++ .../install-output-json-agent-error/script | 9 +++++ .../install-output-json-agent-error/test.toml | 37 +++++++++++++++++++ .../out.test.toml | 2 + .../output.txt | 24 ++++++++++++ .../install-output-json-agents-skipped/script | 9 +++++ .../test.toml | 10 +++++ 8 files changed, 115 insertions(+) create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agent-error/out.test.toml create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agent-error/output.txt create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agent-error/script create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agent-error/test.toml create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agents-skipped/out.test.toml create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agents-skipped/output.txt create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agents-skipped/script create mode 100644 acceptance/experimental/aitools/skills/install-output-json-agents-skipped/test.toml diff --git a/acceptance/experimental/aitools/skills/install-output-json-agent-error/out.test.toml b/acceptance/experimental/aitools/skills/install-output-json-agent-error/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agent-error/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/experimental/aitools/skills/install-output-json-agent-error/output.txt b/acceptance/experimental/aitools/skills/install-output-json-agent-error/output.txt new file mode 100644 index 00000000000..6860d375fa5 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agent-error/output.txt @@ -0,0 +1,22 @@ + +=== install --output json records a per-agent error category +>>> [CLI] aitools install --skills-only --scope=project --agents=claude-code,cursor --output json +{ + "scope": "project", + "agents": [ + { + "name": "claude-code", + "delivery": "skills", + "status": "installed" + }, + { + "name": "cursor", + "delivery": "skip", + "status": "skipped", + "error_category": "UNSUPPORTED_SCOPE", + "message": "does not support project-scoped skills" + } + ] +} + +Exit code: 1 diff --git a/acceptance/experimental/aitools/skills/install-output-json-agent-error/script b/acceptance/experimental/aitools/skills/install-output-json-agent-error/script new file mode 100644 index 00000000000..c84ede5e39b --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agent-error/script @@ -0,0 +1,9 @@ +# Isolate HOME so parallel aitools tests don't race on a shared ~/.databricks. +sethome home + +title "install --output json records a per-agent error category" +# claude-code supports project-scoped skills; cursor does not. At --scope=project, +# claude-code installs while cursor is skipped with an UNSUPPORTED_SCOPE category in +# its own agents[] entry (the top-level error_category stays unset because the +# failure is per-agent). cursor was named explicitly, so the run exits non-zero. +trace $CLI aitools install --skills-only --scope=project --agents=claude-code,cursor --output json diff --git a/acceptance/experimental/aitools/skills/install-output-json-agent-error/test.toml b/acceptance/experimental/aitools/skills/install-output-json-agent-error/test.toml new file mode 100644 index 00000000000..5b9c683e75e --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agent-error/test.toml @@ -0,0 +1,37 @@ +# 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", + ".claude", + ".databricks", +] + +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/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/out.test.toml b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/output.txt b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/output.txt new file mode 100644 index 00000000000..01c511aaf17 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/output.txt @@ -0,0 +1,24 @@ + +=== install --output json: every named agent skipped, each with its own category +>>> [CLI] aitools install --skills-only --scope=project --agents=cursor,codex --output json +{ + "scope": "project", + "agents": [ + { + "name": "cursor", + "delivery": "skip", + "status": "skipped", + "error_category": "UNSUPPORTED_SCOPE", + "message": "does not support project-scoped skills" + }, + { + "name": "codex", + "delivery": "skip", + "status": "skipped", + "error_category": "UNSUPPORTED_SCOPE", + "message": "does not support project-scoped skills" + } + ] +} + +Exit code: 1 diff --git a/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/script b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/script new file mode 100644 index 00000000000..6e23f81762c --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/script @@ -0,0 +1,9 @@ +# Isolate HOME so parallel aitools tests don't race on a shared ~/.databricks. +sethome home + +title "install --output json: every named agent skipped, each with its own category" +# Neither cursor nor codex supports project-scoped skills, so at --scope=project both +# are skipped with a per-agent UNSUPPORTED_SCOPE category and nothing is installed (no +# manifest fetch). Both were named explicitly, so the run exits non-zero, yet the +# top-level error_category stays unset because every failure is per-agent. +trace $CLI aitools install --skills-only --scope=project --agents=cursor,codex --output json diff --git a/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/test.toml b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/test.toml new file mode 100644 index 00000000000..3cbcdf66fc7 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-agents-skipped/test.toml @@ -0,0 +1,10 @@ +# Every named agent is skipped for scope before any manifest fetch, so no skills +# server is needed and nothing is written outside the isolated home. +Env.DATABRICKS_SKILLS_BASE_URL = "$DATABRICKS_HOST" +Env.DATABRICKS_SKILLS_REF = "test-ref" + +Ignore = [ + "home", +] + +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] From ef859bcb27792b24b1006e8acb4ff32efa81d201 Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Tue, 8 Sep 2026 13:44:27 +0000 Subject: [PATCH 9/9] aitools: categorize experimental-skill install failures Address review feedback on PR #6482: - Add EXPERIMENTAL_SKILL error category so a specific experimental skill requested without --experimental no longer classifies as UNCATEGORIZED; installer returns a *SkillError with ReasonExperimentalSkill. - Key buildInstallOutput's category emission on status != outcomeInstalled to match agentResultsField, so the JSON and telemetry views agree. - Rename skipError to skipErrorCategory (it holds a category, not an error). - Add a telemetry wire-format test pinning the snake_case error_category keys. - Add an acceptance test for the experimental-skill JSON output. Co-authored-by: Isaac --- .../out.test.toml | 2 + .../output.txt | 11 ++++++ .../script | 8 ++++ .../test.toml | 28 ++++++++++++++ cmd/aitools/categorize.go | 2 + cmd/aitools/categorize_test.go | 9 ++++- cmd/aitools/install.go | 25 +++++++------ libs/aitools/installer/errors.go | 2 + libs/aitools/installer/installer.go | 2 +- libs/telemetry/protos/aitools_install.go | 1 + libs/telemetry/protos/aitools_install_test.go | 37 +++++++++++++++++++ 11 files changed, 113 insertions(+), 14 deletions(-) create mode 100644 acceptance/experimental/aitools/skills/install-output-json-experimental-error/out.test.toml create mode 100644 acceptance/experimental/aitools/skills/install-output-json-experimental-error/output.txt create mode 100644 acceptance/experimental/aitools/skills/install-output-json-experimental-error/script create mode 100644 acceptance/experimental/aitools/skills/install-output-json-experimental-error/test.toml create mode 100644 libs/telemetry/protos/aitools_install_test.go diff --git a/acceptance/experimental/aitools/skills/install-output-json-experimental-error/out.test.toml b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/experimental/aitools/skills/install-output-json-experimental-error/output.txt b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/output.txt new file mode 100644 index 00000000000..513d2d5b5b3 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/output.txt @@ -0,0 +1,11 @@ + +=== install --output json categorizes an experimental skill requested without --experimental +>>> [CLI] aitools install --skills-only --scope=global --agents=claude-code --skills=test-exp --output json +{ + "scope": "global", + "agents": [], + "error": "skill \"test-exp\" is experimental; use --experimental to install", + "error_category": "EXPERIMENTAL_SKILL" +} + +Exit code: 1 diff --git a/acceptance/experimental/aitools/skills/install-output-json-experimental-error/script b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/script new file mode 100644 index 00000000000..3b68e727fb4 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/script @@ -0,0 +1,8 @@ +# Isolate HOME so parallel aitools tests don't race on a shared ~/.databricks. +sethome home + +title "install --output json categorizes an experimental skill requested without --experimental" +# Requesting an experimental skill without --experimental fails during skill +# resolution, before any agent is touched, so the failure has no per-agent entry +# and surfaces in the top-level error/error_category fields (EXPERIMENTAL_SKILL). +trace $CLI aitools install --skills-only --scope=global --agents=claude-code --skills=test-exp --output json diff --git a/acceptance/experimental/aitools/skills/install-output-json-experimental-error/test.toml b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/test.toml new file mode 100644 index 00000000000..c7f2d6e3fe1 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-output-json-experimental-error/test.toml @@ -0,0 +1,28 @@ +# 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"] + +# The requested skill is experimental (repo_dir=experimental); the script omits +# --experimental so resolution fails before any skill file is fetched. +[[Server]] +Pattern = "GET /test-ref/manifest.json" +Response.Body = ''' +{ + "version": "2", + "updated_at": "2026-01-01T00:00:00Z", + "skills": { + "test-exp": { + "version": "0.0.1", + "description": "Experimental test skill", + "files": ["SKILL.md"], + "repo_dir": "experimental" + } + } +} +''' diff --git a/cmd/aitools/categorize.go b/cmd/aitools/categorize.go index 33bbf297fb5..b4ed1265f7a 100644 --- a/cmd/aitools/categorize.go +++ b/cmd/aitools/categorize.go @@ -27,6 +27,8 @@ func skillErrorCategory(e *installer.SkillError) protos.AitoolsErrorCategory { return protos.AitoolsErrorCategorySkillNotFound case installer.ReasonVersionIncompatible: return protos.AitoolsErrorCategoryVersionIncompatible + case installer.ReasonExperimentalSkill: + return protos.AitoolsErrorCategoryExperimentalSkill default: return protos.AitoolsErrorCategoryUncategorized } diff --git a/cmd/aitools/categorize_test.go b/cmd/aitools/categorize_test.go index 2b6e337fea6..03e6a94904b 100644 --- a/cmd/aitools/categorize_test.go +++ b/cmd/aitools/categorize_test.go @@ -32,8 +32,8 @@ func TestClassifyInstallError(t *testing.T) { want: protos.AitoolsErrorCategoryPluginInstallFailed, }, { - name: "blocked no plugin is uncategorized", - err: &installer.BlockedError{Agent: "codex", Reason: installer.ReasonNoPlugin}, + name: "blocked error with unknown reason is uncategorized", + err: &installer.BlockedError{Agent: "codex", Reason: "some-future-reason"}, want: protos.AitoolsErrorCategoryUncategorized, }, { @@ -46,6 +46,11 @@ func TestClassifyInstallError(t *testing.T) { err: &installer.SkillError{Skill: "databricks", Reason: installer.ReasonVersionIncompatible, Detail: "requires CLI version 0.5 (running 0.4)"}, want: protos.AitoolsErrorCategoryVersionIncompatible, }, + { + name: "experimental skill", + err: &installer.SkillError{Skill: "test-exp", Reason: installer.ReasonExperimentalSkill, Detail: "is experimental; use --experimental to install"}, + want: protos.AitoolsErrorCategoryExperimentalSkill, + }, { name: "skill error with unknown reason is uncategorized", err: &installer.SkillError{Skill: "databricks", Reason: "some-future-reason"}, diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index ad4255eab82..695e58108c9 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -58,12 +58,12 @@ func (d delivery) String() string { // agentPlanItem is the resolved plan for one agent: what we'll do and why. type agentPlanItem struct { - agent *agents.Agent - delivery delivery - scope string // agent-native plugin scope (deliveryPlugin only) - reason string // why the agent is skipped (deliverySkip only) - skipError protos.AitoolsErrorCategory // error category for the skip (deliverySkip only) - explicit bool // named via --agents (blocking it is an error) + agent *agents.Agent + delivery delivery + scope string // agent-native plugin scope (deliveryPlugin only) + reason string // why the agent is skipped (deliverySkip only) + skipErrorCategory protos.AitoolsErrorCategory // error category for the skip (deliverySkip only) + explicit bool // named via --agents (blocking it is an error) } // agentChoice is one row in the interactive agent picker. @@ -218,7 +218,7 @@ Supported agents: ` + strings.Join(agents.SupportedNames(), ", "), }() 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. @@ -415,7 +415,7 @@ func planItemFor(a *agents.Agent, scope string, skillsOnly, explicit bool) agent if scope == installer.ScopeProject && !a.SupportsProjectScope { item.delivery = deliverySkip item.reason = "does not support project-scoped skills" - item.skipError = protos.AitoolsErrorCategoryUnsupportedScope + item.skipErrorCategory = protos.AitoolsErrorCategoryUnsupportedScope } else { item.delivery = deliverySkills } @@ -424,7 +424,7 @@ func planItemFor(a *agents.Agent, scope string, skillsOnly, explicit bool) agent if !ok { item.delivery = deliverySkip item.reason = reason - item.skipError = protos.AitoolsErrorCategoryUnsupportedScope + item.skipErrorCategory = protos.AitoolsErrorCategoryUnsupportedScope } else { item.delivery = deliveryPlugin item.scope = nativeScope @@ -579,7 +579,7 @@ func executePlan(ctx context.Context, src installer.ManifestSource, plan []agent agent: it.agent, delivery: deliverySkip, status: outcomeSkipped, - errorCategory: it.skipError, + errorCategory: it.skipErrorCategory, message: it.reason, }) if it.explicit { @@ -630,7 +630,10 @@ func buildInstallOutput(scope string, outcomes []agentOutcome, runErr error) ins Status: string(o.status), Message: o.message, } - if o.errorCategory != "" { + // Only non-successful outcomes carry a category, keyed on status to match + // agentResultsField in telemetry.go so the JSON and telemetry views of the + // same slice never disagree. + if o.status != outcomeInstalled { entry.ErrorCategory = string(o.errorCategory) } out.Agents = append(out.Agents, entry) diff --git a/libs/aitools/installer/errors.go b/libs/aitools/installer/errors.go index 774b327df1b..a3c7bc41022 100644 --- a/libs/aitools/installer/errors.go +++ b/libs/aitools/installer/errors.go @@ -19,6 +19,8 @@ const ( ReasonSkillNotFound = "skill-not-found" // ReasonVersionIncompatible: the skill requires a newer CLI than the one running. ReasonVersionIncompatible = "version-incompatible" + // ReasonExperimentalSkill: the named skill is experimental and --experimental was not set. + ReasonExperimentalSkill = "experimental-skill" ) func (e *SkillError) Error() string { diff --git a/libs/aitools/installer/installer.go b/libs/aitools/installer/installer.go index accaa086269..e3f39e09435 100644 --- a/libs/aitools/installer/installer.go +++ b/libs/aitools/installer/installer.go @@ -484,7 +484,7 @@ func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts Instal for name, meta := range candidates { if meta.IsExperimental() && !opts.IncludeExperimental { if isSpecific { - return nil, fmt.Errorf("skill %q is experimental; use --experimental to install", name) + return nil, &SkillError{Skill: name, Reason: ReasonExperimentalSkill, Detail: "is experimental; use --experimental to install"} } log.Debugf(ctx, "Skipping experimental skill %s", name) continue diff --git a/libs/telemetry/protos/aitools_install.go b/libs/telemetry/protos/aitools_install.go index 7bd034b3665..152d599671d 100644 --- a/libs/telemetry/protos/aitools_install.go +++ b/libs/telemetry/protos/aitools_install.go @@ -42,6 +42,7 @@ const ( AitoolsErrorCategoryCLINotOnPath AitoolsErrorCategory = "CLI_NOT_ON_PATH" AitoolsErrorCategoryPluginInstallFailed AitoolsErrorCategory = "PLUGIN_INSTALL_FAILED" AitoolsErrorCategoryUnsupportedScope AitoolsErrorCategory = "UNSUPPORTED_SCOPE" + AitoolsErrorCategoryExperimentalSkill AitoolsErrorCategory = "EXPERIMENTAL_SKILL" AitoolsErrorCategoryUncategorized AitoolsErrorCategory = "UNCATEGORIZED" ) diff --git a/libs/telemetry/protos/aitools_install_test.go b/libs/telemetry/protos/aitools_install_test.go new file mode 100644 index 00000000000..19742ccb4c3 --- /dev/null +++ b/libs/telemetry/protos/aitools_install_test.go @@ -0,0 +1,37 @@ +package protos + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The lumberjack proto keys these fields as snake_case error_category, both on +// the event and on each agent_results entry. A hand-edit to a camelCase tag +// marshals under the wrong key and is dropped silently on ingestion, so this +// pins the wire payload for an event with one per-agent result. +func TestAitoolsInstallEventEncodesErrorCategoryKeys(t *testing.T) { + b, err := json.Marshal(AitoolsInstallEvent{ + Agents: []AitoolsAgentType{AitoolsAgentTypeCodex}, + Scope: AitoolsInstallScopeGlobal, + ErrorCategory: AitoolsErrorCategoryPluginInstallFailed, + AgentResults: []AitoolsAgentResult{ + {Agent: AitoolsAgentTypeCodex, ErrorCategory: AitoolsErrorCategoryPluginInstallFailed}, + }, + }) + require.NoError(t, err) + + var got map[string]any + require.NoError(t, json.Unmarshal(b, &got)) + + assert.Equal(t, "PLUGIN_INSTALL_FAILED", got["error_category"]) + + results, ok := got["agent_results"].([]any) + require.True(t, ok, "agent_results must be present") + require.Len(t, results, 1) + entry := results[0].(map[string]any) + assert.Equal(t, "CODEX", entry["agent"]) + assert.Equal(t, "PLUGIN_INSTALL_FAILED", entry["error_category"]) +}