diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 9ed526da6c..258815011e 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -23572,6 +23572,9 @@ const docTemplate = `{ "code_diff_display_mode": { "$ref": "#/definitions/codersdk.AgentDisplayMode" }, + "shell_tool_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, @@ -24060,6 +24063,9 @@ const docTemplate = `{ "code_diff_display_mode": { "$ref": "#/definitions/codersdk.AgentDisplayMode" }, + "shell_tool_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 15d1131f7f..6baef26867 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -21694,6 +21694,9 @@ "code_diff_display_mode": { "$ref": "#/definitions/codersdk.AgentDisplayMode" }, + "shell_tool_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, @@ -22153,6 +22156,9 @@ "code_diff_display_mode": { "$ref": "#/definitions/codersdk.AgentDisplayMode" }, + "shell_tool_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index ab6bd9d05a..bc6f5842c3 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -4546,6 +4546,17 @@ func (q *querier) GetUserSecretsTelemetrySummary(ctx context.Context) (database. return q.db.GetUserSecretsTelemetrySummary(ctx) } +func (q *querier) GetUserShellToolDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + user, err := q.db.GetUserByID(ctx, userID) + if err != nil { + return "", err + } + if err := q.authorizeContext(ctx, policy.ActionReadPersonal, user); err != nil { + return "", err + } + return q.db.GetUserShellToolDisplayMode(ctx, userID) +} + func (q *querier) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return nil, err @@ -7213,6 +7224,17 @@ func (q *querier) UpdateUserSecretByUserIDAndName(ctx context.Context, arg datab return q.db.UpdateUserSecretByUserIDAndName(ctx, arg) } +func (q *querier) UpdateUserShellToolDisplayMode(ctx context.Context, arg database.UpdateUserShellToolDisplayModeParams) (string, error) { + user, err := q.db.GetUserByID(ctx, arg.UserID) + if err != nil { + return "", err + } + if err := q.authorizeContext(ctx, policy.ActionUpdatePersonal, user); err != nil { + return "", err + } + return q.db.UpdateUserShellToolDisplayMode(ctx, arg) +} + func (q *querier) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { fetch := func(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { return q.db.GetUserByID(ctx, arg.ID) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 0f25f59c34..1b57628db8 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2862,6 +2862,19 @@ func (s *MethodTestSuite) TestUser() { dbm.EXPECT().UpdateUserThinkingDisplayMode(gomock.Any(), arg).Return("always_expanded", nil).AnyTimes() check.Args(arg).Asserts(u, policy.ActionUpdatePersonal).Returns("always_expanded") })) + s.Run("GetUserShellToolDisplayMode", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + u := testutil.Fake(s.T(), faker, database.User{}) + dbm.EXPECT().GetUserByID(gomock.Any(), u.ID).Return(u, nil).AnyTimes() + dbm.EXPECT().GetUserShellToolDisplayMode(gomock.Any(), u.ID).Return("auto", nil).AnyTimes() + check.Args(u.ID).Asserts(u, policy.ActionReadPersonal).Returns("auto") + })) + s.Run("UpdateUserShellToolDisplayMode", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + u := testutil.Fake(s.T(), faker, database.User{}) + arg := database.UpdateUserShellToolDisplayModeParams{UserID: u.ID, ShellToolDisplayMode: "always_collapsed"} + dbm.EXPECT().GetUserByID(gomock.Any(), u.ID).Return(u, nil).AnyTimes() + dbm.EXPECT().UpdateUserShellToolDisplayMode(gomock.Any(), arg).Return("always_collapsed", nil).AnyTimes() + check.Args(arg).Asserts(u, policy.ActionUpdatePersonal).Returns("always_collapsed") + })) s.Run("GetUserCodeDiffDisplayMode", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { u := testutil.Fake(s.T(), faker, database.User{}) dbm.EXPECT().GetUserByID(gomock.Any(), u.ID).Return(u, nil).AnyTimes() diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 9cd1bea9f9..9bd20957f4 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2969,6 +2969,14 @@ func (m queryMetricsStore) GetUserSecretsTelemetrySummary(ctx context.Context) ( return r0, r1 } +func (m queryMetricsStore) GetUserShellToolDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + start := time.Now() + r0, r1 := m.s.GetUserShellToolDisplayMode(ctx, userID) + m.queryLatencies.WithLabelValues("GetUserShellToolDisplayMode").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetUserShellToolDisplayMode").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { start := time.Now() r0, r1 := m.s.GetUserStatusCounts(ctx, arg) @@ -5153,6 +5161,14 @@ func (m queryMetricsStore) UpdateUserSecretByUserIDAndName(ctx context.Context, return r0, r1 } +func (m queryMetricsStore) UpdateUserShellToolDisplayMode(ctx context.Context, arg database.UpdateUserShellToolDisplayModeParams) (string, error) { + start := time.Now() + r0, r1 := m.s.UpdateUserShellToolDisplayMode(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateUserShellToolDisplayMode").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateUserShellToolDisplayMode").Inc() + return r0, r1 +} + func (m queryMetricsStore) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { start := time.Now() r0, r1 := m.s.UpdateUserStatus(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 8832a8e8cb..80fc1741ab 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -5553,6 +5553,21 @@ func (mr *MockStoreMockRecorder) GetUserSecretsTelemetrySummary(ctx any) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecretsTelemetrySummary", reflect.TypeOf((*MockStore)(nil).GetUserSecretsTelemetrySummary), ctx) } +// GetUserShellToolDisplayMode mocks base method. +func (m *MockStore) GetUserShellToolDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserShellToolDisplayMode", ctx, userID) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserShellToolDisplayMode indicates an expected call of GetUserShellToolDisplayMode. +func (mr *MockStoreMockRecorder) GetUserShellToolDisplayMode(ctx, userID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserShellToolDisplayMode", reflect.TypeOf((*MockStore)(nil).GetUserShellToolDisplayMode), ctx, userID) +} + // GetUserStatusCounts mocks base method. func (m *MockStore) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { m.ctrl.T.Helper() @@ -9708,6 +9723,21 @@ func (mr *MockStoreMockRecorder) UpdateUserSecretByUserIDAndName(ctx, arg any) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserSecretByUserIDAndName", reflect.TypeOf((*MockStore)(nil).UpdateUserSecretByUserIDAndName), ctx, arg) } +// UpdateUserShellToolDisplayMode mocks base method. +func (m *MockStore) UpdateUserShellToolDisplayMode(ctx context.Context, arg database.UpdateUserShellToolDisplayModeParams) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateUserShellToolDisplayMode", ctx, arg) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateUserShellToolDisplayMode indicates an expected call of UpdateUserShellToolDisplayMode. +func (mr *MockStoreMockRecorder) UpdateUserShellToolDisplayMode(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserShellToolDisplayMode", reflect.TypeOf((*MockStore)(nil).UpdateUserShellToolDisplayMode), ctx, arg) +} + // UpdateUserStatus mocks base method. func (m *MockStore) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 05401a7db2..6bb711b1a5 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -769,6 +769,7 @@ type sqlcQuerier interface { // percentile_disc returns an actual integer count from the underlying // values rather than interpolating between rows. GetUserSecretsTelemetrySummary(ctx context.Context) (GetUserSecretsTelemetrySummaryRow, error) + GetUserShellToolDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) // GetUserStatusCounts returns the count of users in each status over time. // The time range is inclusively defined by the start_time and end_time parameters. GetUserStatusCounts(ctx context.Context, arg GetUserStatusCountsParams) ([]GetUserStatusCountsRow, error) @@ -1223,6 +1224,7 @@ type sqlcQuerier interface { UpdateUserQuietHoursSchedule(ctx context.Context, arg UpdateUserQuietHoursScheduleParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) UpdateUserSecretByUserIDAndName(ctx context.Context, arg UpdateUserSecretByUserIDAndNameParams) (UserSecret, error) + UpdateUserShellToolDisplayMode(ctx context.Context, arg UpdateUserShellToolDisplayModeParams) (string, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) UpdateUserTaskNotificationAlertDismissed(ctx context.Context, arg UpdateUserTaskNotificationAlertDismissedParams) (bool, error) UpdateUserTerminalFont(ctx context.Context, arg UpdateUserTerminalFontParams) (UserConfig, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 92158bc1e9..828db17cbe 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -25953,6 +25953,23 @@ func (q *sqlQuerier) GetUserCount(ctx context.Context, includeSystem bool) (int6 return count, err } +const getUserShellToolDisplayMode = `-- name: GetUserShellToolDisplayMode :one +SELECT + value AS shell_tool_display_mode +FROM + user_configs +WHERE + user_id = $1 + AND key = 'preference_shell_tool_display_mode' +` + +func (q *sqlQuerier) GetUserShellToolDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + row := q.db.QueryRowContext(ctx, getUserShellToolDisplayMode, userID) + var shell_tool_display_mode string + err := row.Scan(&shell_tool_display_mode) + return shell_tool_display_mode, err +} + const getUserTaskNotificationAlertDismissed = `-- name: GetUserTaskNotificationAlertDismissed :one SELECT value::boolean as task_notification_alert_dismissed @@ -26869,6 +26886,33 @@ func (q *sqlQuerier) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesPar return i, err } +const updateUserShellToolDisplayMode = `-- name: UpdateUserShellToolDisplayMode :one +INSERT INTO + user_configs (user_id, key, value) +VALUES + ($1, 'preference_shell_tool_display_mode', $2::text) +ON CONFLICT + ON CONSTRAINT user_configs_pkey +DO UPDATE +SET + value = $2 +WHERE user_configs.user_id = $1 + AND user_configs.key = 'preference_shell_tool_display_mode' +RETURNING value AS shell_tool_display_mode +` + +type UpdateUserShellToolDisplayModeParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + ShellToolDisplayMode string `db:"shell_tool_display_mode" json:"shell_tool_display_mode"` +} + +func (q *sqlQuerier) UpdateUserShellToolDisplayMode(ctx context.Context, arg UpdateUserShellToolDisplayModeParams) (string, error) { + row := q.db.QueryRowContext(ctx, updateUserShellToolDisplayMode, arg.UserID, arg.ShellToolDisplayMode) + var shell_tool_display_mode string + err := row.Scan(&shell_tool_display_mode) + return shell_tool_display_mode, err +} + const updateUserStatus = `-- name: UpdateUserStatus :one UPDATE users diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 2e93da49ba..03f403e145 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -347,6 +347,29 @@ WHERE user_configs.user_id = @user_id AND user_configs.key = 'preference_thinking_display_mode' RETURNING value AS thinking_display_mode; +-- name: GetUserShellToolDisplayMode :one +SELECT + value AS shell_tool_display_mode +FROM + user_configs +WHERE + user_id = @user_id + AND key = 'preference_shell_tool_display_mode'; + +-- name: UpdateUserShellToolDisplayMode :one +INSERT INTO + user_configs (user_id, key, value) +VALUES + (@user_id, 'preference_shell_tool_display_mode', @shell_tool_display_mode::text) +ON CONFLICT + ON CONSTRAINT user_configs_pkey +DO UPDATE +SET + value = @shell_tool_display_mode +WHERE user_configs.user_id = @user_id + AND user_configs.key = 'preference_shell_tool_display_mode' +RETURNING value AS shell_tool_display_mode; + -- name: GetUserCodeDiffDisplayMode :one SELECT value AS code_diff_display_mode diff --git a/coderd/users.go b/coderd/users.go index 839b6c6ab6..4245e6766c 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1306,6 +1306,15 @@ func (api *API) userPreferenceSettings(rw http.ResponseWriter, r *http.Request) return } + shellToolMode, err := api.Database.GetUserShellToolDisplayMode(ctx, user.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Error reading user preference settings.", + Detail: err.Error(), + }) + return + } + codeDiffMode, err := api.Database.GetUserCodeDiffDisplayMode(ctx, user.ID) if err != nil && !errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -1327,6 +1336,7 @@ func (api *API) userPreferenceSettings(rw http.ResponseWriter, r *http.Request) httpapi.Write(ctx, rw, http.StatusOK, codersdk.UserPreferenceSettings{ TaskNotificationAlertDismissed: taskAlertDismissed, ThinkingDisplayMode: sanitizeThinkingDisplayMode(thinkingMode), + ShellToolDisplayMode: sanitizeAgentDisplayMode(shellToolMode), CodeDiffDisplayMode: sanitizeAgentDisplayMode(codeDiffMode), AgentChatSendShortcut: sanitizeAgentChatSendShortcut(agentChatSendShortcut), }) @@ -1363,6 +1373,16 @@ func (api *API) putUserPreferenceSettings(rw http.ResponseWriter, r *http.Reques }) return } + if params.ShellToolDisplayMode != "" && + !slices.Contains(codersdk.ValidAgentDisplayModes, params.ShellToolDisplayMode) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid shell tool display mode.", + Validations: []codersdk.ValidationError{ + {Field: "shell_tool_display_mode", Detail: agentDisplayModeValidationDetail}, + }, + }) + return + } if params.CodeDiffDisplayMode != "" && !slices.Contains(codersdk.ValidAgentDisplayModes, params.CodeDiffDisplayMode) { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -1418,6 +1438,23 @@ func (api *API) putUserPreferenceSettings(rw http.ResponseWriter, r *http.Reques settings.ThinkingDisplayMode = sanitizeThinkingDisplayMode(stored) } + if params.ShellToolDisplayMode != "" { + updated, err := tx.UpdateUserShellToolDisplayMode(ctx, database.UpdateUserShellToolDisplayModeParams{ + UserID: user.ID, + ShellToolDisplayMode: string(params.ShellToolDisplayMode), + }) + if err != nil { + return newUserPreferenceSettingsAPIError("Internal error updating shell tool display mode.", err) + } + settings.ShellToolDisplayMode = sanitizeAgentDisplayMode(updated) + } else { + stored, err := tx.GetUserShellToolDisplayMode(ctx, user.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return newUserPreferenceSettingsAPIError("Error reading shell tool display mode.", err) + } + settings.ShellToolDisplayMode = sanitizeAgentDisplayMode(stored) + } + if params.CodeDiffDisplayMode != "" { updated, err := tx.UpdateUserCodeDiffDisplayMode(ctx, database.UpdateUserCodeDiffDisplayModeParams{ UserID: user.ID, diff --git a/coderd/users_test.go b/coderd/users_test.go index 93f3438066..87ac9b6db5 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -2433,9 +2433,34 @@ func TestAgentDisplayModePreferences(t *testing.T) { settings, err := client.GetUserPreferenceSettings(ctx, codersdk.Me) require.NoError(t, err) + require.Equal(t, codersdk.AgentDisplayModeAuto, settings.ShellToolDisplayMode) require.Equal(t, codersdk.AgentDisplayModeAuto, settings.CodeDiffDisplayMode) }) + t.Run("round-trips shell tool display mode", func(t *testing.T) { + t.Parallel() + + client, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + for _, mode := range []codersdk.AgentDisplayMode{ + codersdk.AgentDisplayModeAlwaysExpanded, + codersdk.AgentDisplayModeAlwaysCollapsed, + } { + updated, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ + ShellToolDisplayMode: mode, + }) + require.NoError(t, err) + require.Equal(t, mode, updated.ShellToolDisplayMode) + + settings, err := client.GetUserPreferenceSettings(ctx, codersdk.Me) + require.NoError(t, err) + require.Equal(t, mode, settings.ShellToolDisplayMode) + } + }) + t.Run("round-trips code diff display mode", func(t *testing.T) { t.Parallel() @@ -2469,24 +2494,57 @@ func TestAgentDisplayModePreferences(t *testing.T) { defer cancel() _, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ - ThinkingDisplayMode: codersdk.ThinkingDisplayModePreview, - CodeDiffDisplayMode: codersdk.AgentDisplayModeAlwaysExpanded, + ThinkingDisplayMode: codersdk.ThinkingDisplayModePreview, + ShellToolDisplayMode: codersdk.AgentDisplayModeAlwaysCollapsed, + CodeDiffDisplayMode: codersdk.AgentDisplayModeAlwaysExpanded, }) require.NoError(t, err) updated, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ - ThinkingDisplayMode: codersdk.ThinkingDisplayModeAlwaysExpanded, + ShellToolDisplayMode: codersdk.AgentDisplayModeAlwaysExpanded, }) require.NoError(t, err) - require.Equal(t, codersdk.ThinkingDisplayModeAlwaysExpanded, updated.ThinkingDisplayMode) + require.Equal(t, codersdk.ThinkingDisplayModePreview, updated.ThinkingDisplayMode) + require.Equal(t, codersdk.AgentDisplayModeAlwaysExpanded, updated.ShellToolDisplayMode) require.Equal(t, codersdk.AgentDisplayModeAlwaysExpanded, updated.CodeDiffDisplayMode) settings, err := client.GetUserPreferenceSettings(ctx, codersdk.Me) require.NoError(t, err) - require.Equal(t, codersdk.ThinkingDisplayModeAlwaysExpanded, settings.ThinkingDisplayMode) + require.Equal(t, codersdk.ThinkingDisplayModePreview, settings.ThinkingDisplayMode) + require.Equal(t, codersdk.AgentDisplayModeAlwaysExpanded, settings.ShellToolDisplayMode) require.Equal(t, codersdk.AgentDisplayModeAlwaysExpanded, settings.CodeDiffDisplayMode) }) + t.Run("rejects invalid shell tool display mode", func(t *testing.T) { + t.Parallel() + + client, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + for _, tt := range []struct { + name string + mode codersdk.AgentDisplayMode + }{ + { + name: "bogus", + mode: codersdk.AgentDisplayMode("bogus"), + }, + { + name: "thinking preview", + mode: codersdk.AgentDisplayMode(codersdk.ThinkingDisplayModePreview), + }, + } { + t.Run(tt.name, func(t *testing.T) { + _, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ + ShellToolDisplayMode: tt.mode, + }) + requireValidationField(t, err, "shell_tool_display_mode") + }) + } + }) + t.Run("rejects invalid code diff display mode", func(t *testing.T) { t.Parallel() diff --git a/codersdk/users.go b/codersdk/users.go index 0bb9f3f046..341b56cb5b 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -305,6 +305,7 @@ type UpdateUserAppearanceSettingsRequest struct { type UserPreferenceSettings struct { TaskNotificationAlertDismissed bool `json:"task_notification_alert_dismissed"` ThinkingDisplayMode ThinkingDisplayMode `json:"thinking_display_mode"` + ShellToolDisplayMode AgentDisplayMode `json:"shell_tool_display_mode"` CodeDiffDisplayMode AgentDisplayMode `json:"code_diff_display_mode"` AgentChatSendShortcut AgentChatSendShortcut `json:"agent_chat_send_shortcut"` } @@ -312,6 +313,7 @@ type UserPreferenceSettings struct { type UpdateUserPreferenceSettingsRequest struct { TaskNotificationAlertDismissed *bool `json:"task_notification_alert_dismissed,omitempty"` ThinkingDisplayMode ThinkingDisplayMode `json:"thinking_display_mode,omitempty"` + ShellToolDisplayMode AgentDisplayMode `json:"shell_tool_display_mode,omitempty"` CodeDiffDisplayMode AgentDisplayMode `json:"code_diff_display_mode,omitempty"` AgentChatSendShortcut AgentChatSendShortcut `json:"agent_chat_send_shortcut,omitempty"` } diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 8c21b01739..35100d38d4 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -12960,6 +12960,7 @@ Restarts will only happen on weekdays in this list on weeks which line up with W { "agent_chat_send_shortcut": "enter", "code_diff_display_mode": "auto", + "shell_tool_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -12971,6 +12972,7 @@ Restarts will only happen on weekdays in this list on weeks which line up with W |-------------------------------------|------------------------------------------------------------------|----------|--------------|-------------| | `agent_chat_send_shortcut` | [codersdk.AgentChatSendShortcut](#codersdkagentchatsendshortcut) | false | | | | `code_diff_display_mode` | [codersdk.AgentDisplayMode](#codersdkagentdisplaymode) | false | | | +| `shell_tool_display_mode` | [codersdk.AgentDisplayMode](#codersdkagentdisplaymode) | false | | | | `task_notification_alert_dismissed` | boolean | false | | | | `thinking_display_mode` | [codersdk.ThinkingDisplayMode](#codersdkthinkingdisplaymode) | false | | | @@ -13556,6 +13558,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| { "agent_chat_send_shortcut": "enter", "code_diff_display_mode": "auto", + "shell_tool_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -13567,6 +13570,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| |-------------------------------------|------------------------------------------------------------------|----------|--------------|-------------| | `agent_chat_send_shortcut` | [codersdk.AgentChatSendShortcut](#codersdkagentchatsendshortcut) | false | | | | `code_diff_display_mode` | [codersdk.AgentDisplayMode](#codersdkagentdisplaymode) | false | | | +| `shell_tool_display_mode` | [codersdk.AgentDisplayMode](#codersdkagentdisplaymode) | false | | | | `task_notification_alert_dismissed` | boolean | false | | | | `thinking_display_mode` | [codersdk.ThinkingDisplayMode](#codersdkthinkingdisplaymode) | false | | | diff --git a/docs/reference/api/users.md b/docs/reference/api/users.md index 6a8d06b40e..ca1ea011d5 100644 --- a/docs/reference/api/users.md +++ b/docs/reference/api/users.md @@ -1312,6 +1312,7 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/preferences \ { "agent_chat_send_shortcut": "enter", "code_diff_display_mode": "auto", + "shell_tool_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -1345,6 +1346,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/preferences \ { "agent_chat_send_shortcut": "enter", "code_diff_display_mode": "auto", + "shell_tool_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -1365,6 +1367,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/preferences \ { "agent_chat_send_shortcut": "enter", "code_diff_display_mode": "auto", + "shell_tool_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index e8892aa546..3ea0fa8220 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -8507,6 +8507,7 @@ export interface UpdateUserPasswordRequest { export interface UpdateUserPreferenceSettingsRequest { readonly task_notification_alert_dismissed?: boolean; readonly thinking_display_mode?: ThinkingDisplayMode; + readonly shell_tool_display_mode?: AgentDisplayMode; readonly code_diff_display_mode?: AgentDisplayMode; readonly agent_chat_send_shortcut?: AgentChatSendShortcut; } @@ -8903,6 +8904,7 @@ export interface UserParameter { export interface UserPreferenceSettings { readonly task_notification_alert_dismissed: boolean; readonly thinking_display_mode: ThinkingDisplayMode; + readonly shell_tool_display_mode: AgentDisplayMode; readonly code_diff_display_mode: AgentDisplayMode; readonly agent_chat_send_shortcut: AgentChatSendShortcut; } diff --git a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx index 93236ee27f..311df2eb8c 100644 --- a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx @@ -10,6 +10,7 @@ import { const preferencesData = { task_notification_alert_dismissed: false, thinking_display_mode: "auto" as const, + shell_tool_display_mode: "auto" as const, code_diff_display_mode: "auto" as const, agent_chat_send_shortcut: "enter" as const, }; @@ -177,6 +178,7 @@ export const RendersAgentDisplayModeSettings: Story = { const canvas = within(canvasElement); expect(await canvas.findByText("Thinking Display")).toBeVisible(); + expect(await canvas.findByText("Shell Output Display")).toBeVisible(); expect(await canvas.findByText("Code Diff Display")).toBeVisible(); }, }; diff --git a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx index d69373cda8..dee0ed2439 100644 --- a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx +++ b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx @@ -5,6 +5,7 @@ import { ChatFullWidthSettings } from "./components/ChatFullWidthSettings"; import { ChatSendShortcutSettings } from "./components/ChatSendShortcutSettings"; import { CodeDiffDisplaySettings, + ShellToolDisplaySettings, ThinkingDisplaySettings, } from "./components/DisplayModeSettings"; import { PersonalInstructionsSettings } from "./components/PersonalInstructionsSettings"; @@ -60,6 +61,7 @@ export const AgentSettingsGeneralPageView: FC< + { const canvas = within(canvasElement); + expect(canvas.getByText("pnpm test")).toBeVisible(); + expect(canvas.queryByText("tests passed")).not.toBeInTheDocument(); expect(canvas.getByText(/Edited config\.ts/)).toBeVisible(); expect(canvas.queryAllByTestId("edit-file-diff")).toHaveLength(0); + const commandOutputButton = canvas.getByRole("button", { + name: "Expand command output", + }); + expect(commandOutputButton).toHaveAttribute("aria-expanded", "false"); + await userEvent.click(commandOutputButton); + await waitFor(() => { + expect(canvas.getByText("tests passed")).toBeVisible(); + }); + const editFilesButton = canvas.getByRole("button", { name: /Edited config\.ts/, }); @@ -1926,6 +1949,7 @@ export const ThinkingBlockAlwaysExpanded: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "always_expanded" as const, + shell_tool_display_mode: "auto" as const, code_diff_display_mode: "auto" as const, agent_chat_send_shortcut: "enter" as const, }, @@ -1975,6 +1999,7 @@ export const ThinkingBlockAlwaysCollapsed: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "always_collapsed" as const, + shell_tool_display_mode: "auto" as const, code_diff_display_mode: "auto" as const, agent_chat_send_shortcut: "enter" as const, }, @@ -2025,6 +2050,7 @@ export const ThinkingBlockWithToolCall: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "always_collapsed" as const, + shell_tool_display_mode: "auto" as const, code_diff_display_mode: "auto" as const, agent_chat_send_shortcut: "enter" as const, }, @@ -2088,6 +2114,7 @@ export const ThinkingBlockAutoMode: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "auto" as const, + shell_tool_display_mode: "auto" as const, code_diff_display_mode: "auto" as const, agent_chat_send_shortcut: "enter" as const, }, @@ -2141,6 +2168,7 @@ export const ThinkingBlockPreviewMode: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "preview" as const, + shell_tool_display_mode: "auto" as const, code_diff_display_mode: "auto" as const, agent_chat_send_shortcut: "enter" as const, }, diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index a0742fb364..9ea957a0ee 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -275,6 +275,8 @@ export const BlockList: FC<{ const prefQuery = useQuery(preferenceSettings()); const thinkingDisplayMode: ThinkingDisplayMode = prefQuery.data?.thinking_display_mode || "auto"; + const shellToolDisplayMode: TypesGen.AgentDisplayMode = + prefQuery.data?.shell_tool_display_mode || "auto"; const codeDiffDisplayMode: TypesGen.AgentDisplayMode = prefQuery.data?.code_diff_display_mode || "auto"; @@ -367,6 +369,7 @@ export const BlockList: FC<{ name="Tool" status="running" isError={false} + shellToolDisplayMode={shellToolDisplayMode} codeDiffDisplayMode={codeDiffDisplayMode} subagentTitles={subagentTitles} subagentVariants={subagentVariants} @@ -384,6 +387,7 @@ export const BlockList: FC<{ status={tool.status} isError={tool.isError} killedBySignal={tool.killedBySignal} + shellToolDisplayMode={shellToolDisplayMode} codeDiffDisplayMode={codeDiffDisplayMode} subagentTitles={subagentTitles} subagentVariants={subagentVariants} @@ -440,6 +444,7 @@ export const BlockList: FC<{ status={tool.status} isError={tool.isError} killedBySignal={tool.killedBySignal} + shellToolDisplayMode={shellToolDisplayMode} codeDiffDisplayMode={codeDiffDisplayMode} subagentTitles={subagentTitles} subagentVariants={subagentVariants} @@ -564,7 +569,8 @@ const ChatMessageItem = memo<{ ) : ( -
+ {/* Keep consecutive shell tools tighter because execute/process_output pairs read as one terminal interaction. */} +
= { title: "components/ai-elements/tool/ExecuteTool", component: ExecuteTool, @@ -29,29 +32,29 @@ export const ShortCommand: Story = { }, }; -/** A long command expanded to show the full text, with the chevron visible on hover. */ export const LongCommand: Story = { args: { - command: - "find /home/coder/project/src -type f -name '*.ts' -not -path '*/node_modules/*' -not -path '*/.git/*' | xargs grep -l 'deprecated' | sort | head -50", + command: longCommand, output: "", }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - const chevron = canvas.getByRole("button", { - name: /expand command/i, - }); - await userEvent.click(chevron); - // Hover the component so the chevron stays visible. - await userEvent.hover(canvasElement.firstElementChild!); + const command = canvas.getByText(longCommand); + expect(command).toBeVisible(); + expect( + canvas.queryByRole("button", { name: longCommand }), + ).not.toBeInTheDocument(); + await userEvent.hover(command); + expect( + await screen.findByRole("tooltip", undefined, { timeout: 2000 }), + ).toHaveTextContent(longCommand); }, }; /** A long truncated command with multi-line output below it. */ export const LongCommandWithOutput: Story = { args: { - command: - "find /home/coder/project/src -type f -name '*.ts' -not -path '*/node_modules/*' -not -path '*/.git/*' | xargs grep -l 'deprecated' | sort | head -50", + command: longCommand, output: [ "src/api/legacyClient.ts", "src/components/OldTable/OldTable.tsx", diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx index 49bec3889e..60e6171649 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx @@ -9,7 +9,8 @@ import { TriangleAlertIcon, } from "lucide-react"; import type React from "react"; -import { useRef, useState } from "react"; +import { useState } from "react"; +import type * as TypesGen from "#/api/typesGenerated"; import { Button } from "#/components/Button/Button"; import { CopyButton } from "#/components/CopyButton/CopyButton"; import { ScrollArea } from "#/components/ScrollArea/ScrollArea"; @@ -20,186 +21,210 @@ import { } from "#/components/Tooltip/Tooltip"; import { cn } from "#/utils/cn"; import { - BORDER_BG_STYLE, - COLLAPSED_OUTPUT_HEIGHT, + type AgentDisplayState, + isAgentDisplayOpen, + resolveAgentDisplayState, +} from "./displayMode"; +import { + formatShellDurationMs, signalTooltipLabel, type ToolStatus, } from "./utils"; -/** - * Specialized rendering for `execute` tool calls. Shows the command - * in a terminal-style block with a copy button. Output is shown in a - * collapsed preview (~3 lines) with an expand chevron at the bottom. - */ -export const ExecuteTool: React.FC<{ +type ExecuteToolProps = { command: string; output: string; status: ToolStatus; isError: boolean; + durationMs?: number; isBackgrounded?: boolean; killedBySignal?: "kill" | "terminate"; -}> = ({ command, output, status, isBackgrounded = false, killedBySignal }) => { - const [expanded, setExpanded] = useState(false); - const outputRef = useRef(null); + shellToolDisplayMode?: TypesGen.AgentDisplayMode; +}; + +type ExecuteToolInnerProps = ExecuteToolProps & { + outputInitiallyOpen: boolean; +}; + +export const ExecuteTool: React.FC = (props) => { + const autoDisplayState: AgentDisplayState = + props.output.length > 0 || + props.status === "running" || + props.isBackgrounded || + !!props.killedBySignal + ? "preview" + : "collapsed"; + const resolvedDisplayState = resolveAgentDisplayState( + props.shellToolDisplayMode, + autoDisplayState, + ); + return ( + + ); +}; + +const ExecuteToolInner: React.FC = ({ + command, + output, + status, + isError, + durationMs, + isBackgrounded = false, + killedBySignal, + outputInitiallyOpen, +}) => { const hasOutput = output.length > 0; const isRunning = status === "running"; - - // Track whether the command text is truncated so we can offer - // a click-to-expand interaction. The ResizeObserver may clear - // commandOverflows while the text is wrapped, but - // canToggleCommand stays true via commandExpanded so the - // collapse affordance remains visible. - const [commandExpanded, setCommandExpanded] = useState(false); - const [commandOverflows, setCommandOverflows] = useState(false); - const canToggleCommand = commandOverflows || commandExpanded; - const commandRef = (node: HTMLElement | null) => { - if (!node) return; - const measure = () => { - setCommandOverflows(node.scrollWidth > node.clientWidth); - }; - measure(); - const ro = new ResizeObserver(measure); - ro.observe(node); - return () => ro.disconnect(); - }; - - // Check whether the output overflows the collapsed height so we - // know if we need to show the expand toggle at all. - const [overflows, setOverflows] = useState(false); - const measureRef = (node: HTMLPreElement | null) => { - outputRef.current = node; - if (node) { - setOverflows(node.scrollHeight > COLLAPSED_OUTPUT_HEIGHT); - } - }; + const showFailureIndicator = isError && !isRunning; + const [outputOpen, setOutputOpen] = useState(outputInitiallyOpen); + const outputToggleLabel = outputOpen + ? "Collapse command output" + : "Expand command output"; + const durationLabel = formatShellDurationMs(durationMs); return ( -
- {/* Header: $ command + copy button */} -
- {/* biome-ignore lint/a11y/useKeyWithClickEvents: Click toggles for mouse users; keyboard users use the chevron button. */} -
setCommandExpanded((v) => !v) : undefined - } - > - - $ - - - {command} - -
-
- {canToggleCommand && ( +
+ + + {hasOutput ? ( + ) : ( +
+ +
)} - {isRunning && ( - - )} - {isBackgrounded && !isRunning && ( - - - - - Running in background - - )} - {killedBySignal && !isRunning && ( - - - - - - {signalTooltipLabel(killedBySignal)} - - - )} - -
+ + + {command} + + +
+ {isRunning && ( + + )} + {showFailureIndicator && ( + + + + + + + Command failed + + )} + {isBackgrounded && !isRunning && ( + + + + + + + Running in background + + )} + {killedBySignal && !isRunning && ( + + + + + + {signalTooltipLabel(killedBySignal)} + + + )} +
- {/* Output preview / expanded */} - {hasOutput && ( - <> -
- - 
-							{output}
-
- - - {/* Expand / collapse toggle at the bottom */} - {overflows && ( - - )} - + {hasOutput && outputOpen && ( + )}
); }; +const ShellCommandLine: React.FC<{ + command: string; + durationLabel: string; + expanded?: boolean; +}> = ({ command, durationLabel, expanded }) => { + return ( + <> + $ + + {command} + + {durationLabel && ( + + {durationLabel} + + )} + {expanded !== undefined && ( + + )} + + ); +}; + +const ShellOutputBody: React.FC<{ + output: string; + isError: boolean; +}> = ({ output, isError }) => { + return ( + + 
+				{output}
+
+ + ); +}; + export const ExecuteAuthRequiredTool: React.FC<{ command: string; output: string; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessKilledIndicator.stories.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessKilledIndicator.stories.tsx index 3aab639684..c4f5434d82 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessKilledIndicator.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessKilledIndicator.stories.tsx @@ -19,7 +19,7 @@ export default meta; type Story = StoryObj; // --------------------------------------------------------------------------- -// Execute tool — killed indicator via killedBySignal prop +// Execute tool, killed indicator via killedBySignal prop. // --------------------------------------------------------------------------- export const ExecuteKilled: Story = { @@ -62,7 +62,7 @@ export const ExecuteTerminated: Story = { }, }; -/** Execute NOT signaled — no indicator. */ +/** Execute not signaled, no indicator. */ export const ExecuteNotSignaled: Story = { args: { name: "execute", @@ -81,7 +81,7 @@ export const ExecuteNotSignaled: Story = { }, }; -/** Running execute — killed indicator should NOT appear yet. */ +/** Running execute, killed indicator should not appear yet. */ export const ExecuteRunningNotYetKilled: Story = { args: { name: "execute", @@ -96,7 +96,7 @@ export const ExecuteRunningNotYetKilled: Story = { }; // --------------------------------------------------------------------------- -// ProcessOutput tool — killed indicator +// ProcessOutput tool, killed indicator. // --------------------------------------------------------------------------- export const ProcessOutputKilled: Story = { @@ -133,7 +133,7 @@ export const ProcessOutputTerminated: Story = { }, }; -/** ProcessOutput with no output and killed — indicator in empty state. */ +/** ProcessOutput with no output and killed, indicator in empty state. */ export const ProcessOutputKilledNoOutput: Story = { args: { name: "process_output", diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessOutputTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessOutputTool.tsx index bdddc388ac..9f80084f5f 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessOutputTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ProcessOutputTool.tsx @@ -1,6 +1,7 @@ import { ChevronDownIcon, LoaderIcon, OctagonXIcon } from "lucide-react"; import type React from "react"; -import { useRef, useState } from "react"; +import { useState } from "react"; +import type * as TypesGen from "#/api/typesGenerated"; import { CopyButton } from "#/components/CopyButton/CopyButton"; import { ScrollArea } from "#/components/ScrollArea/ScrollArea"; import { @@ -9,124 +10,152 @@ import { TooltipTrigger, } from "#/components/Tooltip/Tooltip"; import { cn } from "#/utils/cn"; +import { + type AgentDisplayState, + isAgentDisplayFullyExpanded, + resolveAgentDisplayState, +} from "./displayMode"; +import { AgentDisplayModeToolCollapsible } from "./ToolCollapsible"; import { COLLAPSED_OUTPUT_HEIGHT, signalTooltipLabel } from "./utils"; -/** - * Specialized rendering for `process_output` tool calls. Shows - * process output directly in a terminal-style block with a - * collapsible preview and an expand chevron at the bottom. - */ -export const ProcessOutputTool: React.FC<{ +type ProcessOutputToolProps = { output: string; isRunning: boolean; exitCode: number | null; isError: boolean; killedBySignal?: "kill" | "terminate"; -}> = ({ output, isRunning, exitCode, isError, killedBySignal }) => { - const [expanded, setExpanded] = useState(false); - const outputRef = useRef(null); + shellToolDisplayMode?: TypesGen.AgentDisplayMode; +}; + +type ProcessOutputToolInnerProps = ProcessOutputToolProps & { + autoDisplayState: AgentDisplayState; + outputInitiallyFullyExpanded: boolean; +}; + +export const ProcessOutputTool: React.FC = (props) => { + const autoDisplayState: AgentDisplayState = + props.output.length > 0 ? "preview" : "collapsed"; + const resolvedDisplayState = resolveAgentDisplayState( + props.shellToolDisplayMode, + autoDisplayState, + ); + return ( + + ); +}; + +const ProcessOutputToolInner: React.FC = ({ + output, + isRunning, + exitCode, + isError, + killedBySignal, + shellToolDisplayMode, + autoDisplayState, + outputInitiallyFullyExpanded, +}) => { + const [outputFullyExpanded, setOutputFullyExpanded] = useState( + outputInitiallyFullyExpanded, + ); const hasOutput = output.length > 0; const [overflows, setOverflows] = useState(false); const measureRef = (node: HTMLPreElement | null) => { - outputRef.current = node; if (node) { setOverflows(node.scrollHeight > COLLAPSED_OUTPUT_HEIGHT); } }; const showExitCode = exitCode !== null && exitCode !== 0; + const toggleOutputExpansion = () => { + setOutputFullyExpanded((expanded) => !expanded); + }; + const hasHeaderActions = + isRunning || Boolean(killedBySignal) || showExitCode || hasOutput; return ( -
- {hasOutput ? ( - <> -
- - 
-								{output}
-
- -
- {isRunning && ( - - )} - {killedBySignal && !isRunning && ( - - - - - - {signalTooltipLabel(killedBySignal)} - - - )} - {showExitCode && ( - - exit {exitCode} - - )} - -
-
- - {/* Expand / collapse toggle at the bottom */} - {overflows && ( - + + expanded ? "Collapse process output" : "Expand process output" + } + header={Process output} + headerActions={ + hasHeaderActions ? ( + <> + {isRunning && ( + + )} + {killedBySignal && !isRunning && ( + + + + + + {signalTooltipLabel(killedBySignal)} + + + )} + {showExitCode && ( + + exit {exitCode} + + )} + {hasOutput && } + + ) : undefined + } + > + + 
-			) : (
-				

-					{isRunning && (
-						
-					)}
-					{killedBySignal && !isRunning && (
-						
-							
-								
-							
-							
-								{signalTooltipLabel(killedBySignal)}
-							
-						
-					)}
-					{showExitCode && (
-						
-							exit {exitCode}
-						
-					)}
-				

+				>
+					{output}
+
+ + {overflows && ( + )} -
+ ); }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx index 1f21043869..fc8b23793c 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx @@ -1,11 +1,21 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { expect, fn, spyOn, userEvent, waitFor, within } from "storybook/test"; +import { + expect, + fn, + screen, + spyOn, + userEvent, + waitFor, + within, +} from "storybook/test"; import { reactRouterParameters } from "storybook-addon-remix-react-router"; import { ChatWorkspaceContext } from "../../../context/ChatWorkspaceContext"; import { DesktopPanelContext } from "./DesktopPanelContext"; import { Tool } from "./Tool"; const executeCommand = "git fetch origin"; +const longExecuteCommand = + "docker build --no-cache --build-arg NODE_ENV=production --build-arg API_URL=https://coder.example.com/api --build-arg SENTRY_DSN=https://example.com/sentry --build-arg FEATURE_FLAGS=agents,shell-tools --tag coder-agent:latest ."; const meta: Meta = { title: "pages/AgentsPage/ChatElements/tools/Tool", component: Tool, @@ -46,7 +56,11 @@ export const ExecuteRunning: Story = { export const ExecuteSuccess: Story = { args: { + shellToolDisplayMode: "auto", + args: { command: longExecuteCommand }, result: { + wall_duration_ms: 47200, + exit_code: 0, output: "From github.com:coder/coder\n * [new branch] feature/agent-ui -> origin/feature/agent-ui", }, @@ -54,6 +68,167 @@ export const ExecuteSuccess: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect(canvas.getByText(/From github\.com:coder\/coder/)).toBeVisible(); + expect(canvas.queryByText("exit 0")).not.toBeInTheDocument(); + expect( + canvas.queryByRole("img", { name: "Running in background" }), + ).not.toBeInTheDocument(); + expect(canvas.getByText("47.2s")).toBeVisible(); + expect(canvas.queryByText("2 lines")).not.toBeInTheDocument(); + }, +}; + +export const ExecuteError: Story = { + args: { + name: "execute", + status: "error", + isError: true, + args: { command: longExecuteCommand }, + shellToolDisplayMode: "always_collapsed", + result: { + wall_duration_ms: 8600, + exit_code: 1, + output: Array.from( + { length: 47 }, + (_, index) => `error line ${index + 1}`, + ).join("\n"), + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.queryByText(/error line 1/)).not.toBeInTheDocument(); + expect(canvas.getByRole("img", { name: "Command failed" })).toBeVisible(); + expect(canvas.queryByText("exit 1")).not.toBeInTheDocument(); + await userEvent.click( + canvas.getByRole("button", { name: "Expand command output" }), + ); + await waitFor(() => { + expect(canvas.getByText(/error line 1/)).toBeVisible(); + }); + }, +}; + +export const ExecuteBackgrounded: Story = { + args: { + name: "execute", + status: "completed", + args: { command: "npm start" }, + shellToolDisplayMode: "always_collapsed", + result: { + background_process_id: "process-123", + output: "", + wall_duration_ms: 2100, + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const backgroundIndicator = canvas.getByRole("img", { + name: "Running in background", + }); + expect(backgroundIndicator).toBeVisible(); + await userEvent.hover(backgroundIndicator); + expect(await screen.findByRole("tooltip")).toHaveTextContent( + "Running in background", + ); + }, +}; + +export const ExecuteAlwaysCollapsed: Story = { + args: { + name: "execute", + status: "completed", + args: { command: executeCommand }, + shellToolDisplayMode: "always_collapsed", + result: { + output: "From github.com:coder/coder\nFetching origin/main", + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(executeCommand)).toBeVisible(); + expect(canvas.queryByText("exit 0")).not.toBeInTheDocument(); + expect(canvas.queryByText("2 lines")).not.toBeInTheDocument(); + expect( + canvas.queryByText(/From github\.com:coder\/coder/), + ).not.toBeInTheDocument(); + await userEvent.click(canvas.getByText(executeCommand)); + await waitFor(() => { + expect(canvas.getByText(/From github\.com:coder\/coder/)).toBeVisible(); + }); + }, +}; + +export const ExecuteLongCommandCollapsed: Story = { + args: { + name: "execute", + status: "completed", + args: { command: longExecuteCommand }, + shellToolDisplayMode: "always_collapsed", + result: { + wall_duration_ms: 47200, + exit_code: 0, + output: Array.from( + { length: 61 }, + (_, index) => `output line ${index + 1}`, + ).join("\n"), + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const command = canvas.getByText(longExecuteCommand); + expect(command).toBeVisible(); + expect( + canvas.queryByRole("button", { name: longExecuteCommand }), + ).not.toBeInTheDocument(); + expect(canvas.queryByText("exit 0")).not.toBeInTheDocument(); + expect(canvas.getByText("47.2s")).toBeVisible(); + expect(canvas.queryByText("61 lines")).not.toBeInTheDocument(); + }, +}; + +export const ProcessOutputAlwaysCollapsed: Story = { + args: { + name: "process_output", + status: "completed", + shellToolDisplayMode: "always_collapsed", + result: { + output: "build completed\n0 errors", + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.queryByText(/build completed/)).not.toBeInTheDocument(); + await userEvent.click( + canvas.getByRole("button", { name: "Expand process output" }), + ); + await waitFor(() => { + expect(canvas.getByText(/build completed/)).toBeVisible(); + }); + }, +}; + +export const ProcessOutputAlwaysExpanded: Story = { + args: { + name: "process_output", + status: "completed", + shellToolDisplayMode: "always_expanded", + result: { + output: Array.from( + { length: 30 }, + (_, index) => `process output line ${index + 1}`, + ).join("\n"), + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/process output line 1/)).toBeVisible(); + expect(canvas.getByText(/process output line 30/)).toBeVisible(); + await waitFor(() => { + expect( + canvas.getByRole("button", { + name: "Collapse full process output", + }), + ).toHaveAttribute("aria-expanded", "true"); + }); }, }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx index ca86af1b0c..b3c9179777 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx @@ -92,6 +92,7 @@ interface ToolProps extends Omit, "children"> { previousResponseText?: string; /** Human-readable intent extracted from the model's tool-call args. */ modelIntent?: string; + shellToolDisplayMode?: TypesGen.AgentDisplayMode; codeDiffDisplayMode?: TypesGen.AgentDisplayMode; } @@ -116,6 +117,7 @@ type ToolRendererProps = { mcpServerConfigId?: string; mcpServers?: readonly TypesGen.MCPServerConfig[]; modelIntent?: string; + shellToolDisplayMode?: TypesGen.AgentDisplayMode; codeDiffDisplayMode?: TypesGen.AgentDisplayMode; }; @@ -218,11 +220,19 @@ const ExecuteRenderer: FC = ({ result, isError, killedBySignal, + shellToolDisplayMode, }) => { const parsedArgs = parseArgs(args); const command = parsedArgs ? asString(parsedArgs.command) : ""; const rec = asRecord(result); const output = rec ? asString(rec.output).trim() : ""; + const durationMs = rec + ? (asNumber(rec.wall_duration_ms, { parseString: true }) ?? + asNumber(rec.duration_ms, { parseString: true })) + : undefined; + const isBackgrounded = Boolean( + rec && asString(rec.background_process_id).trim(), + ); const authRequired = rec ? Boolean(rec.auth_required) : false; const authenticateURL = rec ? asString(rec.authenticate_url).trim() : ""; const providerLabel = toProviderLabel( @@ -247,7 +257,10 @@ const ExecuteRenderer: FC = ({ output={output} status={status} isError={isError} + durationMs={durationMs} + isBackgrounded={isBackgrounded} killedBySignal={killedBySignal} + shellToolDisplayMode={shellToolDisplayMode} /> ); }; @@ -257,13 +270,12 @@ const ProcessOutputRenderer: FC = ({ result, isError, killedBySignal, + shellToolDisplayMode, }) => { const rec = asRecord(result); const output = rec ? asString(rec.output).trim() : ""; const exitCode = rec - ? rec.exit_code !== undefined && rec.exit_code !== null - ? Number(rec.exit_code) - : null + ? (asNumber(rec.exit_code, { parseString: true }) ?? null) : null; return ( @@ -273,6 +285,7 @@ const ProcessOutputRenderer: FC = ({ exitCode={exitCode} isError={isError} killedBySignal={killedBySignal} + shellToolDisplayMode={shellToolDisplayMode} /> ); }; @@ -1037,6 +1050,7 @@ export const Tool = memo( isLatestAskUserQuestion, previousResponseText, modelIntent, + shellToolDisplayMode, codeDiffDisplayMode, ref, ...props @@ -1044,21 +1058,18 @@ export const Tool = memo( const Renderer = isSubagentToolName(name) ? SubagentRenderer : (toolRenderers[name] ?? GenericToolRenderer); + const isShellTool = name === "execute" || name === "process_output"; return (
diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx index 5cb279d820..9e223d9f19 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx @@ -9,13 +9,16 @@ import { resolveAgentDisplayState, } from "./displayMode"; +type ToolCollapsibleAriaLabel = string | ((expanded: boolean) => string); type ToolCollapsibleHeader = ReactNode | ((expanded: boolean) => ReactNode); interface ToolCollapsibleProps { children: ReactNode; header: ToolCollapsibleHeader; + headerActions?: ReactNode; hasContent?: boolean; defaultExpanded?: boolean; + ariaLabel?: ToolCollapsibleAriaLabel; className?: string; headerClassName?: string; } @@ -30,6 +33,7 @@ export const AgentDisplayModeToolCollapsible: FC< AgentDisplayModeToolCollapsibleProps > = ({ displayMode, autoDisplayState, ...props }) => { const displayState = resolveAgentDisplayState(displayMode, autoDisplayState); + return ( = ({ children, header, + headerActions, hasContent = true, defaultExpanded = false, + ariaLabel, className, headerClassName, }) => { const [expanded, setExpanded] = useState(defaultExpanded); const renderedHeader = typeof header === "function" ? header(expanded) : header; + const headerButton = hasContent ? ( + + ) : ( +
+ {renderedHeader} +
+ ); + return (
- {hasContent ? ( - - ) : ( -
- {renderedHeader} + {headerActions ? ( +
+ {headerButton} +
+ {headerActions} +
+ ) : ( + headerButton )} {expanded && hasContent && children}
diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/utils.test.ts b/site/src/pages/AgentsPage/components/ChatElements/tools/utils.test.ts index 2f462e255d..09be688333 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/utils.test.ts +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/utils.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it, vi } from "vitest"; import { - BORDER_BG_STYLE, buildEditDiff, buildWriteFileDiff, COLLAPSED_OUTPUT_HEIGHT, @@ -9,6 +8,7 @@ import { diffViewerCSS, fileViewerCSS, formatResultOutput, + formatShellDurationMs, getDiffViewerOptions, getFileContentForViewer, getFileViewerOptions, @@ -70,20 +70,45 @@ describe("shortDurationMs", () => { expect(shortDurationMs(1000)).toBe("1s"); expect(shortDurationMs(30_000)).toBe("30s"); expect(shortDurationMs(59_000)).toBe("59s"); + expect(shortDurationMs(59_499)).toBe("59s"); }); it("formats minutes", () => { + expect(shortDurationMs(59_500)).toBe("1m"); expect(shortDurationMs(60_000)).toBe("1m"); expect(shortDurationMs(300_000)).toBe("5m"); expect(shortDurationMs(3_540_000)).toBe("59m"); + expect(shortDurationMs(3_569_999)).toBe("59m"); }); it("formats hours", () => { + expect(shortDurationMs(3_570_000)).toBe("1h"); expect(shortDurationMs(3_600_000)).toBe("1h"); expect(shortDurationMs(7_200_000)).toBe("2h"); }); }); +describe("formatShellDurationMs", () => { + it("returns empty string for invalid values", () => { + expect(formatShellDurationMs(undefined)).toBe(""); + expect(formatShellDurationMs(-1)).toBe(""); + expect(formatShellDurationMs(Number.NaN)).toBe(""); + expect(formatShellDurationMs(Number.POSITIVE_INFINITY)).toBe(""); + }); + + it("formats milliseconds and rounded seconds", () => { + expect(formatShellDurationMs(100)).toBe("100ms"); + expect(formatShellDurationMs(47_200)).toBe("47.2s"); + expect(formatShellDurationMs(59_949)).toBe("59.9s"); + expect(formatShellDurationMs(59_950)).toBe("1m"); + }); + + it("formats rounded minutes and hours", () => { + expect(formatShellDurationMs(3_596_999)).toBe("59.9m"); + expect(formatShellDurationMs(3_597_000)).toBe("1h"); + }); +}); + describe("normalizeStatus", () => { it("lowercases and trims", () => { expect(normalizeStatus(" COMPLETED ")).toBe("completed"); @@ -840,13 +865,6 @@ describe("constants", () => { expect(DIFFS_FONT_STYLE).toHaveProperty("--diffs-line-height", "1.5"); }); - it("BORDER_BG_STYLE has expected background", () => { - expect(BORDER_BG_STYLE).toHaveProperty( - "background", - "hsl(var(--border-default))", - ); - }); - it("fileViewerCSS is a non-empty string", () => { expect(typeof fileViewerCSS).toBe("string"); expect(fileViewerCSS.length).toBeGreaterThan(0); diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts b/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts index 467a7387bd..87d80e6891 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts @@ -55,11 +55,38 @@ export const shortDurationMs = (durationMs: number | undefined): string => { if (seconds < 60) { return `${seconds}s`; } - const minutes = Math.round(seconds / 60); + const minutes = Math.round(durationMs / 60_000); if (minutes < 60) { return `${minutes}m`; } - const hours = Math.round(minutes / 60); + const hours = Math.round(durationMs / 3_600_000); + return `${hours}h`; +}; + +const roundToTenths = (value: number): number => Number(value.toFixed(1)); + +export const formatShellDurationMs = ( + durationMs: number | undefined, +): string => { + if ( + durationMs === undefined || + durationMs < 0 || + !Number.isFinite(durationMs) + ) { + return ""; + } + if (durationMs < 1000) { + return `${Math.round(durationMs)}ms`; + } + const seconds = roundToTenths(durationMs / 1000); + if (seconds < 60) { + return `${seconds}s`; + } + const minutes = roundToTenths(durationMs / 60_000); + if (minutes < 60) { + return `${minutes}m`; + } + const hours = roundToTenths(durationMs / 3_600_000); return `${hours}h`; }; @@ -439,10 +466,6 @@ export const DIFFS_FONT_STYLE = { "--diffs-line-height": "1.5", } as CSSProperties; -export const BORDER_BG_STYLE = { - background: "hsl(var(--border-default))", -}; - /** * Checks whether a tool result should be rendered as a syntax-highlighted * file viewer. Returns the file path, content, and whether the header diff --git a/site/src/pages/AgentsPage/components/DisplayModeSettings.tsx b/site/src/pages/AgentsPage/components/DisplayModeSettings.tsx index 2e0b861ff5..2db2d8eb30 100644 --- a/site/src/pages/AgentsPage/components/DisplayModeSettings.tsx +++ b/site/src/pages/AgentsPage/components/DisplayModeSettings.tsx @@ -115,6 +115,23 @@ export const ThinkingDisplaySettings: FC = () => { ); }; +export const ShellToolDisplaySettings: FC = () => { + return ( + settings.shell_tool_display_mode} + updateSettings={(value) => ({ + shell_tool_display_mode: value, + })} + /> + ); +}; + export const CodeDiffDisplaySettings: FC = () => { return (