From 92987a872cbb209394eb7cfac22a96c2fe07ea6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fiere?= Date: Mon, 17 Aug 2026 20:28:26 +0000 Subject: [PATCH] feat: add relative last seen display option to account security (#2739) Co-authored-by: Carlos Florencio --- .../internal/backendapp/boot_state_routes.go | 6 + .../boot_state_user_settings_test.go | 35 + .../internal/user/controller/controller.go | 1 + .../user/controller/controller_test.go | 35 + apps/backend/internal/user/dto/dto.go | 3 + apps/backend/internal/user/dto/dto_test.go | 47 + .../internal/user/handlers/handlers.go | 4 + .../internal/user/handlers/handlers_test.go | 76 ++ apps/backend/internal/user/models/models.go | 15 + apps/backend/internal/user/service/service.go | 159 ++-- .../internal/user/service/service_test.go | 55 ++ .../user/service/user_settings_cas_test.go | 821 ++++++++++++++++++ apps/backend/internal/user/store/sqlite.go | 63 +- .../user/store/sqlite_last_seen_cas_test.go | 502 +++++++++++ .../internal/user/store/sqlite_test.go | 293 ++----- apps/backend/internal/user/store/store.go | 7 +- .../account/security-settings.test.tsx | 560 ++++++++++++ .../settings/account/security-settings.tsx | 313 ++++++- .../auth/mobile-relative-last-seen.spec.ts | 112 +++ .../e2e/tests/auth/relative-last-seen.spec.ts | 162 ++++ .../hooks/use-ensure-user-settings.test.ts | 3 + .../lib/settings-discovery/catalog/account.ts | 19 +- apps/web/lib/ssr/user-settings.test.ts | 47 + apps/web/lib/ssr/user-settings.ts | 29 + apps/web/lib/state/slices/settings/types.ts | 2 + apps/web/lib/types/http-user-settings.ts | 3 + apps/web/lib/types/http.ts | 1 + apps/web/lib/ws/handlers/users.test.ts | 42 + apps/web/src/locales/en/account.json | 6 + apps/web/src/locales/pseudo/account.json | 6 + apps/web/src/locales/pt-pt/account.json | 6 + apps/web/src/locales/zh-cn/account.json | 6 + apps/web/src/locales/zh-hk/account.json | 6 + apps/web/src/locales/zh-tw/account.json | 6 + docs/plans/relative-last-seen/plan.md | 282 ++++++ .../task-01-backend-last-seen-setting.md | 173 ++++ .../task-02-frontend-settings-contract.md | 64 ++ ...ask-03-security-page-relative-last-seen.md | 84 ++ .../task-04-e2e-relative-last-seen.md | 79 ++ docs/specs/relative-last-seen/spec.md | 109 +++ 40 files changed, 3890 insertions(+), 352 deletions(-) create mode 100644 apps/backend/internal/user/service/user_settings_cas_test.go create mode 100644 apps/backend/internal/user/store/sqlite_last_seen_cas_test.go create mode 100644 apps/web/components/settings/account/security-settings.test.tsx create mode 100644 apps/web/e2e/tests/auth/mobile-relative-last-seen.spec.ts create mode 100644 apps/web/e2e/tests/auth/relative-last-seen.spec.ts create mode 100644 docs/plans/relative-last-seen/plan.md create mode 100644 docs/plans/relative-last-seen/task-01-backend-last-seen-setting.md create mode 100644 docs/plans/relative-last-seen/task-02-frontend-settings-contract.md create mode 100644 docs/plans/relative-last-seen/task-03-security-page-relative-last-seen.md create mode 100644 docs/plans/relative-last-seen/task-04-e2e-relative-last-seen.md create mode 100644 docs/specs/relative-last-seen/spec.md diff --git a/apps/backend/internal/backendapp/boot_state_routes.go b/apps/backend/internal/backendapp/boot_state_routes.go index e4e26b684a..dd9273791e 100644 --- a/apps/backend/internal/backendapp/boot_state_routes.go +++ b/apps/backend/internal/backendapp/boot_state_routes.go @@ -520,6 +520,7 @@ func mapUserSettingsState(response userdto.UserSettingsResponse, workspaceID str "terminalFontFamily": nullString(settings.TerminalFontFamily), "terminalFontSize": nullInt(settings.TerminalFontSize), "changesPanelLayout": changesPanelLayout(settings.ChangesPanelLayout), + "lastSeenDisplay": lastSeenDisplay(settings.LastSeenDisplay), "azureDevOpsBrowsePreferences": settings.AzureDevOpsBrowsePreferences, "systemMetricsDisplay": map[string]any{ "showInTopbar": settings.SystemMetricsDisplay.ShowInTopbar, @@ -889,6 +890,11 @@ func changesPanelLayout(value string) string { return "tree" } +// lastSeenDisplay normalizes the last-seen display to absolute or relative. +func lastSeenDisplay(value string) string { + return usermodels.NormalizeLastSeenDisplay(value) +} + // logBootError logs a debug entry when optional boot data failed to load. func (b bootStateBuilder) logBootError(operation string, err error) { if err == nil || b.p.log == nil { diff --git a/apps/backend/internal/backendapp/boot_state_user_settings_test.go b/apps/backend/internal/backendapp/boot_state_user_settings_test.go index 8718661d3d..8ae0be523a 100644 --- a/apps/backend/internal/backendapp/boot_state_user_settings_test.go +++ b/apps/backend/internal/backendapp/boot_state_user_settings_test.go @@ -7,6 +7,7 @@ import ( usermodels "github.com/kandev/kandev/internal/user/models" ) +// TestMapUserSettingsStateIncludesArchiveConfirmation verifies boot state carries the archive confirmation flag. func TestMapUserSettingsStateIncludesArchiveConfirmation(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{ConfirmTaskArchive: true}, @@ -18,6 +19,7 @@ func TestMapUserSettingsStateIncludesArchiveConfirmation(t *testing.T) { } } +// TestMapUserSettingsStateIncludesAgentGeneratedTaskTitles verifies boot state carries the agent-generated task titles flag. func TestMapUserSettingsStateIncludesAgentGeneratedTaskTitles(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{AgentGeneratedTaskTitles: true}, @@ -29,6 +31,7 @@ func TestMapUserSettingsStateIncludesAgentGeneratedTaskTitles(t *testing.T) { } } +// TestMapUserSettingsStateIncludesTasksListShowDetails verifies boot state carries the tasks-list show-details flag. func TestMapUserSettingsStateIncludesTasksListShowDetails(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{TasksListShowDetails: true}, @@ -39,6 +42,7 @@ func TestMapUserSettingsStateIncludesTasksListShowDetails(t *testing.T) { } } +// TestMapUserSettingsStateIncludesNormalizedMCPTaskAgentProfileDefault verifies boot state normalizes the MCP task agent profile default. func TestMapUserSettingsStateIncludesNormalizedMCPTaskAgentProfileDefault(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{MCPTaskAgentProfileDefault: "future_value"}, @@ -50,6 +54,7 @@ func TestMapUserSettingsStateIncludesNormalizedMCPTaskAgentProfileDefault(t *tes } } +// TestMapUserSettingsStateIncludesNormalizedStartupPage verifies boot state normalizes the startup page. func TestMapUserSettingsStateIncludesNormalizedStartupPage(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{StartupPage: "future_value"}, @@ -61,6 +66,7 @@ func TestMapUserSettingsStateIncludesNormalizedStartupPage(t *testing.T) { } } +// TestMapUserSettingsStateIncludesHiddenWorkflowStepIds verifies boot state carries hidden workflow step IDs under the store-field key. func TestMapUserSettingsStateIncludesHiddenWorkflowStepIds(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{ @@ -85,6 +91,7 @@ func TestMapUserSettingsStateIncludesHiddenWorkflowStepIds(t *testing.T) { } } +// TestMapUserSettingsStateDefaultsHiddenWorkflowStepIdsToEmptyMap verifies boot state defaults hidden workflow step IDs to an empty map. func TestMapUserSettingsStateDefaultsHiddenWorkflowStepIdsToEmptyMap(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{}, @@ -96,6 +103,7 @@ func TestMapUserSettingsStateDefaultsHiddenWorkflowStepIdsToEmptyMap(t *testing. } } +// TestMapUserSettingsStateIncludesAppStatusBarOrder verifies boot state carries the app status bar order. func TestMapUserSettingsStateIncludesAppStatusBarOrder(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{AppStatusBarOrder: usermodels.AppStatusBarOrder{ @@ -113,6 +121,7 @@ func TestMapUserSettingsStateIncludesAppStatusBarOrder(t *testing.T) { } } +// TestMapUserSettingsStateNormalizesLspStatusLocation verifies boot state normalizes the LSP status location. func TestMapUserSettingsStateNormalizesLspStatusLocation(t *testing.T) { tests := []struct { name string @@ -137,6 +146,32 @@ func TestMapUserSettingsStateNormalizesLspStatusLocation(t *testing.T) { } } +// TestMapUserSettingsStateNormalizesLastSeenDisplay verifies boot state normalizes the last-seen display mode. +func TestMapUserSettingsStateNormalizesLastSeenDisplay(t *testing.T) { + tests := []struct { + name string + value string + want string + }{ + {name: "relative is preserved", value: usermodels.LastSeenDisplayRelative, want: usermodels.LastSeenDisplayRelative}, + {name: "empty uses absolute", value: "", want: usermodels.LastSeenDisplayAbsolute}, + {name: "unknown uses absolute", value: "future_value", want: usermodels.LastSeenDisplayAbsolute}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + state := mapUserSettingsState(userdto.UserSettingsResponse{ + Settings: userdto.UserSettingsDTO{LastSeenDisplay: tt.value}, + }, "workspace-1") + + if got := state["lastSeenDisplay"]; got != tt.want { + t.Fatalf("lastSeenDisplay = %#v, want %q", got, tt.want) + } + }) + } +} + +// TestMapUserSettingsStateIncludesSystemMetricsDisplayPreference verifies boot state carries the system metrics display preference. func TestMapUserSettingsStateIncludesSystemMetricsDisplayPreference(t *testing.T) { state := mapUserSettingsState(userdto.UserSettingsResponse{ Settings: userdto.UserSettingsDTO{SystemMetricsDisplay: usermodels.SystemMetricsDisplaySettings{ diff --git a/apps/backend/internal/user/controller/controller.go b/apps/backend/internal/user/controller/controller.go index bbb6152cfd..2d314cdf0d 100644 --- a/apps/backend/internal/user/controller/controller.go +++ b/apps/backend/internal/user/controller/controller.go @@ -102,6 +102,7 @@ func (c *Controller) UpdateUserSettings(ctx context.Context, req dto.UpdateUserS TerminalFontFamily: req.TerminalFontFamily, TerminalFontSize: req.TerminalFontSize, ChangesPanelLayout: req.ChangesPanelLayout, + LastSeenDisplay: req.LastSeenDisplay, SystemMetricsDisplay: systemMetricsDisplayPatch(req.SystemMetricsDisplay), AppStatusBarEnabled: req.AppStatusBarEnabled, AppStatusBarOrder: req.AppStatusBarOrder, diff --git a/apps/backend/internal/user/controller/controller_test.go b/apps/backend/internal/user/controller/controller_test.go index a8ea7758cb..ce17496302 100644 --- a/apps/backend/internal/user/controller/controller_test.go +++ b/apps/backend/internal/user/controller/controller_test.go @@ -21,35 +21,43 @@ type settingsRepository struct { settings *models.UserSettings } +// GetUser returns an error, asserting the settings service never fetches users. func (r *settingsRepository) GetUser(context.Context, string) (*models.User, error) { return nil, errors.New("unexpected GetUser call") } +// GetDefaultUser returns an error, asserting the settings service never fetches the default user. func (r *settingsRepository) GetDefaultUser(context.Context) (*models.User, error) { return nil, errors.New("unexpected GetDefaultUser call") } +// GetUserSettings returns a copy of the repository's stored settings. func (r *settingsRepository) GetUserSettings(context.Context, string) (*models.UserSettings, error) { copy := *r.settings return ©, nil } +// UpsertUserSettingsPreservingTaskCreateLastUsed stores a copy of the settings and returns it. func (r *settingsRepository) UpsertUserSettingsPreservingTaskCreateLastUsed( _ context.Context, settings *models.UserSettings, _ *models.TaskCreateLastUsed, + _ int64, ) (*models.UserSettings, error) { copy := *settings r.settings = © return ©, nil } +// UpdateTaskCreateLastUsed returns an error, asserting the settings service never updates task-creation history directly. func (r *settingsRepository) UpdateTaskCreateLastUsed(context.Context, string, models.TaskCreateLastUsed) (*models.UserSettings, error) { return nil, errors.New("unexpected UpdateTaskCreateLastUsed call") } +// Close is a no-op, satisfying the repository interface. func (r *settingsRepository) Close() error { return nil } +// TestUpdateUserSettingsMapsMCPTaskAgentProfileDefault verifies the controller persists an MCP task agent profile default patch. func TestUpdateUserSettingsMapsMCPTaskAgentProfileDefault(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -72,6 +80,7 @@ func TestUpdateUserSettingsMapsMCPTaskAgentProfileDefault(t *testing.T) { } } +// TestUpdateUserSettingsMapsLspStatusLocation verifies the controller persists an LSP status location patch. func TestUpdateUserSettingsMapsLspStatusLocation(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -94,6 +103,7 @@ func TestUpdateUserSettingsMapsLspStatusLocation(t *testing.T) { } } +// TestUpdateUserSettingsMapsStartupPage verifies the controller persists a startup page patch. func TestUpdateUserSettingsMapsStartupPage(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -116,6 +126,30 @@ func TestUpdateUserSettingsMapsStartupPage(t *testing.T) { } } +// TestUpdateUserSettingsMapsLastSeenDisplay verifies the controller persists a last-seen display patch. +func TestUpdateUserSettingsMapsLastSeenDisplay(t *testing.T) { + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + t.Fatalf("logger.NewFromZap: %v", err) + } + repo := &settingsRepository{settings: &models.UserSettings{ + LastSeenDisplay: models.LastSeenDisplayAbsolute, + }} + controller := NewController(service.NewService(repo, nil, log)) + want := models.LastSeenDisplayRelative + + response, err := controller.UpdateUserSettings(context.Background(), dto.UpdateUserSettingsRequest{ + LastSeenDisplay: &want, + }) + if err != nil { + t.Fatalf("UpdateUserSettings: %v", err) + } + if response.Settings.LastSeenDisplay != want { + t.Fatalf("LastSeenDisplay = %q, want %q", response.Settings.LastSeenDisplay, want) + } +} + +// TestAzureDevOpsBrowsePreferencesRoundTripThroughSettingsAPI verifies Azure DevOps browse preferences survive update, read-back, omission, and null clearing. func TestAzureDevOpsBrowsePreferencesRoundTripThroughSettingsAPI(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -185,6 +219,7 @@ func TestAzureDevOpsBrowsePreferencesRoundTripThroughSettingsAPI(t *testing.T) { } } +// TestSystemMetricsDisplayPatch verifies systemMetricsDisplayPatch nil, explicit-value, and omitted-field handling. func TestSystemMetricsDisplayPatch(t *testing.T) { t.Run("nil patch stays nil", func(t *testing.T) { if got := systemMetricsDisplayPatch(nil); got != nil { diff --git a/apps/backend/internal/user/dto/dto.go b/apps/backend/internal/user/dto/dto.go index bf27b1b432..7cb13b0441 100644 --- a/apps/backend/internal/user/dto/dto.go +++ b/apps/backend/internal/user/dto/dto.go @@ -67,6 +67,7 @@ type UserSettingsDTO struct { TerminalFontFamily string `json:"terminal_font_family"` TerminalFontSize int `json:"terminal_font_size"` ChangesPanelLayout string `json:"changes_panel_layout"` + LastSeenDisplay string `json:"last_seen_display"` SystemMetricsDisplay models.SystemMetricsDisplaySettings `json:"system_metrics_display"` AppStatusBarEnabled bool `json:"app_status_bar_enabled"` AppStatusBarOrder models.AppStatusBarOrder `json:"app_status_bar_order"` @@ -142,6 +143,7 @@ type UpdateUserSettingsRequest struct { TerminalFontFamily *string `json:"terminal_font_family,omitempty"` TerminalFontSize *int `json:"terminal_font_size,omitempty"` ChangesPanelLayout *string `json:"changes_panel_layout,omitempty"` + LastSeenDisplay *string `json:"last_seen_display,omitempty"` SystemMetricsDisplay *SystemMetricsDisplaySettingsPatch `json:"system_metrics_display,omitempty"` AppStatusBarEnabled *bool `json:"app_status_bar_enabled,omitempty"` AppStatusBarOrder *models.AppStatusBarOrder `json:"app_status_bar_order,omitempty"` @@ -291,6 +293,7 @@ func FromUserSettings(settings *models.UserSettings) UserSettingsDTO { TerminalFontFamily: settings.TerminalFontFamily, TerminalFontSize: settings.TerminalFontSize, ChangesPanelLayout: settings.ChangesPanelLayout, + LastSeenDisplay: models.NormalizeLastSeenDisplay(settings.LastSeenDisplay), SystemMetricsDisplay: settings.SystemMetricsDisplay, AppStatusBarEnabled: settings.AppStatusBarEnabled, AppStatusBarOrder: settings.AppStatusBarOrder, diff --git a/apps/backend/internal/user/dto/dto_test.go b/apps/backend/internal/user/dto/dto_test.go index 7c71368fdc..34815c6f4e 100644 --- a/apps/backend/internal/user/dto/dto_test.go +++ b/apps/backend/internal/user/dto/dto_test.go @@ -8,6 +8,7 @@ import ( "github.com/kandev/kandev/internal/user/models" ) +// TestFromUserSettingsIncludesAtomicRevision verifies the DTO carries the settings revision. func TestFromUserSettingsIncludesAtomicRevision(t *testing.T) { got := FromUserSettings(&models.UserSettings{Revision: 42}).Revision if got != 42 { @@ -15,6 +16,7 @@ func TestFromUserSettingsIncludesAtomicRevision(t *testing.T) { } } +// TestUpdateUserSettingsRequestExposesAzureDevOpsBrowsePreferences verifies the patch request exposes the Azure DevOps browse preferences field. func TestUpdateUserSettingsRequestExposesAzureDevOpsBrowsePreferences(t *testing.T) { field, ok := reflect.TypeFor[UpdateUserSettingsRequest]().FieldByName("AzureDevOpsBrowsePreferences") if !ok || field.Tag.Get("json") != "azure_devops_browse_preferences,omitempty" { @@ -22,6 +24,7 @@ func TestUpdateUserSettingsRequestExposesAzureDevOpsBrowsePreferences(t *testing } } +// TestFromUserSettingsMapsAzureDevOpsBrowsePreferences verifies the DTO passes the Azure DevOps browse preferences through unchanged. func TestFromUserSettingsMapsAzureDevOpsBrowsePreferences(t *testing.T) { preferences := json.RawMessage(`{"project-1":{"teamId":"team-1"}}`) settings := FromUserSettings(&models.UserSettings{AzureDevOpsBrowsePreferences: preferences}) @@ -30,6 +33,7 @@ func TestFromUserSettingsMapsAzureDevOpsBrowsePreferences(t *testing.T) { } } +// TestUpdateUserSettingsRequestExposesKanbanHiddenStepIDs verifies the patch request exposes the kanban hidden step IDs field. func TestUpdateUserSettingsRequestExposesKanbanHiddenStepIDs(t *testing.T) { field, ok := reflect.TypeFor[UpdateUserSettingsRequest]().FieldByName("KanbanHiddenStepIDs") if !ok || field.Tag.Get("json") != "kanban_hidden_step_ids,omitempty" { @@ -37,6 +41,7 @@ func TestUpdateUserSettingsRequestExposesKanbanHiddenStepIDs(t *testing.T) { } } +// TestFromUserSettingsMapsKanbanHiddenStepIDs verifies the DTO carries the per-workflow hidden step ID map. func TestFromUserSettingsMapsKanbanHiddenStepIDs(t *testing.T) { hidden := map[string][]string{"wf-1": {"step-a", "step-b"}} settings := FromUserSettings(&models.UserSettings{KanbanHiddenStepIDs: hidden}) @@ -45,6 +50,7 @@ func TestFromUserSettingsMapsKanbanHiddenStepIDs(t *testing.T) { } } +// TestUserSettingsDTOKanbanHiddenStepIDsAlwaysSerializesEvenWhenEmpty verifies an empty hidden step ID map serializes as {} instead of being omitted. func TestUserSettingsDTOKanbanHiddenStepIDsAlwaysSerializesEvenWhenEmpty(t *testing.T) { // Regression test: the response field must never use `omitempty`. A // client that clears its hidden set to {} needs that {} to actually @@ -72,6 +78,7 @@ func TestUserSettingsDTOKanbanHiddenStepIDsAlwaysSerializesEvenWhenEmpty(t *test } } +// TestKanbanHiddenStepIDsRequestDecode verifies decoding distinguishes an omitted field from an explicit empty map. func TestKanbanHiddenStepIDsRequestDecode(t *testing.T) { t.Run("omitted value stays nil", func(t *testing.T) { var req UpdateUserSettingsRequest @@ -94,6 +101,7 @@ func TestKanbanHiddenStepIDsRequestDecode(t *testing.T) { }) } +// TestTasksListShowDetailsDTO verifies the DTO mapping and the nil-versus-explicit-false patch semantics. func TestTasksListShowDetailsDTO(t *testing.T) { if !FromUserSettings(&models.UserSettings{TasksListShowDetails: true}).TasksListShowDetails { t.Fatal("TasksListShowDetails = false, want true") @@ -120,6 +128,7 @@ func TestTasksListShowDetailsDTO(t *testing.T) { }) } +// TestAppStatusBarOrderDTOAndPatchSemantics verifies the status bar order DTO mapping and its patch semantics. func TestAppStatusBarOrderDTOAndPatchSemantics(t *testing.T) { want := models.AppStatusBarOrder{ LeftItemIDs: []string{"builtin:connection", "plugin:left"}, @@ -151,6 +160,7 @@ func TestAppStatusBarOrderDTOAndPatchSemantics(t *testing.T) { }) } +// TestLspStatusLocationDTOAndPatchSemantics verifies the LSP status location DTO normalization and its patch semantics. func TestLspStatusLocationDTOAndPatchSemantics(t *testing.T) { t.Run("response normalizes missing and unknown values to toolbar", func(t *testing.T) { for _, value := range []string{"", "future_location"} { @@ -191,6 +201,7 @@ func TestLspStatusLocationDTOAndPatchSemantics(t *testing.T) { }) } +// TestUpdateUserSettingsRequestSystemMetricsDisplayPreservesOmittedFields verifies omitted system metrics display fields stay absent when re-marshaled. func TestUpdateUserSettingsRequestSystemMetricsDisplayPreservesOmittedFields(t *testing.T) { var req UpdateUserSettingsRequest if err := json.Unmarshal([]byte(`{"system_metrics_display":{"show_in_topbar":true}}`), &req); err != nil { @@ -210,6 +221,7 @@ func TestUpdateUserSettingsRequestSystemMetricsDisplayPreservesOmittedFields(t * } } +// TestFromUserSettingsIncludesArchiveConfirmation verifies the DTO carries the task archive confirmation flag. func TestFromUserSettingsIncludesArchiveConfirmation(t *testing.T) { for _, want := range []bool{true, false} { dto := FromUserSettings(&models.UserSettings{ConfirmTaskArchive: want}) @@ -219,6 +231,7 @@ func TestFromUserSettingsIncludesArchiveConfirmation(t *testing.T) { } } +// TestAgentGeneratedTaskTitlesDTOAndPatchSemantics verifies the agent-generated task titles DTO mapping and its patch semantics. func TestAgentGeneratedTaskTitlesDTOAndPatchSemantics(t *testing.T) { if !FromUserSettings(&models.UserSettings{AgentGeneratedTaskTitles: true}).AgentGeneratedTaskTitles { t.Fatal("AgentGeneratedTaskTitles = false, want true") @@ -249,6 +262,7 @@ func TestAgentGeneratedTaskTitlesDTOAndPatchSemantics(t *testing.T) { } } +// TestFromUserSettingsIncludesNormalizedMCPTaskAgentProfileDefault verifies the DTO normalizes the MCP task agent profile default. func TestFromUserSettingsIncludesNormalizedMCPTaskAgentProfileDefault(t *testing.T) { tests := []struct { name string @@ -276,6 +290,7 @@ func TestFromUserSettingsIncludesNormalizedMCPTaskAgentProfileDefault(t *testing } } +// TestFromUserSettingsIncludesNormalizedStartupPage verifies the DTO normalizes the startup page. func TestFromUserSettingsIncludesNormalizedStartupPage(t *testing.T) { tests := []struct { name string @@ -303,6 +318,35 @@ func TestFromUserSettingsIncludesNormalizedStartupPage(t *testing.T) { } } +// TestFromUserSettingsIncludesNormalizedLastSeenDisplay verifies the DTO normalizes the last-seen display mode. +func TestFromUserSettingsIncludesNormalizedLastSeenDisplay(t *testing.T) { + tests := []struct { + name string + value string + want string + }{ + {name: "relative", value: models.LastSeenDisplayRelative, want: models.LastSeenDisplayRelative}, + {name: "unknown defaults to absolute", value: "future_value", want: models.LastSeenDisplayAbsolute}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + raw, err := json.Marshal(FromUserSettings(&models.UserSettings{LastSeenDisplay: tt.value})) + if err != nil { + t.Fatalf("marshal DTO: %v", err) + } + var payload map[string]any + if err := json.Unmarshal(raw, &payload); err != nil { + t.Fatalf("decode DTO: %v", err) + } + if got := payload["last_seen_display"]; got != tt.want { + t.Fatalf("last_seen_display = %#v, want %q", got, tt.want) + } + }) + } +} + +// TestUpdateUserSettingsRequestStartupPagePatchSemantics verifies startup page patch decoding distinguishes omitted and explicit values. func TestUpdateUserSettingsRequestStartupPagePatchSemantics(t *testing.T) { t.Run("omitted value stays nil", func(t *testing.T) { var req UpdateUserSettingsRequest @@ -325,6 +369,7 @@ func TestUpdateUserSettingsRequestStartupPagePatchSemantics(t *testing.T) { }) } +// TestUpdateUserSettingsRequestMCPTaskAgentProfileDefaultPatchSemantics verifies MCP task agent profile default patch decoding distinguishes omitted and explicit values. func TestUpdateUserSettingsRequestMCPTaskAgentProfileDefaultPatchSemantics(t *testing.T) { t.Run("omitted value stays nil", func(t *testing.T) { var req UpdateUserSettingsRequest @@ -347,6 +392,7 @@ func TestUpdateUserSettingsRequestMCPTaskAgentProfileDefaultPatchSemantics(t *te }) } +// TestNullableSidebarDraft verifies sidebar draft decoding distinguishes omitted, null, and object values. func TestNullableSidebarDraft(t *testing.T) { t.Run("omitted field is not set", func(t *testing.T) { var req UpdateUserSettingsRequest @@ -385,6 +431,7 @@ func TestNullableSidebarDraft(t *testing.T) { }) } +// TestNullableRawMessage verifies raw-message decoding distinguishes omitted, null, and JSON values. func TestNullableRawMessage(t *testing.T) { t.Run("omitted field is not set", func(t *testing.T) { var req UpdateUserSettingsRequest diff --git a/apps/backend/internal/user/handlers/handlers.go b/apps/backend/internal/user/handlers/handlers.go index 8e8afb74f3..8f4d3bba89 100644 --- a/apps/backend/internal/user/handlers/handlers.go +++ b/apps/backend/internal/user/handlers/handlers.go @@ -89,6 +89,8 @@ func (h *Handlers) httpUpdateUserSettings(c *gin.Context) { status := http.StatusInternalServerError if errors.Is(err, service.ErrValidation) { status = http.StatusBadRequest + } else if errors.Is(err, service.ErrUserSettingsConflict) { + status = http.StatusConflict } h.logger.Error("failed to update user settings", zap.Error(err)) c.JSON(status, gin.H{"error": "failed to update user settings"}) @@ -115,6 +117,8 @@ func (h *Handlers) wsUpdateUserSettings(ctx context.Context, msg *ws.Message) (* code := ws.ErrorCodeInternalError if errors.Is(err, service.ErrValidation) { code = ws.ErrorCodeBadRequest + } else if errors.Is(err, service.ErrUserSettingsConflict) { + code = ws.ErrorCodeConflict } return ws.NewError(msg.ID, msg.Action, code, "Failed to update user settings", nil) } diff --git a/apps/backend/internal/user/handlers/handlers_test.go b/apps/backend/internal/user/handlers/handlers_test.go index 9a10bb1211..981c3df880 100644 --- a/apps/backend/internal/user/handlers/handlers_test.go +++ b/apps/backend/internal/user/handlers/handlers_test.go @@ -2,6 +2,7 @@ package handlers import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -14,8 +15,10 @@ import ( "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/user/controller" "github.com/kandev/kandev/internal/user/dto" + "github.com/kandev/kandev/internal/user/models" "github.com/kandev/kandev/internal/user/service" userstore "github.com/kandev/kandev/internal/user/store" + ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" ) @@ -171,3 +174,76 @@ func TestHTTPUpdateUserSettingsBodyTooLarge(t *testing.T) { t.Fatalf("PATCH oversized body status = %d, want %d: %s", response.Code, http.StatusRequestEntityTooLarge, response.Body.String()) } } + +// conflictSettingsRepository forces every conditional settings write to +// exercise the service retry limit without requiring a live database race. +type conflictSettingsRepository struct { + userstore.Repository +} + +func (conflictSettingsRepository) GetUserSettings(_ context.Context, userID string) (*models.UserSettings, error) { + return &models.UserSettings{UserID: userID}, nil +} + +func (conflictSettingsRepository) UpsertUserSettingsPreservingTaskCreateLastUsed( + context.Context, + *models.UserSettings, + *models.TaskCreateLastUsed, + int64, +) (*models.UserSettings, error) { + return nil, userstore.ErrUserSettingsRevisionConflict +} + +func newConflictHandlers(t *testing.T) *Handlers { + t.Helper() + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + t.Fatalf("create logger: %v", err) + } + svc := service.NewService(conflictSettingsRepository{}, nil, log) + return NewHandlers(controller.NewController(svc), log) +} + +func TestHTTPUpdateUserSettingsRevisionConflict(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + newConflictHandlers(t).registerHTTP(router) + + request := httptest.NewRequest( + http.MethodPatch, + "/api/v1/user/settings", + bytes.NewReader([]byte(`{"last_seen_display":"relative"}`)), + ) + request.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + + if response.Code != http.StatusConflict { + t.Fatalf("PATCH revision conflict status = %d, want %d: %s", response.Code, http.StatusConflict, response.Body.String()) + } +} + +func TestWSUpdateUserSettingsRevisionConflict(t *testing.T) { + h := newConflictHandlers(t) + msg, err := ws.NewRequest("settings-1", ws.ActionUserSettingsUpdate, map[string]string{ + "last_seen_display": "relative", + }) + if err != nil { + t.Fatalf("create request: %v", err) + } + + response, err := h.wsUpdateUserSettings(context.Background(), msg) + if err != nil { + t.Fatalf("wsUpdateUserSettings returned error: %v", err) + } + if response.Type != ws.MessageTypeError { + t.Fatalf("response type = %q, want %q", response.Type, ws.MessageTypeError) + } + var payload ws.ErrorPayload + if err := response.ParsePayload(&payload); err != nil { + t.Fatalf("parse error payload: %v", err) + } + if payload.Code != ws.ErrorCodeConflict { + t.Fatalf("error code = %q, want %q", payload.Code, ws.ErrorCodeConflict) + } +} diff --git a/apps/backend/internal/user/models/models.go b/apps/backend/internal/user/models/models.go index 86404fd3a4..38552f3abb 100644 --- a/apps/backend/internal/user/models/models.go +++ b/apps/backend/internal/user/models/models.go @@ -45,6 +45,20 @@ func NormalizeStartupPage(value string) string { return StartupPageTaskOverview } +const ( + LastSeenDisplayAbsolute = "absolute" + LastSeenDisplayRelative = "relative" +) + +// NormalizeLastSeenDisplay returns the canonical last-seen display mode: +// relative is accepted as-is, anything else is coerced to absolute. +func NormalizeLastSeenDisplay(value string) string { + if value == LastSeenDisplayRelative { + return value + } + return LastSeenDisplayAbsolute +} + const ( // RoleAdmin unlocks user management and system settings mutation when // authentication is enabled. It does NOT grant visibility into other @@ -123,6 +137,7 @@ type UserSettings struct { TerminalFontFamily string `json:"terminal_font_family"` TerminalFontSize int `json:"terminal_font_size"` ChangesPanelLayout string `json:"changes_panel_layout"` // "flat" | "tree" + LastSeenDisplay string `json:"last_seen_display"` // "absolute" | "relative" SystemMetricsDisplay SystemMetricsDisplaySettings `json:"system_metrics_display"` AppStatusBarEnabled bool `json:"app_status_bar_enabled"` AppStatusBarOrder AppStatusBarOrder `json:"app_status_bar_order"` diff --git a/apps/backend/internal/user/service/service.go b/apps/backend/internal/user/service/service.go index 37d59061fa..a02178b957 100644 --- a/apps/backend/internal/user/service/service.go +++ b/apps/backend/internal/user/service/service.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "reflect" "strings" "time" @@ -18,14 +19,18 @@ import ( "go.uber.org/zap" ) +// ErrUserSettingsConflict marks an exhausted conditional settings update. var ( - ErrUserNotFound = errors.New("user not found") - ErrValidation = errors.New("validation error") + ErrUserNotFound = errors.New("user not found") + ErrValidation = errors.New("validation error") + ErrUserSettingsConflict = store.ErrUserSettingsRevisionConflict ) const ( changesPanelLayoutFlat = "flat" changesPanelLayoutTree = "tree" + + maxUserSettingsCASAttempts = 3 ) type Service struct { @@ -87,6 +92,7 @@ type UpdateUserSettingsRequest struct { TerminalFontFamily *string TerminalFontSize *int ChangesPanelLayout *string + LastSeenDisplay *string SystemMetricsDisplay *SystemMetricsDisplaySettingsPatch AppStatusBarEnabled *bool AppStatusBarOrder *models.AppStatusBarOrder @@ -166,45 +172,87 @@ func (s *Service) GetDefaultUtilityAgentProfileID(ctx context.Context) (string, } // UpdateUserSettings applies a partial settings patch field by field, -// validates each group, persists the result, and publishes the settings -// update event. +// validates each group, persists the result under expected-revision CAS, and +// publishes the settings update event. func (s *Service) UpdateUserSettings(ctx context.Context, req *UpdateUserSettingsRequest) (*models.UserSettings, error) { - settings, err := s.repo.GetUserSettings(ctx, s.settingsUserID(ctx)) - if err != nil { - return nil, err - } - if err := applyBasicSettings(settings, req); err != nil { - return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error()) - } - if err := s.applyChatSubmitKey(settings, req); err != nil { - return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error()) - } - if err := applyLSPSettings(settings, req); err != nil { - return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error()) - } - if err := applySavedLayouts(settings, req); err != nil { - return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error()) - } - if err := applySidebarViews(settings, req); err != nil { - return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error()) - } - if err := applySidebarViewState(settings, req); err != nil { - return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error()) - } - if err := applyUserPreferenceBlobs(settings, req); err != nil { - return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error()) - } - settings.UpdatedAt = time.Now().UTC() var taskCreatePatch *models.TaskCreateLastUsed if req.TaskCreateLastUsed != nil && !taskCreateLastUsedPatchEmpty(*req.TaskCreateLastUsed) { taskCreatePatch = req.TaskCreateLastUsed } - settings, err = s.repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, taskCreatePatch) - if err != nil { - return nil, err + return s.updateUserSettingsCAS(ctx, func(settings *models.UserSettings) (bool, error) { + // The shallow copy is safe only while every apply* helper replaces + // reference fields (slices, maps, and json.RawMessage) instead of + // mutating them in place. In-place mutation would alias before and make + // DeepEqual miss the write. + before := *settings + if err := applyBasicSettings(settings, req); err != nil { + return false, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + } + if err := s.applyChatSubmitKey(settings, req); err != nil { + return false, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + } + if err := applyLSPSettings(settings, req); err != nil { + return false, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + } + if err := applySavedLayouts(settings, req); err != nil { + return false, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + } + if err := applySidebarViews(settings, req); err != nil { + return false, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + } + if err := applySidebarViewState(settings, req); err != nil { + return false, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + } + if err := applyUserPreferenceBlobs(settings, req); err != nil { + return false, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + } + return !reflect.DeepEqual(*settings, before), nil + }, taskCreatePatch) +} + +// updateUserSettingsCAS applies a full-blob user-settings write under +// expected-revision CAS with bounded retry. apply is evaluated against every +// freshly read model and reports whether it mutated it; the immutable +// taskCreatePatch is passed to every attempt so a retry re-applies the same +// task-create merge against the fresh row. A no-op (apply returned false and +// no patch) writes and publishes nothing and returns the freshly read +// (current) settings. Exactly one event is published per successful write. +func (s *Service) updateUserSettingsCAS( + ctx context.Context, + apply func(*models.UserSettings) (bool, error), + taskCreatePatch *models.TaskCreateLastUsed, +) (*models.UserSettings, error) { + userID := s.settingsUserID(ctx) + var lastErr error + for attempt := 0; attempt < maxUserSettingsCASAttempts; attempt++ { + settings, err := s.repo.GetUserSettings(ctx, userID) + if err != nil { + return nil, err + } + applied, err := apply(settings) + if err != nil { + return nil, err + } + if !applied && taskCreatePatch == nil { + // No-op: report the freshly read (current) settings, write nothing, + // publish nothing. + return settings, nil + } + updated, err := s.repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, taskCreatePatch, settings.Revision) + if err == nil { + s.publishUserSettingsEvent(ctx, updated) + return updated, nil + } + if !errors.Is(err, store.ErrUserSettingsRevisionConflict) { + return nil, err + } + lastErr = err } - s.publishUserSettingsEvent(ctx, settings) - return settings, nil + s.logger.Warn("user settings update exhausted CAS retries", + zap.Int("attempts", maxUserSettingsCASAttempts), + zap.Error(lastErr), + ) + return nil, fmt.Errorf("user settings update exhausted after %d attempts: %w", maxUserSettingsCASAttempts, lastErr) } // RecordTaskCreateLastUsed persists the last task-creation choices (a no-op @@ -254,6 +302,9 @@ func applyBasicSettings(settings *models.UserSettings, req *UpdateUserSettingsRe if err := applyChangesPanelLayout(settings, req.ChangesPanelLayout); err != nil { return err } + if err := applyLastSeenDisplay(settings, req.LastSeenDisplay); err != nil { + return err + } applySystemMetricsDisplay(settings, req.SystemMetricsDisplay) if req.AppStatusBarEnabled != nil { settings.AppStatusBarEnabled = *req.AppStatusBarEnabled @@ -546,6 +597,20 @@ func applyChangesPanelLayout(settings *models.UserSettings, value *string) error return nil } +// applyLastSeenDisplay validates and applies the last-seen display enum +// (absolute or relative). +func applyLastSeenDisplay(settings *models.UserSettings, value *string) error { + if value == nil { + return nil + } + v := strings.TrimSpace(*value) + if v != models.LastSeenDisplayAbsolute && v != models.LastSeenDisplayRelative { + return errors.New("last_seen_display must be 'absolute' or 'relative'") + } + settings.LastSeenDisplay = v + return nil +} + // applyChatSubmitKey validates and applies the chat_submit_key setting. func (s *Service) applyChatSubmitKey(settings *models.UserSettings, req *UpdateUserSettingsRequest) error { if req.ChatSubmitKey == nil { @@ -813,6 +878,7 @@ func (s *Service) publishUserSettingsEvent(ctx context.Context, settings *models "terminal_font_family": settings.TerminalFontFamily, "terminal_font_size": settings.TerminalFontSize, "changes_panel_layout": settings.ChangesPanelLayout, + "last_seen_display": models.NormalizeLastSeenDisplay(settings.LastSeenDisplay), "system_metrics_display": settings.SystemMetricsDisplay, "app_status_bar_enabled": settings.AppStatusBarEnabled, "app_status_bar_order": settings.AppStatusBarOrder, @@ -884,19 +950,12 @@ func (s *Service) ClearDefaultEditorID(ctx context.Context, editorID string) err if editorID == "" { return nil } - settings, err := s.repo.GetUserSettings(ctx, s.settingsUserID(ctx)) - if err != nil { - return err - } - if settings.DefaultEditorID != editorID { - return nil - } - settings.DefaultEditorID = "" - settings.UpdatedAt = time.Now().UTC() - settings, err = s.repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, nil) - if err != nil { - return err - } - s.publishUserSettingsEvent(ctx, settings) - return nil + _, err := s.updateUserSettingsCAS(ctx, func(settings *models.UserSettings) (bool, error) { + if settings.DefaultEditorID != editorID { + return false, nil + } + settings.DefaultEditorID = "" + return true, nil + }, nil) + return err } diff --git a/apps/backend/internal/user/service/service_test.go b/apps/backend/internal/user/service/service_test.go index e5dd56581e..3a8b6dc485 100644 --- a/apps/backend/internal/user/service/service_test.go +++ b/apps/backend/internal/user/service/service_test.go @@ -19,16 +19,20 @@ import ( "go.uber.org/zap" ) +// ptr returns a pointer to a copy of v. func ptr[T any](v T) *T { return &v } +// rawPatch wraps v in a double pointer for use as a raw JSON patch value. func rawPatch(v json.RawMessage) **json.RawMessage { return ptr(ptr(v)) } +// rawClear returns a double pointer to a nil RawMessage for clearing a blob preference. func rawClear() **json.RawMessage { return ptr((*json.RawMessage)(nil)) } +// TestApplyBasicSettingsTasksListShowDetails verifies applyBasicSettings preserves TasksListShowDetails when omitted and applies explicit values. func TestApplyBasicSettingsTasksListShowDetails(t *testing.T) { t.Run("omission preserves saved value", func(t *testing.T) { settings := &models.UserSettings{TasksListShowDetails: true} @@ -53,6 +57,7 @@ func TestApplyBasicSettingsTasksListShowDetails(t *testing.T) { } } +// TestApplyBasicSettingsSystemMetricsDisplayPreservesOmittedFields verifies omitted SystemMetricsDisplay subfields are preserved by applyBasicSettings. func TestApplyBasicSettingsSystemMetricsDisplayPreservesOmittedFields(t *testing.T) { settings := &models.UserSettings{ SystemMetricsDisplay: models.SystemMetricsDisplaySettings{ @@ -72,6 +77,7 @@ func TestApplyBasicSettingsSystemMetricsDisplayPreservesOmittedFields(t *testing } } +// makeLayouts builds n SavedLayout fixtures with distinct IDs and names. func makeLayouts(n int) []models.SavedLayout { layouts := make([]models.SavedLayout, n) for i := range layouts { @@ -86,6 +92,7 @@ func makeLayouts(n int) []models.SavedLayout { return layouts } +// TestApplyBasicSettings_ReleaseNotes verifies release-note fields are preserved when nil and set or cleared when provided. func TestApplyBasicSettings_ReleaseNotes(t *testing.T) { t.Run("nil fields leave settings unchanged", func(t *testing.T) { settings := &models.UserSettings{ @@ -149,6 +156,7 @@ func TestApplyBasicSettings_ReleaseNotes(t *testing.T) { }) } +// TestApplyBasicSettings_ConfirmTaskArchive verifies archive confirmation stays enabled when omitted and is toggled by explicit values. func TestApplyBasicSettings_ConfirmTaskArchive(t *testing.T) { t.Run("omitted value leaves confirmation enabled", func(t *testing.T) { settings := &models.UserSettings{ConfirmTaskArchive: true} @@ -185,6 +193,7 @@ func TestApplyBasicSettings_ConfirmTaskArchive(t *testing.T) { }) } +// TestApplyBasicSettings_PreventAutoStartAgentOnOpen verifies the auto-start setting is preserved when omitted and toggled by explicit values. func TestApplyBasicSettings_PreventAutoStartAgentOnOpen(t *testing.T) { t.Run("omitted value leaves the setting unchanged", func(t *testing.T) { settings := &models.UserSettings{PreventAutoStartAgentOnOpen: true} @@ -221,6 +230,7 @@ func TestApplyBasicSettings_PreventAutoStartAgentOnOpen(t *testing.T) { }) } +// TestApplyBasicSettingsAgentGeneratedTaskTitles verifies AgentGeneratedTaskTitles is preserved when omitted and applied when provided. func TestApplyBasicSettingsAgentGeneratedTaskTitles(t *testing.T) { settings := &models.UserSettings{AgentGeneratedTaskTitles: false} if err := applyBasicSettings(settings, &UpdateUserSettingsRequest{}); err != nil { @@ -245,6 +255,7 @@ func TestApplyBasicSettingsAgentGeneratedTaskTitles(t *testing.T) { } } +// TestApplyBasicSettingsAppStatusBarOrder verifies the status bar order is preserved when omitted and replaced when provided. func TestApplyBasicSettingsAppStatusBarOrder(t *testing.T) { saved := models.AppStatusBarOrder{ LeftItemIDs: []string{"builtin:connection"}, @@ -275,6 +286,7 @@ func TestApplyBasicSettingsAppStatusBarOrder(t *testing.T) { }) } +// TestApplyLSPSettingsRejectsManualOnlyAutoInstallLanguage verifies manual-install-only languages are rejected without mutating settings. func TestApplyLSPSettingsRejectsManualOnlyAutoInstallLanguage(t *testing.T) { settings := &models.UserSettings{} req := &UpdateUserSettingsRequest{ @@ -290,6 +302,7 @@ func TestApplyLSPSettingsRejectsManualOnlyAutoInstallLanguage(t *testing.T) { } } +// TestApplyLSPSettingsAcceptsTaskHostAutoInstallPreference verifies a task-host language auto-install preference is accepted and stored. func TestApplyLSPSettingsAcceptsTaskHostAutoInstallPreference(t *testing.T) { settings := &models.UserSettings{} req := &UpdateUserSettingsRequest{ @@ -304,6 +317,7 @@ func TestApplyLSPSettingsAcceptsTaskHostAutoInstallPreference(t *testing.T) { } } +// TestApplyLspStatusLocation verifies LspStatusLocation is preserved when omitted, applied when valid, and rejected when invalid. func TestApplyLspStatusLocation(t *testing.T) { t.Run("omission preserves saved value", func(t *testing.T) { settings := &models.UserSettings{LspStatusLocation: models.LspStatusLocationStatusBar} @@ -347,6 +361,7 @@ func TestApplyLspStatusLocation(t *testing.T) { }) } +// TestApplyBasicSettingsMCPTaskAgentProfileDefault verifies the MCP task-agent profile default is preserved, applied, and validated. func TestApplyBasicSettingsMCPTaskAgentProfileDefault(t *testing.T) { t.Run("omission preserves saved value", func(t *testing.T) { settings := &models.UserSettings{MCPTaskAgentProfileDefault: models.MCPTaskAgentProfileDefaultWorkspaceDefault} @@ -386,6 +401,7 @@ func TestApplyBasicSettingsMCPTaskAgentProfileDefault(t *testing.T) { }) } +// TestApplyBasicSettingsShowAnchoredPromptBar verifies ShowAnchoredPromptBar is preserved when omitted and toggled by explicit values. func TestApplyBasicSettingsShowAnchoredPromptBar(t *testing.T) { t.Run("omission preserves saved value", func(t *testing.T) { settings := &models.UserSettings{ShowAnchoredPromptBar: true} @@ -418,6 +434,7 @@ func TestApplyBasicSettingsShowAnchoredPromptBar(t *testing.T) { }) } +// TestApplyBasicSettingsTodoListPanel verifies ShowTodoListPanel is preserved when omitted and toggled by explicit values. func TestApplyBasicSettingsTodoListPanel(t *testing.T) { t.Run("omission preserves saved value", func(t *testing.T) { settings := &models.UserSettings{ShowTodoListPanel: true} @@ -450,6 +467,7 @@ func TestApplyBasicSettingsTodoListPanel(t *testing.T) { }) } +// TestApplyBasicSettingsTodoListPanelOnlyWhenNotEmpty verifies ShowTodoListPanelOnlyWhenNotEmpty is preserved when omitted and toggled by explicit values. func TestApplyBasicSettingsTodoListPanelOnlyWhenNotEmpty(t *testing.T) { t.Run("omission preserves saved value", func(t *testing.T) { settings := &models.UserSettings{ShowTodoListPanelOnlyWhenNotEmpty: true} @@ -482,6 +500,7 @@ func TestApplyBasicSettingsTodoListPanelOnlyWhenNotEmpty(t *testing.T) { }) } +// TestApplyBasicSettingsTranscriptNavigation verifies transcript navigation flags are applied while omitted flags are preserved. func TestApplyBasicSettingsTranscriptNavigation(t *testing.T) { settings := &models.UserSettings{ ShowScrollToLastPrompt: true, @@ -507,6 +526,7 @@ func TestApplyBasicSettingsTranscriptNavigation(t *testing.T) { } } +// TestApplyBasicSettings_TasksListPreferences verifies tasks list sort and group are applied and invalid values rejected. func TestApplyBasicSettings_TasksListPreferences(t *testing.T) { t.Run("sets valid sort and group", func(t *testing.T) { settings := &models.UserSettings{} @@ -542,6 +562,7 @@ func TestApplyBasicSettings_TasksListPreferences(t *testing.T) { }) } +// TestApplyBasicSettings_TerminalFontFamily verifies the terminal font family is preserved, set, trimmed, and cleared. func TestApplyBasicSettings_TerminalFontFamily(t *testing.T) { t.Run("nil leaves settings unchanged", func(t *testing.T) { settings := &models.UserSettings{TerminalFontFamily: "Fira Code"} @@ -588,6 +609,7 @@ func TestApplyBasicSettings_TerminalFontFamily(t *testing.T) { }) } +// TestApplyChangesPanelLayout verifies the changes panel layout is preserved, set, trimmed, and validated. func TestApplyChangesPanelLayout(t *testing.T) { t.Run("nil leaves settings unchanged", func(t *testing.T) { settings := &models.UserSettings{ChangesPanelLayout: "tree"} @@ -642,6 +664,7 @@ func TestApplyChangesPanelLayout(t *testing.T) { }) } +// TestApplyStartupPage verifies the startup page is preserved when omitted, applied when valid, and rejected when invalid. func TestApplyStartupPage(t *testing.T) { t.Run("omission preserves saved value", func(t *testing.T) { settings := &models.UserSettings{StartupPage: models.StartupPageLastTask} @@ -677,6 +700,7 @@ func TestApplyStartupPage(t *testing.T) { }) } +// TestApplyBasicSettings_TerminalFontSize verifies font size application, reset to zero, and bounds validation. func TestApplyBasicSettings_TerminalFontSize(t *testing.T) { t.Run("nil leaves settings unchanged", func(t *testing.T) { settings := &models.UserSettings{TerminalFontSize: 14} @@ -728,6 +752,7 @@ func TestApplyBasicSettings_TerminalFontSize(t *testing.T) { }) } +// TestApplySavedLayouts table-tests saved layout count limits and validation errors. func TestApplySavedLayouts(t *testing.T) { tests := []struct { name string @@ -907,6 +932,7 @@ func TestApplySavedLayouts(t *testing.T) { } } +// TestApplyWorkspaceAndTaskListPreferencesKanbanHiddenStepIDs table-tests kanban hidden step ID count and byte-budget limits. func TestApplyWorkspaceAndTaskListPreferencesKanbanHiddenStepIDs(t *testing.T) { makeIDs := func(n int) []string { ids := make([]string, n) @@ -1034,6 +1060,7 @@ func TestApplyWorkspaceAndTaskListPreferencesKanbanHiddenStepIDs(t *testing.T) { } } +// makeSidebarViews builds n SidebarView fixtures with distinct IDs and names. func makeSidebarViews(n int) []models.SidebarView { views := make([]models.SidebarView, n) for i := range views { @@ -1049,6 +1076,7 @@ func makeSidebarViews(n int) []models.SidebarView { return views } +// TestApplySidebarViews table-tests sidebar view count limits and validation errors. func TestApplySidebarViews(t *testing.T) { tests := []struct { name string @@ -1139,6 +1167,7 @@ func TestApplySidebarViews(t *testing.T) { } } +// TestApplySidebarViewState verifies active view and draft application, clearing, and validation. func TestApplySidebarViewState(t *testing.T) { t.Run("nil fields leave active view and draft unchanged", func(t *testing.T) { settings := &models.UserSettings{ @@ -1221,6 +1250,7 @@ func TestApplySidebarViewState(t *testing.T) { }) } +// TestApplyUserPreferenceBlobs verifies blob preferences are patched while task-create last-used is preserved. func TestApplyUserPreferenceBlobs(t *testing.T) { settings := &models.UserSettings{ TaskCreateLastUsed: models.TaskCreateLastUsed{ @@ -1250,6 +1280,7 @@ func TestApplyUserPreferenceBlobs(t *testing.T) { } } +// TestUpdateUserSettingsCombinesSettingsAndTaskCreatePatch verifies UpdateUserSettings folds the task-create patch into a single preserving write. func TestUpdateUserSettingsCombinesSettingsAndTaskCreatePatch(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1302,6 +1333,7 @@ func TestUpdateUserSettingsCombinesSettingsAndTaskCreatePatch(t *testing.T) { } } +// TestPublishUserSettingsEventIncludesArchiveConfirmation verifies the settings event carries confirm_task_archive. func TestPublishUserSettingsEventIncludesArchiveConfirmation(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1323,6 +1355,7 @@ func TestPublishUserSettingsEventIncludesArchiveConfirmation(t *testing.T) { } } +// TestPublishUserSettingsEventIncludesTasksListShowDetails verifies the settings event carries tasks_list_show_details. func TestPublishUserSettingsEventIncludesTasksListShowDetails(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1344,6 +1377,7 @@ func TestPublishUserSettingsEventIncludesTasksListShowDetails(t *testing.T) { } } +// TestPublishUserSettingsEventIncludesNormalizedMCPTaskAgentProfileDefault verifies the event normalizes unknown profile defaults to current_task. func TestPublishUserSettingsEventIncludesNormalizedMCPTaskAgentProfileDefault(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1367,6 +1401,7 @@ func TestPublishUserSettingsEventIncludesNormalizedMCPTaskAgentProfileDefault(t } } +// TestPublishUserSettingsEventIncludesNormalizedStartupPage verifies the event normalizes unknown startup pages to task_overview. func TestPublishUserSettingsEventIncludesNormalizedStartupPage(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1385,6 +1420,7 @@ func TestPublishUserSettingsEventIncludesNormalizedStartupPage(t *testing.T) { } } +// TestPublishUserSettingsEventIncludesAppStatusBarOrder verifies the settings event carries the app status bar order. func TestPublishUserSettingsEventIncludesAppStatusBarOrder(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1407,6 +1443,7 @@ func TestPublishUserSettingsEventIncludesAppStatusBarOrder(t *testing.T) { } } +// TestPublishUserSettingsEventIncludesLspStatusLocation verifies the settings event carries lsp_status_location. func TestPublishUserSettingsEventIncludesLspStatusLocation(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1427,6 +1464,7 @@ func TestPublishUserSettingsEventIncludesLspStatusLocation(t *testing.T) { } } +// TestUpdateUserSettingsRejectsInvalidMCPTaskAgentProfileDefaultWithoutPersisting verifies an invalid profile default is rejected without persisting or publishing. func TestUpdateUserSettingsRejectsInvalidMCPTaskAgentProfileDefaultWithoutPersisting(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1455,6 +1493,7 @@ func TestUpdateUserSettingsRejectsInvalidMCPTaskAgentProfileDefaultWithoutPersis } } +// TestClearDefaultEditorIDPreservesTaskCreateLastUsed verifies clearing the default editor preserves task-create last-used in the write and event. func TestClearDefaultEditorIDPreservesTaskCreateLastUsed(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1501,6 +1540,7 @@ func TestClearDefaultEditorIDPreservesTaskCreateLastUsed(t *testing.T) { } } +// TestRecordTaskCreateLastUsed verifies RecordTaskCreateLastUsed skips empty patches and updates and publishes non-empty ones. func TestRecordTaskCreateLastUsed(t *testing.T) { newTestService := func(repo *recordingUserRepository, eventBus *recordingEventBus) *Service { log, err := logger.NewFromZap(zap.NewNop()) @@ -1624,6 +1664,7 @@ func TestRecordTaskCreateLastUsed(t *testing.T) { }) } +// TestApplyUserPreferenceBlobsValidation verifies blob preferences accept arrays, objects, and null and reject scalars and oversized blobs. func TestApplyUserPreferenceBlobsValidation(t *testing.T) { t.Run("accepts arrays objects and null", func(t *testing.T) { settings := &models.UserSettings{} @@ -1669,6 +1710,7 @@ func TestApplyUserPreferenceBlobsValidation(t *testing.T) { } +// TestAzureDevOpsBrowsePreferencesArePatched verifies Azure DevOps browse preferences are patched, persisted, and published. func TestAzureDevOpsBrowsePreferencesArePatched(t *testing.T) { log, err := logger.NewFromZap(zap.NewNop()) if err != nil { @@ -1728,16 +1770,19 @@ type recordingUserRepository struct { closeCalls int } +// GetUser records the call and returns an unexpected-call error. func (r *recordingUserRepository) GetUser(context.Context, string) (*models.User, error) { r.getUserCalls++ return nil, errors.New("unexpected GetUser call") } +// GetDefaultUser records the call and returns an unexpected-call error. func (r *recordingUserRepository) GetDefaultUser(context.Context) (*models.User, error) { r.getDefaultUserCalls++ return nil, errors.New("unexpected GetDefaultUser call") } +// GetUserSettings records the call and returns the configured settings or error. func (r *recordingUserRepository) GetUserSettings(_ context.Context, userID string) (*models.UserSettings, error) { r.getUserSettingsCalls++ r.getSettingsUserID = userID @@ -1750,10 +1795,12 @@ func (r *recordingUserRepository) GetUserSettings(_ context.Context, userID stri return nil, errors.New("unexpected GetUserSettings call") } +// UpsertUserSettingsPreservingTaskCreateLastUsed records the preserving-write inputs and returns the configured result or error. func (r *recordingUserRepository) UpsertUserSettingsPreservingTaskCreateLastUsed( _ context.Context, settings *models.UserSettings, patch *models.TaskCreateLastUsed, + _ int64, ) (*models.UserSettings, error) { r.upsertUserSettingsPreservingLastUsedCalls++ settingsCopy := *settings @@ -1771,6 +1818,7 @@ func (r *recordingUserRepository) UpsertUserSettingsPreservingTaskCreateLastUsed return nil, errors.New("unexpected UpsertUserSettingsPreservingTaskCreateLastUsed call") } +// UpdateTaskCreateLastUsed records the update inputs and returns the configured result or error. func (r *recordingUserRepository) UpdateTaskCreateLastUsed( _ context.Context, userID string, @@ -1785,6 +1833,7 @@ func (r *recordingUserRepository) UpdateTaskCreateLastUsed( return r.updateSettings, nil } +// Close records the close call. func (r *recordingUserRepository) Close() error { r.closeCalls++ return nil @@ -1795,26 +1844,32 @@ type recordingEventBus struct { publishedEvents []*bus.Event } +// Publish records the published subject and event. func (b *recordingEventBus) Publish(_ context.Context, subject string, event *bus.Event) error { b.publishedSubjects = append(b.publishedSubjects, subject) b.publishedEvents = append(b.publishedEvents, event) return nil } +// Subscribe returns an unexpected-call error. func (b *recordingEventBus) Subscribe(string, bus.EventHandler) (bus.Subscription, error) { return nil, errors.New("unexpected Subscribe call") } +// QueueSubscribe returns an unexpected-call error. func (b *recordingEventBus) QueueSubscribe(string, string, bus.EventHandler) (bus.Subscription, error) { return nil, errors.New("unexpected QueueSubscribe call") } +// Request returns an unexpected-call error. func (b *recordingEventBus) Request(context.Context, string, *bus.Event, time.Duration) (*bus.Event, error) { return nil, errors.New("unexpected Request call") } +// Close is a no-op. func (b *recordingEventBus) Close() {} +// IsConnected reports the fake bus as connected. func (b *recordingEventBus) IsConnected() bool { return true } diff --git a/apps/backend/internal/user/service/user_settings_cas_test.go b/apps/backend/internal/user/service/user_settings_cas_test.go new file mode 100644 index 0000000000..63b2c7c9b5 --- /dev/null +++ b/apps/backend/internal/user/service/user_settings_cas_test.go @@ -0,0 +1,821 @@ +package service + +import ( + "context" + "encoding/json" + "errors" + "strings" + "sync" + "testing" + "time" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/user/models" + "github.com/kandev/kandev/internal/user/store" + "go.uber.org/zap" +) + +// casRead records the revision and editor observed by a single read. +type casRead struct { + revision int64 + defaultEditorID string +} + +// casUpsert records one revision-conditional write attempt. +type casUpsert struct { + expectedRevision int64 + defaultEditorID string + committed bool +} + +// casFakeRepo is a synchronized store.Repository fake for concurrent CAS +// service tests. It clones the current row on every read and commits an +// upsert only when the expected revision matches the current row revision +// (otherwise it returns store.ErrUserSettingsRevisionConflict), modeling the +// repository's revision-conditional write without a database. +type casFakeRepo struct { + mu sync.Mutex + row *models.UserSettings + + reads []casRead + upserts []casUpsert + + // forceConflictNext makes the next upsert return a conflict without + // committing, then clears itself (used to force a retry deterministically). + forceConflictNext bool + + // forceConflictCount makes the next N upserts return a conflict without + // committing (used to force retry exhaustion deterministically). + forceConflictCount int + + // blockNextUpsert, when true, blocks the next upsert until releaseUpsert + // is closed, then clears itself. Used to force a stale-CAS interleaving. + blockNextUpsert bool + releaseUpsert chan struct{} + + // blockUpsert, when non-nil, blocks every upsert until the channel is + // closed. Used to force both concurrent readers to observe the same + // revision before either write commits. + blockUpsert chan struct{} + + // upsertStarted and readDone are buffered phase-observation signals. + upsertStarted chan struct{} + readDone chan struct{} +} + +// newCASFakeRepo builds a casFakeRepo seeded with a clone of row. +func newCASFakeRepo(row *models.UserSettings) *casFakeRepo { + return &casFakeRepo{row: cloneSettings(row)} +} + +// snapshot returns a deep clone of the current row. +func (f *casFakeRepo) snapshot() *models.UserSettings { + f.mu.Lock() + defer f.mu.Unlock() + return cloneSettings(f.row) +} + +// readLog returns a copy of the recorded reads. +func (f *casFakeRepo) readLog() []casRead { + f.mu.Lock() + defer f.mu.Unlock() + return append([]casRead(nil), f.reads...) +} + +// upsertLog returns a copy of the recorded upsert attempts. +func (f *casFakeRepo) upsertLog() []casUpsert { + f.mu.Lock() + defer f.mu.Unlock() + return append([]casUpsert(nil), f.upserts...) +} + +// GetUser returns an unexpected-call error. +func (f *casFakeRepo) GetUser(context.Context, string) (*models.User, error) { + return nil, errors.New("unexpected GetUser call") +} + +// GetDefaultUser returns an unexpected-call error. +func (f *casFakeRepo) GetDefaultUser(context.Context) (*models.User, error) { + return nil, errors.New("unexpected GetDefaultUser call") +} + +// GetUserSettings returns a clone of the row, records the read, and signals phase observers. +func (f *casFakeRepo) GetUserSettings(_ context.Context, _ string) (*models.UserSettings, error) { + f.mu.Lock() + row := cloneSettings(f.row) + f.reads = append(f.reads, casRead{revision: row.Revision, defaultEditorID: row.DefaultEditorID}) + sig := f.readDone + f.mu.Unlock() + if sig != nil { + select { + case sig <- struct{}{}: + default: + } + } + return row, nil +} + +// UpsertUserSettingsPreservingTaskCreateLastUsed commits revision-conditionally, merging the task-create patch, or returns a revision conflict. +func (f *casFakeRepo) UpsertUserSettingsPreservingTaskCreateLastUsed( + _ context.Context, + settings *models.UserSettings, + patch *models.TaskCreateLastUsed, + expectedRevision int64, +) (*models.UserSettings, error) { + f.mu.Lock() + block := f.blockNextUpsert + if block { + f.blockNextUpsert = false + } + rel := f.releaseUpsert + sig := f.upsertStarted + gate := f.blockUpsert + f.mu.Unlock() + + if sig != nil { + select { + case sig <- struct{}{}: + default: + } + } + if block { + <-rel + } + if gate != nil { + <-gate + } + + f.mu.Lock() + defer f.mu.Unlock() + + record := casUpsert{expectedRevision: expectedRevision, defaultEditorID: settings.DefaultEditorID} + if f.forceConflictNext { + f.forceConflictNext = false + f.upserts = append(f.upserts, record) + return nil, store.ErrUserSettingsRevisionConflict + } + if f.forceConflictCount > 0 { + f.forceConflictCount-- + f.upserts = append(f.upserts, record) + return nil, store.ErrUserSettingsRevisionConflict + } + if f.row.Revision != expectedRevision { + f.upserts = append(f.upserts, record) + return nil, store.ErrUserSettingsRevisionConflict + } + next := cloneSettings(settings) + next.TaskCreateLastUsed = applyTaskCreatePatchToFake(next.TaskCreateLastUsed, patch) + next.Revision = f.row.Revision + 1 + f.row = next + record.committed = true + f.upserts = append(f.upserts, record) + return cloneSettings(next), nil +} + +// UpdateTaskCreateLastUsed returns an unexpected-call error. +func (f *casFakeRepo) UpdateTaskCreateLastUsed(context.Context, string, models.TaskCreateLastUsed) (*models.UserSettings, error) { + return nil, errors.New("unexpected UpdateTaskCreateLastUsed call") +} + +// Close is a no-op. +func (f *casFakeRepo) Close() error { return nil } + +// casEventBus is a thread-safe recording bus for concurrent service tests. +type casEventBus struct { + mu sync.Mutex + subjects []string + events []*bus.Event +} + +// Publish records the published subject and event. +func (b *casEventBus) Publish(_ context.Context, subject string, event *bus.Event) error { + b.mu.Lock() + defer b.mu.Unlock() + b.subjects = append(b.subjects, subject) + b.events = append(b.events, event) + return nil +} + +// count returns the number of published events. +func (b *casEventBus) count() int { + b.mu.Lock() + defer b.mu.Unlock() + return len(b.events) +} + +// Subscribe returns an unexpected-call error. +func (b *casEventBus) Subscribe(string, bus.EventHandler) (bus.Subscription, error) { + return nil, errors.New("unexpected Subscribe call") +} + +// QueueSubscribe returns an unexpected-call error. +func (b *casEventBus) QueueSubscribe(string, string, bus.EventHandler) (bus.Subscription, error) { + return nil, errors.New("unexpected QueueSubscribe call") +} + +// Request returns an unexpected-call error. +func (b *casEventBus) Request(context.Context, string, *bus.Event, time.Duration) (*bus.Event, error) { + return nil, errors.New("unexpected Request call") +} + +// Close is a no-op. +func (b *casEventBus) Close() {} + +// IsConnected reports the fake bus as connected. +func (b *casEventBus) IsConnected() bool { return true } + +// newCASService builds a Service wired to the given repository and bus with a nop logger. +func newCASService(repo store.Repository, eventBus bus.EventBus) *Service { + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + panic(err) + } + return NewService(repo, eventBus, log) +} + +// cloneSettings deep-clones settings via JSON so nested maps and slices are never aliased. +func cloneSettings(s *models.UserSettings) *models.UserSettings { + if s == nil { + return nil + } + // Deep clone via JSON so nested slices/maps (TaskCreateLastUsed workflow + // maps, KanbanHiddenStepIDs, LspServerConfigs, KeyboardShortcuts, ...) are + // never aliased with the fake's stored row: a caller mutating a read result + // or a concurrent task-create patch must not corrupt the shared row. + raw, err := json.Marshal(s) + if err != nil { + panic(err) + } + var clone models.UserSettings + if err := json.Unmarshal(raw, &clone); err != nil { + panic(err) + } + return &clone +} + +// applyTaskCreatePatchToFake mirrors the repository's task_create_last_used +// json merge for the fake: non-empty patch fields override the current value, +// and an empty branch is applied when the repository changed (clearing it). +func applyTaskCreatePatchToFake(current models.TaskCreateLastUsed, patch *models.TaskCreateLastUsed) models.TaskCreateLastUsed { + if patch == nil { + return current + } + if patch.RepositoryID != "" { + current.RepositoryID = patch.RepositoryID + } + if patch.Branch != "" || patch.RepositoryID != "" { + current.Branch = patch.Branch + } + if patch.AgentProfileID != "" { + current.AgentProfileID = patch.AgentProfileID + } + if patch.ExecutorProfileID != "" { + current.ExecutorProfileID = patch.ExecutorProfileID + } + if patch.WorkflowIDsByWorkspace != nil { + if current.WorkflowIDsByWorkspace == nil { + current.WorkflowIDsByWorkspace = map[string]string{} + } + for k, v := range patch.WorkflowIDsByWorkspace { + if v == "" { + delete(current.WorkflowIDsByWorkspace, k) + } else { + current.WorkflowIDsByWorkspace[k] = v + } + } + } + return current +} + +// TestApplyLastSeenDisplay verifies applyLastSeenDisplay preserves nil, accepts absolute and relative, rejects invalid, and trims whitespace. +func TestApplyLastSeenDisplay(t *testing.T) { + t.Run("nil leaves settings unchanged", func(t *testing.T) { + settings := &models.UserSettings{LastSeenDisplay: models.LastSeenDisplayRelative} + if err := applyLastSeenDisplay(settings, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if settings.LastSeenDisplay != models.LastSeenDisplayRelative { + t.Fatalf("LastSeenDisplay = %q, want relative", settings.LastSeenDisplay) + } + }) + + t.Run("accepts absolute", func(t *testing.T) { + settings := &models.UserSettings{LastSeenDisplay: models.LastSeenDisplayRelative} + if err := applyLastSeenDisplay(settings, ptr("absolute")); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if settings.LastSeenDisplay != models.LastSeenDisplayAbsolute { + t.Fatalf("LastSeenDisplay = %q, want absolute", settings.LastSeenDisplay) + } + }) + + t.Run("accepts relative", func(t *testing.T) { + settings := &models.UserSettings{} + if err := applyLastSeenDisplay(settings, ptr("relative")); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if settings.LastSeenDisplay != models.LastSeenDisplayRelative { + t.Fatalf("LastSeenDisplay = %q, want relative", settings.LastSeenDisplay) + } + }) + + t.Run("rejects invalid", func(t *testing.T) { + if err := applyLastSeenDisplay(&models.UserSettings{}, ptr("garbage")); err == nil { + t.Fatal("expected error for invalid display, got nil") + } + }) + + t.Run("trims whitespace", func(t *testing.T) { + settings := &models.UserSettings{} + if err := applyLastSeenDisplay(settings, ptr(" relative ")); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if settings.LastSeenDisplay != models.LastSeenDisplayRelative { + t.Fatalf("LastSeenDisplay = %q, want relative", settings.LastSeenDisplay) + } + }) +} + +// TestPublishUserSettingsEventIncludesNormalizedLastSeenDisplay verifies the event normalizes an unknown last_seen_display to absolute. +func TestPublishUserSettingsEventIncludesNormalizedLastSeenDisplay(t *testing.T) { + eventBus := &casEventBus{} + svc := newCASService(&casFakeRepo{row: &models.UserSettings{}}, eventBus) + svc.publishUserSettingsEvent(context.Background(), &models.UserSettings{LastSeenDisplay: "future_value"}) + + if eventBus.count() != 1 { + t.Fatalf("events = %d, want 1", eventBus.count()) + } + eventData, ok := eventBus.events[0].Data.(map[string]interface{}) + if !ok { + t.Fatalf("expected event data map, got %T", eventBus.events[0].Data) + } + if got := eventData["last_seen_display"]; got != models.LastSeenDisplayAbsolute { + t.Fatalf("last_seen_display = %#v, want %q", got, models.LastSeenDisplayAbsolute) + } +} + +// TestUpdateUserSettingsPublishesLastSeenDisplay verifies UpdateUserSettings publishes the applied last_seen_display. +func TestUpdateUserSettingsPublishesLastSeenDisplay(t *testing.T) { + updated := &models.UserSettings{ + UserID: store.DefaultUserID, + LastSeenDisplay: models.LastSeenDisplayRelative, + } + repo := &recordingUserRepository{ + getSettings: &models.UserSettings{UserID: store.DefaultUserID}, + preservingSettings: updated, + } + eventBus := &recordingEventBus{} + svc := newCASService(repo, eventBus) + + if _, err := svc.UpdateUserSettings(context.Background(), &UpdateUserSettingsRequest{LastSeenDisplay: ptr(models.LastSeenDisplayRelative)}); err != nil { + t.Fatalf("UpdateUserSettings: %v", err) + } + if len(eventBus.publishedEvents) != 1 { + t.Fatalf("events = %d, want 1", len(eventBus.publishedEvents)) + } + eventData, ok := eventBus.publishedEvents[0].Data.(map[string]interface{}) + if !ok { + t.Fatalf("expected event data map, got %T", eventBus.publishedEvents[0].Data) + } + if got := eventData["last_seen_display"]; got != models.LastSeenDisplayRelative { + t.Fatalf("last_seen_display = %#v, want %q", got, models.LastSeenDisplayRelative) + } +} + +// TestUpdateUserSettingsRejectsInvalidLastSeenDisplay verifies an invalid last_seen_display is rejected without an upsert. +func TestUpdateUserSettingsRejectsInvalidLastSeenDisplay(t *testing.T) { + repo := &recordingUserRepository{getSettings: &models.UserSettings{UserID: store.DefaultUserID}} + eventBus := &recordingEventBus{} + svc := newCASService(repo, eventBus) + + if _, err := svc.UpdateUserSettings(context.Background(), &UpdateUserSettingsRequest{LastSeenDisplay: ptr("garbage")}); err == nil { + t.Fatal("expected validation error, got nil") + } + if repo.upsertUserSettingsPreservingLastUsedCalls != 0 { + t.Fatalf("expected no upsert on validation failure, got %d", repo.upsertUserSettingsPreservingLastUsedCalls) + } +} + +// TestUpdateUserSettingsEmptyPatchIsNoop verifies an empty patch performs no write or publish. +func TestUpdateUserSettingsEmptyPatchIsNoop(t *testing.T) { + base := &models.UserSettings{ + UserID: store.DefaultUserID, + LastSeenDisplay: models.LastSeenDisplayAbsolute, + Revision: 7, + } + repo := &recordingUserRepository{getSettings: base} + eventBus := &recordingEventBus{} + svc := newCASService(repo, eventBus) + + got, err := svc.UpdateUserSettings(context.Background(), &UpdateUserSettingsRequest{}) + if err != nil { + t.Fatalf("UpdateUserSettings: %v", err) + } + if repo.upsertUserSettingsPreservingLastUsedCalls != 0 { + t.Fatalf("empty patch upserts = %d, want 0", repo.upsertUserSettingsPreservingLastUsedCalls) + } + if len(eventBus.publishedEvents) != 0 { + t.Fatalf("empty patch events = %d, want 0", len(eventBus.publishedEvents)) + } + if got == nil || got.Revision != base.Revision { + t.Fatalf("no-op returned settings = %+v, want current revision %d", got, base.Revision) + } +} + +// TestUpdateUserSettingsSameValuePatchIsNoop verifies a same-value patch performs no write or publish. +func TestUpdateUserSettingsSameValuePatchIsNoop(t *testing.T) { + base := &models.UserSettings{ + UserID: store.DefaultUserID, + LastSeenDisplay: models.LastSeenDisplayRelative, + Revision: 7, + } + repo := &recordingUserRepository{getSettings: base} + eventBus := &recordingEventBus{} + svc := newCASService(repo, eventBus) + + relative := models.LastSeenDisplayRelative + got, err := svc.UpdateUserSettings(context.Background(), &UpdateUserSettingsRequest{LastSeenDisplay: &relative}) + if err != nil { + t.Fatalf("UpdateUserSettings: %v", err) + } + if repo.upsertUserSettingsPreservingLastUsedCalls != 0 { + t.Fatalf("same-value patch upserts = %d, want 0", repo.upsertUserSettingsPreservingLastUsedCalls) + } + if len(eventBus.publishedEvents) != 0 { + t.Fatalf("same-value patch events = %d, want 0", len(eventBus.publishedEvents)) + } + if got == nil || got.Revision != base.Revision { + t.Fatalf("no-op returned settings = %+v, want current revision %d", got, base.Revision) + } +} + +// TestUpdateUserSettingsCASExhaustionWrapsConflict verifies callers can +// distinguish a bounded retry exhaustion from an unexpected storage failure. +func TestUpdateUserSettingsCASExhaustionWrapsConflict(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{UserID: store.DefaultUserID}) + repo.forceConflictCount = maxUserSettingsCASAttempts + svc := newCASService(repo, &casEventBus{}) + + _, err := svc.UpdateUserSettings(context.Background(), &UpdateUserSettingsRequest{ + LastSeenDisplay: ptr(models.LastSeenDisplayRelative), + }) + if err == nil { + t.Fatal("expected retry exhaustion error, got nil") + } + if !errors.Is(err, store.ErrUserSettingsRevisionConflict) { + t.Fatalf("error = %v, want ErrUserSettingsRevisionConflict", err) + } + if !strings.Contains(err.Error(), "exhausted") { + t.Fatalf("error = %v, want retry exhaustion context", err) + } + if got := len(repo.upsertLog()); got != maxUserSettingsCASAttempts { + t.Fatalf("upsert attempts = %d, want %d", got, maxUserSettingsCASAttempts) + } +} + +// TestConcurrentPatchMergesLastSeenDisplay verifies concurrent patches merge via a single CAS retry. +func TestConcurrentPatchMergesLastSeenDisplay(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{ + UserID: store.DefaultUserID, + LastSeenDisplay: models.LastSeenDisplayAbsolute, + }) + repo.readDone = make(chan struct{}, 4) + repo.blockUpsert = make(chan struct{}) + eventBus := &casEventBus{} + svc := newCASService(repo, eventBus) + + ctx := context.Background() + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{LastSeenDisplay: ptr(models.LastSeenDisplayRelative)}); err != nil { + t.Errorf("relative patch: %v", err) + } + }() + go func() { + defer wg.Done() + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{AppStatusBarEnabled: ptr(true)}); err != nil { + t.Errorf("status-bar patch: %v", err) + } + }() + + // Barrier: both initial reads must observe the same revision before either + // write commits, so one stale upsert is forced to conflict and retry. + <-repo.readDone + <-repo.readDone + close(repo.blockUpsert) + wg.Wait() + + row := repo.snapshot() + if row.LastSeenDisplay != models.LastSeenDisplayRelative { + t.Fatalf("final LastSeenDisplay = %q, want relative", row.LastSeenDisplay) + } + if !row.AppStatusBarEnabled { + t.Fatal("final AppStatusBarEnabled = false, want true") + } + if row.Revision != 2 { + t.Fatalf("final revision = %d, want 2", row.Revision) + } + if got := eventBus.count(); got != 2 { + t.Fatalf("events = %d, want 2", got) + } + assertOneStaleConflictAndRetry(t, repo) +} + +// TestCASFakeCloneIsDeep pins the fake's clone contract: mutating a read +// result's nested maps must not corrupt the fake's stored row. +func TestCASFakeCloneIsDeep(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{ + UserID: store.DefaultUserID, + TaskCreateLastUsed: models.TaskCreateLastUsed{ + WorkflowIDsByWorkspace: map[string]string{"w1": "f1"}, + }, + KeyboardShortcuts: map[string]interface{}{}, + }) + + got, err := repo.GetUserSettings(context.Background(), store.DefaultUserID) + if err != nil { + t.Fatalf("read: %v", err) + } + got.TaskCreateLastUsed.WorkflowIDsByWorkspace["w1"] = "mutated" + got.KeyboardShortcuts["x"] = map[string]interface{}{"key": "y"} + + row := repo.snapshot() + if row.TaskCreateLastUsed.WorkflowIDsByWorkspace["w1"] != "f1" { + t.Fatalf("read clone aliased the workflow map: %+v", row.TaskCreateLastUsed.WorkflowIDsByWorkspace) + } + if _, ok := row.KeyboardShortcuts["x"]; ok { + t.Fatal("read clone aliased the keyboard shortcuts map") + } +} + +// TestConcurrentTaskCreateMapPatchesMerge proves two concurrent task-create +// patches with workflow maps survive the CAS retry and merge without +// corrupting the fake's stored row (deep clone requirement). +func TestConcurrentTaskCreateMapPatchesMerge(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{ + UserID: store.DefaultUserID, + TaskCreateLastUsed: models.TaskCreateLastUsed{ + WorkflowIDsByWorkspace: map[string]string{"w0": "f0"}, + }, + }) + repo.readDone = make(chan struct{}, 4) + repo.blockUpsert = make(chan struct{}) + eventBus := &casEventBus{} + svc := newCASService(repo, eventBus) + + ctx := context.Background() + patchA := models.TaskCreateLastUsed{WorkflowIDsByWorkspace: map[string]string{"wA": "fA"}} + patchB := models.TaskCreateLastUsed{WorkflowIDsByWorkspace: map[string]string{"wB": "fB"}} + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{TaskCreateLastUsed: &patchA}); err != nil { + t.Errorf("patch A: %v", err) + } + }() + go func() { + defer wg.Done() + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{TaskCreateLastUsed: &patchB}); err != nil { + t.Errorf("patch B: %v", err) + } + }() + + <-repo.readDone + <-repo.readDone + close(repo.blockUpsert) + wg.Wait() + + row := repo.snapshot() + workflows := row.TaskCreateLastUsed.WorkflowIDsByWorkspace + if workflows["w0"] != "f0" || workflows["wA"] != "fA" || workflows["wB"] != "fB" { + t.Fatalf("workflow maps did not merge: %+v", workflows) + } + assertOneStaleConflictAndRetry(t, repo) +} + +// assertOneStaleConflictAndRetry pins the CAS retry interleaving: one write +// commits at the shared initial revision, the other conflicts once and +// re-reads/re-applies, so the repo sees two commits, exactly one stale +// conflict, and a fresh retry re-read. +func assertOneStaleConflictAndRetry(t *testing.T, repo *casFakeRepo) { + t.Helper() + var commits, conflicts int + for _, u := range repo.upsertLog() { + if u.committed { + commits++ + } else { + conflicts++ + } + } + if commits != 2 { + t.Fatalf("commits = %d, want 2", commits) + } + if conflicts != 1 { + t.Fatalf("stale conflicts = %d, want exactly 1", conflicts) + } + if reads := len(repo.readLog()); reads != 3 { + t.Fatalf("reads = %d, want 3 (two initial + one retry re-read)", reads) + } +} + +// TestConcurrentClearEditorAndPatchMerge verifies a concurrent Clear and patch both commit via a single CAS retry. +func TestConcurrentClearEditorAndPatchMerge(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{ + UserID: store.DefaultUserID, + DefaultEditorID: "editor-1", + LastSeenDisplay: models.LastSeenDisplayAbsolute, + }) + repo.readDone = make(chan struct{}, 4) + repo.blockUpsert = make(chan struct{}) + eventBus := &casEventBus{} + svc := newCASService(repo, eventBus) + + ctx := context.Background() + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + if err := svc.ClearDefaultEditorID(ctx, "editor-1"); err != nil { + t.Errorf("clear editor: %v", err) + } + }() + go func() { + defer wg.Done() + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{LastSeenDisplay: ptr(models.LastSeenDisplayRelative)}); err != nil { + t.Errorf("relative patch: %v", err) + } + }() + + // Barrier: both initial reads must observe the same revision before either + // write commits, so one stale upsert is forced to conflict and retry. + <-repo.readDone + <-repo.readDone + close(repo.blockUpsert) + wg.Wait() + + row := repo.snapshot() + if row.DefaultEditorID != "" { + t.Fatalf("final DefaultEditorID = %q, want empty (cleared)", row.DefaultEditorID) + } + if row.LastSeenDisplay != models.LastSeenDisplayRelative { + t.Fatalf("final LastSeenDisplay = %q, want relative", row.LastSeenDisplay) + } + if row.Revision != 2 { + t.Fatalf("final revision = %d, want 2", row.Revision) + } + if got := eventBus.count(); got != 2 { + t.Fatalf("events = %d, want 2", got) + } + assertOneStaleConflictAndRetry(t, repo) +} + +// TestClearDefaultEditorIDConflictNoop forces a real CAS conflict for a Clear +// whose stale read saw the old editor: the callback re-evaluates against the +// retry's fresh read and returns applied=false, so the Clear writes and +// publishes nothing while the competing PATCH's effect survives. +func TestClearDefaultEditorIDConflictNoop(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{ + UserID: store.DefaultUserID, + DefaultEditorID: "old-editor", + }) + repo.blockNextUpsert = true + repo.releaseUpsert = make(chan struct{}) + repo.upsertStarted = make(chan struct{}, 4) + repo.readDone = make(chan struct{}, 4) + eventBus := &casEventBus{} + svc := newCASService(repo, eventBus) + + ctx := context.Background() + clearDone := make(chan error, 1) + go func() { + clearDone <- svc.ClearDefaultEditorID(ctx, "old-editor") + }() + + // PHASE 1: Clear reads the old editor, then blocks at its upsert. + <-repo.readDone + <-repo.upsertStarted + + // PHASE 2: a competing PATCH commits a new editor. + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{DefaultEditorID: ptr("new-editor")}); err != nil { + t.Fatalf("competing patch: %v", err) + } + + // PHASE 3: release the stale Clear upsert; it conflicts and re-evaluates. + close(repo.releaseUpsert) + if err := <-clearDone; err != nil { + t.Fatalf("ClearDefaultEditorID: %v", err) + } + + row := repo.snapshot() + if row.DefaultEditorID != "new-editor" { + t.Fatalf("final DefaultEditorID = %q, want new-editor", row.DefaultEditorID) + } + if row.Revision != 1 { + t.Fatalf("final revision = %d, want 1", row.Revision) + } + if got := eventBus.count(); got != 1 { + t.Fatalf("events = %d, want exactly 1 (the PATCH)", got) + } + + // The Clear contributed exactly one upsert attempt (the forced stale CAS) + // and no successful write. + var clearAttempts, clearCommits int + for _, u := range repo.upsertLog() { + if u.defaultEditorID == "" { + clearAttempts++ + if u.committed { + clearCommits++ + } + } + } + if clearAttempts != 1 { + t.Fatalf("clear upsert attempts = %d, want 1", clearAttempts) + } + if clearCommits != 0 { + t.Fatalf("clear successful upserts = %d, want 0", clearCommits) + } + if len(repo.readLog()) < 3 { + t.Fatalf("reads = %d, want at least 3 (clear read, patch read, clear retry re-read)", len(repo.readLog())) + } +} + +// TestClearDefaultEditorIDFreshReadNoop verifies Clear after the competing patch committed is an immediate no-op. +func TestClearDefaultEditorIDFreshReadNoop(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{ + UserID: store.DefaultUserID, + DefaultEditorID: "old-editor", + }) + eventBus := &casEventBus{} + svc := newCASService(repo, eventBus) + + ctx := context.Background() + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{DefaultEditorID: ptr("new-editor")}); err != nil { + t.Fatalf("competing patch: %v", err) + } + + // Clear starts after the PATCH committed: immediate no-op, no conflict. + if err := svc.ClearDefaultEditorID(ctx, "old-editor"); err != nil { + t.Fatalf("ClearDefaultEditorID: %v", err) + } + + row := repo.snapshot() + if row.DefaultEditorID != "new-editor" { + t.Fatalf("final DefaultEditorID = %q, want new-editor", row.DefaultEditorID) + } + if got := eventBus.count(); got != 1 { + t.Fatalf("events = %d, want exactly 1 (the PATCH)", got) + } + if len(repo.upsertLog()) != 1 { + t.Fatalf("upserts = %d, want exactly 1 (the PATCH)", len(repo.upsertLog())) + } +} + +// TestUpdateUserSettingsTaskCreateOnlyConflictRetry pins the task-create-only +// PATCH under the CAS rewrite: a forced conflict survives the retry, merges +// against the winner's row, and increments the revision once on its own +// successful retry. +func TestUpdateUserSettingsTaskCreateOnlyConflictRetry(t *testing.T) { + repo := newCASFakeRepo(&models.UserSettings{ + UserID: store.DefaultUserID, + TaskCreateLastUsed: models.TaskCreateLastUsed{ + RepositoryID: "repo-1", + }, + }) + eventBus := &casEventBus{} + svc := newCASService(repo, eventBus) + + ctx := context.Background() + + // Winner commits first, bumping R to R+1. + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{AppStatusBarEnabled: ptr(true)}); err != nil { + t.Fatalf("winner patch: %v", err) + } + + // Force the task-create-only PATCH's first upsert into a conflict. + repo.mu.Lock() + repo.forceConflictNext = true + repo.mu.Unlock() + + patch := models.TaskCreateLastUsed{RepositoryID: "repo-2"} + if _, err := svc.UpdateUserSettings(ctx, &UpdateUserSettingsRequest{TaskCreateLastUsed: &patch}); err != nil { + t.Fatalf("task-create patch: %v", err) + } + + row := repo.snapshot() + if !row.AppStatusBarEnabled { + t.Fatal("winner's AppStatusBarEnabled was reverted by the retry") + } + if row.TaskCreateLastUsed.RepositoryID != "repo-2" { + t.Fatalf("final task-create repository = %q, want repo-2", row.TaskCreateLastUsed.RepositoryID) + } + if row.Revision != 2 { + t.Fatalf("final revision = %d, want 2", row.Revision) + } + if got := eventBus.count(); got != 2 { + t.Fatalf("events = %d, want 2", got) + } +} diff --git a/apps/backend/internal/user/store/sqlite.go b/apps/backend/internal/user/store/sqlite.go index 59f0f3f79f..749545c9ca 100644 --- a/apps/backend/internal/user/store/sqlite.go +++ b/apps/backend/internal/user/store/sqlite.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "sort" "strings" @@ -272,6 +273,7 @@ func (r *sqliteRepository) UpsertUserSettingsPreservingTaskCreateLastUsed( ctx context.Context, settings *models.UserSettings, patch *models.TaskCreateLastUsed, + expectedRevision int64, ) (*models.UserSettings, error) { settings.UpdatedAt = time.Now().UTC() if settings.CreatedAt.IsZero() { @@ -282,7 +284,7 @@ func (r *sqliteRepository) UpsertUserSettingsPreservingTaskCreateLastUsed( return nil, err } if dialect.IsPostgres(r.db.DriverName()) { - return r.upsertUserSettingsPreservingTaskCreateLastUsedPostgres(ctx, settings, settingsPayload, patch) + return r.upsertUserSettingsPreservingTaskCreateLastUsedPostgres(ctx, settings, settingsPayload, patch, expectedRevision) } settingsExpr := `json_set( json(?), @@ -304,29 +306,30 @@ func (r *sqliteRepository) UpsertUserSettingsPreservingTaskCreateLastUsed( query := fmt.Sprintf(` UPDATE users SET settings = %s, updated_at = ?, settings_revision = settings_revision + 1 - WHERE id = ? + WHERE id = ? AND settings_revision = ? RETURNING settings, updated_at, settings_revision `, settingsExpr) - args = append(args, settings.UpdatedAt, settings.UserID) - return scanUpdatedUserSettings( + args = append(args, settings.UpdatedAt, settings.UserID, expectedRevision) + return r.scanConditionalSettingsUpdate( + ctx, r.db.QueryRowContext(ctx, r.db.Rebind(query), args...), settings.UserID, ) } -// upsertUserSettingsPreservingTaskCreateLastUsedPostgres writes settings via -// a jsonb_set expression so task_create_last_used merges instead of -// replacing. +// upsertUserSettingsPreservingTaskCreateLastUsedPostgres writes the settings via a Postgres jsonb_set UPDATE that merges the task_create_last_used patch. func (r *sqliteRepository) upsertUserSettingsPreservingTaskCreateLastUsedPostgres( ctx context.Context, settings *models.UserSettings, settingsPayload []byte, patch *models.TaskCreateLastUsed, + expectedRevision int64, ) (*models.UserSettings, error) { query, patchArgs := buildPostgresUserSettingsPreservingTaskCreateLastUsedUpdate(patch) args := append([]any{string(settingsPayload)}, patchArgs...) - args = append(args, settings.UpdatedAt, settings.UserID) - return scanUpdatedUserSettings( + args = append(args, settings.UpdatedAt, settings.UserID, expectedRevision) + return r.scanConditionalSettingsUpdate( + ctx, r.db.QueryRowContext(ctx, r.db.Rebind(query), args...), settings.UserID, ) @@ -471,7 +474,7 @@ func buildPostgresUserSettingsPreservingTaskCreateLastUsedUpdate(patch *models.T query := fmt.Sprintf(` UPDATE users SET settings = %s::text, updated_at = ?, settings_revision = settings_revision + 1 - WHERE id = ? + WHERE id = ? AND settings_revision = ? RETURNING settings, updated_at, settings_revision `, expr) return query, args @@ -611,6 +614,7 @@ func marshalUserSettingsPayload(settings *models.UserSettings) ([]byte, error) { "terminal_font_family": settings.TerminalFontFamily, "terminal_font_size": settings.TerminalFontSize, "changes_panel_layout": settings.ChangesPanelLayout, + "last_seen_display": models.NormalizeLastSeenDisplay(settings.LastSeenDisplay), "system_metrics_display": settings.SystemMetricsDisplay, "app_status_bar_enabled": settings.AppStatusBarEnabled, "app_status_bar_order": normalizeAppStatusBarOrder(settings.AppStatusBarOrder), @@ -628,6 +632,26 @@ func scanUpdatedUserSettings(scanner interface{ Scan(dest ...any) error }, userI return settings, err } +// scanConditionalSettingsUpdate scans a revision-conditional settings UPDATE, +// mapping a zero-row result to either a missing user (user-not-found) or a +// stale revision (ErrUserSettingsRevisionConflict). +func (r *sqliteRepository) scanConditionalSettingsUpdate(ctx context.Context, scanner interface{ Scan(dest ...any) error }, userID string) (*models.UserSettings, error) { + settings, err := scanUserSettings(scanner, userID) + if err == nil { + return settings, nil + } + if !errors.Is(err, sql.ErrNoRows) { + return nil, err + } + if _, uerr := r.getUserFromWriter(ctx, userID); uerr != nil { + if errors.Is(uerr, sql.ErrNoRows) { + return nil, fmt.Errorf("user not found: %s", userID) + } + return nil, uerr + } + return nil, ErrUserSettingsRevisionConflict +} + // scanUser scans a single user row into a models.User. func scanUser(scanner interface{ Scan(dest ...any) error }) (*models.User, error) { user := &models.User{} @@ -667,6 +691,7 @@ func defaultUserSettings(userID string) *models.UserSettings { KeyboardShortcuts: map[string]interface{}{}, TerminalLinkBehavior: "new_tab", ChangesPanelLayout: defaultChangesPanelLayout, + LastSeenDisplay: models.LastSeenDisplayAbsolute, SidebarViews: DefaultSidebarViews(), SidebarActiveViewID: DefaultSidebarViewID, SidebarTaskPrefs: normalizeSidebarTaskPrefs(models.SidebarTaskPrefs{}), @@ -751,6 +776,7 @@ func scanUserSettings(scanner interface{ Scan(dest ...any) error }, userID strin TerminalFontFamily string `json:"terminal_font_family"` TerminalFontSize int `json:"terminal_font_size"` ChangesPanelLayout string `json:"changes_panel_layout"` + LastSeenDisplay json.RawMessage `json:"last_seen_display"` SystemMetricsDisplay models.SystemMetricsDisplaySettings `json:"system_metrics_display"` AppStatusBarEnabled *bool `json:"app_status_bar_enabled"` AppStatusBarOrder models.AppStatusBarOrder `json:"app_status_bar_order"` @@ -884,10 +910,27 @@ func scanUserSettings(scanner interface{ Scan(dest ...any) error }, userID strin } else { settings.ChangesPanelLayout = defaultChangesPanelLayout } + settings.LastSeenDisplay = normalizeLastSeenDisplayStored(payload.LastSeenDisplay) settings.KanbanHiddenStepIDs = decodeKanbanHiddenStepIDs(payload.KanbanHiddenStepIDs) return settings, nil } +// normalizeLastSeenDisplayStored maps a stored JSON value to the canonical +// display mode: only a string exactly equal to "relative" is accepted. Every +// other JSON type (number, object, boolean, null) and every other string +// coerces to "absolute", so a hand-edited blob can never fail the settings +// read or produce a broken column. +func normalizeLastSeenDisplayStored(raw json.RawMessage) string { + if len(raw) == 0 { + return models.LastSeenDisplayAbsolute + } + var value string + if err := json.Unmarshal(raw, &value); err != nil { + return models.LastSeenDisplayAbsolute + } + return models.NormalizeLastSeenDisplay(value) +} + // decodeKanbanHiddenStepIDs parses the persisted per-workflow hidden-step-id // map, defaulting to an empty map on a missing or malformed value instead of // failing the whole settings read. Unlike most structured settings fields, diff --git a/apps/backend/internal/user/store/sqlite_last_seen_cas_test.go b/apps/backend/internal/user/store/sqlite_last_seen_cas_test.go new file mode 100644 index 0000000000..e161b0b818 --- /dev/null +++ b/apps/backend/internal/user/store/sqlite_last_seen_cas_test.go @@ -0,0 +1,502 @@ +package store + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + + "github.com/kandev/kandev/internal/testutil" + "github.com/kandev/kandev/internal/user/models" +) + +// TestDefaultUserSettingsLastSeenDisplay verifies the default LastSeenDisplay is absolute. +func TestDefaultUserSettingsLastSeenDisplay(t *testing.T) { + settings := defaultUserSettings(DefaultUserID) + if settings.LastSeenDisplay != models.LastSeenDisplayAbsolute { + t.Fatalf("default LastSeenDisplay = %q, want %q", settings.LastSeenDisplay, models.LastSeenDisplayAbsolute) + } +} + +// TestScanUserSettingsLastSeenDisplay verifies last_seen_display defaults to absolute, loads stored values, and coerces unknown values. +func TestScanUserSettingsLastSeenDisplay(t *testing.T) { + tests := []struct { + name string + raw string + want string + }{ + {name: "default empty", raw: "{}", want: models.LastSeenDisplayAbsolute}, + {name: "relative stored", raw: `{"last_seen_display":"relative"}`, want: models.LastSeenDisplayRelative}, + {name: "absolute stored", raw: `{"last_seen_display":"absolute"}`, want: models.LastSeenDisplayAbsolute}, + {name: "unknown coerced", raw: `{"last_seen_display":"garbage"}`, want: models.LastSeenDisplayAbsolute}, + {name: "empty coerced", raw: `{"last_seen_display":""}`, want: models.LastSeenDisplayAbsolute}, + {name: "number coerced", raw: `{"last_seen_display":123}`, want: models.LastSeenDisplayAbsolute}, + {name: "object coerced", raw: `{"last_seen_display":{"x":1}}`, want: models.LastSeenDisplayAbsolute}, + {name: "boolean coerced", raw: `{"last_seen_display":true}`, want: models.LastSeenDisplayAbsolute}, + {name: "null coerced", raw: `{"last_seen_display":null}`, want: models.LastSeenDisplayAbsolute}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + settings, err := scanUserSettings(settingsScanner{raw: tt.raw}, DefaultUserID) + if err != nil { + t.Fatalf("scan settings: %v", err) + } + if settings.LastSeenDisplay != tt.want { + t.Fatalf("LastSeenDisplay = %q, want %q", settings.LastSeenDisplay, tt.want) + } + }) + } +} + +// TestMarshalUserSettingsPersistsLastSeenDisplay verifies a relative LastSeenDisplay survives a marshal and scan round trip. +func TestMarshalUserSettingsPersistsLastSeenDisplay(t *testing.T) { + raw, err := marshalUserSettingsPayload(&models.UserSettings{LastSeenDisplay: models.LastSeenDisplayRelative}) + if err != nil { + t.Fatalf("marshal settings: %v", err) + } + settings, err := scanUserSettings(settingsScanner{raw: string(raw)}, DefaultUserID) + if err != nil { + t.Fatalf("scan settings: %v", err) + } + if settings.LastSeenDisplay != models.LastSeenDisplayRelative { + t.Fatalf("round-trip LastSeenDisplay = %q, want %q", settings.LastSeenDisplay, models.LastSeenDisplayRelative) + } +} + +// TestMarshalUserSettingsNormalizesUnknownLastSeenDisplay verifies an unknown LastSeenDisplay is marshaled as absolute. +func TestMarshalUserSettingsNormalizesUnknownLastSeenDisplay(t *testing.T) { + raw, err := marshalUserSettingsPayload(&models.UserSettings{LastSeenDisplay: "garbage"}) + if err != nil { + t.Fatalf("marshal settings: %v", err) + } + if !strings.Contains(string(raw), `"last_seen_display":"absolute"`) { + t.Fatalf("marshaled payload = %s, want normalized absolute", raw) + } +} + +// TestSQLiteRepositoryLastSeenDisplayRoundTrip verifies LastSeenDisplay round-trips through the SQLite repository. +func TestSQLiteRepositoryLastSeenDisplayRoundTrip(t *testing.T) { + conn, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + conn.SetMaxOpenConns(1) + t.Cleanup(func() { _ = conn.Close() }) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new repo: %v", err) + } + + ctx := context.Background() + settings, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get defaults: %v", err) + } + if settings.LastSeenDisplay != models.LastSeenDisplayAbsolute { + t.Fatalf("default LastSeenDisplay = %q, want %q", settings.LastSeenDisplay, models.LastSeenDisplayAbsolute) + } + + settings.LastSeenDisplay = models.LastSeenDisplayRelative + updated, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, nil, settings.Revision) + if err != nil { + t.Fatalf("write relative: %v", err) + } + if updated.LastSeenDisplay != models.LastSeenDisplayRelative { + t.Fatalf("persisted LastSeenDisplay = %q, want %q", updated.LastSeenDisplay, models.LastSeenDisplayRelative) + } + + updated.LastSeenDisplay = models.LastSeenDisplayAbsolute + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, updated, nil, updated.Revision); err != nil { + t.Fatalf("write absolute: %v", err) + } + + reloaded, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("reload settings: %v", err) + } + if reloaded.LastSeenDisplay != models.LastSeenDisplayAbsolute { + t.Fatalf("reloaded LastSeenDisplay = %q, want %q", reloaded.LastSeenDisplay, models.LastSeenDisplayAbsolute) + } +} + +// TestSQLiteRepositoryUpsertSettingsRevisionConflict verifies a stale revision write fails with ErrUserSettingsRevisionConflict. +func TestSQLiteRepositoryUpsertSettingsRevisionConflict(t *testing.T) { + conn, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + conn.SetMaxOpenConns(1) + t.Cleanup(func() { _ = conn.Close() }) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new repo: %v", err) + } + + ctx := context.Background() + settings, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get defaults: %v", err) + } + staleRevision := settings.Revision + settings.AppStatusBarEnabled = true + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, nil, settings.Revision); err != nil { + t.Fatalf("initial write: %v", err) + } + + stale := *settings + stale.AppStatusBarEnabled = false + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, &stale, nil, staleRevision); !errors.Is(err, ErrUserSettingsRevisionConflict) { + t.Fatalf("stale write error = %v, want ErrUserSettingsRevisionConflict", err) + } +} + +// TestSQLiteRepositoryUpsertSettingsMissingUser verifies upserting settings for a missing user fails with a user-not-found error. +func TestSQLiteRepositoryUpsertSettingsMissingUser(t *testing.T) { + conn, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + conn.SetMaxOpenConns(1) + t.Cleanup(func() { _ = conn.Close() }) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new repo: %v", err) + } + + ctx := context.Background() + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, &models.UserSettings{UserID: "missing-user"}, nil, 0); err == nil { + t.Fatal("missing user upsert = nil, want user not found") + } else if errors.Is(err, ErrUserSettingsRevisionConflict) { + t.Fatalf("missing user upsert returned conflict, want user not found: %v", err) + } else if !strings.Contains(err.Error(), "user not found") { + t.Fatalf("missing user upsert error = %v, want user not found", err) + } +} + +// TestPostgresRepositoryUpsertSettingsRevisionConflict verifies a stale revision write fails with ErrUserSettingsRevisionConflict on Postgres. +func TestPostgresRepositoryUpsertSettingsRevisionConflict(t *testing.T) { + conn := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new postgres repo: %v", err) + } + + ctx := context.Background() + settings, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get defaults: %v", err) + } + staleRevision := settings.Revision + settings.AppStatusBarEnabled = true + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, nil, settings.Revision); err != nil { + t.Fatalf("initial write: %v", err) + } + + stale := *settings + stale.AppStatusBarEnabled = false + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, &stale, nil, staleRevision); !errors.Is(err, ErrUserSettingsRevisionConflict) { + t.Fatalf("stale write error = %v, want ErrUserSettingsRevisionConflict", err) + } +} + +// TestPostgresRepositoryUpsertSettingsMissingUser verifies upserting settings for a missing user fails with a user-not-found error on Postgres. +func TestPostgresRepositoryUpsertSettingsMissingUser(t *testing.T) { + conn := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new postgres repo: %v", err) + } + + ctx := context.Background() + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, &models.UserSettings{UserID: "missing-user"}, nil, 0); err == nil { + t.Fatal("missing user upsert = nil, want user not found") + } else if errors.Is(err, ErrUserSettingsRevisionConflict) { + t.Fatalf("missing user upsert returned conflict, want user not found: %v", err) + } else if !strings.Contains(err.Error(), "user not found") { + t.Fatalf("missing user upsert error = %v, want user not found", err) + } +} + +// TestSQLiteRepositoryConcurrentUpsertOneSucceedsOneConflicts is the migrated +// unique-revision test: two candidates read the same revision, exactly one +// write commits and the other reports a revision conflict (retry lives in the +// service layer). +func TestSQLiteRepositoryConcurrentUpsertOneSucceedsOneConflicts(t *testing.T) { + conn, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + conn.SetMaxOpenConns(1) + t.Cleanup(func() { _ = conn.Close() }) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new repo: %v", err) + } + + ctx := context.Background() + settings, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get defaults: %v", err) + } + expectedRevision := settings.Revision + first := *settings + second := *settings + first.AppStatusBarEnabled = true + second.ChangesPanelLayout = "flat" + + start := make(chan struct{}) + revisions := make(chan int64, 2) + conflicts := make(chan error, 2) + other := make(chan error, 2) + var workers sync.WaitGroup + for _, candidate := range []*models.UserSettings{&first, &second} { + workers.Add(1) + go func(candidate *models.UserSettings) { + defer workers.Done() + <-start + updated, updateErr := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, candidate, nil, expectedRevision) + if updateErr != nil { + if errors.Is(updateErr, ErrUserSettingsRevisionConflict) { + conflicts <- updateErr + } else { + other <- updateErr + } + return + } + revisions <- updated.Revision + }(candidate) + } + close(start) + workers.Wait() + close(revisions) + close(conflicts) + close(other) + + for updateErr := range other { + t.Fatalf("unexpected concurrent update error: %v", updateErr) + } + var got []int64 + for revision := range revisions { + got = append(got, revision) + } + conflictCount := 0 + for range conflicts { + conflictCount++ + } + if len(got) != 1 || got[0] != 1 { + t.Fatalf("successful revisions = %v, want exactly [1]", got) + } + if conflictCount != 1 { + t.Fatalf("conflict count = %d, want exactly 1", conflictCount) + } + final, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get final settings: %v", err) + } + if final.Revision != 1 { + t.Fatalf("final revision = %d, want 1", final.Revision) + } +} + +// TestPostgresRepositoryTaskCreateLastUsedRoundTrip verifies the full-blob +// write merges the current task_create_last_used over the passed blob when a +// stale caller re-reads the current revision. +func TestPostgresRepositoryTaskCreateLastUsedRoundTrip(t *testing.T) { + conn := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new postgres repo: %v", err) + } + + ctx := context.Background() + staleSettings, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get defaults: %v", err) + } + staleSettings.SidebarActiveViewID = "view-before" + staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{ + RepositoryID: "repo-before", + Branch: "main", + AgentProfileID: "agent-before", + ExecutorProfileID: "exec-before", + } + upsertUserSettingsForTest(t, repo, ctx, staleSettings) + + got, err := repo.UpdateTaskCreateLastUsed(ctx, DefaultUserID, models.TaskCreateLastUsed{ + RepositoryID: "repo-after", + Branch: "feature", + AgentProfileID: "agent-after", + }) + if err != nil { + t.Fatalf("update postgres task-create last-used: %v", err) + } + if got.SidebarActiveViewID != "view-before" { + t.Fatalf("unrelated postgres setting should be preserved, got %q", got.SidebarActiveViewID) + } + if got.TaskCreateLastUsed.RepositoryID != "repo-after" || + got.TaskCreateLastUsed.Branch != "feature" || + got.TaskCreateLastUsed.AgentProfileID != "agent-after" || + got.TaskCreateLastUsed.ExecutorProfileID != "exec-before" { + t.Fatalf("postgres task-create update mismatch: %+v", got.TaskCreateLastUsed) + } + if got.Revision < 2 { + t.Fatalf("postgres settings revision = %d, want at least 2", got.Revision) + } + firstRevision := got.Revision + + // A stale caller re-reads the current revision, then writes an unrelated + // change carrying stale task-create fields; the merge must keep the stored + // current values. + current, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("re-read current settings: %v", err) + } + current.SidebarActiveViewID = "view-after" + current.TaskCreateLastUsed = models.TaskCreateLastUsed{ + RepositoryID: "repo-stale", + Branch: "stale", + } + got, err = repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, current, nil, current.Revision) + if err != nil { + t.Fatalf("upsert preserving postgres task-create last-used: %v", err) + } + if got.SidebarActiveViewID != "view-after" { + t.Fatalf("expected unrelated postgres setting to update, got %q", got.SidebarActiveViewID) + } + if got.TaskCreateLastUsed.RepositoryID != "repo-after" || + got.TaskCreateLastUsed.Branch != "feature" || + got.TaskCreateLastUsed.AgentProfileID != "agent-after" || + got.TaskCreateLastUsed.ExecutorProfileID != "exec-before" { + t.Fatalf("postgres preserving upsert should keep current task-create values: %+v", got.TaskCreateLastUsed) + } + if got.Revision != firstRevision+1 { + t.Fatalf("postgres settings revision = %d, want %d", got.Revision, firstRevision+1) + } +} + +// TestSQLiteRepositoryUpsertSettingsPreservesCurrentTaskCreateLastUsed is the +// migrated stale-blob merge test: the caller re-reads the current revision +// before its unrelated write so the task_create_last_used merge survives. +func TestSQLiteRepositoryUpsertSettingsPreservesCurrentTaskCreateLastUsed(t *testing.T) { + conn, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + conn.SetMaxOpenConns(1) + t.Cleanup(func() { _ = conn.Close() }) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new repo: %v", err) + } + + ctx := context.Background() + staleSettings, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get defaults: %v", err) + } + staleSettings.SidebarActiveViewID = "view-before" + staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{RepositoryID: "repo-before"} + upsertUserSettingsForTest(t, repo, ctx, staleSettings) + + // Simulate a task-create write that lands after another settings caller + // read the old blob but before that caller writes its unrelated change. + if _, err := repo.UpdateTaskCreateLastUsed(ctx, DefaultUserID, models.TaskCreateLastUsed{ + RepositoryID: "repo-after", + Branch: "feature", + }); err != nil { + t.Fatalf("update task-create last-used: %v", err) + } + + current, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("re-read current settings: %v", err) + } + current.SidebarActiveViewID = "view-after" + got, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, current, nil, current.Revision) + if err != nil { + t.Fatalf("upsert preserving task-create last-used: %v", err) + } + + if got.SidebarActiveViewID != "view-after" { + t.Fatalf("expected unrelated setting to update, got %q", got.SidebarActiveViewID) + } + if got.TaskCreateLastUsed.RepositoryID != "repo-after" { + t.Fatalf("expected current task-create repository to survive stale write, got %q", got.TaskCreateLastUsed.RepositoryID) + } + if got.TaskCreateLastUsed.Branch != "feature" { + t.Fatalf("expected current task-create branch to survive stale write, got %q", got.TaskCreateLastUsed.Branch) + } +} + +// TestSQLiteRepositoryUpsertSettingsPreservingTaskCreateLastUsedAppliesPatch is +// the migrated patch-path test, re-reading the current revision before the +// direct upsert. +func TestSQLiteRepositoryUpsertSettingsPreservingTaskCreateLastUsedAppliesPatch(t *testing.T) { + conn, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + conn.SetMaxOpenConns(1) + t.Cleanup(func() { _ = conn.Close() }) + repo, err := newSQLiteRepositoryWithDB(conn, conn) + if err != nil { + t.Fatalf("new repo: %v", err) + } + + ctx := context.Background() + staleSettings, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("get defaults: %v", err) + } + staleSettings.SidebarActiveViewID = "view-before" + staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{ + RepositoryID: "repo-before", + Branch: "main", + AgentProfileID: "agent-before", + ExecutorProfileID: "exec-before", + } + upsertUserSettingsForTest(t, repo, ctx, staleSettings) + if _, err := repo.UpdateTaskCreateLastUsed(ctx, DefaultUserID, models.TaskCreateLastUsed{ + RepositoryID: "repo-current", + Branch: "current", + ExecutorProfileID: "exec-current", + }); err != nil { + t.Fatalf("update current task-create last-used: %v", err) + } + + current, err := repo.GetUserSettings(ctx, DefaultUserID) + if err != nil { + t.Fatalf("re-read current settings: %v", err) + } + current.SidebarActiveViewID = "view-after" + current.TaskCreateLastUsed = models.TaskCreateLastUsed{ + RepositoryID: "repo-stale", + Branch: "stale", + AgentProfileID: "agent-stale", + } + patch := models.TaskCreateLastUsed{AgentProfileID: "agent-after"} + got, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, current, &patch, current.Revision) + if err != nil { + t.Fatalf("upsert preserving task-create last-used with patch: %v", err) + } + + if got.SidebarActiveViewID != "view-after" { + t.Fatalf("expected unrelated setting to update, got %q", got.SidebarActiveViewID) + } + if got.TaskCreateLastUsed.RepositoryID != "repo-current" { + t.Fatalf("expected current repository to survive stale write, got %q", got.TaskCreateLastUsed.RepositoryID) + } + if got.TaskCreateLastUsed.Branch != "current" { + t.Fatalf("expected current branch to survive stale write, got %q", got.TaskCreateLastUsed.Branch) + } + if got.TaskCreateLastUsed.AgentProfileID != "agent-after" { + t.Fatalf("expected patch agent profile to apply, got %q", got.TaskCreateLastUsed.AgentProfileID) + } + if got.TaskCreateLastUsed.ExecutorProfileID != "exec-current" { + t.Fatalf("expected current executor profile to survive stale write, got %q", got.TaskCreateLastUsed.ExecutorProfileID) + } +} diff --git a/apps/backend/internal/user/store/sqlite_test.go b/apps/backend/internal/user/store/sqlite_test.go index 25c2a6ec46..71bc4ee839 100644 --- a/apps/backend/internal/user/store/sqlite_test.go +++ b/apps/backend/internal/user/store/sqlite_test.go @@ -4,9 +4,7 @@ import ( "context" "encoding/json" "reflect" - "sort" "strings" - "sync" "testing" "time" @@ -22,17 +20,23 @@ type settingsScanner struct { revision int64 } +// upsertUserSettingsForTest writes settings via UpsertUserSettingsPreservingTaskCreateLastUsed at the current stored revision. func upsertUserSettingsForTest(t *testing.T, repo *sqliteRepository, ctx context.Context, settings *models.UserSettings) { t.Helper() + current, err := repo.GetUserSettings(ctx, settings.UserID) + if err != nil { + t.Fatalf("read current settings: %v", err) + } var patch *models.TaskCreateLastUsed if !reflect.DeepEqual(settings.TaskCreateLastUsed, models.TaskCreateLastUsed{}) { patch = &settings.TaskCreateLastUsed } - if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, patch); err != nil { + if _, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, patch, current.Revision); err != nil { t.Fatalf("upsert settings: %v", err) } } +// Scan copies the scanner's raw settings string, zero time, and revision into dest. func (s settingsScanner) Scan(dest ...any) error { *(dest[0].(*string)) = s.raw *(dest[1].(*time.Time)) = time.Time{} @@ -40,6 +44,7 @@ func (s settingsScanner) Scan(dest ...any) error { return nil } +// TestSQLiteRepositoryMigratesLegacySettingsRevision verifies the legacy settings revision migration against an in-memory SQLite database. func TestSQLiteRepositoryMigratesLegacySettingsRevision(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -50,11 +55,13 @@ func TestSQLiteRepositoryMigratesLegacySettingsRevision(t *testing.T) { assertLegacySettingsRevisionMigration(t, conn) } +// TestPostgresRepositoryMigratesLegacySettingsRevision verifies the legacy settings revision migration against Postgres. func TestPostgresRepositoryMigratesLegacySettingsRevision(t *testing.T) { conn := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) assertLegacySettingsRevisionMigration(t, conn) } +// assertLegacySettingsRevisionMigration verifies a legacy settings blob migrates to revision 0, bumps to 1 on write, and survives a migration replay. func assertLegacySettingsRevisionMigration(t *testing.T, conn *sqlx.DB) { t.Helper() now := time.Now().UTC() @@ -93,7 +100,7 @@ func assertLegacySettingsRevisionMigration(t *testing.T, conn *sqlx.DB) { } settings.AppStatusBarEnabled = true settings.UpdatedAt = now.Add(time.Second) - updated, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, nil) + updated, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, settings, nil, settings.Revision) if err != nil { t.Fatalf("write migrated settings: %v", err) } @@ -117,70 +124,7 @@ func assertLegacySettingsRevisionMigration(t *testing.T, conn *sqlx.DB) { } } -func TestSQLiteRepositoryAssignsUniqueAtomicSettingsRevisions(t *testing.T) { - conn, err := sqlx.Open("sqlite3", ":memory:") - if err != nil { - t.Fatalf("open sqlite: %v", err) - } - conn.SetMaxOpenConns(1) - t.Cleanup(func() { _ = conn.Close() }) - repo, err := newSQLiteRepositoryWithDB(conn, conn) - if err != nil { - t.Fatalf("new repo: %v", err) - } - - ctx := context.Background() - settings, err := repo.GetUserSettings(ctx, DefaultUserID) - if err != nil { - t.Fatalf("get defaults: %v", err) - } - first := *settings - second := *settings - first.AppStatusBarEnabled = true - second.ChangesPanelLayout = "flat" - - start := make(chan struct{}) - revisions := make(chan int64, 2) - errors := make(chan error, 2) - var workers sync.WaitGroup - for _, candidate := range []*models.UserSettings{&first, &second} { - workers.Add(1) - go func(candidate *models.UserSettings) { - defer workers.Done() - <-start - updated, updateErr := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, candidate, nil) - if updateErr != nil { - errors <- updateErr - return - } - revisions <- updated.Revision - }(candidate) - } - close(start) - workers.Wait() - close(errors) - close(revisions) - for updateErr := range errors { - t.Fatalf("concurrent update: %v", updateErr) - } - - got := make([]int64, 0, 2) - for revision := range revisions { - got = append(got, revision) - } - sort.Slice(got, func(i, j int) bool { return got[i] < got[j] }) - if !reflect.DeepEqual(got, []int64{1, 2}) { - t.Fatalf("revisions = %v, want unique commit order [1 2]", got) - } - final, err := repo.GetUserSettings(ctx, DefaultUserID) - if err != nil { - t.Fatalf("get final settings: %v", err) - } - if final.Revision != 2 { - t.Fatalf("final revision = %d, want 2", final.Revision) - } -} - +// TestScanUserSettingsStartupPage verifies startup_page defaults to task_overview and preserves an explicit last_task value. func TestScanUserSettingsStartupPage(t *testing.T) { tests := []struct { name string @@ -214,6 +158,7 @@ func TestScanUserSettingsStartupPage(t *testing.T) { } } +// TestScanUserSettingsSidebarDefaults verifies the canonical default sidebar view and that explicit sidebar settings are preserved. func TestScanUserSettingsSidebarDefaults(t *testing.T) { defaultView := models.SidebarView{ ID: "view-all-tasks", @@ -260,6 +205,7 @@ func TestScanUserSettingsSidebarDefaults(t *testing.T) { } } +// TestScanUserSettingsPreservesExplicitEmptySidebarSettings verifies explicitly empty sidebar view lists survive scanning. func TestScanUserSettingsPreservesExplicitEmptySidebarSettings(t *testing.T) { for _, raw := range []string{ `{"sidebar_views":[],"sidebar_active_view_id":""}`, @@ -280,6 +226,7 @@ func TestScanUserSettingsPreservesExplicitEmptySidebarSettings(t *testing.T) { } } +// TestScanUserSettingsChangesPanelLayoutDefault verifies changes_panel_layout defaults to tree and preserves an explicit flat value. func TestScanUserSettingsChangesPanelLayoutDefault(t *testing.T) { t.Run("empty settings default to tree", func(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) @@ -312,6 +259,7 @@ func TestScanUserSettingsChangesPanelLayoutDefault(t *testing.T) { }) } +// TestScanUserSettingsConfirmTaskArchiveDefault verifies confirm_task_archive defaults to true and preserves an explicit false. func TestScanUserSettingsConfirmTaskArchiveDefault(t *testing.T) { tests := []struct { name string @@ -336,6 +284,7 @@ func TestScanUserSettingsConfirmTaskArchiveDefault(t *testing.T) { } } +// TestScanUserSettingsUnreadDividerDefault verifies unread_divider defaults to false and honors explicit values. func TestScanUserSettingsUnreadDividerDefault(t *testing.T) { tests := []struct { name string @@ -361,6 +310,7 @@ func TestScanUserSettingsUnreadDividerDefault(t *testing.T) { } } +// TestScanUserSettingsAgentGeneratedTaskTitlesDefault verifies agent_generated_task_titles defaults to true and honors explicit values. func TestScanUserSettingsAgentGeneratedTaskTitlesDefault(t *testing.T) { tests := []struct { name string @@ -386,6 +336,7 @@ func TestScanUserSettingsAgentGeneratedTaskTitlesDefault(t *testing.T) { } } +// TestScanUserSettingsMCPTaskAgentProfileDefault verifies mcp_task_agent_profile_default defaults to current_task and preserves workspace_default. func TestScanUserSettingsMCPTaskAgentProfileDefault(t *testing.T) { tests := []struct { name string @@ -419,6 +370,7 @@ func TestScanUserSettingsMCPTaskAgentProfileDefault(t *testing.T) { } } +// TestScanUserSettingsShowAnchoredPromptBarDefault verifies show_anchored_prompt_bar defaults to false. func TestScanUserSettingsShowAnchoredPromptBarDefault(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) if err != nil { @@ -437,6 +389,7 @@ func TestScanUserSettingsShowAnchoredPromptBarDefault(t *testing.T) { } } +// TestScanUserSettingsTranscriptNavigationDefaults verifies transcript navigation control defaults and stored preferences. func TestScanUserSettingsTranscriptNavigationDefaults(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) if err != nil { @@ -480,6 +433,7 @@ func TestScanUserSettingsTranscriptNavigationDefaults(t *testing.T) { } } +// TestScanUserSettingsDefaultsTranscriptAutoScrollControlToHidden verifies the transcript auto-scroll control defaults to hidden. func TestScanUserSettingsDefaultsTranscriptAutoScrollControlToHidden(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) if err != nil { @@ -490,6 +444,7 @@ func TestScanUserSettingsDefaultsTranscriptAutoScrollControlToHidden(t *testing. } } +// TestTranscriptNavigationSettingsRoundTripThroughMarshalAndScan verifies transcript navigation settings survive a marshal and scan round trip. func TestTranscriptNavigationSettingsRoundTripThroughMarshalAndScan(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{ ShowScrollToLastPrompt: false, @@ -513,6 +468,7 @@ func TestTranscriptNavigationSettingsRoundTripThroughMarshalAndScan(t *testing.T } } +// TestScanUserSettingsTodoListPanelDefault verifies show_todo_list_panel defaults to false and honors explicit values. func TestScanUserSettingsTodoListPanelDefault(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) if err != nil { @@ -545,6 +501,7 @@ func TestScanUserSettingsTodoListPanelDefault(t *testing.T) { } } +// TestScanUserSettingsTodoListPanelOnlyWhenNotEmptyDefault verifies show_todo_list_panel_only_when_not_empty defaults to false and honors explicit values. func TestScanUserSettingsTodoListPanelOnlyWhenNotEmptyDefault(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) if err != nil { @@ -577,6 +534,7 @@ func TestScanUserSettingsTodoListPanelOnlyWhenNotEmptyDefault(t *testing.T) { } } +// TestTodoListPanelSettingRoundTripThroughMarshalAndScan verifies the todo list panel setting survives a marshal and scan round trip. func TestTodoListPanelSettingRoundTripThroughMarshalAndScan(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{ShowTodoListPanel: true}) if err != nil { @@ -591,6 +549,7 @@ func TestTodoListPanelSettingRoundTripThroughMarshalAndScan(t *testing.T) { } } +// TestTodoListPanelOnlyWhenNotEmptyRoundTripThroughMarshalAndScan verifies the todo list panel visibility setting survives a marshal and scan round trip. func TestTodoListPanelOnlyWhenNotEmptyRoundTripThroughMarshalAndScan(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{ ShowTodoListPanelOnlyWhenNotEmpty: true, @@ -607,6 +566,7 @@ func TestTodoListPanelOnlyWhenNotEmptyRoundTripThroughMarshalAndScan(t *testing. } } +// TestMarshalUserSettingsPersistsDisabledArchiveConfirmation verifies marshaling preserves an explicit false confirm_task_archive. func TestMarshalUserSettingsPersistsDisabledArchiveConfirmation(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{ConfirmTaskArchive: false}) if err != nil { @@ -622,6 +582,7 @@ func TestMarshalUserSettingsPersistsDisabledArchiveConfirmation(t *testing.T) { } } +// TestShowAnchoredPromptBarRoundTripsThroughMarshalAndScan verifies show_anchored_prompt_bar survives a marshal and scan round trip. func TestShowAnchoredPromptBarRoundTripsThroughMarshalAndScan(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{ShowAnchoredPromptBar: true}) if err != nil { @@ -636,6 +597,7 @@ func TestShowAnchoredPromptBarRoundTripsThroughMarshalAndScan(t *testing.T) { } } +// TestMarshalUserSettingsPersistsTasksListShowDetails verifies marshaling preserves tasks_list_show_details true. func TestMarshalUserSettingsPersistsTasksListShowDetails(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{TasksListShowDetails: true}) if err != nil { @@ -651,6 +613,7 @@ func TestMarshalUserSettingsPersistsTasksListShowDetails(t *testing.T) { } } +// TestScanUserSettingsTasksListShowDetailsDefaultsAndLoads verifies tasks_list_show_details defaults to false and loads a stored true. func TestScanUserSettingsTasksListShowDetailsDefaultsAndLoads(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) if err != nil { @@ -669,6 +632,7 @@ func TestScanUserSettingsTasksListShowDetailsDefaultsAndLoads(t *testing.T) { } } +// TestMarshalUserSettingsPersistsMCPTaskAgentProfileDefault verifies marshaling preserves the workspace_default MCP task agent profile. func TestMarshalUserSettingsPersistsMCPTaskAgentProfileDefault(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{ MCPTaskAgentProfileDefault: models.MCPTaskAgentProfileDefaultWorkspaceDefault, @@ -686,6 +650,7 @@ func TestMarshalUserSettingsPersistsMCPTaskAgentProfileDefault(t *testing.T) { } } +// TestSQLiteRepositoryMCPTaskAgentProfileDefaultRoundTrip verifies the MCP task agent profile default round-trips through the SQLite repository. func TestSQLiteRepositoryMCPTaskAgentProfileDefaultRoundTrip(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -715,6 +680,7 @@ func TestSQLiteRepositoryMCPTaskAgentProfileDefaultRoundTrip(t *testing.T) { } } +// TestScanUserSettingsLspStatusLocationDefaultsAndLoads verifies lsp_status_location defaults to toolbar and preserves stored values. func TestScanUserSettingsLspStatusLocationDefaultsAndLoads(t *testing.T) { tests := []struct { name string @@ -740,6 +706,7 @@ func TestScanUserSettingsLspStatusLocationDefaultsAndLoads(t *testing.T) { } } +// TestMarshalUserSettingsLspStatusLocation verifies marshaling preserves the stored LSP status location. func TestMarshalUserSettingsLspStatusLocation(t *testing.T) { raw, err := marshalUserSettingsPayload(&models.UserSettings{ LspStatusLocation: models.LspStatusLocationStatusBar, @@ -756,6 +723,7 @@ func TestMarshalUserSettingsLspStatusLocation(t *testing.T) { } } +// TestSQLiteRepositoryLspStatusLocationRoundTrip verifies the LSP status location round-trips through the SQLite repository. func TestSQLiteRepositoryLspStatusLocationRoundTrip(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -785,6 +753,7 @@ func TestSQLiteRepositoryLspStatusLocationRoundTrip(t *testing.T) { } } +// TestScanUserSettingsSystemMetricsDisplayDefault verifies the system metrics display defaults to disabled and honors stored preferences. func TestScanUserSettingsSystemMetricsDisplayDefault(t *testing.T) { settings, err := scanUserSettings(settingsScanner{raw: "{}"}, DefaultUserID) if err != nil { @@ -817,6 +786,7 @@ func TestScanUserSettingsSystemMetricsDisplayDefault(t *testing.T) { } } +// TestSQLiteRepositorySystemMetricsDisplayRoundTrip verifies the system metrics display preference round-trips through the SQLite repository. func TestSQLiteRepositorySystemMetricsDisplayRoundTrip(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -845,6 +815,7 @@ func TestSQLiteRepositorySystemMetricsDisplayRoundTrip(t *testing.T) { } } +// TestSQLiteRepositoryAppStatusBarOrderDefaultAndRoundTrip verifies the status bar order defaults to non-nil empty arrays and round-trips through the SQLite repository. func TestSQLiteRepositoryAppStatusBarOrderDefaultAndRoundTrip(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -879,6 +850,7 @@ func TestSQLiteRepositoryAppStatusBarOrderDefaultAndRoundTrip(t *testing.T) { } } +// TestSQLiteRepositoryKanbanHiddenStepIDsDefaultAndRoundTrip verifies kanban hidden step IDs default to empty and round-trip through the SQLite repository. func TestSQLiteRepositoryKanbanHiddenStepIDsDefaultAndRoundTrip(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -913,6 +885,7 @@ func TestSQLiteRepositoryKanbanHiddenStepIDsDefaultAndRoundTrip(t *testing.T) { } } +// TestScanUserSettingsKanbanHiddenStepIDsCorruptFallsBackToEmpty verifies corrupt kanban_hidden_step_ids values fall back to empty while sibling fields still load. func TestScanUserSettingsKanbanHiddenStepIDsCorruptFallsBackToEmpty(t *testing.T) { tests := []struct { name string @@ -960,6 +933,7 @@ func TestScanUserSettingsKanbanHiddenStepIDsCorruptFallsBackToEmpty(t *testing.T } } +// TestSQLiteRepositoryUpdateTaskCreateLastUsedPatchesNonEmptyFields verifies updating task-create last-used patches only non-empty fields. func TestSQLiteRepositoryUpdateTaskCreateLastUsedPatchesNonEmptyFields(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -1011,6 +985,7 @@ func TestSQLiteRepositoryUpdateTaskCreateLastUsedPatchesNonEmptyFields(t *testin } } +// TestSQLiteRepositoryUpdateTaskCreateLastUsedPreservesWorkflowHistory verifies workflow history entries are merged rather than replaced. func TestSQLiteRepositoryUpdateTaskCreateLastUsedPreservesWorkflowHistory(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -1057,6 +1032,7 @@ func TestSQLiteRepositoryUpdateTaskCreateLastUsedPreservesWorkflowHistory(t *tes } } +// TestSQLiteRepositoryUpdateTaskCreateLastUsedClearsBranchOnRepositoryChange verifies the branch is cleared when the repository changes. func TestSQLiteRepositoryUpdateTaskCreateLastUsedClearsBranchOnRepositoryChange(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { @@ -1103,6 +1079,7 @@ func TestSQLiteRepositoryUpdateTaskCreateLastUsedClearsBranchOnRepositoryChange( } } +// TestBuildPostgresTaskCreateLastUsedUpdatePatchesNonEmptyFields verifies the generated Postgres update patches task-create fields with jsonb_set. func TestBuildPostgresTaskCreateLastUsedUpdatePatchesNonEmptyFields(t *testing.T) { query, args := buildPostgresTaskCreateLastUsedUpdate(models.TaskCreateLastUsed{ RepositoryID: "repo-1", @@ -1132,6 +1109,7 @@ func TestBuildPostgresTaskCreateLastUsedUpdatePatchesNonEmptyFields(t *testing.T } } +// TestBuildPostgresTaskCreateLastUsedUpdatePatchesWorkflowHistoryEntries verifies the generated Postgres update patches workflow history entries. func TestBuildPostgresTaskCreateLastUsedUpdatePatchesWorkflowHistoryEntries(t *testing.T) { query, args := buildPostgresTaskCreateLastUsedUpdate(models.TaskCreateLastUsed{ WorkflowIDsByWorkspace: map[string]string{ @@ -1154,6 +1132,7 @@ func TestBuildPostgresTaskCreateLastUsedUpdatePatchesWorkflowHistoryEntries(t *t } } +// TestMakeTaskCreateLastUsedJSONSetArgsRejectsUnsafeWorkspacePathKeys verifies unsafe workspace keys are excluded from JSON set arguments. func TestMakeTaskCreateLastUsedJSONSetArgsRejectsUnsafeWorkspacePathKeys(t *testing.T) { args := makeTaskCreateLastUsedJSONSetArgs(models.TaskCreateLastUsed{ WorkflowIDsByWorkspace: map[string]string{ @@ -1172,6 +1151,7 @@ func TestMakeTaskCreateLastUsedJSONSetArgsRejectsUnsafeWorkspacePathKeys(t *test } } +// TestBuildPostgresTaskCreateLastUsedUpdateClearsBranchOnRepositoryChange verifies the generated Postgres update clears the branch when the repository changes. func TestBuildPostgresTaskCreateLastUsedUpdateClearsBranchOnRepositoryChange(t *testing.T) { query, args := buildPostgresTaskCreateLastUsedUpdate(models.TaskCreateLastUsed{ RepositoryID: "repo-after", @@ -1193,6 +1173,7 @@ func TestBuildPostgresTaskCreateLastUsedUpdateClearsBranchOnRepositoryChange(t * } } +// TestBuildPostgresUserSettingsPreservingTaskCreateLastUsedUpdateUsesJSONB verifies the generated Postgres preserving update merges the payload with jsonb_set. func TestBuildPostgresUserSettingsPreservingTaskCreateLastUsedUpdateUsesJSONB(t *testing.T) { patch := models.TaskCreateLastUsed{ RepositoryID: "repo-1", @@ -1223,181 +1204,7 @@ func TestBuildPostgresUserSettingsPreservingTaskCreateLastUsedUpdateUsesJSONB(t } } -func TestPostgresRepositoryTaskCreateLastUsedRoundTrip(t *testing.T) { - conn := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) - repo, err := newSQLiteRepositoryWithDB(conn, conn) - if err != nil { - t.Fatalf("new postgres repo: %v", err) - } - - ctx := context.Background() - staleSettings, err := repo.GetUserSettings(ctx, DefaultUserID) - if err != nil { - t.Fatalf("get defaults: %v", err) - } - staleSettings.SidebarActiveViewID = "view-before" - staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{ - RepositoryID: "repo-before", - Branch: "main", - AgentProfileID: "agent-before", - ExecutorProfileID: "exec-before", - } - upsertUserSettingsForTest(t, repo, ctx, staleSettings) - - got, err := repo.UpdateTaskCreateLastUsed(ctx, DefaultUserID, models.TaskCreateLastUsed{ - RepositoryID: "repo-after", - Branch: "feature", - AgentProfileID: "agent-after", - }) - if err != nil { - t.Fatalf("update postgres task-create last-used: %v", err) - } - if got.SidebarActiveViewID != "view-before" { - t.Fatalf("unrelated postgres setting should be preserved, got %q", got.SidebarActiveViewID) - } - if got.TaskCreateLastUsed.RepositoryID != "repo-after" || - got.TaskCreateLastUsed.Branch != "feature" || - got.TaskCreateLastUsed.AgentProfileID != "agent-after" || - got.TaskCreateLastUsed.ExecutorProfileID != "exec-before" { - t.Fatalf("postgres task-create update mismatch: %+v", got.TaskCreateLastUsed) - } - if got.Revision < 2 { - t.Fatalf("postgres settings revision = %d, want at least 2", got.Revision) - } - firstRevision := got.Revision - - staleSettings.SidebarActiveViewID = "view-after" - staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{ - RepositoryID: "repo-stale", - Branch: "stale", - } - got, err = repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, staleSettings, nil) - if err != nil { - t.Fatalf("upsert preserving postgres task-create last-used: %v", err) - } - if got.SidebarActiveViewID != "view-after" { - t.Fatalf("expected unrelated postgres setting to update, got %q", got.SidebarActiveViewID) - } - if got.TaskCreateLastUsed.RepositoryID != "repo-after" || - got.TaskCreateLastUsed.Branch != "feature" || - got.TaskCreateLastUsed.AgentProfileID != "agent-after" || - got.TaskCreateLastUsed.ExecutorProfileID != "exec-before" { - t.Fatalf("postgres preserving upsert should keep current task-create values: %+v", got.TaskCreateLastUsed) - } - if got.Revision != firstRevision+1 { - t.Fatalf("postgres settings revision = %d, want %d", got.Revision, firstRevision+1) - } -} - -func TestSQLiteRepositoryUpsertSettingsPreservesCurrentTaskCreateLastUsed(t *testing.T) { - conn, err := sqlx.Open("sqlite3", ":memory:") - if err != nil { - t.Fatalf("open sqlite: %v", err) - } - conn.SetMaxOpenConns(1) - t.Cleanup(func() { _ = conn.Close() }) - repo, err := newSQLiteRepositoryWithDB(conn, conn) - if err != nil { - t.Fatalf("new repo: %v", err) - } - - ctx := context.Background() - staleSettings, err := repo.GetUserSettings(ctx, DefaultUserID) - if err != nil { - t.Fatalf("get defaults: %v", err) - } - staleSettings.SidebarActiveViewID = "view-before" - staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{RepositoryID: "repo-before"} - upsertUserSettingsForTest(t, repo, ctx, staleSettings) - - // Simulate a task-create write that lands after another settings caller - // read the old blob but before that caller writes its unrelated change. - if _, err := repo.UpdateTaskCreateLastUsed(ctx, DefaultUserID, models.TaskCreateLastUsed{ - RepositoryID: "repo-after", - Branch: "feature", - }); err != nil { - t.Fatalf("update task-create last-used: %v", err) - } - - staleSettings.SidebarActiveViewID = "view-after" - got, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, staleSettings, nil) - if err != nil { - t.Fatalf("upsert preserving task-create last-used: %v", err) - } - - if got.SidebarActiveViewID != "view-after" { - t.Fatalf("expected unrelated setting to update, got %q", got.SidebarActiveViewID) - } - if got.TaskCreateLastUsed.RepositoryID != "repo-after" { - t.Fatalf("expected current task-create repository to survive stale write, got %q", got.TaskCreateLastUsed.RepositoryID) - } - if got.TaskCreateLastUsed.Branch != "feature" { - t.Fatalf("expected current task-create branch to survive stale write, got %q", got.TaskCreateLastUsed.Branch) - } -} - -func TestSQLiteRepositoryUpsertSettingsPreservingTaskCreateLastUsedAppliesPatch(t *testing.T) { - conn, err := sqlx.Open("sqlite3", ":memory:") - if err != nil { - t.Fatalf("open sqlite: %v", err) - } - conn.SetMaxOpenConns(1) - t.Cleanup(func() { _ = conn.Close() }) - repo, err := newSQLiteRepositoryWithDB(conn, conn) - if err != nil { - t.Fatalf("new repo: %v", err) - } - - ctx := context.Background() - staleSettings, err := repo.GetUserSettings(ctx, DefaultUserID) - if err != nil { - t.Fatalf("get defaults: %v", err) - } - staleSettings.SidebarActiveViewID = "view-before" - staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{ - RepositoryID: "repo-before", - Branch: "main", - AgentProfileID: "agent-before", - ExecutorProfileID: "exec-before", - } - upsertUserSettingsForTest(t, repo, ctx, staleSettings) - if _, err := repo.UpdateTaskCreateLastUsed(ctx, DefaultUserID, models.TaskCreateLastUsed{ - RepositoryID: "repo-current", - Branch: "current", - ExecutorProfileID: "exec-current", - }); err != nil { - t.Fatalf("update current task-create last-used: %v", err) - } - - staleSettings.SidebarActiveViewID = "view-after" - staleSettings.TaskCreateLastUsed = models.TaskCreateLastUsed{ - RepositoryID: "repo-stale", - Branch: "stale", - AgentProfileID: "agent-stale", - } - patch := models.TaskCreateLastUsed{AgentProfileID: "agent-after"} - got, err := repo.UpsertUserSettingsPreservingTaskCreateLastUsed(ctx, staleSettings, &patch) - if err != nil { - t.Fatalf("upsert preserving task-create last-used with patch: %v", err) - } - - if got.SidebarActiveViewID != "view-after" { - t.Fatalf("expected unrelated setting to update, got %q", got.SidebarActiveViewID) - } - if got.TaskCreateLastUsed.RepositoryID != "repo-current" { - t.Fatalf("expected current repository to survive stale write, got %q", got.TaskCreateLastUsed.RepositoryID) - } - if got.TaskCreateLastUsed.Branch != "current" { - t.Fatalf("expected current branch to survive stale write, got %q", got.TaskCreateLastUsed.Branch) - } - if got.TaskCreateLastUsed.AgentProfileID != "agent-after" { - t.Fatalf("expected patch agent profile to apply, got %q", got.TaskCreateLastUsed.AgentProfileID) - } - if got.TaskCreateLastUsed.ExecutorProfileID != "exec-current" { - t.Fatalf("expected current executor profile to survive stale write, got %q", got.TaskCreateLastUsed.ExecutorProfileID) - } -} - +// TestSQLiteRepositorySidebarViewStateRoundTrip verifies sidebar view state round-trips through the SQLite repository. func TestSQLiteRepositorySidebarViewStateRoundTrip(t *testing.T) { conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { diff --git a/apps/backend/internal/user/store/store.go b/apps/backend/internal/user/store/store.go index de6baab7af..b71804a4bc 100644 --- a/apps/backend/internal/user/store/store.go +++ b/apps/backend/internal/user/store/store.go @@ -2,15 +2,20 @@ package store import ( "context" + "errors" "github.com/kandev/kandev/internal/user/models" ) +// ErrUserSettingsRevisionConflict reports a conditional settings write that +// matched no rows because the expected revision was stale. +var ErrUserSettingsRevisionConflict = errors.New("user settings revision conflict") + type Repository interface { GetUser(ctx context.Context, id string) (*models.User, error) GetDefaultUser(ctx context.Context) (*models.User, error) GetUserSettings(ctx context.Context, userID string) (*models.UserSettings, error) - UpsertUserSettingsPreservingTaskCreateLastUsed(ctx context.Context, settings *models.UserSettings, patch *models.TaskCreateLastUsed) (*models.UserSettings, error) + UpsertUserSettingsPreservingTaskCreateLastUsed(ctx context.Context, settings *models.UserSettings, patch *models.TaskCreateLastUsed, expectedRevision int64) (*models.UserSettings, error) UpdateTaskCreateLastUsed(ctx context.Context, userID string, patch models.TaskCreateLastUsed) (*models.UserSettings, error) Close() error } diff --git a/apps/web/components/settings/account/security-settings.test.tsx b/apps/web/components/settings/account/security-settings.test.tsx new file mode 100644 index 0000000000..5a6c33cc13 --- /dev/null +++ b/apps/web/components/settings/account/security-settings.test.tsx @@ -0,0 +1,560 @@ +import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createDefaultUserSettings } from "@/lib/ssr/user-settings"; +import type { UserSettingsState } from "@/lib/state/slices/settings/types"; +import { StateProvider, useAppStoreApi } from "@/components/state-provider"; +import { SettingsTargetProvider } from "@/components/settings/settings-target-provider"; +import { emitSettingsTargetRequest } from "@/lib/settings-discovery/target"; +import { formatDateTime } from "@/lib/i18n/formats"; +import { ApiError } from "@/lib/api/client"; +import type { AuthSession } from "@/lib/api/domains/auth-api"; +import { registerUsersHandlers } from "@/lib/ws/handlers/users"; +import type { BackendMessageMap } from "@/lib/types/backend"; +import { SecuritySettings } from "./security-settings"; +import { SettingsSaveProvider } from "@/components/settings/settings-save-provider"; + +const authApiMocks = vi.hoisted(() => ({ + listSessions: vi.fn(), + revokeSession: vi.fn(), + changePassword: vi.fn(), +})); +const settingsApiMocks = vi.hoisted(() => ({ updateUserSettings: vi.fn() })); +const toastMocks = vi.hoisted(() => ({ error: vi.fn() })); + +vi.mock("@/lib/api/domains/auth-api", () => ({ + listSessions: (...args: unknown[]) => authApiMocks.listSessions(...args), + revokeSession: (...args: unknown[]) => authApiMocks.revokeSession(...args), + changePassword: (...args: unknown[]) => authApiMocks.changePassword(...args), +})); +vi.mock("@/lib/api/domains/settings-api", () => ({ + updateUserSettings: (...args: unknown[]) => settingsApiMocks.updateUserSettings(...args), +})); +vi.mock("@/lib/toast/sonner", () => ({ + toast: { error: (...args: unknown[]) => toastMocks.error(...args) }, +})); + +const LAST_SEEN_AT = "2026-08-15T15:42:00Z"; +const RELATIVE_TEST_ID = "last-seen-relative"; +const SELECT_TEST_ID = "last-seen-display-select"; +const EMPTY_TEST_ID = "last-seen-empty"; +const SESSIONS_CARD_TEST_ID = "account-sessions-card"; +const DIRTY_ATTRIBUTE = "data-settings-dirty"; +const DIRTY_TRUE = "true"; +const DIRTY_FALSE = "false"; +const RELATIVE_OPTION = "Relative time"; +const ABSOLUTE_OPTION = "Absolute time"; + +const SESSION: AuthSession = { + id: "sess-1", + created_at: "2026-01-01T00:00:00Z", + last_seen_at: LAST_SEEN_AT, + user_agent: "Chrome on macOS", + ip: "203.0.113.7", + current: true, +}; + +/** Returns default user settings merged with the given overrides. */ +function makeUserSettings(overrides: Partial = {}): UserSettingsState { + return { ...createDefaultUserSettings(), ...overrides }; +} + +type StoreHolder = { current: ReturnType | null }; + +/** Stores the app store API in the holder and renders nothing. */ +function StoreCapture({ holder }: { holder: StoreHolder }) { + holder.current = useAppStoreApi(); + return null; +} + +/** Renders SecuritySettings inside a store provider, optionally capturing the store API. */ +function renderSecurity(overrides: Partial = {}, holder?: StoreHolder) { + return render( + + + + {holder ? : null} + + + + , + ); +} + +/** Renders SecuritySettings and waits for the last-seen display select to appear. */ +async function renderLoaded(overrides: Partial = {}, holder?: StoreHolder) { + renderSecurity(overrides, holder); + await screen.findByTestId(SELECT_TEST_ID); +} + +// RemountHarness keeps one StateProvider (and thus one store) mounted while +// toggling only SecuritySettings, so a remounted component reads the same +// converged store the pre-unmount component mutated. +function RemountHarness({ holder, mounted }: { holder: StoreHolder; mounted: boolean }) { + return ( + + + + + {mounted ? : null} + + + + ); +} + +// renderRelativeWithFakeTime mounts the card in relative mode under fake +// timers at a fixed clock, optionally overriding the session's last_seen_at. +async function renderRelativeWithFakeTime(lastSeenAt?: string) { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-08-15T15:45:00Z")); + if (lastSeenAt) { + authApiMocks.listSessions.mockResolvedValue({ + sessions: [{ ...SESSION, last_seen_at: lastSeenAt }], + }); + } + renderSecurity({ lastSeenDisplay: "relative" }); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); +} + +/** Opens the last-seen display select and clicks the option with the given name. */ +async function chooseDisplay(name: "Absolute time" | "Relative time") { + fireEvent.click(screen.getByTestId(SELECT_TEST_ID)); + fireEvent.click(await screen.findByRole("option", { name })); +} + +/** Runs the shared settings save action for the current draft. */ +async function saveChanges() { + fireEvent.click(await screen.findByRole("button", { name: "Save changes" })); +} + +type PatchResponse = { + settings: { + user_id: string; + workspace_id: string; + repository_ids: string[]; + updated_at: string; + revision: number; + [key: string]: unknown; + }; +}; + +/** Returns a default settings PATCH response merged with the given overrides. */ +function patchResponse(overrides: Record = {}): PatchResponse { + return { + settings: { + user_id: "default-user", + workspace_id: "workspace-1", + repository_ids: [], + updated_at: "2026-08-15T15:43:00Z", + last_seen_display: "relative", + revision: 2, + ...overrides, + }, + }; +} + +/** Builds a user.settings.updated WebSocket message from the given payload. */ +function settingsUpdatedMessage( + payload: Partial, +): BackendMessageMap["user.settings.updated"] { + return { + type: "notification", + action: "user.settings.updated", + payload: { + user_id: "default-user", + workspace_id: "workspace-1", + repository_ids: [], + ...payload, + }, + }; +} + +/** Feeds a user.settings.updated snapshot through the registered WS handler. */ +function dispatchUserSettingsSnapshot( + holder: StoreHolder, + payload: Partial, +) { + registerUsersHandlers(holder.current!)["user.settings.updated"]?.( + settingsUpdatedMessage(payload), + ); +} + +/** Creates a PATCH promise with externally callable resolve and reject functions. */ +function deferredPatch(): { + promise: Promise; + resolve: (value: PatchResponse) => void; + reject: (error: ApiError) => void; +} { + let resolve!: (value: PatchResponse) => void; + let reject!: (error: ApiError) => void; + const promise = new Promise((complete, fail) => { + resolve = complete; + reject = fail; + }); + return { promise, resolve, reject }; +} + +beforeEach(() => { + authApiMocks.listSessions.mockReset().mockResolvedValue({ sessions: [SESSION] }); + authApiMocks.revokeSession.mockReset().mockResolvedValue(undefined); + authApiMocks.changePassword.mockReset().mockResolvedValue(undefined); + settingsApiMocks.updateUserSettings.mockReset().mockResolvedValue(patchResponse()); + toastMocks.error.mockReset(); +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +describe("Last seen display rendering", () => { + it("renders absolute time by default", async () => { + await renderLoaded(); + + expect(screen.queryByTestId(RELATIVE_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(EMPTY_TEST_ID)).toBeNull(); + expect(screen.getByText(formatDateTime(new Date(LAST_SEEN_AT)))).toBeTruthy(); + }); + + it("renders a relative label when the store baseline is relative", async () => { + await renderLoaded({ lastSeenDisplay: "relative" }); + + const trigger = screen.getByTestId(RELATIVE_TEST_ID); + expect(trigger.textContent).not.toBe(""); + }); + + it("exposes the absolute timestamp as accessible name and native title", async () => { + await renderLoaded({ lastSeenDisplay: "relative" }); + + const trigger = screen.getByTestId(RELATIVE_TEST_ID); + const absolute = formatDateTime(new Date(LAST_SEEN_AT)); + // The trigger is a semantic, focusable time element, not a bare span. + expect(trigger.tagName).toBe("TIME"); + expect(trigger.getAttribute("dateTime")).toBe(new Date(LAST_SEEN_AT).toISOString()); + expect(trigger.getAttribute("aria-label")).toBe(absolute); + expect(trigger.getAttribute("title")).toBe(absolute); + }); + + it("opens the absolute-time tooltip on keyboard focus of the relative trigger", async () => { + await renderLoaded({ lastSeenDisplay: "relative" }); + + const trigger = screen.getByTestId(RELATIVE_TEST_ID); + fireEvent.focus(trigger); + await waitFor(() => expect(screen.getByRole("tooltip")).toBeTruthy()); + expect(screen.getByRole("tooltip").textContent).toBe(formatDateTime(new Date(LAST_SEEN_AT))); + }); + + it("renders an empty cell for an unparseable last_seen_at", async () => { + authApiMocks.listSessions.mockResolvedValue({ + sessions: [{ ...SESSION, last_seen_at: "not-a-date" }], + }); + await renderLoaded({ lastSeenDisplay: "relative" }); + + expect(screen.getByTestId(EMPTY_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId(RELATIVE_TEST_ID)).toBeNull(); + }); + + it("keeps the Last seen select accessible when no sessions are returned", async () => { + authApiMocks.listSessions.mockResolvedValue({ sessions: [] }); + await renderLoaded({ lastSeenDisplay: "absolute" }); + + // The display option lives on the card independent of table rows. + expect(screen.getByTestId(SELECT_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId("account-sessions-table")).toBeNull(); + }); + + it("keeps the Last seen select reachable when loading sessions fails", async () => { + authApiMocks.listSessions.mockRejectedValue(new Error("boom")); + await renderLoaded({ lastSeenDisplay: "absolute" }); + await screen.findByTestId("account-sessions-error"); + + // A transient sessions API failure must not hide the persisted preference. + expect(screen.getByTestId(SELECT_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId("account-sessions-table")).toBeNull(); + }); +}); + +describe("Last seen display persistence", () => { + it("keeps a selection local until the shared save action runs", async () => { + await renderLoaded(); + + await chooseDisplay(RELATIVE_OPTION); + + expect(settingsApiMocks.updateUserSettings).not.toHaveBeenCalled(); + expect(screen.getByTestId(SESSIONS_CARD_TEST_ID).getAttribute(DIRTY_ATTRIBUTE)).toBe( + DIRTY_TRUE, + ); + fireEvent.click(await screen.findByRole("button", { name: "Save changes" })); + + await waitFor(() => + expect(settingsApiMocks.updateUserSettings).toHaveBeenCalledWith({ + last_seen_display: "relative", + }), + ); + }); + + it("discards a local selection through the shared reset action", async () => { + await renderLoaded(); + + await chooseDisplay(RELATIVE_OPTION); + fireEvent.click(await screen.findByRole("button", { name: "Reset" })); + + expect(settingsApiMocks.updateUserSettings).not.toHaveBeenCalled(); + expect(screen.getByTestId(SELECT_TEST_ID).textContent).toContain(ABSOLUTE_OPTION); + expect(screen.getByTestId(SESSIONS_CARD_TEST_ID).getAttribute(DIRTY_ATTRIBUTE)).toBe( + DIRTY_FALSE, + ); + }); + + it("persists a relative selection through the shared save action", async () => { + await renderLoaded(); + + await chooseDisplay(RELATIVE_OPTION); + await saveChanges(); + + await waitFor(() => + expect(settingsApiMocks.updateUserSettings).toHaveBeenCalledWith({ + last_seen_display: "relative", + }), + ); + }); + + it("surfaces a revoke failure in the sessions card", async () => { + authApiMocks.revokeSession.mockRejectedValue(new ApiError("boom", 400, null)); + authApiMocks.listSessions.mockResolvedValue({ + sessions: [{ ...SESSION, current: false, id: "sess-2" }], + }); + await renderLoaded(); + + fireEvent.click(screen.getByTestId("account-sessions-revoke")); + await waitFor(() => + expect(screen.getByTestId("account-sessions-error").textContent).toContain("boom"), + ); + }); + + it("shows the error toast and keeps the draft for a failed save", async () => { + settingsApiMocks.updateUserSettings.mockRejectedValue(new Error("boom")); + await renderLoaded(); + + await chooseDisplay(RELATIVE_OPTION); + await saveChanges(); + // The draft stays visible while the write is in flight. + expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy(); + + await waitFor(() => expect(toastMocks.error).toHaveBeenCalled()); + // The failed draft remains available for a retry through Save changes. + expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId(SESSIONS_CARD_TEST_ID).getAttribute(DIRTY_ATTRIBUTE)).toBe( + DIRTY_TRUE, + ); + }); + + it("keeps the optimistic value while a write is in flight even when the store re-asserts the baseline", async () => { + const { promise: pending, resolve: resolvePatch } = deferredPatch(); + settingsApiMocks.updateUserSettings.mockReturnValue(pending); + const holder: StoreHolder = { current: null }; + await renderLoaded({}, holder); + + await chooseDisplay(RELATIVE_OPTION); + await saveChanges(); + await waitFor(() => expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy()); + + // An unrelated full snapshot re-asserts the baseline value while the write + // is still in flight; the optimistic override must survive and still render. + act(() => { + dispatchUserSettingsSnapshot(holder, { last_seen_display: "absolute", revision: 3 }); + }); + expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy(); + + // The write settles: the confirmed relative value renders. + await act(async () => { + resolvePatch(patchResponse({ last_seen_display: "relative", revision: 4 })); + }); + await waitFor(() => expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy()); + expect(holder.current!.getState().userSettings.lastSeenDisplay).toBe("relative"); + }); +}); + +describe("Last seen display queued write ordering", () => { + it("keeps a newer draft while an earlier save is in flight", async () => { + const { promise: pendingA, resolve: resolveA } = deferredPatch(); + settingsApiMocks.updateUserSettings + .mockReturnValueOnce(pendingA) + .mockResolvedValueOnce(patchResponse({ last_seen_display: "absolute", revision: 3 })); + const holder: StoreHolder = { current: null }; + await renderLoaded({}, holder); + + await chooseDisplay(RELATIVE_OPTION); + await saveChanges(); + expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy(); + + await chooseDisplay(ABSOLUTE_OPTION); + expect(screen.queryByTestId(RELATIVE_TEST_ID)).toBeNull(); + + // A settles first. The newer local draft remains visible and dirty. + await act(async () => { + resolveA(patchResponse({ last_seen_display: "relative", revision: 2 })); + }); + expect(screen.queryByTestId(RELATIVE_TEST_ID)).toBeNull(); + expect(screen.getByTestId(SESSIONS_CARD_TEST_ID).getAttribute(DIRTY_ATTRIBUTE)).toBe( + DIRTY_TRUE, + ); + + await saveChanges(); + await waitFor(() => + expect(holder.current!.getState().userSettings.lastSeenDisplay).toBe("absolute"), + ); + }); +}); + +describe("Last seen display write and lifecycle races", () => { + it("discards a deferred PATCH response older than a newer WS snapshot", async () => { + const { promise: pending, resolve: resolvePatch } = deferredPatch(); + settingsApiMocks.updateUserSettings.mockReturnValue(pending); + const holder: StoreHolder = { current: null }; + await renderLoaded({}, holder); + + await chooseDisplay(RELATIVE_OPTION); + await saveChanges(); + + // A newer WS snapshot (foreign change at rev 5) lands first through the + // real user.settings.updated handler. + act(() => { + dispatchUserSettingsSnapshot(holder, { last_seen_display: "absolute", revision: 5 }); + }); + + // The deferred PATCH response (rev 2) is discarded by the revision guard; + // the cell renders the newer foreign value. + await act(async () => { + resolvePatch(patchResponse({ last_seen_display: "relative", revision: 2 })); + }); + await waitFor(() => expect(screen.queryByTestId(RELATIVE_TEST_ID)).toBeNull()); + expect(screen.getByText(formatDateTime(new Date(LAST_SEEN_AT)))).toBeTruthy(); + expect(holder.current!.getState().userSettings.revision).toBe(5); + expect(holder.current!.getState().userSettings.lastSeenDisplay).toBe("absolute"); + }); + + it("converges the store when the component unmounts mid-write and remounts", async () => { + const { promise: pending, resolve: resolvePatch } = deferredPatch(); + settingsApiMocks.updateUserSettings.mockReturnValue(pending); + const holder: StoreHolder = { current: null }; + + const view = render(); + await screen.findByTestId(SELECT_TEST_ID); + + await chooseDisplay(RELATIVE_OPTION); + await saveChanges(); + await waitFor(() => expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy()); + + // Unmount only the component; the provider and store stay mounted. A newer + // WS snapshot lands after unmount through the real handler, then the PATCH + // resolves (its older revision is discarded), so the store converges. + view.rerender(); + act(() => { + dispatchUserSettingsSnapshot(holder, { last_seen_display: "absolute", revision: 5 }); + }); + await act(async () => { + resolvePatch(patchResponse({ last_seen_display: "relative", revision: 2 })); + }); + await waitFor(() => + expect(holder.current!.getState().userSettings.lastSeenDisplay).toBe("absolute"), + ); + expect(holder.current!.getState().userSettings.revision).toBe(5); + + // Re-mount the component under the SAME store: it must render the converged + // store value, not a seeded baseline. + view.rerender(); + await screen.findByTestId(SELECT_TEST_ID); + expect(screen.queryByTestId(RELATIVE_TEST_ID)).toBeNull(); + expect(screen.getByText(formatDateTime(new Date(LAST_SEEN_AT)))).toBeTruthy(); + view.unmount(); + }); + + it("handles a rejected write after unmount and remounts without a stale override", async () => { + const { promise: pending, reject: rejectPatch } = deferredPatch(); + settingsApiMocks.updateUserSettings.mockReturnValue(pending); + const holder: StoreHolder = { current: null }; + + const view = render(); + await screen.findByTestId(SELECT_TEST_ID); + + await chooseDisplay(RELATIVE_OPTION); + await saveChanges(); + await waitFor(() => expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy()); + + // Unmount only the component; the provider and store stay mounted. A newer + // WS snapshot lands after unmount through the real handler, then the PATCH + // is rejected: the toast fires, the store keeps the WS value. + view.rerender(); + act(() => { + dispatchUserSettingsSnapshot(holder, { last_seen_display: "relative", revision: 3 }); + }); + await act(async () => { + rejectPatch(new ApiError("boom", 400, null)); + }); + await waitFor(() => expect(toastMocks.error).toHaveBeenCalled()); + await waitFor(() => + expect(holder.current!.getState().userSettings.lastSeenDisplay).toBe("relative"), + ); + + // Re-mount the component under the SAME store: it renders the converged WS + // value, with no stale optimistic override. + view.rerender(); + await screen.findByTestId(SELECT_TEST_ID); + expect(screen.getByTestId(RELATIVE_TEST_ID)).toBeTruthy(); + view.unmount(); + }); + + it("ticks every second while the timestamp is under a minute old", async () => { + // last_seen 55s before now: "55 seconds ago", well past the just-now + // threshold so a one-second advance changes the label. + await renderRelativeWithFakeTime("2026-08-15T15:44:05Z"); + + const before = screen.getByTestId(RELATIVE_TEST_ID).textContent; + act(() => { + vi.advanceTimersByTime(1_000); + }); + expect(screen.getByTestId(RELATIVE_TEST_ID).textContent).not.toBe(before); + }); + + it("ticks every minute once the timestamp is a minute or more old", async () => { + await renderRelativeWithFakeTime(); + + const before = screen.getByTestId(RELATIVE_TEST_ID).textContent; + act(() => { + vi.advanceTimersByTime(60_000); + }); + expect(screen.getByTestId(RELATIVE_TEST_ID).textContent).not.toBe(before); + }); +}); + +describe("Last seen display discovery", () => { + it("registers the sessions and last-seen-display targets", async () => { + const reveal = vi.fn(); + authApiMocks.listSessions.mockResolvedValue({ sessions: [SESSION] }); + render( + + + + + + + + + , + ); + await screen.findByTestId(SELECT_TEST_ID); + + emitSettingsTargetRequest("setting-account-sessions"); + await waitFor(() => expect(reveal).toHaveBeenCalledTimes(1)); + + emitSettingsTargetRequest("setting-account-last-seen-display"); + await waitFor(() => expect(reveal).toHaveBeenCalledTimes(2)); + }); +}); diff --git a/apps/web/components/settings/account/security-settings.tsx b/apps/web/components/settings/account/security-settings.tsx index 3fac621860..2f7dcd5bf5 100644 --- a/apps/web/components/settings/account/security-settings.tsx +++ b/apps/web/components/settings/account/security-settings.tsx @@ -1,12 +1,23 @@ "use client"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { Button } from "@kandev/ui/button"; import { CardContent } from "@kandev/ui/card"; import { Input } from "@kandev/ui/input"; +import { Label } from "@kandev/ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; import { Spinner } from "@kandev/ui/spinner"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@kandev/ui/table"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { + Drawer, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, + DrawerTrigger, +} from "@kandev/ui/drawer"; import { Badge } from "@kandev/ui/badge"; import { IconDevices, IconKey } from "@tabler/icons-react"; import { ApiError } from "@/lib/api/client"; @@ -17,11 +28,26 @@ import { type AuthSession, } from "@/lib/api/domains/auth-api"; import { SettingsCard } from "@/components/settings/settings-card"; +import { useSettingsSaveContributor } from "@/components/settings/settings-save-provider"; +import { useSettingsTargetRegistration } from "@/components/settings/settings-target-provider"; import { ACCOUNT_SETTINGS_TARGETS } from "@/lib/settings-discovery/catalog/account"; -import { formatDateTime } from "@/lib/i18n/formats"; +import { formatDateTime, formatRelativeTime } from "@/lib/i18n/formats"; import { SettingsCardHeader } from "@/components/settings/settings-card-header"; import { SettingsErrorText, SettingsFieldLabel } from "@/components/settings/settings-typography"; +import { useNow } from "@/hooks/use-now"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; +import { createQueuedUserSettingsSyncWithResponse } from "@/lib/user-settings-sync"; +import { mapUserSettingsResponse } from "@/lib/ssr/user-settings"; +import { toast } from "@/lib/toast/sonner"; +import type { LastSeenDisplay } from "@/lib/types/http"; +/** Queued sync that persists the last-seen display preference to the backend. */ +const syncLastSeenDisplay = createQueuedUserSettingsSyncWithResponse( + (lastSeenDisplay) => ({ last_seen_display: lastSeenDisplay }), +); + +/** Renders the change-password form card. */ function ChangePasswordCard() { const { t } = useTranslation(); const [current, setCurrent] = useState(""); @@ -30,6 +56,7 @@ function ChangePasswordCard() { const [success, setSuccess] = useState(false); const [submitting, setSubmitting] = useState(false); + /** Submits the password change and updates success/error state. */ const onSubmit = async (e: React.FormEvent) => { e.preventDefault(); setError(null); @@ -108,12 +135,14 @@ function ChangePasswordCard() { ); } +/** Loads the active sessions list and exposes reload/error state. */ function useSessionsList() { const { t } = useTranslation(); const [sessions, setSessions] = useState([]); const [loaded, setLoaded] = useState(false); const [error, setError] = useState(null); + /** Fetches the latest sessions from the API and updates state. */ const reload = useCallback(async () => { setError(null); try { @@ -129,21 +158,248 @@ function useSessionsList() { void reload(); }, [reload]); - return { sessions, loaded, error, reload }; + return { sessions, loaded, error, reload, setError }; } +/** Parses a last-seen timestamp into a Date, or null when invalid. */ +function parseLastSeenAt(lastSeenAt: string): Date | null { + const parsed = new Date(lastSeenAt); + return Number.isNaN(parsed.getTime()) ? null : parsed; +} + +/** Renders a session's last-seen timestamp in the selected display format. */ +function LastSeenCell({ lastSeenAt, display }: { lastSeenAt: string; display: LastSeenDisplay }) { + const absolute = useMemo(() => parseLastSeenAt(lastSeenAt), [lastSeenAt]); + + if (!absolute) { + // Guard formatDateTime, which throws RangeError on invalid dates. + return ; + } + if (display === "absolute") { + return {formatDateTime(absolute)}; + } + return ; +} + +/** Renders a live-updating relative last-seen time with an absolute-time tooltip. */ +function RelativeLastSeenCell({ lastSeenAt }: { lastSeenAt: Date }) { + const { t } = useTranslation(); + const usesTouchDrawer = useTouchDrawer(); + const [open, setOpen] = useState(false); + // Tick every second while the timestamp is under a minute old (second-scale + // labels like "45 seconds ago" need live updates), then every minute once + // the age crosses a minute. The interval is recomputed on each render, so + // useNow re-creates its timer exactly when the cadence changes. + const ageMs = Math.abs(Date.now() - lastSeenAt.getTime()); + const intervalMs = ageMs < 60_000 ? 1_000 : 60_000; + const now = useNow(intervalMs); + const absolute = formatDateTime(lastSeenAt); + const relative = formatRelativeTime(lastSeenAt, now); + + if (usesTouchDrawer) { + return ( + + + + + + + + {t("account:lastSeen")} + {absolute} + + + + + ); + } + + return ( + + + + + + {absolute} + + + ); +} + +/** Renders the last-seen display format preference selector. */ +function LastSeenDisplaySelect({ + value, + onChange, +}: { + value: LastSeenDisplay; + onChange: (value: LastSeenDisplay) => void; +}) { + const { t } = useTranslation(); + /** Ref callback that registers this element as the last-seen display discovery target. */ + const registerTarget = useSettingsTargetRegistration(ACCOUNT_SETTINGS_TARGETS.lastSeenDisplay); + return ( +
+ + +

{t("account:lastSeenDisplayDescription")}

+
+ ); +} + +/** Renders the active sessions table with revoke actions. */ +function SessionsTable({ + sessions, + display, + onRevoke, +}: { + sessions: AuthSession[]; + display: LastSeenDisplay; + onRevoke: (id: string) => void; +}) { + const { t } = useTranslation(); + return ( + + + + {t("account:device")} + {t("account:ipAddress")} + {t("account:lastSeen")} + {t("account:actions")} + + + + {sessions.map((session) => ( + + + {session.user_agent} + {session.current && ( + + {t("account:thisDevice")} + + )} + + {session.ip} + + + {!session.current && ( + + )} + + + ))} + +
+ ); +} + +/** Renders the active-sessions settings card with display preference and revoke. */ function SessionsCard() { const { t } = useTranslation(); - const { sessions, loaded, error, reload } = useSessionsList(); + const { sessions, loaded, error, reload, setError } = useSessionsList(); + const savedDisplay = useAppStore((state) => state.userSettings.lastSeenDisplay); + const store = useAppStoreApi(); + const [draftDisplay, setDraftDisplay] = useState(savedDisplay); + const hasLocalDraft = useRef(false); + const isDirty = draftDisplay !== savedDisplay; + useEffect(() => { + if (!hasLocalDraft.current) setDraftDisplay(savedDisplay); + }, [savedDisplay]); + + const onDisplayChange = useCallback((next: LastSeenDisplay) => { + hasLocalDraft.current = true; + setDraftDisplay(next); + }, []); + + useSettingsSaveContributor({ + id: "account-last-seen-display", + order: 30, + revision: draftDisplay, + isDirty, + save: async (revision) => { + const submitted = revision as LastSeenDisplay; + try { + const response = await syncLastSeenDisplay(submitted); + const state = store.getState(); + state.setUserSettings(mapUserSettingsResponse(response, state.userSettings)); + const confirmed = store.getState().userSettings.lastSeenDisplay; + setDraftDisplay((current) => { + if (current !== submitted) return current; + hasLocalDraft.current = false; + return confirmed; + }); + } catch (error) { + toast.error(t("account:failedToSaveLastSeenDisplay")); + throw error; + } + }, + discard: () => { + hasLocalDraft.current = false; + setDraftDisplay(savedDisplay); + }, + }); + + /** Revokes a session and reloads the sessions list. */ const onRevoke = async (id: string) => { - await revokeSession(id); - await reload(); + try { + await revokeSession(id); + await reload(); + } catch (err) { + setError(err instanceof ApiError ? err.message : t("account:failedToRevokeSession")); + } }; return ( + {/* The display preference is independent of session rows, so the + select (and its discovery target) stays reachable during loading + and when the sessions API fails. */} + {error && ( {error} )} @@ -163,51 +423,14 @@ function SessionsCard() { )} {loaded && sessions.length > 0 && ( - - - - {t("account:device")} - {t("account:ipAddress")} - {t("account:lastSeen")} - {t("account:actions")} - - - - {sessions.map((session) => ( - - - {session.user_agent} - {session.current && ( - - {t("account:thisDevice")} - - )} - - {session.ip} - {formatDateTime(session.last_seen_at)} - - {!session.current && ( - - )} - - - ))} - -
+ )} ); } +/** Renders the account security settings section. */ export function SecuritySettings() { return (
diff --git a/apps/web/e2e/tests/auth/mobile-relative-last-seen.spec.ts b/apps/web/e2e/tests/auth/mobile-relative-last-seen.spec.ts new file mode 100644 index 0000000000..aa297cbbff --- /dev/null +++ b/apps/web/e2e/tests/auth/mobile-relative-last-seen.spec.ts @@ -0,0 +1,112 @@ +import { devices, expect } from "@playwright/test"; +import path from "node:path"; +import { backendFixture as test } from "../../fixtures/backend"; +import { login, setupAdmin } from "../../helpers/auth"; + +/** + * Mobile parity for the relative "Last seen" option: the select is operated + * with touch-native `tap()` (mouse-semantics clicks would not prove touch), + * relative labels render without hover, and the trigger/option meet the 44px + * active-dimension touch target at the Pixel 5 viewport. + * + * Named `mobile-*` so the `mobile-chrome` project routes it away from the + * desktop `auth` project. Manual contexts do not inherit project device + * options, so the Pixel 5 viewport is spread explicitly. + */ +const ADMIN = { email: "admin@demo.dev", password: "adminpass123", displayName: "Ada Admin" }; +const SECURITY_PATH = "/settings/account/security"; +const CURRENT_SESSION_ROW = '[data-testid="account-sessions-row"][data-current-session="true"]'; + +test.describe.serial("relative last seen (mobile)", () => { + test.beforeAll(async ({ backend }) => { + await backend.restart({ + KANDEV_FEATURES_AUTH: "true", + KANDEV_DATABASE_PATH: path.join(backend.tmpDir, "kandev-mobile-relative-last-seen.db"), + }); + }); + + test.afterAll(async ({ backend }) => { + await backend.restart(); + }); + + test("operates the Last seen select with touch and renders relative labels without hover", async ({ + browser, + backend, + }) => { + const ctx = await browser.newContext({ + ...devices["Pixel 5"], + baseURL: backend.frontendUrl, + }); + try { + await setupAdmin(ctx, backend.baseUrl, ADMIN); + await login(ctx, backend.baseUrl, ADMIN); + let original = "absolute"; + try { + const originalRes = await ctx.request.get(`${backend.baseUrl}/api/v1/user/settings`); + expect(originalRes.ok(), await originalRes.text()).toBeTruthy(); + const originalBody = (await originalRes.json()) as { + settings: { last_seen_display?: string }; + }; + original = originalBody.settings.last_seen_display ?? "absolute"; + // Pin a known absolute baseline so the relative transition below is a + // real persisted change, not a pre-existing value. + const baselineRes = await ctx.request.patch(`${backend.baseUrl}/api/v1/user/settings`, { + data: { last_seen_display: "absolute" }, + }); + expect(baselineRes.ok(), await baselineRes.text()).toBeTruthy(); + + const page = await ctx.newPage(); + // Manual contexts do not inherit project device options; pin the width. + expect((await page.viewportSize())?.width).toBe(393); + + await page.goto(SECURITY_PATH); + await expect(page.getByTestId("last-seen-relative")).toHaveCount(0); + + const trigger = page.getByTestId("last-seen-display-select"); + await expect(trigger).toBeVisible({ timeout: 15_000 }); + + // The trigger meets the 44px active-dimension touch target. + const triggerBox = await trigger.boundingBox(); + expect(triggerBox).not.toBeNull(); + expect(triggerBox!.height).toBeGreaterThanOrEqual(44); + + await trigger.tap(); + const option = page.getByRole("listbox").getByRole("option", { name: "Relative time" }); + await expect(option).toBeVisible(); + // The dropdown entrance animation (zoom-in-95 over 100ms) scales the + // whole content, so a one-shot boundingBox() can measure mid-flight + // and read ~42px. Poll until the box settles at the real size before + // asserting the 44px active-dimension touch target. + await expect + .poll(async () => Math.round((await option.boundingBox())?.height ?? 0)) + .toBeGreaterThanOrEqual(44); + await option.tap(); + await page.getByRole("button", { name: "Save changes" }).tap(); + + // Relative labels render without hover, with no horizontal overflow. + const relative = page.locator(CURRENT_SESSION_ROW).getByTestId("last-seen-relative"); + await expect(relative).toBeVisible({ timeout: 15_000 }); + const overflow = await page.evaluate( + () => document.documentElement.scrollWidth > document.documentElement.clientWidth, + ); + expect(overflow).toBe(false); + + // The absolute stamp stays reachable through an explicit touch drawer. + const absolute = await relative.getAttribute("title"); + expect(absolute).toBeTruthy(); + await expect(relative).toHaveAttribute("aria-label", absolute!); + await relative.tap(); + const absoluteDetails = page.getByTestId("last-seen-absolute"); + await expect(absoluteDetails).toBeVisible(); + await expect(absoluteDetails).toHaveText(absolute!); + } finally { + const res = await ctx.request.patch(`${backend.baseUrl}/api/v1/user/settings`, { + data: { last_seen_display: original }, + }); + expect(res.ok(), await res.text()).toBeTruthy(); + } + } finally { + await ctx.close(); + } + }); +}); diff --git a/apps/web/e2e/tests/auth/relative-last-seen.spec.ts b/apps/web/e2e/tests/auth/relative-last-seen.spec.ts new file mode 100644 index 0000000000..c48b68f4a6 --- /dev/null +++ b/apps/web/e2e/tests/auth/relative-last-seen.spec.ts @@ -0,0 +1,162 @@ +import { expect, type BrowserContext } from "@playwright/test"; +import path from "node:path"; +import { backendFixture as test } from "../../fixtures/backend"; +import { login, setupAdmin } from "../../helpers/auth"; +import { watchWs } from "../../helpers/causal-waits"; + +/** + * Relative "Last seen" on the account security page (desktop + cross-tab). + * + * Runs in the `auth` project with the worker backend restarted to require + * auth and use its own database. Serial: auth setup is single-shot per + * database and the worker DB survives restarts, so the file owns its own + * `KANDEV_DATABASE_PATH` (the users-self-actions isolation pattern) and + * restores the baseline backend in afterAll. + */ +const ADMIN = { email: "admin@demo.dev", password: "adminpass123", displayName: "Ada Admin" }; +const SECURITY_PATH = "/settings/account/security"; +const CURRENT_SESSION_ROW = '[data-testid="account-sessions-row"][data-current-session="true"]'; + +/** Fetches the user's current last_seen_display setting from the settings API. */ +async function readLastSeenDisplay(ctx: BrowserContext, baseUrl: string): Promise { + const res = await ctx.request.get(`${baseUrl}/api/v1/user/settings`); + expect(res.ok(), await res.text()).toBeTruthy(); + const body = (await res.json()) as { settings: { last_seen_display?: string } }; + return body.settings.last_seen_display ?? "absolute"; +} + +/** Persists the given last_seen_display value through the settings API. */ +async function restoreLastSeenDisplay( + ctx: BrowserContext, + baseUrl: string, + value: string, +): Promise { + const res = await ctx.request.patch(`${baseUrl}/api/v1/user/settings`, { + data: { last_seen_display: value }, + }); + expect(res.ok(), await res.text()).toBeTruthy(); +} + +test.describe.serial("relative last seen (desktop)", () => { + test.beforeAll(async ({ browser, backend }) => { + await backend.restart({ + KANDEV_FEATURES_AUTH: "true", + KANDEV_DATABASE_PATH: path.join(backend.tmpDir, "kandev-relative-last-seen.db"), + }); + const setupContext = await browser.newContext({ baseURL: backend.frontendUrl }); + try { + await setupAdmin(setupContext, backend.baseUrl, ADMIN); + } finally { + await setupContext.close(); + } + }); + + test.afterAll(async ({ backend }) => { + await backend.restart(); + }); + + test("switches Last seen to relative time and shows a relative label with an absolute tooltip", async ({ + browser, + backend, + }) => { + const ctx = await browser.newContext({ baseURL: backend.frontendUrl }); + try { + await login(ctx, backend.baseUrl, ADMIN); + let original = "absolute"; + try { + original = await readLastSeenDisplay(ctx, backend.baseUrl); + // Pin a known absolute baseline so selecting Relative time is a real + // transition and PATCH, not a no-op on a pre-existing relative value. + await restoreLastSeenDisplay(ctx, backend.baseUrl, "absolute"); + + const page = await ctx.newPage(); + await page.goto(SECURITY_PATH); + await expect(page.getByTestId("last-seen-relative")).toHaveCount(0); + + const select = page.getByTestId("last-seen-display-select"); + await expect(select).toBeVisible({ timeout: 15_000 }); + + await select.click(); + await page.getByRole("listbox").getByRole("option", { name: "Relative time" }).click(); + await page.getByRole("button", { name: "Save changes" }).click(); + + const relative = page.locator(CURRENT_SESSION_ROW).getByTestId("last-seen-relative"); + await expect(relative).toBeVisible({ timeout: 15_000 }); + + // Hover reveals the absolute timestamp in a tooltip, matching the + // trigger's accessible name/native title exactly. + const absolute = await relative.getAttribute("title"); + expect(absolute).toBeTruthy(); + await relative.hover(); + const tooltip = page.getByRole("tooltip"); + await expect(tooltip).toBeVisible(); + await expect(tooltip).toHaveText(absolute!); + + // The choice persists to the user-settings API. + await expect + .poll(() => readLastSeenDisplay(ctx, backend.baseUrl), { timeout: 15_000 }) + .toBe("relative"); + } finally { + await restoreLastSeenDisplay(ctx, backend.baseUrl, original); + } + } finally { + await ctx.close(); + } + }); + + test("syncs a Last seen change to another tab over WebSocket without a reload", async ({ + browser, + backend, + }) => { + const ctxA = await browser.newContext({ baseURL: backend.frontendUrl }); + const ctxB = await browser.newContext({ baseURL: backend.frontendUrl }); + try { + await login(ctxA, backend.baseUrl, ADMIN); + await login(ctxB, backend.baseUrl, ADMIN); + let original = "absolute"; + try { + original = await readLastSeenDisplay(ctxA, backend.baseUrl); + // Pin a known absolute baseline so tab A's pre-mutation display is + // established: the relative assertion below can only pass via a real + // WS event, not a stale pre-existing value. + await restoreLastSeenDisplay(ctxA, backend.baseUrl, "absolute"); + + const pageA = await ctxA.newPage(); + // watchWs only observes sockets opened after it is armed, and the + // subscription wait itself must be armed before navigation. + const watcher = watchWs(pageA); + const subscriptionAck = watcher.waitForResponse("user.subscribe"); + await pageA.goto(SECURITY_PATH); + await subscriptionAck; + await expect(pageA.getByTestId("last-seen-relative")).toHaveCount(0); + + // Arm the WS event wait before tab B mutates; frames are not buffered, + // so a wait armed after the mutation could miss the push entirely. + const settingsUpdated = watcher.waitForEvent("user.settings.updated"); + + const pageB = await ctxB.newPage(); + await pageB.goto(SECURITY_PATH); + await expect(pageB.getByTestId("last-seen-relative")).toHaveCount(0); + await pageB.getByTestId("last-seen-display-select").click(); + await pageB.getByRole("listbox").getByRole("option", { name: "Relative time" }).click(); + await pageB.getByRole("button", { name: "Save changes" }).click(); + await expect( + pageB.locator(CURRENT_SESSION_ROW).getByTestId("last-seen-relative"), + ).toBeVisible({ + timeout: 15_000, + }); + + // Tab A observes the change over WebSocket, without a reload. + await settingsUpdated; + await expect( + pageA.locator(CURRENT_SESSION_ROW).getByTestId("last-seen-relative"), + ).toBeVisible(); + } finally { + await restoreLastSeenDisplay(ctxA, backend.baseUrl, original); + } + } finally { + await ctxA.close(); + await ctxB.close(); + } + }); +}); diff --git a/apps/web/hooks/use-ensure-user-settings.test.ts b/apps/web/hooks/use-ensure-user-settings.test.ts index d5ec8cc9ea..ae510ae860 100644 --- a/apps/web/hooks/use-ensure-user-settings.test.ts +++ b/apps/web/hooks/use-ensure-user-settings.test.ts @@ -44,6 +44,7 @@ import { useEnsureUserSettings, } from "./use-ensure-user-settings"; +/** Builds a full default user-settings state marked as not loaded. */ function makeUnloadedSettings(): UserSettingsState { return { revision: null, @@ -98,6 +99,7 @@ function makeUnloadedSettings(): UserSettingsState { terminalFontFamily: null, terminalFontSize: null, changesPanelLayout: "tree", + lastSeenDisplay: "absolute", systemMetricsDisplay: { showInTopbar: false, simplified: false }, appStatusBarEnabled: false, appStatusBarOrder: { leftItemIds: [], rightItemIds: [] }, @@ -110,6 +112,7 @@ function makeUnloadedSettings(): UserSettingsState { }; } +/** Builds a mock settings API response wrapping the given task-create last-used payload. */ function userSettingsResponse(taskCreateLastUsed = {}) { return { shell_options: [], diff --git a/apps/web/lib/settings-discovery/catalog/account.ts b/apps/web/lib/settings-discovery/catalog/account.ts index 2597f1fbb0..d37410ce92 100644 --- a/apps/web/lib/settings-discovery/catalog/account.ts +++ b/apps/web/lib/settings-discovery/catalog/account.ts @@ -2,14 +2,16 @@ import type { SettingsDiscoveryDefinition } from "../types"; export const ACCOUNT_SETTINGS_HREF = "/settings/account"; export const ACCOUNT_SECURITY_SETTINGS_HREF = `${ACCOUNT_SETTINGS_HREF}/security`; +const ACCOUNT_SECURITY_ID = "account-security"; export const ACCOUNT_SETTINGS_TARGETS = { password: "setting-account-password", sessions: "setting-account-sessions", + lastSeenDisplay: "setting-account-last-seen-display", } as const; export const ACCOUNT_DISCOVERY_DEFINITIONS: SettingsDiscoveryDefinition[] = [ { - id: "account-security", + id: ACCOUNT_SECURITY_ID, kind: "page", labelKey: "sidebar:profileAndPassword", groupId: "access", @@ -30,7 +32,7 @@ export const ACCOUNT_DISCOVERY_DEFINITIONS: SettingsDiscoveryDefinition[] = [ id: "account-password", kind: "control", labelKey: "settings:password", - parentId: "account-security", + parentId: ACCOUNT_SECURITY_ID, groupId: "access", href: ACCOUNT_SECURITY_SETTINGS_HREF, targetId: ACCOUNT_SETTINGS_TARGETS.password, @@ -41,11 +43,22 @@ export const ACCOUNT_DISCOVERY_DEFINITIONS: SettingsDiscoveryDefinition[] = [ id: "account-sessions", kind: "section", labelKey: "settings:activeSessions", - parentId: "account-security", + parentId: ACCOUNT_SECURITY_ID, groupId: "access", href: ACCOUNT_SECURITY_SETTINGS_HREF, targetId: ACCOUNT_SETTINGS_TARGETS.sessions, order: 812, requires: "account", }, + { + id: "account-last-seen-display", + kind: "control", + labelKey: "account:lastSeenDisplay", + parentId: ACCOUNT_SECURITY_ID, + groupId: "access", + href: ACCOUNT_SECURITY_SETTINGS_HREF, + targetId: ACCOUNT_SETTINGS_TARGETS.lastSeenDisplay, + order: 813, + requires: "account", + }, ]; diff --git a/apps/web/lib/ssr/user-settings.test.ts b/apps/web/lib/ssr/user-settings.test.ts index 1080ffac4d..4bf8cedc0a 100644 --- a/apps/web/lib/ssr/user-settings.test.ts +++ b/apps/web/lib/ssr/user-settings.test.ts @@ -1,8 +1,11 @@ import { describe, it, expect } from "vitest"; import { buildCoreFields, + createDefaultUserSettings, + mapUserSettingsData, mapUserSettingsResponse, parseChangesPanelLayout, + parseLastSeenDisplay, parseLspStatusLocation, parseStartupPage, parseSystemMetricsDisplay, @@ -528,6 +531,50 @@ describe("parseChangesPanelLayout", () => { }); }); +describe("parseLastSeenDisplay", () => { + it('returns "relative" for "relative"', () => { + expect(parseLastSeenDisplay("relative")).toBe("relative"); + }); + + it('returns "absolute" for "absolute"', () => { + expect(parseLastSeenDisplay("absolute")).toBe("absolute"); + }); + + it('returns "absolute" for undefined or unknown values', () => { + expect(parseLastSeenDisplay(undefined)).toBe("absolute"); + expect(parseLastSeenDisplay("grid")).toBe("absolute"); + expect(parseLastSeenDisplay("")).toBe("absolute"); + }); +}); + +describe("last seen display hydration", () => { + it("defaults to absolute", () => { + expect(createDefaultUserSettings().lastSeenDisplay).toBe("absolute"); + }); + + it("maps a stored relative value", () => { + const mapped = mapUserSettingsData( + { last_seen_display: "relative" }, + createDefaultUserSettings(), + ); + expect(mapped.lastSeenDisplay).toBe("relative"); + }); + + it("normalizes unknown stored values to absolute", () => { + const mapped = mapUserSettingsData( + { last_seen_display: "garbage" as "absolute" | "relative" }, + createDefaultUserSettings(), + ); + expect(mapped.lastSeenDisplay).toBe("absolute"); + }); + + it("preserves the current value when omitted", () => { + const current = { ...createDefaultUserSettings(), lastSeenDisplay: "relative" as const }; + const mapped = mapUserSettingsData({}, current); + expect(mapped.lastSeenDisplay).toBe("relative"); + }); +}); + describe("prevent auto-start on open preference", () => { it("defaults the missing preference to false", () => { expect(buildCoreFields({}).preventAutoStartAgentOnOpen).toBe(false); diff --git a/apps/web/lib/ssr/user-settings.ts b/apps/web/lib/ssr/user-settings.ts index 9c60bfc8f2..50a50151a2 100644 --- a/apps/web/lib/ssr/user-settings.ts +++ b/apps/web/lib/ssr/user-settings.ts @@ -10,6 +10,7 @@ import { type UserSettingsState } from "@/lib/state/slices/settings/types"; import type { SidebarTaskPrefsApi, UserSettings, UserSettingsResponse } from "@/lib/types/http"; import type { LspStatusLocation, + LastSeenDisplay, MCPTaskAgentProfileDefault, StartupPage, } from "@/lib/types/http-user-settings"; @@ -18,6 +19,7 @@ export type UserSettingsData = Omit, "workspace_id"> & { workspace_id?: string; }; +/** Builds a fresh UserSettingsState with default values. */ export function createDefaultUserSettings(): UserSettingsState { return { revision: null, @@ -77,6 +79,7 @@ export function createDefaultUserSettings(): UserSettingsState { terminalFontFamily: null, terminalFontSize: null, changesPanelLayout: "tree", + lastSeenDisplay: "absolute", systemMetricsDisplay: { showInTopbar: false, simplified: false }, appStatusBarEnabled: false, appStatusBarOrder: { leftItemIds: [], rightItemIds: [] }, @@ -85,28 +88,39 @@ export function createDefaultUserSettings(): UserSettingsState { }; } +/** Parses the terminal link behavior, defaulting to "new_tab". */ export function parseTerminalLinkBehavior(value: string | undefined): "new_tab" | "browser_panel" { return value === "browser_panel" ? "browser_panel" : "new_tab"; } +/** Parses the changes panel layout, defaulting to "tree". */ export function parseChangesPanelLayout(value: string | undefined): "flat" | "tree" { return value === "flat" ? "flat" : "tree"; } +/** Parses the last-seen display format, defaulting to "absolute". */ +export function parseLastSeenDisplay(value: string | undefined): LastSeenDisplay { + return value === "relative" ? "relative" : "absolute"; +} + +/** Parses the MCP task agent profile default, defaulting to "current_task". */ export function parseMCPTaskAgentProfileDefault( value: string | undefined, ): MCPTaskAgentProfileDefault { return value === "workspace_default" ? "workspace_default" : "current_task"; } +/** Parses the startup page preference, defaulting to "task_overview". */ export function parseStartupPage(value: string | undefined): StartupPage { return value === "last_task" ? "last_task" : "task_overview"; } +/** Parses the LSP status location, defaulting to "toolbar". */ export function parseLspStatusLocation(value: string | undefined): LspStatusLocation { return value === "status_bar" ? "status_bar" : "toolbar"; } +/** Parses the system metrics display fields with defaults. */ export function parseSystemMetricsDisplay(value: UserSettingsData["system_metrics_display"]) { return { showInTopbar: value?.show_in_topbar ?? false, @@ -114,6 +128,7 @@ export function parseSystemMetricsDisplay(value: UserSettingsData["system_metric }; } +/** Parses the app status bar order fields with empty defaults. */ export function parseAppStatusBarOrder(value: UserSettingsData["app_status_bar_order"]) { return { leftItemIds: value?.left_item_ids ?? [], @@ -121,6 +136,7 @@ export function parseAppStatusBarOrder(value: UserSettingsData["app_status_bar_o }; } +/** Maps terminal-related API fields onto state, falling back to current values. */ function buildTerminalFields(s: UserSettingsData, current: UserSettingsState) { return { terminalLinkBehavior: @@ -140,6 +156,7 @@ function buildTerminalFields(s: UserSettingsData, current: UserSettingsState) { }; } +/** Maps the system metrics display field onto state, falling back to current. */ function buildSystemMetricsDisplayFields( s: UserSettingsData | undefined, current: UserSettingsState, @@ -152,6 +169,7 @@ function buildSystemMetricsDisplayFields( }; } +/** Parses sidebar task preferences with empty-array defaults. */ function parseSidebarTaskPrefs(value: SidebarTaskPrefsApi | undefined) { return { pinnedTaskIds: value?.pinned_task_ids ?? [], @@ -160,6 +178,7 @@ function parseSidebarTaskPrefs(value: SidebarTaskPrefsApi | undefined) { }; } +/** Returns true when the task-create last-used payload carries any value. */ export function taskCreateLastUsedHasValue( value: UserSettingsData["task_create_last_used"] | undefined, ) { @@ -172,6 +191,7 @@ export function taskCreateLastUsedHasValue( ); } +/** Parses task-create last-used fields, deriving the synced flag from presence. */ function parseTaskCreateLastUsed(value: UserSettingsData["task_create_last_used"] | undefined) { return { repositoryId: value?.repository_id || null, @@ -183,6 +203,7 @@ function parseTaskCreateLastUsed(value: UserSettingsData["task_create_last_used" }; } +/** Returns current when value is undefined, otherwise maps it. */ function mapDefined( value: TInput | undefined, current: TOutput, @@ -191,10 +212,12 @@ function mapDefined( return value === undefined ? current : map(value); } +/** Maps an optional string, normalizing empty strings to null. */ function mapNullableString(value: string | undefined, current: string | null) { return mapDefined(value, current, (defined) => defined || null); } +/** Maps identity-related API fields onto state, falling back to current values. */ function buildIdentityFields(s: UserSettingsData, current: UserSettingsState) { return { workspaceId: mapNullableString(s.workspace_id, current.workspaceId), @@ -213,6 +236,7 @@ function buildIdentityFields(s: UserSettingsData, current: UserSettingsState) { }; } +/** Maps behavior-related API fields onto state, falling back to current values. */ function buildBehaviorFields(s: UserSettingsData, current: UserSettingsState) { return { enablePreviewOnClick: s.enable_preview_on_click ?? current.enablePreviewOnClick, @@ -233,6 +257,7 @@ function buildBehaviorFields(s: UserSettingsData, current: UserSettingsState) { }; } +/** Maps appearance-related API fields onto state, falling back to current values. */ function buildAppearanceFields(s: UserSettingsData, current: UserSettingsState) { return { showAnchoredPromptBar: s.show_anchored_prompt_bar ?? current.showAnchoredPromptBar, @@ -248,9 +273,11 @@ function buildAppearanceFields(s: UserSettingsData, current: UserSettingsState) s.release_notes_last_seen_version, current.releaseNotesLastSeenVersion, ), + lastSeenDisplay: mapDefined(s.last_seen_display, current.lastSeenDisplay, parseLastSeenDisplay), }; } +/** Maps the core user-settings API fields onto state, falling back to current values. */ export function buildCoreFields( s: UserSettingsData, current: UserSettingsState = createDefaultUserSettings(), @@ -312,6 +339,7 @@ export function buildCoreFields( }; } +/** Maps LSP-related API fields onto state, falling back to current values. */ export function buildLspFields( s: UserSettingsData | undefined, current: UserSettingsState = createDefaultUserSettings(), @@ -327,6 +355,7 @@ export function buildLspFields( }; } +/** Maps API settings data onto a complete UserSettingsState, marking it loaded. */ export function mapUserSettingsData( settings: UserSettingsData, current: UserSettingsState = createDefaultUserSettings(), diff --git a/apps/web/lib/state/slices/settings/types.ts b/apps/web/lib/state/slices/settings/types.ts index a1f9239b7b..f5d87c1a58 100644 --- a/apps/web/lib/state/slices/settings/types.ts +++ b/apps/web/lib/state/slices/settings/types.ts @@ -11,6 +11,7 @@ import type { SavedLayout, ToolStatus, LspStatusLocation, + LastSeenDisplay, MCPTaskAgentProfileDefault, StartupPage, } from "@/lib/types/http"; @@ -297,6 +298,7 @@ export type UserSettingsState = { terminalFontFamily: string | null; terminalFontSize: number | null; changesPanelLayout: "flat" | "tree"; + lastSeenDisplay: LastSeenDisplay; systemMetricsDisplay: { showInTopbar: boolean; simplified: boolean }; appStatusBarEnabled: boolean; appStatusBarOrder: AppStatusBarOrderState; diff --git a/apps/web/lib/types/http-user-settings.ts b/apps/web/lib/types/http-user-settings.ts index b7ee5e72cd..4d361eccd0 100644 --- a/apps/web/lib/types/http-user-settings.ts +++ b/apps/web/lib/types/http-user-settings.ts @@ -3,6 +3,7 @@ import type { WorkspaceId } from "./ids"; export type MCPTaskAgentProfileDefault = "current_task" | "workspace_default"; export type StartupPage = "task_overview" | "last_task"; export type LspStatusLocation = "toolbar" | "status_bar"; +export type LastSeenDisplay = "absolute" | "relative"; export type SavedLayout = { id: string; @@ -100,6 +101,7 @@ export type UserSettings = { terminal_font_family?: string; terminal_font_size?: number; changes_panel_layout?: "flat" | "tree"; + last_seen_display?: LastSeenDisplay; system_metrics_display?: { show_in_topbar?: boolean; simplified?: boolean }; app_status_bar_enabled?: boolean; app_status_bar_order?: AppStatusBarOrderApi; @@ -164,6 +166,7 @@ export type UserSettingsUpdatePayload = { terminal_font_family?: string; terminal_font_size?: number; changes_panel_layout?: "flat" | "tree"; + last_seen_display?: LastSeenDisplay; system_metrics_display?: { show_in_topbar?: boolean; simplified?: boolean }; app_status_bar_enabled?: boolean; app_status_bar_order?: AppStatusBarOrderApi; diff --git a/apps/web/lib/types/http.ts b/apps/web/lib/types/http.ts index f6e44a8da5..3d0e9a54d4 100644 --- a/apps/web/lib/types/http.ts +++ b/apps/web/lib/types/http.ts @@ -28,6 +28,7 @@ export type { TaskCreateLastUsedApi, AppStatusBarOrderApi, LspStatusLocation, + LastSeenDisplay, MCPTaskAgentProfileDefault, StartupPage, UserSettings, diff --git a/apps/web/lib/ws/handlers/users.test.ts b/apps/web/lib/ws/handlers/users.test.ts index 05611caa01..ac65c17504 100644 --- a/apps/web/lib/ws/handlers/users.test.ts +++ b/apps/web/lib/ws/handlers/users.test.ts @@ -5,10 +5,12 @@ import { defaultState } from "@/lib/state/default-state"; import type { AppState } from "@/lib/state/store"; import type { BackendMessageMap } from "@/lib/types/backend"; +/** Creates a vanilla Zustand store seeded with a structured clone of the default app state. */ function makeStore() { return createStore(() => structuredClone(defaultState) as AppState); } +/** Builds a `user.settings.updated` notification frame merging the given payload over default values. */ function userSettingsMessage( payload: Partial, ): BackendMessageMap["user.settings.updated"] { @@ -103,6 +105,46 @@ describe("status bar visibility websocket sync", () => { }); }); +describe("last seen display websocket sync", () => { + it("applies a relative value and normalizes unknown values", () => { + const store = makeStore(); + expect(store.getState().userSettings.lastSeenDisplay).toBe("absolute"); + + registerUsersHandlers(store)["user.settings.updated"]?.( + userSettingsMessage({ last_seen_display: "relative" }), + ); + expect(store.getState().userSettings.lastSeenDisplay).toBe("relative"); + + registerUsersHandlers(store)["user.settings.updated"]?.( + userSettingsMessage({ + last_seen_display: "unexpected", + } as unknown as Partial), + ); + expect(store.getState().userSettings.lastSeenDisplay).toBe("absolute"); + }); + + it("preserves the current value when omitted", () => { + const store = makeStore(); + store.setState((state) => ({ + ...state, + userSettings: { ...state.userSettings, lastSeenDisplay: "relative" }, + })); + registerUsersHandlers(store)["user.settings.updated"]?.(userSettingsMessage({})); + expect(store.getState().userSettings.lastSeenDisplay).toBe("relative"); + }); + + it("ignores older settings revisions", () => { + const store = makeStore(); + store.setState((state) => ({ + ...state, + userSettings: { ...state.userSettings, lastSeenDisplay: "absolute", revision: 2 }, + })); + const handler = registerUsersHandlers(store)["user.settings.updated"]; + handler?.(userSettingsMessage({ last_seen_display: "relative", revision: 1 })); + expect(store.getState().userSettings.lastSeenDisplay).toBe("absolute"); + }); +}); + describe("user settings websocket handler", () => { it("updates LSP status location, normalizes unknown values, and preserves omissions", () => { const store = makeStore(); diff --git a/apps/web/src/locales/en/account.json b/apps/web/src/locales/en/account.json index c9aedeb2a8..70650bc2ec 100644 --- a/apps/web/src/locales/en/account.json +++ b/apps/web/src/locales/en/account.json @@ -1,6 +1,7 @@ { "actions": "Actions", "activeSessions": "Active sessions", + "absoluteTime": "Absolute time", "apiTokens": "API tokens", "apiTokensBlurb": "Personal access tokens authenticate scripts and CLIs as you. Revoke a token immediately if it may have leaked.", "cancel": "Cancel", @@ -17,8 +18,12 @@ "done": "Done", "failedToLoadSessions": "Failed to load sessions.", "failedToLoadTokens": "Failed to load tokens.", + "failedToSaveLastSeenDisplay": "Failed to save the last seen display.", + "failedToRevokeSession": "Failed to revoke the session.", "ipAddress": "IP", "lastSeen": "Last seen", + "lastSeenDisplay": "Last seen display", + "lastSeenDisplayDescription": "Choose how session activity times are shown.", "lastUsed": "Last used", "loadingSessions": "Loading sessions...", "loadingTokens": "Loading tokens...", @@ -29,6 +34,7 @@ "noTokensYet": "No tokens yet.", "password": "Password", "passwordUpdated": "Password updated.", + "relativeTime": "Relative time", "revoke": "Revoke", "saving": "Saving...", "securityPageDescription": "Change your password and review devices signed in to your account.", diff --git a/apps/web/src/locales/pseudo/account.json b/apps/web/src/locales/pseudo/account.json index f567ce397d..a134a8f8d2 100644 --- a/apps/web/src/locales/pseudo/account.json +++ b/apps/web/src/locales/pseudo/account.json @@ -1,6 +1,7 @@ { "actions": "Àćţĩōńś", "activeSessions": "Àćţĩvē śēśśĩōńś", + "absoluteTime": "Àƀśōĺũţē ţĩḿē", "apiTokens": "ÀƤĨ ţōķēńś", "apiTokensBlurb": "Ƥēŕśōńàĺ àććēśś ţōķēńś àũţĥēńţĩćàţē śćŕĩƥţś àńď ĆĹĨś àś ŷōũ. Ŕēvōķē à ţōķēń ĩḿḿēďĩàţēĺŷ ĩƒ ĩţ ḿàŷ ĥàvē ĺēàķēď.", "cancel": "Ćàńćēĺ", @@ -17,8 +18,12 @@ "done": "Ďōńē", "failedToLoadSessions": "Ƒàĩĺēď ţō ĺōàď śēśśĩōńś.", "failedToLoadTokens": "Ƒàĩĺēď ţō ĺōàď ţōķēńś.", + "failedToSaveLastSeenDisplay": "Ƒàĩĺēď ţō śàvē ţĥē ĺàśţ śēēń ďĩśƥĺàŷ.", + "failedToRevokeSession": "Ƒàĩĺēď ţō ŕēvōķē ţĥē śēśśĩōń.", "ipAddress": "ĨƤ", "lastSeen": "Ĺàśţ śēēń", + "lastSeenDisplay": "Ĺàśţ śēēń ďĩśƥĺàŷ", + "lastSeenDisplayDescription": "Ćĥōōśē ĥōŵ śēśśĩōń àćţĩvĩţŷ ţĩḿēś àŕē śĥōŵń.", "lastUsed": "Ĺàśţ ũśēď", "loadingSessions": "Ĺōàďĩńĝ śēśśĩōńś...", "loadingTokens": "Ĺōàďĩńĝ ţōķēńś...", @@ -29,6 +34,7 @@ "noTokensYet": "Ńō ţōķēńś ŷēţ.", "password": "Ƥàśśŵōŕď", "passwordUpdated": "Ƥàśśŵōŕď ũƥďàţēď.", + "relativeTime": "Ŕēĺàţĩvē ţĩḿē", "revoke": "Ŕēvōķē", "saving": "Śàvĩńĝ...", "securityPageDescription": "Ćĥàńĝē ŷōũŕ ƥàśśŵōŕď àńď ŕēvĩēŵ ďēvĩćēś śĩĝńēď ĩń ţō ŷōũŕ àććōũńţ.", diff --git a/apps/web/src/locales/pt-pt/account.json b/apps/web/src/locales/pt-pt/account.json index 2f3b74862b..9bf00977fe 100644 --- a/apps/web/src/locales/pt-pt/account.json +++ b/apps/web/src/locales/pt-pt/account.json @@ -1,6 +1,7 @@ { "actions": "Ações", "activeSessions": "Sessões ativas", + "absoluteTime": "Hora absoluta", "apiTokens": "Tokens de API", "apiTokensBlurb": "Os tokens de acesso pessoal autenticam scripts e CLI como se fosse o utilizador. Revogue um token de imediato se houver a possibilidade de ter sido divulgado.", "cancel": "Cancelar", @@ -17,8 +18,12 @@ "done": "Concluído", "failedToLoadSessions": "Não foi possível carregar as sessões.", "failedToLoadTokens": "Não foi possível carregar os tokens.", + "failedToSaveLastSeenDisplay": "Não foi possível guardar a forma de apresentação da última atividade.", + "failedToRevokeSession": "Não foi possível revogar a sessão.", "ipAddress": "IP", "lastSeen": "Visto pela última vez", + "lastSeenDisplay": "Apresentação da última atividade", + "lastSeenDisplayDescription": "Escolha como as horas de atividade das sessões são apresentadas.", "lastUsed": "Última utilização", "loadingSessions": "A carregar as sessões...", "loadingTokens": "A carregar os tokens...", @@ -29,6 +34,7 @@ "noTokensYet": "Ainda não há tokens.", "password": "Palavra-passe", "passwordUpdated": "Palavra-passe atualizada.", + "relativeTime": "Hora relativa", "revoke": "Revogar", "saving": "A guardar...", "securityPageDescription": "Altere a sua palavra-passe e reveja os dispositivos com sessão iniciada na sua conta.", diff --git a/apps/web/src/locales/zh-cn/account.json b/apps/web/src/locales/zh-cn/account.json index e30c2023e7..67bc46540a 100644 --- a/apps/web/src/locales/zh-cn/account.json +++ b/apps/web/src/locales/zh-cn/account.json @@ -1,6 +1,7 @@ { "actions": "操作", "activeSessions": "活动会话", + "absoluteTime": "绝对时间", "apiTokens": "API 令牌", "apiTokensBlurb": "个人访问令牌可让脚本和 CLI 以你的身份认证。若令牌可能已泄露，请立即吊销。", "cancel": "取消", @@ -17,8 +18,12 @@ "done": "完成", "failedToLoadSessions": "加载会话失败。", "failedToLoadTokens": "加载令牌失败。", + "failedToSaveLastSeenDisplay": "无法保存上次活动的显示方式。", + "failedToRevokeSession": "无法吊销会话。", "ipAddress": "IP", "lastSeen": "上次活动", + "lastSeenDisplay": "上次活动显示", + "lastSeenDisplayDescription": "选择会话活动时间的显示方式。", "lastUsed": "上次使用", "loadingSessions": "正在加载会话…", "loadingTokens": "正在加载令牌…", @@ -29,6 +34,7 @@ "noTokensYet": "暂无令牌。", "password": "密码", "passwordUpdated": "密码已更新。", + "relativeTime": "相对时间", "revoke": "吊销", "saving": "正在保存…", "securityPageDescription": "修改密码并查看已登录你账户的设备。", diff --git a/apps/web/src/locales/zh-hk/account.json b/apps/web/src/locales/zh-hk/account.json index b3beb8f3cd..8f73830775 100644 --- a/apps/web/src/locales/zh-hk/account.json +++ b/apps/web/src/locales/zh-hk/account.json @@ -1,6 +1,7 @@ { "actions": "操作", "activeSessions": "活動工作階段", + "absoluteTime": "絕對時間", "apiTokens": "API 權杖", "apiTokensBlurb": "個人存取權杖可讓腳本和 CLI 以你的身份驗證。若權杖可能已泄露，請立即吊銷。", "cancel": "取消", @@ -17,8 +18,12 @@ "done": "完成", "failedToLoadSessions": "載入工作階段失敗。", "failedToLoadTokens": "載入權杖失敗。", + "failedToSaveLastSeenDisplay": "無法儲存上次活動的顯示方式。", + "failedToRevokeSession": "無法吊銷工作階段。", "ipAddress": "IP", "lastSeen": "上次活動", + "lastSeenDisplay": "上次活動顯示", + "lastSeenDisplayDescription": "選擇工作階段活動時間的顯示方式。", "lastUsed": "上次使用", "loadingSessions": "正在載入工作階段…", "loadingTokens": "正在載入權杖…", @@ -29,6 +34,7 @@ "noTokensYet": "暫無權杖。", "password": "密碼", "passwordUpdated": "密碼已更新。", + "relativeTime": "相對時間", "revoke": "吊銷", "saving": "正在儲存…", "securityPageDescription": "修改密碼並查看已登入你帳戶的設備。", diff --git a/apps/web/src/locales/zh-tw/account.json b/apps/web/src/locales/zh-tw/account.json index 4c7f035ae9..4e4b25aadf 100644 --- a/apps/web/src/locales/zh-tw/account.json +++ b/apps/web/src/locales/zh-tw/account.json @@ -1,6 +1,7 @@ { "actions": "操作", "activeSessions": "活動工作階段", + "absoluteTime": "絕對時間", "apiTokens": "API 權杖", "apiTokensBlurb": "個人存取權杖可讓指令碼和 CLI 以你的身分驗證。若權杖可能已洩露，請立即吊銷。", "cancel": "取消", @@ -17,8 +18,12 @@ "done": "完成", "failedToLoadSessions": "載入工作階段失敗。", "failedToLoadTokens": "載入權杖失敗。", + "failedToSaveLastSeenDisplay": "無法儲存上次活動的顯示方式。", + "failedToRevokeSession": "無法吊銷工作階段。", "ipAddress": "IP", "lastSeen": "上次活動", + "lastSeenDisplay": "上次活動顯示", + "lastSeenDisplayDescription": "選擇工作階段活動時間的顯示方式。", "lastUsed": "上次使用", "loadingSessions": "正在載入工作階段…", "loadingTokens": "正在載入權杖…", @@ -29,6 +34,7 @@ "noTokensYet": "暫無權杖。", "password": "密碼", "passwordUpdated": "密碼已更新。", + "relativeTime": "相對時間", "revoke": "吊銷", "saving": "正在儲存…", "securityPageDescription": "修改密碼並檢視已登入你帳戶的裝置。", diff --git a/docs/plans/relative-last-seen/plan.md b/docs/plans/relative-last-seen/plan.md new file mode 100644 index 0000000000..d10283fe0e --- /dev/null +++ b/docs/plans/relative-last-seen/plan.md @@ -0,0 +1,282 @@ +--- +spec: docs/specs/relative-last-seen/spec.md +created: 2026-08-16 +status: done +--- + +# Implementation Plan: Relative Last Seen in Account Security + +## Overview + +Add a persisted per-user enum setting `last_seen_display` (`"absolute"` default | `"relative"`) and +a select on the Active sessions card of `/settings/account/security`. In relative mode the Last seen +column renders a live-updating, locale-aware relative label with the absolute timestamp in a +fine-pointer tooltip or a coarse-pointer touch drawer. The setting follows the established +`changes_panel_layout` pattern end to end: JSON-blob user settings on the backend, boot-payload + +WS-push hydration on the frontend, and the shared manual-save settings flow in the UI. + +## Design decisions + +- **Shared manual save.** The select edits a local draft. The shared `Save changes` action calls + `useSettingsSaveContributor` and persists the draft. The shared `Reset` action restores the + confirmed value. A failed save keeps the draft dirty so the user can retry. +- **Queued write with revision-aware confirmation.** The contributor save uses + `createQueuedUserSettingsSyncWithResponse`. The draft lives in local component state and is + never written into the store directly. The store's `userSettings.lastSeenDisplay` is the + confirmed baseline, updated by exactly two ingestion paths: a successful PATCH response mapped + with `mapUserSettingsResponse(response, current)` and applied through `setUserSettings` (which + discards older revisions), and a newer `user.settings.updated` WS snapshot through the existing + handler path (`mapUserSettingsData` + handler revision gate, unchanged per task 02). A save + response reconciles the draft with the revision-gated store result, so a newer foreign snapshot + wins. A failed save leaves the submitted draft in place. +- **Revision-gated draft reconciliation; no writer-id correlation or store metadata.** The backend + publishes the settings event before the PATCH response and broadcasts a full snapshot to every + subscriber. A newer local draft stays visible while Save is in flight. The unchanged WebSocket + handler applies own echoes and foreign snapshots to the confirmed store. After a successful + response, the component reconciles the draft with the revision-gated store result. A failed save + keeps the draft dirty for retry. A writer-id scheme is not needed because the shared settings + provider owns Save and Reset, and the store remains the confirmed baseline. +- **Default `"absolute"`** preserves current behavior for users who never opt in. +- **`formatRelativeTime` from `@/lib/i18n/formats`** (locale-aware, accepts `now`) renders the + relative label; the shared `useNow` hook drives the live update only while the cell is in + relative mode (mounted in a dedicated child component so the interval is never created in + absolute mode), with an age-aware cadence: tick every second while the timestamp is under a + minute old (second-scale labels need live updates) and every minute once the age crosses a + minute. +- **Exact time disclosure** uses `@kandev/ui/tooltip` with `formatDateTime(last_seen_at)` on + fine-pointer devices. Coarse-pointer devices use the shipped `useTouchDrawer` and Drawer + pattern to show the same timestamp after a tap. + +## Backend + +- `apps/backend/internal/user/models/models.go`: add `LastSeenDisplay string` to `UserSettings` + with `json:"last_seen_display"`; add `LastSeenDisplayAbsolute = "absolute"` / + `LastSeenDisplayRelative = "relative"` constants and a `NormalizeLastSeenDisplay` helper + (anything other than `relative` coerces to `absolute`). +- `apps/backend/internal/user/store/sqlite.go`: default `"absolute"` in `defaultUserSettings`; add + the field to BOTH hand-built JSON codec paths (the model struct alone does not persist it): + `marshalUserSettingsPayload` map entry, the `scanUserSettings` payload struct field, and the + scan assignment with `NormalizeLastSeenDisplay` coercion. Guard the PATCH write against the + read-modify-write lost-update race: `UpsertUserSettingsPreservingTaskCreateLastUsed` gains an + `expectedRevision` parameter and its UPDATE gains `AND settings_revision = ?` (SQLite and + Postgres paths), returning a revision-conflict sentinel when zero rows match. The conflict is + distinguished from a missing user explicitly: after a zero-row conditional UPDATE the repo + existence-checks the user row; missing → the existing user-not-found error, present → + `ErrUserSettingsRevisionConflict` (both drivers tested). The service routes ALL full-blob + writers through one bounded read-apply-CAS-retry helper (read → apply patch → upsert with the + read revision → on conflict re-read and re-apply the ORIGINAL patch, ~3 attempts): both + `UpdateUserSettings` and `ClearDefaultEditorID` (service.go:883-900 currently upserts once and + would otherwise either fail to clear on conflict or bypass CAS and overwrite the setting), with + exactly one event publication after the successful final write. The helper carries the IMMUTABLE + `taskCreatePatch` extracted from the original request (service.go:198-202) as a separate input + passed to EVERY CAS attempt, and treats a non-empty task patch as `applied` even when the blob + is otherwise unchanged (a task-create-only PATCH must not be classified no-op and dropped); a + task-create-only PATCH under a forced conflict survives the retry, merges with the winner, + increments the revision once on its own successful retry (the winner already bumped R to R+1, so + the final revision is R+2), and emits the snapshot event (existing public behavior, + service.go:197-206 / service_test.go:1257-1283). Every upsert callsite is + updated: the store interface, both driver implementations, the controller/service test fakes, + and the direct sqlite_test.go calls (~31/96/151/1274/1323/1379, passing the revision from the + preceding read; the existing concurrent test at ~150 asserts conflict-and-retry semantics). + Without this, a concurrent omitted-field PATCH (app-status-bar/review save) that read the + pre-PATCH row can serialize a stale snapshot over `last_seen_display` at a newer revision, + which the frontend then accepts as truth. +- `apps/backend/internal/user/service/service.go`: add `LastSeenDisplay *string` to + `UpdateUserSettingsRequest`; add `applyLastSeenDisplay` (trim, validate against the two values, + reject otherwise) and call it from `applyBasicSettings`; add + `"last_seen_display": models.NormalizeLastSeenDisplay(settings.LastSeenDisplay)` to the + hand-built `publishUserSettingsEvent` snapshot map so the WS `user.settings.updated` payload + carries the field to every other tab. NO writer-id plumbing: the publisher signature stays + `publishUserSettingsEvent(ctx, settings)` (no direct test callsite changes needed: + `app_status_bar_visibility_test.go:44`, `service_test.go:1312, 1333, 1354, 1377, 1399, 1417`), + and pending-selection protection is entirely client-side (pending-write gating). +- `apps/backend/internal/user/dto/dto.go`: add `LastSeenDisplay` to `UserSettingsDTO` + (`json:"last_seen_display"`), map it in `FromUserSettings` via + `models.NormalizeLastSeenDisplay(settings.LastSeenDisplay)` (matching how every other enum-like + setting is normalized at the API boundary, so a model assembled outside the store can never emit + a non-canonical value); add `LastSeenDisplay *string` to the DTO `UpdateUserSettingsRequest`. +- `apps/backend/internal/user/controller/controller.go`: pass `req.LastSeenDisplay` into the + service request. The controller test asserts `req.LastSeenDisplay` reaches the service request + (the REST handler and the WS `wsUpdateUserSettings` handler share this one controller path, + handlers.go:113, so one test covers both transports). +- `apps/backend/internal/backendapp/boot_state_routes.go`: map `lastSeenDisplay` in + `mapUserSettingsState` via a small normalize helper (mirroring `changesPanelLayout(...)`). + +## Frontend contracts + +- `apps/web/lib/types/http-user-settings.ts`: `LastSeenDisplay` type + (`"absolute" | "relative"`); `last_seen_display?: LastSeenDisplay` on `UserSettings` and + `UserSettingsUpdatePayload`. +- `apps/web/lib/types/http.ts`: add `LastSeenDisplay` to the explicit `http-user-settings` + re-export list (the established import surface; omitting it breaks type resolution for barrel + consumers). +- `apps/web/lib/state/slices/settings/types.ts`: `lastSeenDisplay: LastSeenDisplay` on + `UserSettingsState` (default `"absolute"`). NO client-only metadata field: pending-selection + protection lives in the component (pending-write gating), so no writer-id/lastWriterId state + exists to be wiped by `mapUserSettingsResponse` callers that build from a fresh default state + (use-layout-settings, layout-preset-selector, use-ensure-user-settings, hydration). +- `apps/web/lib/ssr/user-settings.ts`: default `"absolute"` for `lastSeenDisplay` in + `createDefaultUserSettings`; `parseLastSeenDisplay` (only `"relative"` accepted, else + `"absolute"`); map `lastSeenDisplay` in `buildAppearanceFields` via `mapDefined`. +- The WS `user.settings.updated` handler in `apps/web/lib/ws/handlers/users.ts` picks the field up + through `mapUserSettingsData` with its existing revision gate, UNCHANGED otherwise (own echoes + are applied normally, so the store converges even if the component unmounts mid-write); add + `users.test.ts` assertions (valid value applies, unknown value normalizes to `"absolute"`, + omitted field leaves the current value, stale revision ignored), so the cross-tab + producer→handler path is tested on both sides. +- `apps/web/hooks/use-ensure-user-settings.test.ts` (`makeUnloadedSettings`, lines ~47-110) builds + a complete typed `UserSettingsState` and gains `lastSeenDisplay: "absolute"`; audit remaining + typed fixtures for the new required field. + +## Security page UI + +- `apps/web/components/settings/account/security-settings.tsx`: + - Read `userSettings.lastSeenDisplay` from the store in `SessionsCard`. + - Add a labeled select (Absolute time / Relative time) in the card, above the table, with a + discovery target id `ACCOUNT_SETTINGS_TARGETS.lastSeenDisplay` and self-documenting copy. + - On change: keep the draft in local component state and mark the `SessionsCard` dirty. Register + the field with `useSettingsSaveContributor`. The contributor's `save` sends a queued + `createQueuedUserSettingsSyncWithResponse({ last_seen_display })` write. The contributor's + `discard` restores the confirmed store value. A failed save keeps the draft dirty and shows + error copy so the shared Save action can retry. + - Apply successful responses through `mapUserSettingsResponse` into `setUserSettings` only + (never a raw store write). `setUserSettings` discards older revisions, so a deferred PATCH + response arriving after a newer cross-tab WS event cannot regress the value or revision. After + the response, reconcile the draft with the revision-gated store result. A newer foreign value + therefore wins when it supersedes the submitted response. + - Extract a small `LastSeenCell` component: absolute mode renders `formatDateTime`; relative mode + mounts a ticking `useNow` wrapper and renders a `formatRelativeTime(last_seen_at, now)` label. + Fine-pointer devices use a focusable semantic trigger and a `Tooltip` with the absolute time. + Coarse-pointer devices use an explicit tap trigger and the shipped `useTouchDrawer` Drawer + pattern to show the same absolute time. The cell validates the timestamp once; when unparseable + it renders an empty cell with no disclosure surface (guarding the `formatDateTime` content, + which throws `RangeError` on invalid dates). +- `apps/web/lib/settings-discovery/catalog/account.ts`: add `lastSeenDisplay: + "setting-account-last-seen-display"` target and a `kind: "control"` discovery definition. The + SessionsCard's `SettingsCard` already registers the `sessions` target and accepts exactly one + `discoveryTargetId` (settings-card.tsx:7-30), so the select's wrapper registers the new target + separately with `useSettingsTargetRegistration(ACCOUNT_SETTINGS_TARGETS.lastSeenDisplay)` (from + `settings-target-provider.tsx`) rather than replacing the card's target; add a discovery-target + test asserting both targets register and reveal. +- i18n: new keys in `apps/web/src/locales/en/account.json` (+ pseudo/pt-pt/zh-cn/zh-hk/zh-tw + mirrors). Use plain punctuation (no em dash). + +## Tests + +- **Backend:** `service` test for `applyLastSeenDisplay` (accept both values, reject invalid, nil + no-op); a `publishUserSettingsEvent` test asserting the published event map carries the + normalized `last_seen_display` (in `service/user_settings_cas_test.go`; the existing publisher + tests at `service_test.go:1312-1417` stay untouched); store tests asserting `defaultUserSettings`, + a JSON round-trip of the new field through `marshalUserSettingsPayload`/`scanUserSettings` + (stored `"relative"` reads back as `"relative"`, stored unknown/empty coerces to `"absolute"`, a + later PATCH updates the stored JSON); a `dto` test asserting `FromUserSettings` emits the + canonical value for an unknown model value; a `controller` mapping test asserting + `req.LastSeenDisplay` reaches the service request (the controller is a manual field-by-field + adapter; an omission compiles and silently no-ops); `boot_state_routes_test.go` mapping test + (relative passes through, unknown coerces to absolute); a barrier-based concurrent PATCH test + proving an unrelated omitted-field write (started from the same initial revision) cannot revert + `last_seen_display` — both fields are present in the final row and the retry loop merges the + loser's patch; a concurrent `ClearDefaultEditorID` + settings PATCH test (the editor is cleared + AND `last_seen_display` survives; TWO events are published, one per successful write with + distinct revisions — the helper contract is exactly one event per successful final write, so two + successful writes emit two events; a one-event outcome is only valid in the no-op scenario); a + CAS-conflict no-op barrier test that FORCES callback re-evaluation (Clear's initial read + completes seeing `old-editor`, its upsert is blocked; the PATCH commits `new-editor` at R+1 and + publishes; Clear's stale upsert then conflicts, the retry fresh-reads the row, the callback + re-evaluates with the immutable `old-editor` and returns `applied=false` → no Clear write/event; + assert read count >= 2, `clearUpsertAttempts == 1` (the forced stale CAS invocation that + conflicts) with `clearSuccessfulUpserts == 0`, `clearEvents == 0`, exactly one PATCH event — + counters named to distinguish attempts from successful commits) PLUS a separate + fresh-read no-op test (Clear starts after the PATCH committed; immediate no-op, no conflict); + repo zero-row tests for both drivers (missing user → user-not-found, + revision mismatch → `ErrUserSettingsRevisionConflict`); direct repo tests migrated to CAS and + MOVED to new focused files (the unique-revision concurrent test becomes one success + one + conflict; the stale-blob merge tests re-read the current revision or assert the conflict) + because `sqlite_test.go`/`service_test.go` are over Revive's 800-line test-file limit and new + code there fails the changed-file lint — new files: `store/sqlite_last_seen_cas_test.go`, + `service/user_settings_cas_test.go`. The concurrent SERVICE tests use a dedicated synchronized + fake (clones settings per read, models revision-conditional writes, records reads/upserts and + events through synchronized state/channels, channel barriers for phase observation) — NOT the + existing `recordingUserRepository`/`recordingEventBus` fakes, which share one mutable + `getSettings` pointer, mutate unprotected fields, and append events unsynchronized + (service_test.go:1741-1801), racing under `go test -race`. +- **Frontend:** `ssr/user-settings.test.ts` covers `parseLastSeenDisplay` and + `mapUserSettingsData`; `users.test.ts` covers valid, unknown, omitted, and stale WebSocket + snapshots. The `SessionsCard` tests cover the absolute default, relative labels, fine-pointer + tooltip and keyboard focus, coarse-pointer drawer disclosure, live ticking, invalid timestamps, + local draft state, shared Save and Reset actions, failed-save retry state, revision-gated PATCH + responses, foreign WebSocket updates, and unmount/remount convergence. +- **E2E (desktop + mobile + cross-tab):** add `apps/web/e2e/tests/auth/relative-last-seen.spec.ts` + (Desktop Chrome, `auth` project) grouped in `test.describe.serial` (single-shot `setupAdmin` + + shared worker backend/DB require serial ordering), restarting the worker backend with auth on and + its OWN `KANDEV_DATABASE_PATH` (auth setup is single-shot and worker DB is preserved across + restart; without a dedicated DB the focused command collides with sibling auth specs), calling + `setupAdmin` in `beforeAll` so focused test runs are independent, logging in, opening + `/settings/account/security`, switching to Relative time, saving through the shared Save action, + and asserting a relative label renders with an absolute tooltip on hover. Restore the original + setting in `finally`, and restart the baseline backend in `afterAll`. The same file adds a + two-context cross-tab scenario that arms `watchWs(pageA)` BEFORE `pageA.goto` (it only sees + sockets opened after it is called) AND arms the subscription wait (`const subscriptionAck = + watcher.waitForResponse("user.subscribe")`) before the same `goto`, then awaits `subscriptionAck` + after navigation before tab B mutates (frames are not buffered and `waitForResponse` only records + request ids once armed, so arming the wait before navigation is required to catch the ACK); the + assertion is causal: tab B changes the setting, tab A observes the update via WS without a + reload. Add `apps/web/e2e/tests/auth/mobile-relative-last-seen.spec.ts` (`mobile-chrome` + project, named `mobile-*` so the project routes it) that spreads `...devices["Pixel 5"]` into a + manual `browser.newContext` (manual contexts do not inherit project device options), asserts the + resulting viewport width, uses its own DB + `setupAdmin`, operates the select with touch-native + `tap()` on the trigger and option, saves through the shared Save action, proves relative labels + render without hover and without horizontal overflow, taps a relative label, and asserts the + absolute stamp in the touch drawer. Restore the setting. The Last seen trigger and options get a + mobile-appropriate touch target + (min-h-11 / 44px at phone widths via responsive classes on this instance, per /mobile-parity — + the shared Select defaults to 28px trigger/items), and the mobile E2E asserts the trigger's and + option's bounding boxes meet 44px in the active dimension. + +## Implementation waves + +Wave 1: + +- [x] [Task 01: Backend last_seen_display setting](task-01-backend-last-seen-setting.md) + +Wave 2: + +- [x] [Task 02: Frontend settings contract and SSR mapping](task-02-frontend-settings-contract.md) + +Wave 3: + +- [x] [Task 03: Security page option, relative column, tooltip, live update](task-03-security-page-relative-last-seen.md) + +Wave 4: + +- [x] [Task 04: E2E coverage (desktop + mobile)](task-04-e2e-relative-last-seen.md) + +## Verification + +```bash +make -C apps/backend test +cd apps/web && pnpm run typecheck && pnpm run lint:i18n components/settings/account/security-settings.tsx +cd apps/web && pnpm vitest run components/settings/account lib/ssr/user-settings.test.ts +cd apps/web && pnpm e2e:run --project auth tests/auth/relative-last-seen.spec.ts +cd apps/web && pnpm e2e:run --project mobile-chrome tests/auth/mobile-relative-last-seen.spec.ts +``` + +## Risks + +- The select shares the floating Save and Reset actions with the other `/settings` configuration + controls. Its table preview updates from the local draft, while the durable setting changes only + after Save. +- Live ticking is wasted work if the user never switches to relative mode; the ticking interval is + created only inside the relative-mode cell component, so absolute mode stays inert. +- Tooltip timing/positioning inside a table cell: use the standard Radix tooltip for fine-pointer + devices. Coarse-pointer devices use the shipped Drawer pattern, and mobile E2E taps the label and + asserts the visible absolute stamp. +- The hand-built backend JSON codec paths and the WS event snapshot are easy to miss because the + model struct change alone looks sufficient; task 01 lists each path explicitly and tests the + stored-JSON round trip and the published event map. +- The auth E2E project shares one worker backend whose SQLite DB survives restarts, and auth setup + is single-shot; both new specs must use dedicated `KANDEV_DATABASE_PATH` values or the focused + commands become order-dependent (the `users-self-actions` pattern). +- The PATCH write path is read-modify-write over a full settings blob; without expected-revision + CAS, a concurrent omitted-field PATCH can revert `last_seen_display` at a newer revision that + the frontend accepts. CAS + bounded retry is in task 01; the barrier test pins it. diff --git a/docs/plans/relative-last-seen/task-01-backend-last-seen-setting.md b/docs/plans/relative-last-seen/task-01-backend-last-seen-setting.md new file mode 100644 index 0000000000..4e149b72e7 --- /dev/null +++ b/docs/plans/relative-last-seen/task-01-backend-last-seen-setting.md @@ -0,0 +1,173 @@ +--- +id: "01-backend-last-seen-setting" +title: "Backend last_seen_display setting" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/relative-last-seen/spec.md" +--- + +# Task 01: Backend `last_seen_display` setting + +## Acceptance + +- `UserSettings` model, DTO, store default, service patch, controller mapping, boot-state + mapping, AND the two hand-built store codec paths and the hand-built WS event snapshot all carry + `last_seen_display`; the store default and any unknown value normalize to `"absolute"`. +- The two manual JSON codec paths in `store/sqlite.go` are covered explicitly: the + `marshalUserSettingsPayload` map entry, the `scanUserSettings` payload struct field, and the + scan assignment with `NormalizeLastSeenDisplay` coercion. A model/DTO-only change silently drops + the field on write and falls back to the default on read. +- The PATCH write is expected-revision CAS: `UpsertUserSettingsPreservingTaskCreateLastUsed` gains + `expectedRevision` and its UPDATE gains `AND settings_revision = ?` (SQLite + Postgres paths), + returning a revision-conflict sentinel on zero rows. Zero rows is ambiguous, so the repo + existence-checks the user after a zero-row conditional UPDATE: missing user → the existing + user-not-found error (preserved for direct callers/tests); present → `ErrUserSettingsRevisionConflict` + (both drivers tested, including the RETURNING zero-row case). +- ALL full-blob writers route through one bounded read-apply-CAS-retry helper in the service + (read → apply patch → upsert with the read revision → on conflict re-read and re-apply the + ORIGINAL patch, ~3 attempts): `UpdateUserSettings` AND `ClearDefaultEditorID` (service.go:883-900 + currently upserts once; without the shared helper it either fails to clear on conflict or + bypasses CAS and can still overwrite `last_seen_display`). Exactly one event publication after + the successful final write. Every upsert callsite is updated: the store interface, both driver + implementations, the controller/service test fakes, and the direct sqlite_test.go calls + (~31/96/151/1274/1323/1379, passing the revision from the preceding read; the existing + concurrent test at ~150 asserts conflict-and-retry semantics). +- Barrier-based concurrent tests prove it: two PATCHes (one with `last_seen_display: "relative"`, + one unrelated omitted-field) started from the same initial revision — the final row contains + both fields, and the losing write merged via the retry loop rather than reverting + `last_seen_display`; and a concurrent `ClearDefaultEditorID` + settings PATCH (the Clear applies + `default_editor_id` while the PATCH applies `last_seen_display`, different fields) — the final + row contains BOTH effects (editor cleared AND `last_seen_display` set), and TWO events are + published, one per successful write with distinct revisions (each CAS winner publishes its own + successful write; the loser retries and publishes its own — the helper contract is exactly one + event per successful final write, so two successful writes emit two events; a one-event outcome + is only valid in the two-phase no-op scenario above). +- The shared helper's apply/no-op contract is explicit and covers the task-create patch: the + helper signature is `updateUserSettingsCAS(ctx, apply func(*models.UserSettings) (bool, error), + taskCreatePatch *models.TaskCreateLastUsed)`, where `apply` is evaluated against EVERY freshly + read model, captures immutable inputs (the original `editorID`), and returns `applied=false` for + a no-op. The IMMUTABLE `taskCreatePatch` (extracted once from the original request, as + `UpdateUserSettings` does today at service.go:198-202 with `taskCreateLastUsedPatchEmpty`) is + passed to EVERY CAS attempt as the upsert's separate patch argument, so a retry re-applies the + same merge against the fresh row. `applied` is true when the callback mutated the model OR the + task-create patch is non-empty (the json merge is still a write even when the blob is otherwise + unchanged — a task-create-only PATCH must NOT be classified no-op and dropped). The helper + writes and publishes ONLY when applied — a no-op means NO write and NO event. + `ClearDefaultEditorID`'s existing `settings.DefaultEditorID != editorID` early-return + (service.go:883-902) moves inside the callback and is re-evaluated per attempt, so a competing + PATCH that changes `default_editor_id` to another value on a retry read makes the Clear a no-op. +- Service retry test for the task-create path: a task-create-only PATCH (no other field changes) + forced into a revision conflict survives via the retry loop, merges with the concurrent winner's + row, increments the revision exactly once on its own successful retry (the winner already + bumped R to R+1, so the final revision is R+2 — two total increments across two successful + writes), and emits the resulting snapshot event (this pins the existing public PATCH behavior at + service.go:197-206 / service_test.go:1257-1283 under the CAS rewrite). +- Barrier test for the no-op path FORCES a real CAS conflict and callback re-evaluation (the + earlier fresh-read-only ordering could pass even with the editorID check left OUTSIDE the shared + helper, because Clear's first read already saw the new value): initial `DefaultEditorID = + "old-editor"` at revision R. PHASE 1: release Clear's initial `GetUserSettings` (its read + completes seeing `"old-editor"`), then BLOCK its upsert/CAS attempt on a channel. PHASE 2: + release the full PATCH (`default_editor_id: "new-editor"`), wait for its commit + event (row now + R+1). PHASE 3: release Clear's stale upsert — it conflicts (expected R vs current R+1), the + retry loop fresh-reads the row (sees `"new-editor"`), the callback re-evaluates with the + immutable `"old-editor"` and returns `applied=false` → NO Clear upsert and NO Clear event. + Assert: final row keeps `"new-editor"` at R+1; Clear's read count >= 2 (initial + retry + re-read); `clearUpsertAttempts == 1` (the forced stale CAS invocation that conflicts — the + counter records invocations, so a zero total would contradict the required conflict); + `clearSuccessfulUpserts == 0` (the stale attempt never commits); `clearEvents == 0`; exactly one + event total, with the PATCH's revision. Name the fake's counters accordingly (attempts vs + successful commits). KEEP a separate deterministic fresh-read no-op test (Clear starts only + after the PATCH committed; immediate no-op, no conflict) as a distinct scenario — both prove + different halves of the contract. +- The existing direct repository tests are MIGRATED to the CAS contract, not just listed, and the + migrated tests MOVE to new focused files: `TestSQLiteRepositoryAssignsUniqueAtomicSettingsRevisions` + (~120-180) starts two candidates from one read and currently asserts both writes succeed with + revisions [1,2]; it becomes one success + one `ErrUserSettingsRevisionConflict`, with the + fresh-read/reapply retry behavior covered by the service barrier tests (the direct repo layer has + no retry). The stale-blob merge tests — Postgres round trip (staleSettings upsert ~1274), SQLite + stale write after `UpdateTaskCreateLastUsed` (~1323), and the SQLite patch path (~1379) — re-read + the CURRENT revision before the direct upsert where their purpose is the `task_create_last_used` + json merge (the merge assertions must keep passing), or assert `ErrUserSettingsRevisionConflict` + where staleness is the point. MOVING them (deletion from `sqlite_test.go` + migrated copy in the + new file) is mandatory: `sqlite_test.go` (1,466 lines) and `service_test.go` (1,820 lines) are + already over Revive's 800-effective-line limit, which applies to test files + (`apps/backend/AGENTS.md` "Code-quality limits", `.golangci.yml` revive file-length-limit; the + `_test.go` exclusions do not disable revive), so ANY added code there fails the changed-file + lint. New tests land in new files: `apps/backend/internal/user/store/sqlite_last_seen_cas_test.go` + (codec round-trip, CAS conflict/zero-row, migrated repo tests) and + `apps/backend/internal/user/service/user_settings_cas_test.go` (CAS retry, barrier tests, + no-op contract, event-count assertions); the controller mapping test may stay in + `controller_test.go` (under the limit). +- `publishUserSettingsEvent` in `service/service.go` includes the normalized `last_seen_display`, + so the WS `user.settings.updated` payload reaches every other tab (the initiating tab alone + getting the PATCH response is not cross-tab sync). +- NO writer-id plumbing: the publisher signature stays `publishUserSettingsEvent(ctx, settings)` + (no direct test callsite changes; `app_status_bar_visibility_test.go:44` and + `service_test.go:1312, 1333, 1354, 1377, 1399, 1417` keep their current shape). Pending-selection + protection is entirely client-side (pending-write gating in task 03), because the event is a + full snapshot that any user-settings write re-publishes and no writer-id scheme can tell whether + a foreign snapshot changed `last_seen_display` or merely re-asserted the old value. +- Covered by an UpdateUserSettings integration test asserting the PUBLISHED event map (not just a + direct publisher test): a PATCH with `last_seen_display: "relative"` produces the normalized + `last_seen_display` in the published snapshot. +- The controller test asserts `req.LastSeenDisplay` reaches the service request (the manual + adapter can silently drop the field; the REST handler and `wsUpdateUserSettings` share this one + controller path, handlers.go:113). +- A PATCH with `last_seen_display: "relative"` persists and reads back as `"relative"`; a PATCH + with an invalid value is rejected by the service; a PATCH omitting the field leaves it untouched. +- `FromUserSettings` emits the canonical value via `NormalizeLastSeenDisplay` (matching every other + enum-like setting at the API boundary), so a model assembled outside the store cannot leak an + invalid wire value. +- The boot payload includes `lastSeenDisplay` keyed from the DTO. + +## Verification + +```bash +make -C apps/backend test +``` + +## Files likely touched + +- `apps/backend/internal/user/models/models.go` +- `apps/backend/internal/user/store/sqlite.go` (+ NEW `store/sqlite_last_seen_cas_test.go`: + codec round-trip tests (stored JSON, unknown coercion, later-PATCH update), CAS conflict and + zero-row driver tests, and the MIGRATED repo tests moved out of `sqlite_test.go`) +- `apps/backend/internal/user/service/service.go` (+ NEW `service/user_settings_cas_test.go`: + the shared-CAS-retry tests, barrier tests (PATCH/PATCH, Clear/PATCH, two-phase no-op), the + published event-map integration test, and the new synchronized CAS fakes; the existing + publisher tests at `service_test.go:1312-1417` and `app_status_bar_visibility_test.go:37-52` + are UNCHANGED and stay where they are) +- `apps/backend/internal/user/dto/dto.go` (+ `dto_test.go` regression: unknown model value emits + canonical `"absolute"`) +- `apps/backend/internal/user/controller/controller.go` (+ `controller_test.go`: focused mapping + test asserting `req.LastSeenDisplay` reaches the service request, mirroring the existing enum + mapping tests; the controller is a manual field-by-field adapter, so an omission compiles and + silently no-ops) +- `apps/backend/internal/backendapp/boot_state_routes.go` (+ `boot_state_routes_test.go`) + +The concurrent CAS service tests use a DEDICATED synchronized fake (in `user_settings_cas_test.go`), +NOT the existing `recordingUserRepository`/`recordingEventBus` fakes: those return one shared +mutable `getSettings` pointer on every read (service_test.go:1741-1750), mutate/read unprotected +fields (1753-1771), and append to `publishedEvents` unsynchronized (1793-1801), so concurrent +callers race under `go test -race`, one candidate can observe the other's patch before the CAS +decision, and event counts are unsafe. The new fake: clones settings for every read, models +revision-conditional writes, records reads/upserts through synchronized state/channels, and uses a +thread-safe event recorder; channel barriers gate phase-1 commit/event observation before releasing +phase 2 (no timing/polling). + +## Dependencies + +None. + +## Inputs + +- Spec "What", "Failure modes" (normalization), "Out of scope" (JSON blob, no schema migration) +- Existing enum precedent: `applyChangesPanelLayout`, `NormalizeLspStatusLocation`, + `changesPanelLayout(...)` boot mapping + +## Output contract + +Return a compact handoff capsule with acceptance status, exact test command/results, risk tags, +uncertainties, and set this task to `done`. diff --git a/docs/plans/relative-last-seen/task-02-frontend-settings-contract.md b/docs/plans/relative-last-seen/task-02-frontend-settings-contract.md new file mode 100644 index 0000000000..af57871830 --- /dev/null +++ b/docs/plans/relative-last-seen/task-02-frontend-settings-contract.md @@ -0,0 +1,64 @@ +--- +id: "02-frontend-settings-contract" +title: "Frontend settings contract and SSR mapping" +status: done +wave: 2 +depends_on: ["01-backend-last-seen-setting"] +plan: "plan.md" +spec: "../../specs/relative-last-seen/spec.md" +--- + +# Task 02: Frontend settings contract and SSR mapping + +## Acceptance + +- `UserSettings` / `UserSettingsUpdatePayload` carry `last_seen_display?: "absolute" | "relative"`; + `LastSeenDisplay` is re-exported from the `apps/web/lib/types/http.ts` barrel (the established + import surface for these types). `UserSettingsState` carries `lastSeenDisplay` (default + `"absolute"`) and NO client-only metadata field: pending-selection protection lives in the + component (pending-write gating), so no writer-id/lastWriterId state exists to be wiped by + `mapUserSettingsResponse` callers that build from a fresh default state (use-layout-settings, + layout-preset-selector, use-ensure-user-settings, hydration). +- `parseLastSeenDisplay` accepts only `"relative"` and coerces everything else (including + `undefined`) to `"absolute"`; `mapUserSettingsData` maps the snake_case field into the store + shape. +- The WS `user.settings.updated` handler (`registerUsersHandlers`) propagates the field through + `mapUserSettingsData` with its existing revision gate and is otherwise UNCHANGED (own echoes are + applied normally, so the store converges even if the component unmounts mid-write): valid value + applies, unknown value normalizes to `"absolute"`, omitted field leaves the current value, stale + revision ignored. Covered in `users.test.ts`, not just by SSR tests. +- `apps/web/hooks/use-ensure-user-settings.test.ts` `makeUnloadedSettings` (lines ~47-110) builds a + complete typed `UserSettingsState` and gains `lastSeenDisplay: "absolute"`; audit remaining typed + fixtures for the new required field. +- SSR unit tests cover default, round-trip, and unknown-value coercion. + +## Verification + +```bash +cd apps/web && pnpm run typecheck +cd apps/web && pnpm vitest run lib/ssr/user-settings.test.ts lib/ws/handlers/users.test.ts +``` + +## Files likely touched + +- `apps/web/lib/types/http-user-settings.ts` +- `apps/web/lib/types/http.ts` (barrel re-export) +- `apps/web/lib/state/slices/settings/types.ts` (`lastSeenDisplay`) +- `apps/web/lib/ssr/user-settings.ts` (+ `user-settings.test.ts`) +- `apps/web/lib/ws/handlers/users.ts` (+ `users.test.ts`: `last_seen_display` assertions) +- `apps/web/hooks/use-ensure-user-settings.test.ts` (typed fixture gains `lastSeenDisplay`) + +## Dependencies + +Task 01 (backend field must exist so the payload type matches the wire contract). + +## Inputs + +- Spec "What" (per-user persisted setting, WS sync) +- Existing precedent: `changesPanelLayout` through + `http-user-settings.ts` → `types.ts` → `ssr/user-settings.ts` → `ws/handlers/users.ts` + +## Output contract + +Return a compact handoff capsule with acceptance status, exact test command/results, risk tags, +uncertainties, and set this task to `done`. diff --git a/docs/plans/relative-last-seen/task-03-security-page-relative-last-seen.md b/docs/plans/relative-last-seen/task-03-security-page-relative-last-seen.md new file mode 100644 index 0000000000..bba8b5ef90 --- /dev/null +++ b/docs/plans/relative-last-seen/task-03-security-page-relative-last-seen.md @@ -0,0 +1,84 @@ +--- +id: "03-security-page-relative-last-seen" +title: "Security page option, relative column, tooltip, live update" +status: done +wave: 3 +depends_on: ["02-frontend-settings-contract"] +plan: "plan.md" +spec: "../../specs/relative-last-seen/spec.md" +--- + +# Task 03: Security page option, relative column, tooltip, live update + +## Acceptance + +- The Active sessions card on `/settings/account/security` shows a labeled **Last seen** select + (Absolute time / Relative time) above the table, with self-documenting copy and a discovery + target. The card's `SettingsCard` keeps its existing `sessions` target (it accepts exactly one + `discoveryTargetId`); the select's wrapper registers `ACCOUNT_SETTINGS_TARGETS.lastSeenDisplay` + separately via `useSettingsTargetRegistration` from `settings-target-provider.tsx`, and a + discovery-target test asserts both the sessions and the last-seen-display targets register and + reveal. The trigger and options get a mobile-appropriate touch target (min-h-11 / 44px at + phone widths via responsive classes on this instance; the shared Select defaults to 28px, below + the /mobile-parity 44px active-dimension guidance). +- Default (`absolute`) renders `formatDateTime(last_seen_at)` exactly as before. +- Relative mode renders a locale-aware `formatRelativeTime(last_seen_at, now)` label inside a + tooltip whose content is the absolute `formatDateTime` on fine-pointer devices. On + coarse-pointer devices, tapping the label opens a Drawer with the same absolute timestamp. The + fine-pointer trigger is focusable (`tabIndex={0}`, semantic element) and exposes the absolute + timestamp as its accessible name and native `title` fallback. The label advances while the page + stays open. An unparseable `last_seen_at` renders an empty cell with no disclosure surface + (guards the `formatDateTime` `RangeError` on invalid dates). +- Changing the select keeps a local draft and marks the card dirty. Register the field with + `useSettingsSaveContributor`. The shared Save action persists the draft through a queued + `createQueuedUserSettingsSyncWithResponse({ last_seen_display })` write. The shared Reset action + restores the confirmed store value. The store's `userSettings.lastSeenDisplay` is the confirmed + baseline, updated by exactly two unchanged ingestion paths: (1) a successful PATCH response + mapped with `mapUserSettingsResponse(response, current)` and applied THROUGH `setUserSettings` + (never a raw store write; `setUserSettings` discards older revisions, + `settings-slice.ts:245-250`); (2) a newer `user.settings.updated` WS snapshot through the + existing handler path (`mapUserSettingsData` inside `store.setState` with the handler's own + revision gate, `users.ts:12-24`), which Task 02 requires to stay UNCHANGED. On failure, keep the + draft dirty and show error copy so the shared Save action can retry. +- While a Save request is in flight, a newer local draft stays visible. The backend event remains a + full snapshot, and the unchanged WebSocket handler still applies own echoes and newer foreign + values to the confirmed store. After a successful response, reconcile the draft with the + revision-gated store result. A failed response keeps the draft dirty for retry. The component + tests cover this behavior, PATCH-response ingestion through `setUserSettings`, WebSocket + ingestion through the unchanged handler, Reset, and unmount/remount convergence. +- Component tests cover absolute default, relative label, tooltip content on hover, Tab focus, + coarse-pointer Drawer disclosure, accessible name/title carrying the absolute timestamp, live + ticking (fake timers), local draft state, shared Save and Reset actions, failed-save retry state, + invalid timestamps, revision-gated PATCH responses, foreign WebSocket snapshots, and + unmount/remount convergence. The tests exercise the PATCH-response ingestion path through + `setUserSettings` and the WebSocket ingestion path through the unchanged handler. + +## Verification + +```bash +cd apps/web && pnpm run typecheck +cd apps/web && pnpm vitest run components/settings/account +cd apps/web && pnpm run lint:i18n components/settings/account/security-settings.tsx +``` + +## Files likely touched + +- `apps/web/components/settings/account/security-settings.tsx` (+ new `security-settings.test.tsx`) +- `apps/web/lib/settings-discovery/catalog/account.ts` +- `apps/web/src/locales/{en,pseudo,pt-pt,zh-cn,zh-hk,zh-tw}/account.json` + +## Dependencies + +Task 02 (store field and payload type must exist). + +## Inputs + +- Spec "What", "Failure modes", "Scenarios" +- Existing precedent: `ChangesPanelLayoutCard` select anatomy, `useNow(30_000)` live labels, + `formatRelativeTime`/`formatDateTime` from `@/lib/i18n/formats`, Radix tooltip usage rules in + `apps/web/AGENTS.md` + +## Output contract + +Return a compact handoff capsule with acceptance status, exact test command/results, risk tags, +uncertainties, and set this task to `done`. diff --git a/docs/plans/relative-last-seen/task-04-e2e-relative-last-seen.md b/docs/plans/relative-last-seen/task-04-e2e-relative-last-seen.md new file mode 100644 index 0000000000..87caa9855b --- /dev/null +++ b/docs/plans/relative-last-seen/task-04-e2e-relative-last-seen.md @@ -0,0 +1,79 @@ +--- +id: "04-e2e-relative-last-seen" +title: "E2E coverage for relative last seen" +status: done +wave: 4 +depends_on: ["03-security-page-relative-last-seen"] +plan: "plan.md" +spec: "../../specs/relative-last-seen/spec.md" +--- + +# Task 04: E2E coverage for relative last seen (desktop + mobile + cross-tab) + +## Acceptance + +- **Desktop** (`auth` project, Desktop Chrome): the file groups its tests in + `test.describe.serial` (shared worker backend/DB and backend restart still require serial + ordering), restarts the worker backend with auth on and its OWN `KANDEV_DATABASE_PATH`, creates + the administrator in `beforeAll` so a focused test run is independent, then logs in per test. + Opening `/settings/account/security` shows the Last seen select. Switching to Relative time + updates the local draft. The shared Save action persists the setting. A relative label appears in + the Last seen column, and hovering it reveals a tooltip containing the absolute timestamp. +- **Cross-tab** (same desktop spec, serial): two browser contexts on the same backend, BOTH + authenticated (Playwright contexts have isolated cookie stores and the auth helpers set cookies + per context, so context B must `login` separately — or be created from context A's captured + `storageState` — or it cannot PATCH the setting or receive the user-scoped WS event). Arm + `watchWs(pageA)` BEFORE `pageA.goto` (it only sees sockets opened after it is called), and arm + the subscription wait itself before the same `goto` (`const subscriptionAck = + watcher.waitForResponse("user.subscribe")`), then await `subscriptionAck` after navigation before + tab B mutates: frames are not buffered and `waitForResponse` only records request ids once armed, + so a wait created after `goto` can miss the ACK entirely. Tab B switches the setting, tab A + observes the update via WS without a reload — causal, not timing-based. +- **Mobile** (`mobile-chrome` project, Pixel 5): the spec spreads `...devices["Pixel 5"]` into a + manual `browser.newContext` (manual contexts do not inherit project device options) and asserts + the resulting viewport width; with its own DB + `setupAdmin`, the Last seen select is operated + with touch-native `tap()` on the trigger and the Relative time option, then saved with the shared + Save action. Relative labels render without hover, there is no horizontal document overflow, and + tapping a relative label opens a drawer with the exact timestamp. The Last seen trigger and + options get a mobile-appropriate touch target (min-h-11 / 44px at phone widths via responsive + classes on this instance — the shared Select defaults to a 28px trigger and items — per + /mobile-parity), and the mobile spec asserts the trigger's and option's bounding boxes are >= + 44px in the active dimension. +- The original setting is restored in `finally` and the baseline backend is restarted in + `afterAll` in both specs so the worker is reusable. + +## Verification + +```bash +cd apps/web && pnpm e2e:run --project auth tests/auth/relative-last-seen.spec.ts +cd apps/web && pnpm e2e:run --project mobile-chrome tests/auth/mobile-relative-last-seen.spec.ts +``` + +## Files likely touched + +- `apps/web/e2e/tests/auth/relative-last-seen.spec.ts` (new; `backendFixture`, `setupAdmin`/`login` + from `e2e/helpers/auth.ts`, own DB path under `backend.tmpDir`, `finally` setting restore via raw + PATCH, `afterAll` baseline restart; two-context cross-tab scenario) +- `apps/web/e2e/tests/auth/mobile-relative-last-seen.spec.ts` (new; MUST be named `mobile-*` so the + `mobile-chrome` project routes it away from the desktop `auth` project; own DB + `setupAdmin`; + `...devices["Pixel 5"]` spread into `browser.newContext` + viewport width assertion; `afterAll` + baseline restart, mirroring `mobile-users-self-actions.spec.ts`) + +## Dependencies + +Task 03 (UI exists to exercise). + +## Inputs + +- Spec "Scenarios" (switch, tooltip, persistence, restore, cross-tab) and "Failure modes" (touch + path) +- Existing precedent: `users-self-actions.spec.ts` (own-DB isolation rationale), + `auth-lifecycle.spec.ts` (serial auth project), `auth-screenshots.spec.ts` (security page + visit), `mobile-users-self-actions.spec.ts` (mobile auth project + own DB + Pixel 5 device + spread), `settings-manual-save.spec.ts` (restore settings via raw PATCH), `/mobile-parity` + naming and routing rules + +## Output contract + +Return a compact handoff capsule with acceptance status, exact E2E command/results, risk tags, +failure artifacts when applicable, uncertainties, and set this task to `done`. diff --git a/docs/specs/relative-last-seen/spec.md b/docs/specs/relative-last-seen/spec.md new file mode 100644 index 0000000000..0b411a942d --- /dev/null +++ b/docs/specs/relative-last-seen/spec.md @@ -0,0 +1,109 @@ +--- +status: draft +created: 2026-08-16 +owner: kandev +--- + +# Relative Last Seen in Account Security + +## Why + +The **Active sessions** table on `/settings/account/security` renders the "Last seen" column as an +absolute timestamp (`formatDateTime`, e.g. "Aug 15, 2026, 3:42 PM"). Absolute stamps are precise but +hard to scan: a user reviewing devices signed in to their account must mentally subtract to know +whether a session was active moments ago or a month ago. Relative human-readable time ("5 minutes +ago", "yesterday") makes session freshness legible at a glance, which is the primary security +question a user is asking. + +Relative time is inherently stale the moment it renders, so the label must live-update, and users +who need the exact moment still need access to it. This feature adds a per-user display option on +the security page that switches the column to relative time with an absolute-time tooltip on fine +pointer devices and a touch drawer on coarse-pointer devices. + +## What + +- The **Active sessions** card on `/settings/account/security` gains a **Last seen** display option + (a select) offering **Absolute time** and **Relative time**. +- The default remains **Absolute time**; existing behavior is unchanged for users who never touch + the option. +- With **Relative time** selected, the "Last seen" column renders a locale-aware relative label + ("now", "5 minutes ago", "yesterday") that re-renders as time passes while the page is open. +- Hovering (or keyboard-focusing) a relative label shows a tooltip with the absolute timestamp on + fine-pointer devices. Tapping the label opens a touch drawer with the absolute timestamp on + coarse-pointer devices. +- The choice is a per-user persisted setting (`last_seen_display`), shared across the account and + synced between tabs via the existing user-settings WebSocket push. +- Changing the select updates a local draft and the table preview. The shared **Save changes** + action persists the draft. The shared **Reset** action discards the draft. + +## Permissions + +The option is a per-user preference on an account page; it requires only an authenticated session +(the same requirement as the security page itself). No admin or workspace authorization applies. + +## Failure modes + +- **Settings write fails:** the select keeps the draft and shows the error copy. The page remains + usable, and the user can retry the shared **Save changes** action. The relative/absolute rendering + never depends on a successful write to display. +- **Own-write echo vs unrelated snapshots:** the backend publishes a `user.settings.updated` + event for this tab's own write before the PATCH response returns, and the gateway broadcasts it + to every subscriber, including the initiating tab; the event is a full snapshot that any + user-settings write re-publishes. A pending local selection is therefore never cleared while its + write is in flight (own echoes and unrelated snapshots re-assert the pre-write value or the + write's own confirmation); the selection settles when the write settles, and the page then + always renders server truth, including any change another tab made meanwhile. +- **Unknown persisted value:** the backend coerces any value other than `"relative"` to + `"absolute"` on read (normalization), so a stale or hand-edited blob cannot produce a broken + column. +- **Concurrent unrelated PATCH (lost update):** user-settings updates are read-modify-write over a + full settings blob; a concurrent PATCH that omits `last_seen_display` (for example an + app-status-bar or review-top-bar save) could otherwise serialize a stale snapshot over this + setting, and the frontend would accept it because its revision is newer. The PATCH write must be + expected-revision CAS with bounded retry so an omitted-field write merges against the latest + row and can never revert `last_seen_display`. +- **Unparseable `last_seen_at`:** the relative formatter returns an empty string for invalid input, + but `formatDateTime` (used for the tooltip) throws `RangeError` on an invalid date. The relative + cell must validate the timestamp once and render neither the tooltip nor the absolute label when + it is unparseable, so the cell degrades to an empty cell instead of crashing the render. +- **Tooltip unavailable (touch/coarse pointer):** the absolute time remains reachable through the + relative label's explicit tap action. The tap opens a drawer with the exact timestamp. The + trigger also carries the absolute timestamp as its accessible name, and the fine-pointer path + keeps the native `title` fallback. Mobile E2E proves the tap path at the Pixel 5 viewport. + +## Scenarios + +- **GIVEN** a user on `/settings/account/security` who has never changed the option, **WHEN** the + Active sessions table renders, **THEN** the Last seen column shows absolute timestamps exactly as + before. +- **GIVEN** the Last seen select, **WHEN** the user picks **Relative time**, **THEN** the column + switches to relative labels immediately and the local draft becomes dirty. +- **GIVEN** a dirty Last seen draft, **WHEN** the user activates **Save changes**, **THEN** the + choice persists to user settings and the draft becomes clean. +- **GIVEN** a dirty Last seen draft, **WHEN** the user activates **Reset**, **THEN** the select and + table return to the confirmed setting without a settings write. +- **GIVEN** Relative time selected and a session last seen 3 minutes ago, **WHEN** the page stays + open for a minute, **THEN** the label advances (for example from "3 minutes ago" to "4 minutes + ago") without a page reload. +- **GIVEN** a relative label on a fine-pointer device, **WHEN** the user hovers or keyboard-focuses + it (the trigger is focusable), **THEN** a tooltip shows the absolute timestamp for that session, + and the absolute timestamp is also exposed as the trigger's accessible name and native title. +- **GIVEN** a coarse-pointer device with no hover, **WHEN** the user taps a relative label, **THEN** + a drawer shows the absolute timestamp, and the select and relative labels remain usable at the + phone viewport. +- **GIVEN** the user selects **Absolute time** again, **WHEN** the table renders, **THEN** absolute + timestamps return and the persisted value flips back. +- **GIVEN** the option changed in another tab, **WHEN** the user-settings push arrives, **THEN** + this page reflects the new value without a reload. + +## Out of scope + +- Changing how "Last seen" renders anywhere else (task surfaces, agent lists). +- Relative time for the **Created** or **Last used** columns elsewhere in account settings. +- A custom relative formatter: the shared locale-aware `formatRelativeTime` is used, consistent with + the platform i18n spec. +- Backend storage schema changes: `last_seen_display` rides the existing JSON user-settings blob. + +## Implementation plan + +See [the implementation plan](../../plans/relative-last-seen/plan.md).