From 433c8b5551eba6dd8c852a260c9f7c28b5d7a11e Mon Sep 17 00:00:00 2001 From: mike-delbalso_data Date: Wed, 28 Oct 2026 12:16:03 -0700 Subject: [PATCH 1/2] Add authenticated AIR product feedback command --- .nextchanges/air/feedback.md | 1 + acceptance/air/feedback/out.test.toml | 1 + acceptance/air/feedback/output.txt | 28 ++++ acceptance/air/feedback/script | 11 ++ acceptance/air/feedback/test.toml | 6 + acceptance/air/help/output.txt | 37 +++++ acceptance/air/help/script | 3 + cmd/air/air.go | 1 + cmd/air/air_test.go | 2 +- cmd/air/feedback.go | 188 ++++++++++++++++++++++++++ cmd/air/feedback_test.go | 156 +++++++++++++++++++++ 11 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 .nextchanges/air/feedback.md create mode 100644 acceptance/air/feedback/out.test.toml create mode 100644 acceptance/air/feedback/output.txt create mode 100644 acceptance/air/feedback/script create mode 100644 acceptance/air/feedback/test.toml create mode 100644 cmd/air/feedback.go create mode 100644 cmd/air/feedback_test.go diff --git a/.nextchanges/air/feedback.md b/.nextchanges/air/feedback.md new file mode 100644 index 00000000000..e8068e17710 --- /dev/null +++ b/.nextchanges/air/feedback.md @@ -0,0 +1 @@ +* Add `databricks air feedback` to submit feedback with workspace authentication, agent or human attribution, and optional run context. diff --git a/acceptance/air/feedback/out.test.toml b/acceptance/air/feedback/out.test.toml new file mode 100644 index 00000000000..18b1a88417e --- /dev/null +++ b/acceptance/air/feedback/out.test.toml @@ -0,0 +1 @@ +Cloud = false diff --git a/acceptance/air/feedback/output.txt b/acceptance/air/feedback/output.txt new file mode 100644 index 00000000000..ba1276273eb --- /dev/null +++ b/acceptance/air/feedback/output.txt @@ -0,0 +1,28 @@ + +=== agent report with a receipt +>>> [CLI] air feedback --message Logs omitted the failure reason --reporter agent --agent-name codex --run-id 123 -o json +{ + "v": 1, + "ts": "[TIMESTAMP]", + "data": { + "received": true, + "feedback_id": "[UUID]" + } +} + +=== human report +>>> [CLI] air feedback --message The workaround succeeded +Feedback received. Reference: [UUID] + +=== validation errors +>>> musterr [CLI] air feedback --message -o json +true + +>>> musterr [CLI] air feedback --message Logs omitted the failure reason --reporter robot -o json +true + +>>> musterr [CLI] air feedback --message Logs omitted the failure reason --agent-name codex -o json +true + +>>> musterr [CLI] air feedback unexpected --message Logs omitted the failure reason -o json +true diff --git a/acceptance/air/feedback/script b/acceptance/air/feedback/script new file mode 100644 index 00000000000..a02df8138b0 --- /dev/null +++ b/acceptance/air/feedback/script @@ -0,0 +1,11 @@ +title "agent report with a receipt" +trace $CLI air feedback --message "Logs omitted the failure reason" --reporter agent --agent-name codex --run-id 123 -o json + +title "human report" +trace $CLI air feedback --message "The workaround succeeded" + +title "validation errors" +trace musterr $CLI air feedback --message " " -o json | jq -e '.error.code == "INVALID_ARGUMENT"' +trace musterr $CLI air feedback --message "Logs omitted the failure reason" --reporter robot -o json | jq -e '.error.code == "INVALID_ARGUMENT"' +trace musterr $CLI air feedback --message "Logs omitted the failure reason" --agent-name codex -o json | jq -e '.error.code == "INVALID_ARGUMENT"' +trace musterr $CLI air feedback unexpected --message "Logs omitted the failure reason" -o json | jq -e '.error.code == "INVALID_ARGUMENT"' diff --git a/acceptance/air/feedback/test.toml b/acceptance/air/feedback/test.toml new file mode 100644 index 00000000000..a89a9465c4e --- /dev/null +++ b/acceptance/air/feedback/test.toml @@ -0,0 +1,6 @@ +# Explicit feedback still submits when automatic usage telemetry is disabled. +Env.DATABRICKS_CLI_DISABLE_TELEMETRY = "1" + +[[Repls]] +Old = '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}' +New = '[FEEDBACK_ID]' diff --git a/acceptance/air/help/output.txt b/acceptance/air/help/output.txt index 5bb6f40b4f7..26803d96ecc 100644 --- a/acceptance/air/help/output.txt +++ b/acceptance/air/help/output.txt @@ -11,6 +11,7 @@ Usage: Available Commands: cancel Cancel one or more runs convert-to-dabs Convert an AIR run YAML into a Databricks Asset Bundle + feedback Send feedback about AI Runtime to Databricks get Show status, configuration, and timing details for a specified run images Manage container images for AI Runtime list List active runs for the current user @@ -210,3 +211,39 @@ Global Flags: -o, --output type output type: text or json (default text) -p, --profile string ~/.databrickscfg profile -t, --target string bundle target to use (if applicable) + +=== feedback help +>>> [CLI] air feedback --help +Send feedback about AI Runtime using your workspace authentication. + +Include the command, observed problem, expected behavior, and any workaround +in --message. Use --reporter agent for reports written by an agent. + +Sends your message, CLI version, reporter type, and optional run ID and agent +name to Fast Feedback. The service associates it with your workspace and +authenticated principal. No transcripts, source files, or logs are collected. +Do not include credentials or sensitive data. Follow-up contact is opt-in. + +This is an explicit submission, independent of automatic usage telemetry. +Success confirms receipt by the collector; processing into review tools is +asynchronous. + +Usage: + databricks air feedback [flags] + +Examples: + databricks air feedback --message "air logs omitted the failure reason" --reporter agent --agent-name codex --run-id 123 -o json + +Flags: + --agent-name string Agent name; requires --reporter agent + --can-be-contacted Allow follow-up about this feedback + -h, --help help for feedback + --message string Feedback message (1–10,000 characters) + --reporter string Who wrote the feedback: human or agent (default "human") + --run-id string Related job run ID + +Global Flags: + --debug enable debug logging + -o, --output type output type: text or json (default text) + -p, --profile string ~/.databrickscfg profile + -t, --target string bundle target to use (if applicable) diff --git a/acceptance/air/help/script b/acceptance/air/help/script index fd1ae1514ea..8e330d657db 100644 --- a/acceptance/air/help/script +++ b/acceptance/air/help/script @@ -24,3 +24,6 @@ trace $CLI air logs --help title "cancel help" trace $CLI air cancel --help + +title "feedback help" +trace $CLI air feedback --help diff --git a/cmd/air/air.go b/cmd/air/air.go index dd8ff3a0e1d..d22a655cd62 100644 --- a/cmd/air/air.go +++ b/cmd/air/air.go @@ -28,6 +28,7 @@ These commands are under active development.`, cmd.AddCommand(newCancelCommand()) cmd.AddCommand(newConvertToDabsCommand()) cmd.AddCommand(newImagesCommand()) + cmd.AddCommand(newFeedbackCommand()) return cmd } diff --git a/cmd/air/air_test.go b/cmd/air/air_test.go index b7ba790980b..6da518f8a5b 100644 --- a/cmd/air/air_test.go +++ b/cmd/air/air_test.go @@ -16,7 +16,7 @@ func TestNewRegistersAllSubcommands(t *testing.T) { registered[c.Name()] = true } - want := []string{"run", "get", "list", "pools", "logs", "cancel", "convert-to-dabs", "images"} + want := []string{"run", "get", "list", "pools", "logs", "cancel", "convert-to-dabs", "images", "feedback"} for _, name := range want { assert.True(t, registered[name], "subcommand %q is not registered", name) } diff --git a/cmd/air/feedback.go b/cmd/air/feedback.go new file mode 100644 index 00000000000..16ece538a9f --- /dev/null +++ b/cmd/air/feedback.go @@ -0,0 +1,188 @@ +package aircmd + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "strconv" + "strings" + "time" + "unicode/utf8" + "uuid" + + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/cmdctx" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/telemetry" + "github.com/databricks/databricks-sdk-go/client" + "github.com/spf13/cobra" +) + +const ( + feedbackMessageLimit = 10000 + feedbackAgentNameLimit = 100 + feedbackTimeout = 30 * time.Second +) + +type feedbackOptions struct { + message string + reporter string + runID string + agentName string + canBeContacted bool +} + +func (o *feedbackOptions) validate() error { + o.message = strings.TrimSpace(o.message) + if o.message == "" || !utf8.ValidString(o.message) || utf8.RuneCountInString(o.message) > feedbackMessageLimit { + return fmt.Errorf("--message must contain 1 to %d UTF-8 characters", feedbackMessageLimit) + } + if o.reporter != "human" && o.reporter != "agent" { + return errors.New("--reporter must be human or agent") + } + if o.runID != "" { + id, err := strconv.ParseInt(o.runID, 10, 64) + if err != nil || id <= 0 { + return errors.New("--run-id must be a positive job run ID") + } + } + o.agentName = strings.TrimSpace(o.agentName) + if o.agentName != "" && o.reporter != "agent" { + return errors.New("--agent-name requires --reporter agent") + } + if !utf8.ValidString(o.agentName) || utf8.RuneCountInString(o.agentName) > feedbackAgentNameLimit { + return fmt.Errorf("--agent-name must contain at most %d UTF-8 characters", feedbackAgentNameLimit) + } + return nil +} + +// These bindings mirror FastFeedbackLog in universe. Feedback text is sent only +// through that explicitly submitted feedback schema, never a CLI usage event. +type fastFeedbackLog struct { + ReferenceEventID string `json:"reference_event_id"` + ParentUI string `json:"parent_ui"` + Origin string `json:"origin"` + Text string `json:"text"` + CanBeContacted bool `json:"can_be_contacted"` + Product string `json:"product"` + FeedbackSource string `json:"feedback_source"` + ReporterType string `json:"reporter_type"` + CLIVersion string `json:"cli_version"` + RunID string `json:"run_id,omitempty"` + AgentName string `json:"agent_name,omitempty"` +} + +type feedbackEvent struct { + FrontendLogEventID string `json:"frontend_log_event_id"` + Entry struct { + FastFeedbackLog fastFeedbackLog `json:"fast_feedback_log"` + } `json:"entry"` +} + +type feedbackReceipt struct { + Received bool `json:"received"` + FeedbackID string `json:"feedback_id"` +} + +func newFeedbackCommand() *cobra.Command { + var o feedbackOptions + cmd := &cobra.Command{ + Use: "feedback", + Short: "Send feedback about AI Runtime to Databricks", + Long: `Send feedback about AI Runtime using your workspace authentication. + +Include the command, observed problem, expected behavior, and any workaround +in --message. Use --reporter agent for reports written by an agent. + +Sends your message, CLI version, reporter type, and optional run ID and agent +name to Fast Feedback. The service associates it with your workspace and +authenticated principal. No transcripts, source files, or logs are collected. +Do not include credentials or sensitive data. Follow-up contact is opt-in. + +This is an explicit submission, independent of automatic usage telemetry. +Success confirms receipt by the collector; processing into review tools is +asynchronous.`, + Example: ` databricks air feedback --message "air logs omitted the failure reason" --reporter agent --agent-name codex --run-id 123 -o json`, + } + cmd.Flags().StringVar(&o.message, "message", "", "Feedback message (1–10,000 characters)") + cmd.Flags().StringVar(&o.reporter, "reporter", "human", "Who wrote the feedback: human or agent") + cmd.Flags().StringVar(&o.runID, "run-id", "", "Related job run ID") + cmd.Flags().StringVar(&o.agentName, "agent-name", "", "Agent name; requires --reporter agent") + cmd.Flags().BoolVar(&o.canBeContacted, "can-be-contacted", false, "Allow follow-up about this feedback") + // Cobra runs Args before root initializes command IO. Validate here so JSON + // errors can be rendered, while still rejecting input before authentication. + cmd.PreRunE = func(cmd *cobra.Command, args []string) error { + if err := cobra.NoArgs(cmd, args); err != nil { + return renderError(cmd.Context(), cmd, "INVALID_ARGUMENT", "VALIDATION", false, err) + } + if err := o.validate(); err != nil { + return renderError(cmd.Context(), cmd, "INVALID_ARGUMENT", "VALIDATION", false, err) + } + err := root.MustWorkspaceClient(cmd, args) + if err == nil || errors.Is(err, root.ErrAlreadyPrinted) { + return err + } + return renderError(cmd.Context(), cmd, "AUTHENTICATION_ERROR", "AUTHENTICATION", false, err) + } + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + apiClient, err := client.New(cmdctx.WorkspaceClient(ctx).Config) + if err != nil { + return renderError(ctx, cmd, "AUTHENTICATION_ERROR", "AUTHENTICATION", false, err) + } + id := uuid.New().String() + if err := submitFeedback(ctx, apiClient, o, id); err != nil { + // A timeout can occur after acceptance. Do not invite automated retries + // that would create duplicate reports under a new submission ID. + return renderError(ctx, cmd, "FEEDBACK_SUBMISSION_FAILED", "SUBMISSION", false, + fmt.Errorf("feedback %s could not be confirmed: %w", id, err)) + } + if root.OutputType(cmd) == flags.OutputJSON { + return renderEnvelope(ctx, feedbackReceipt{Received: true, FeedbackID: id}) + } + cmdio.LogString(ctx, "Feedback received. Reference: "+id) + return nil + } + return cmd +} + +func submitFeedback(ctx context.Context, apiClient *client.DatabricksClient, o feedbackOptions, id string) error { + ctx, cancel := context.WithTimeout(ctx, feedbackTimeout) + defer cancel() + event := feedbackEvent{FrontendLogEventID: id} + event.Entry.FastFeedbackLog = fastFeedbackLog{ + ReferenceEventID: id, + ParentUI: "ModelTraining.AIRuntime", + Origin: "air-cli", + Text: o.message, + CanBeContacted: o.canBeContacted, + Product: "PRODUCT_AIR", + FeedbackSource: "SOURCE_AIR_CLI", + ReporterType: "REPORTER_" + strings.ToUpper(o.reporter), + CLIVersion: build.GetInfo().Version, + RunID: o.runID, + AgentName: o.agentName, + } + data, err := json.Marshal(event) + if err != nil { + return fmt.Errorf("encode feedback: %w", err) + } + var response telemetry.ResponseBody + err = apiClient.Do(ctx, http.MethodPost, "/telemetry-ext", auth.WorkspaceIDHeaders(apiClient.Config), nil, telemetry.RequestBody{ + UploadTime: time.Now().UnixMilli(), + Items: []string{}, + ProtoLogs: []string{string(data)}, + }, &response) + if err != nil { + return fmt.Errorf("submit feedback: %w", err) + } + if len(response.Errors) != 0 || response.NumProtoSuccess != 1 { + return errors.New("collector did not acknowledge the feedback") + } + return nil +} diff --git a/cmd/air/feedback_test.go b/cmd/air/feedback_test.go new file mode 100644 index 00000000000..7285118895f --- /dev/null +++ b/cmd/air/feedback_test.go @@ -0,0 +1,156 @@ +package aircmd_test + +import ( + "bytes" + "context" + "encoding/json" + "strings" + "testing" + + aircmd "github.com/databricks/cli/cmd/air" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/telemetry" + "github.com/databricks/cli/libs/testserver" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func feedbackCommand(t *testing.T, args ...string) (*cobra.Command, *bytes.Buffer) { + t.Helper() + cmd, _, err := aircmd.New().Find([]string{"feedback"}) + require.NoError(t, err) + output := flags.OutputJSON + cmd.Flags().Var(&output, "output", "") + require.NoError(t, cmd.ParseFlags(args)) + buf := new(bytes.Buffer) + ctx := cmdio.InContext(t.Context(), cmdio.NewIO(t.Context(), output, nil, buf, buf, "", "")) + cmd.SetContext(ctx) + return cmd, buf +} + +func TestFeedbackValidation(t *testing.T) { + server := testserver.New(t) + t.Cleanup(server.Close) + t.Setenv("DATABRICKS_HOST", server.URL) + t.Setenv("DATABRICKS_TOKEN", "test-token") + for _, tc := range []struct { + name string + args []string + valid bool + }{ + {"missing", nil, false}, + {"blank", []string{"--message", " \n\t"}, false}, + {"too long", []string{"--message", strings.Repeat("a", 10001)}, false}, + {"unicode limit", []string{"--message", strings.Repeat("界", 10000)}, true}, + {"invalid reporter", []string{"--message", "broken", "--reporter", "robot"}, false}, + {"agent on human", []string{"--message", "broken", "--agent-name", "codex"}, false}, + {"invalid run", []string{"--message", "broken", "--run-id", "0"}, false}, + {"overflow run", []string{"--message", "broken", "--run-id", "9223372036854775808"}, false}, + {"agent name limit", []string{"--message", "broken", "--reporter", "agent", "--agent-name", strings.Repeat("a", 101)}, false}, + {"human", []string{"--message", "broken"}, true}, + } { + t.Run(tc.name, func(t *testing.T) { + cmd, buf := feedbackCommand(t, tc.args...) + err := cmd.PreRunE(cmd, nil) + if tc.valid { + require.NoError(t, err) + return + } + require.ErrorIs(t, err, root.ErrAlreadyPrinted) + var got struct { + Error struct { + Code string + Retryable bool + } + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) + assert.Equal(t, "INVALID_ARGUMENT", got.Error.Code) + assert.False(t, got.Error.Retryable) + }) + } +} + +func TestFeedbackSubmission(t *testing.T) { + for _, tc := range []struct { + name string + response telemetry.ResponseBody + accepted bool + }{ + {"accepted", telemetry.ResponseBody{NumProtoSuccess: 1}, true}, + {"unacknowledged", telemetry.ResponseBody{}, false}, + {"rejected", telemetry.ResponseBody{Errors: []telemetry.LogError{{ErrorType: "INVALID_LOG"}}}, false}, + {"inconsistent", telemetry.ResponseBody{NumProtoSuccess: 1, Errors: []telemetry.LogError{{ErrorType: "INVALID_LOG"}}}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + server := testserver.New(t) + t.Cleanup(server.Close) + t.Setenv("DATABRICKS_HOST", server.URL) + t.Setenv("DATABRICKS_TOKEN", "test-token") + t.Setenv("DATABRICKS_WORKSPACE_ID", "123") + var event struct { + ID string `json:"frontend_log_event_id"` + Entry struct { + Feedback map[string]any `json:"fast_feedback_log"` + } `json:"entry"` + } + server.Handle("POST", "/telemetry-ext", func(req testserver.Request) any { + assert.Equal(t, "Bearer test-token", req.Headers.Get("Authorization")) + assert.Equal(t, "123", req.Headers.Get("X-Databricks-Workspace-Id")) + var body telemetry.RequestBody + require.NoError(t, json.Unmarshal(req.Body, &body)) + require.NotNil(t, body.Items) + require.Len(t, body.ProtoLogs, 1) + require.NoError(t, json.Unmarshal([]byte(body.ProtoLogs[0]), &event)) + return tc.response + }) + cmd, buf := feedbackCommand(t, "--message", " logs omitted the failure reason ", "--reporter", "agent", "--agent-name", "codex", "--run-id", "9007199254740993") + require.NoError(t, cmd.PreRunE(cmd, nil)) + err := cmd.RunE(cmd, nil) + assert.NotEmpty(t, event.ID) + assert.Equal(t, event.ID, event.Entry.Feedback["reference_event_id"]) + assert.Equal(t, "logs omitted the failure reason", event.Entry.Feedback["text"]) + assert.Equal(t, "PRODUCT_AIR", event.Entry.Feedback["product"]) + assert.Equal(t, "SOURCE_AIR_CLI", event.Entry.Feedback["feedback_source"]) + assert.Equal(t, "REPORTER_AGENT", event.Entry.Feedback["reporter_type"]) + assert.Equal(t, "codex", event.Entry.Feedback["agent_name"]) + assert.Equal(t, "9007199254740993", event.Entry.Feedback["run_id"]) + assert.Equal(t, false, event.Entry.Feedback["can_be_contacted"]) + assert.Contains(t, event.Entry.Feedback, "cli_version") + if !tc.accepted { + require.ErrorIs(t, err, root.ErrAlreadyPrinted) + assert.Contains(t, buf.String(), "FEEDBACK_SUBMISSION_FAILED") + assert.NotContains(t, buf.String(), `"received": true`) + return + } + require.NoError(t, err) + var receipt struct { + Data struct { + Received bool + ID string `json:"feedback_id"` + } + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &receipt)) + assert.True(t, receipt.Data.Received) + assert.Equal(t, event.ID, receipt.Data.ID) + }) + } +} + +func TestFeedbackCancelledSubmissionDoesNotReportSuccess(t *testing.T) { + server := testserver.New(t) + t.Cleanup(server.Close) + t.Setenv("DATABRICKS_HOST", server.URL) + t.Setenv("DATABRICKS_TOKEN", "test-token") + cmd, buf := feedbackCommand(t, "--message", "logs omitted the failure") + require.NoError(t, cmd.PreRunE(cmd, nil)) + ctx, cancel := context.WithCancel(cmd.Context()) + cancel() + cmd.SetContext(ctx) + err := cmd.RunE(cmd, nil) + require.ErrorIs(t, err, root.ErrAlreadyPrinted) + assert.Contains(t, buf.String(), "FEEDBACK_SUBMISSION_FAILED") + assert.NotContains(t, buf.String(), `"received": true`) +} From 6af851af86dfb2874c1bd0930ab00d075cec4972 Mon Sep 17 00:00:00 2001 From: mike-delbalso_data Date: Thu, 29 Oct 2026 09:44:24 -0700 Subject: [PATCH 2/2] Link AIR feedback changelog to PR 7254 --- .nextchanges/air/feedback.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/air/feedback.md b/.nextchanges/air/feedback.md index e8068e17710..5c7c5b9d4b1 100644 --- a/.nextchanges/air/feedback.md +++ b/.nextchanges/air/feedback.md @@ -1 +1 @@ -* Add `databricks air feedback` to submit feedback with workspace authentication, agent or human attribution, and optional run context. +* Add `databricks air feedback` to submit feedback with workspace authentication, agent or human attribution, and optional run context. ([#7254](https://github.com/databricks/cli/pull/7254))