From a9c52949a02da820fec87be77a1f106258bf36d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Coatelen?= <91121759+jcoatelen-ledger@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:30:38 +0200 Subject: [PATCH] feat(tasks): add pre-defined repository sets for bulk task repository selection (#2774) Co-authored-by: Carlos Florencio --- .../backend/internal/backendapp/boot_state.go | 18 + .../boot_state_repository_sets_test.go | 183 ++++++++ .../internal/backendapp/boot_state_routes.go | 59 ++- apps/backend/internal/backendapp/e2e_reset.go | 11 + apps/backend/internal/backendapp/helpers.go | 1 + .../internal/backendapp/helpers_test.go | 1 + apps/backend/internal/backendapp/services.go | 1 + apps/backend/internal/events/types.go | 7 + .../gateway/websocket/task_notifications.go | 3 + .../websocket/task_notifications_test.go | 2 +- apps/backend/internal/task/dto/dto.go | 43 ++ .../internal/task/dto/repository_set_test.go | 69 +++ .../task/handlers/repository_set_handlers.go | 365 +++++++++++++++ .../handlers/repository_set_handlers_test.go | 278 +++++++++++ .../handlers/repository_set_readonly_test.go | 130 ++++++ apps/backend/internal/task/models/models.go | 37 ++ .../internal/task/repository/interface.go | 21 + .../task/repository/repoerrors/errors.go | 3 + .../task/repository/sqlite/base_migrations.go | 13 + .../task/repository/sqlite/base_schema.go | 51 +++ .../repository/sqlite/repository_entity.go | 27 +- .../task/repository/sqlite/repository_set.go | 342 ++++++++++++++ .../sqlite/repository_set_postgres_test.go | 84 ++++ .../sqlite/repository_set_review_test.go | 155 +++++++ .../repository/sqlite/repository_set_test.go | 432 ++++++++++++++++++ apps/backend/internal/task/service/service.go | 3 + .../task/service/service_repository_sets.go | 338 ++++++++++++++ .../service_repository_sets_review_test.go | 124 +++++ .../service/service_repository_sets_test.go | 373 +++++++++++++++ .../task/service/service_resources.go | 6 + .../internal/task/service/service_test.go | 1 + apps/backend/pkg/websocket/actions.go | 10 + .../use-workspace-repository-sets.ts | 153 +++++++ .../workspace-add-local-repository-dialog.tsx | 37 ++ .../workspace-repositories-client.tsx | 56 +-- .../workspace-repositories-dirty.test.ts | 8 + .../workspace/workspace-repositories-dirty.ts | 12 + .../workspace-repository-set-delete.tsx | 68 +++ .../workspace-repository-set-editor.tsx | 229 ++++++++++ ...workspace-repository-sets-section.test.tsx | 205 +++++++++ .../workspace-repository-sets-section.tsx | 172 +++++++ .../task-create-dialog-prop-builders.ts | 3 + .../task-create-dialog-repo-chips.tsx | 71 ++- ...ask-create-dialog-repository-sets-apply.ts | 63 +++ ...te-dialog-repository-sets-control.test.tsx | 208 +++++++++ ...-create-dialog-repository-sets-control.tsx | 160 +++++++ ...ate-dialog-repository-sets-save-action.tsx | 24 + ...reate-dialog-repository-sets-save.test.tsx | 160 +++++++ ...ask-create-dialog-repository-sets-save.tsx | 193 ++++++++ ...task-create-dialog-repository-sets.test.ts | 236 ++++++++++ .../task-create-dialog-repository-sets.ts | 130 ++++++ .../task-create-dialog-setup.test.ts | 8 + .../components/task-create-dialog-setup.ts | 54 ++- .../components/task-create-dialog-types.ts | 17 + .../components/task-create-dialog.test.tsx | 15 +- apps/web/components/task-create-dialog.tsx | 1 + .../task/new-subtask-form-parts.test.tsx | 10 + .../task/new-subtask-form-parts.tsx | 11 + apps/web/e2e/helpers/api-client.ts | 40 ++ .../workspace-repository-sets.spec.ts | 108 +++++ .../task/create-task-repository-sets.spec.ts | 235 ++++++++++ ...mobile-create-task-repository-sets.spec.ts | 66 +++ .../workspace/use-repository-sets.test.ts | 138 ++++++ .../domains/workspace/use-repository-sets.ts | 78 ++++ .../api/domains/repository-sets-api.test.ts | 107 +++++ apps/web/lib/api/domains/workspace-api.ts | 63 +++ apps/web/lib/state/default-state.ts | 2 + .../repository-sets-revision.test.ts | 120 +++++ .../workspace/repository-sets-slice.test.ts | 132 ++++++ apps/web/lib/state/slices/workspace/types.ts | 34 +- .../state/slices/workspace/workspace-slice.ts | 87 ++++ apps/web/lib/state/store.ts | 11 + apps/web/lib/types/backend.ts | 17 + apps/web/lib/types/http.ts | 28 ++ .../lib/ws/handlers/repository-sets.test.ts | 125 +++++ apps/web/lib/ws/handlers/repository-sets.ts | 65 +++ apps/web/lib/ws/router.ts | 2 + apps/web/src/boot-payload.ts | 4 +- apps/web/src/locales/en/task.json | 26 ++ apps/web/src/locales/en/workspaces.json | 27 +- apps/web/src/locales/pseudo/task.json | 26 ++ apps/web/src/locales/pseudo/workspaces.json | 27 +- apps/web/src/locales/pt-pt/task.json | 26 ++ apps/web/src/locales/pt-pt/workspaces.json | 27 +- apps/web/src/locales/zh-cn/task.json | 26 ++ apps/web/src/locales/zh-cn/workspaces.json | 27 +- apps/web/src/locales/zh-hk/task.json | 26 ++ apps/web/src/locales/zh-hk/workspaces.json | 27 +- apps/web/src/locales/zh-tw/task.json | 26 ++ apps/web/src/locales/zh-tw/workspaces.json | 27 +- docs/plans/repository-sets/plan.md | 273 +++++++++++ .../task-01-persistence-contracts.md | 72 +++ .../task-02-service-and-events.md | 69 +++ .../task-03-http-ws-surface.md | 62 +++ .../task-04-boot-and-web-data-layer.md | 73 +++ .../task-05-apply-set-in-picker.md | 78 ++++ .../task-06-save-selection-as-set.md | 66 +++ .../task-07-settings-management.md | 66 +++ .../task-08-end-to-end-coverage.md | 62 +++ .../task-09-public-documentation.md | 53 +++ .../task-10-final-verification.md | 60 +++ docs/public/tasks-and-workflows.md | 59 ++- docs/public/websocket-api.md | 8 + docs/specs/INDEX.md | 1 + docs/specs/repository-sets/spec.md | 216 +++++++++ 105 files changed, 8434 insertions(+), 59 deletions(-) create mode 100644 apps/backend/internal/backendapp/boot_state_repository_sets_test.go create mode 100644 apps/backend/internal/task/dto/repository_set_test.go create mode 100644 apps/backend/internal/task/handlers/repository_set_handlers.go create mode 100644 apps/backend/internal/task/handlers/repository_set_handlers_test.go create mode 100644 apps/backend/internal/task/handlers/repository_set_readonly_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/repository_set.go create mode 100644 apps/backend/internal/task/repository/sqlite/repository_set_postgres_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/repository_set_review_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/repository_set_test.go create mode 100644 apps/backend/internal/task/service/service_repository_sets.go create mode 100644 apps/backend/internal/task/service/service_repository_sets_review_test.go create mode 100644 apps/backend/internal/task/service/service_repository_sets_test.go create mode 100644 apps/web/app/settings/workspace/use-workspace-repository-sets.ts create mode 100644 apps/web/app/settings/workspace/workspace-add-local-repository-dialog.tsx create mode 100644 apps/web/app/settings/workspace/workspace-repository-set-delete.tsx create mode 100644 apps/web/app/settings/workspace/workspace-repository-set-editor.tsx create mode 100644 apps/web/app/settings/workspace/workspace-repository-sets-section.test.tsx create mode 100644 apps/web/app/settings/workspace/workspace-repository-sets-section.tsx create mode 100644 apps/web/components/task-create-dialog-repository-sets-apply.ts create mode 100644 apps/web/components/task-create-dialog-repository-sets-control.test.tsx create mode 100644 apps/web/components/task-create-dialog-repository-sets-control.tsx create mode 100644 apps/web/components/task-create-dialog-repository-sets-save-action.tsx create mode 100644 apps/web/components/task-create-dialog-repository-sets-save.test.tsx create mode 100644 apps/web/components/task-create-dialog-repository-sets-save.tsx create mode 100644 apps/web/components/task-create-dialog-repository-sets.test.ts create mode 100644 apps/web/components/task-create-dialog-repository-sets.ts create mode 100644 apps/web/e2e/tests/settings/workspace-repository-sets.spec.ts create mode 100644 apps/web/e2e/tests/task/create-task-repository-sets.spec.ts create mode 100644 apps/web/e2e/tests/task/mobile-create-task-repository-sets.spec.ts create mode 100644 apps/web/hooks/domains/workspace/use-repository-sets.test.ts create mode 100644 apps/web/hooks/domains/workspace/use-repository-sets.ts create mode 100644 apps/web/lib/api/domains/repository-sets-api.test.ts create mode 100644 apps/web/lib/state/slices/workspace/repository-sets-revision.test.ts create mode 100644 apps/web/lib/state/slices/workspace/repository-sets-slice.test.ts create mode 100644 apps/web/lib/ws/handlers/repository-sets.test.ts create mode 100644 apps/web/lib/ws/handlers/repository-sets.ts create mode 100644 docs/plans/repository-sets/plan.md create mode 100644 docs/plans/repository-sets/task-01-persistence-contracts.md create mode 100644 docs/plans/repository-sets/task-02-service-and-events.md create mode 100644 docs/plans/repository-sets/task-03-http-ws-surface.md create mode 100644 docs/plans/repository-sets/task-04-boot-and-web-data-layer.md create mode 100644 docs/plans/repository-sets/task-05-apply-set-in-picker.md create mode 100644 docs/plans/repository-sets/task-06-save-selection-as-set.md create mode 100644 docs/plans/repository-sets/task-07-settings-management.md create mode 100644 docs/plans/repository-sets/task-08-end-to-end-coverage.md create mode 100644 docs/plans/repository-sets/task-09-public-documentation.md create mode 100644 docs/plans/repository-sets/task-10-final-verification.md create mode 100644 docs/specs/repository-sets/spec.md diff --git a/apps/backend/internal/backendapp/boot_state.go b/apps/backend/internal/backendapp/boot_state.go index 98f8cc37c9..9d08ab35f1 100644 --- a/apps/backend/internal/backendapp/boot_state.go +++ b/apps/backend/internal/backendapp/boot_state.go @@ -317,6 +317,7 @@ func (b bootStateBuilder) addHomeKanbanRouteState(ctx context.Context, req *http state["userSettings"] = mapUserSettingsStateWithWorkflow(settings, activeWorkspaceID, activeWorkflowID) } b.addRepositoriesState(ctx, state, activeWorkspaceID) + b.addRepositorySetsState(ctx, state, activeWorkspaceID) b.addKanbanSnapshotsState(ctx, state, workflows, activeWorkflowID) } @@ -375,6 +376,23 @@ func (b bootStateBuilder) addRepositoriesState(ctx context.Context, state map[st } } +// addRepositorySetsState hydrates the home/kanban route with the workspace's +// repository sets, so the create dialog can offer them without a fetch. +func (b bootStateBuilder) addRepositorySetsState(ctx context.Context, state map[string]any, workspaceID string) { + items := repositorySetsToDTOs(nil) + loaded := false + sets, err := b.p.taskSvc.ListRepositorySets(ctx, workspaceID) + if err != nil { + // Not loaded, so the client's hook still fetches; see + // repositorySetsForState. + b.logBootError("list home repository sets", err) + } else { + items = repositorySetsToDTOs(sets) + loaded = true + } + state["repositorySets"] = repositorySetsState(workspaceID, items, loaded) +} + func (b bootStateBuilder) addQuickChatState( ctx context.Context, req *http.Request, diff --git a/apps/backend/internal/backendapp/boot_state_repository_sets_test.go b/apps/backend/internal/backendapp/boot_state_repository_sets_test.go new file mode 100644 index 0000000000..56890b7978 --- /dev/null +++ b/apps/backend/internal/backendapp/boot_state_repository_sets_test.go @@ -0,0 +1,183 @@ +package backendapp + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + taskmodels "github.com/kandev/kandev/internal/task/models" + taskservice "github.com/kandev/kandev/internal/task/service" + "github.com/kandev/kandev/internal/webapp" +) + +// bootRepositorySetsPayload drives /api/v1/app-state for one route and returns +// the hydrated repositorySets slice shape. +type bootRepositorySetsPayload struct { + InitialState struct { + RepositorySets struct { + ItemsByWorkspaceID map[string][]struct { + ID string `json:"id"` + Name string `json:"name"` + Repositories []struct { + RepositoryID string `json:"repository_id"` + Position int `json:"position"` + } `json:"repositories"` + } `json:"itemsByWorkspaceId"` + LoadingByWorkspaceID map[string]bool `json:"loadingByWorkspaceId"` + LoadedByWorkspaceID map[string]bool `json:"loadedByWorkspaceId"` + } `json:"repositorySets"` + } `json:"initialState"` + RouteData struct { + TasksPage struct { + RepositorySets []struct { + ID string `json:"id"` + } `json:"repositorySets"` + } `json:"tasksPage"` + } `json:"routeData"` +} + +// bootActiveWorkspace returns the workspace boot selects with no cookie, query +// parameter, or saved setting: the first one the service lists. +func bootActiveWorkspace(t *testing.T, harness bootStateTestHarness) *taskmodels.Workspace { + t.Helper() + workspaces, err := harness.taskSvc.ListWorkspaces(context.Background()) + if err != nil { + t.Fatalf("ListWorkspaces: %v", err) + } + if len(workspaces) == 0 || workspaces[0] == nil { + t.Fatal("no workspace to boot into") + } + return workspaces[0] +} + +func decodeBootRepositorySets(t *testing.T, harness bootStateTestHarness, routePath string) bootRepositorySetsPayload { + t.Helper() + // The query value is escaped; ClassifyRoute takes the real path. Passing the + // escaped form to ClassifyRoute silently classifies every route as the + // default one, which made an earlier version of this test assert nothing. + req := httptest.NewRequest(http.MethodGet, + "/api/v1/app-state?path="+url.QueryEscape(routePath), nil) + payload := bootPayload(context.Background(), req, routeParams{ + taskSvc: harness.taskSvc, + services: &Services{Workflow: harness.workflowSvc}, + }, webapp.ClassifyRoute(routePath)) + + raw, err := json.Marshal(payload) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + var decoded bootRepositorySetsPayload + if err := json.Unmarshal(raw, &decoded); err != nil { + t.Fatalf("decode payload: %v", err) + } + return decoded +} + +// TestBootPayloadHydratesRepositorySets pins the hydrated shape rather than just +// the status code: the boot mappers are explicit whitelists, so a field that is +// not listed is silently absent and the client refetches on every dialog open. +func TestBootPayloadHydratesRepositorySets(t *testing.T) { + harness := newBootStateTestHarness(t) + ctx := context.Background() + workspace := bootActiveWorkspace(t, harness) + + for _, name := range []string{"web", "gateway"} { + if err := harness.taskRepo.CreateRepository(ctx, &taskmodels.Repository{ + ID: "repo-" + name, WorkspaceID: workspace.ID, Name: name, + SourceType: "local", LocalPath: t.TempDir(), + }); err != nil { + t.Fatalf("CreateRepository %s: %v", name, err) + } + } + set, err := harness.taskSvc.CreateRepositorySet(ctx, &taskservice.CreateRepositorySetRequest{ + WorkspaceID: workspace.ID, + Name: "Full-stack", + RepositoryIDs: []string{"repo-web", "repo-gateway"}, + }) + if err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + for _, routePath := range []string{"/tasks", "/"} { + t.Run(routePath, func(t *testing.T) { + decoded := decodeBootRepositorySets(t, harness, routePath) + items := decoded.InitialState.RepositorySets.ItemsByWorkspaceID[workspace.ID] + if len(items) != 1 { + t.Fatalf("hydrated sets = %+v, want one", items) + } + if items[0].ID != set.ID || items[0].Name != "Full-stack" { + t.Fatalf("hydrated set = %+v", items[0]) + } + if len(items[0].Repositories) != 2 { + t.Fatalf("hydrated membership = %+v", items[0].Repositories) + } + if items[0].Repositories[0].RepositoryID != "repo-web" || + items[0].Repositories[1].RepositoryID != "repo-gateway" { + t.Fatalf("hydrated membership order = %+v", items[0].Repositories) + } + if decoded.InitialState.RepositorySets.LoadingByWorkspaceID[workspace.ID] { + t.Fatal("hydrated workspace is marked loading") + } + if !decoded.InitialState.RepositorySets.LoadedByWorkspaceID[workspace.ID] { + t.Fatal("hydrated workspace is not marked loaded") + } + }) + } +} + +// TestBootPayloadDoesNotMarkAFailedRepositorySetLoadAsLoaded pins the error path. +// `useRepositorySets` skips its fallback request when the workspace reports as +// loaded, so marking a failed load as loaded hides the workspace's real sets +// until an explicit refresh. +func TestBootPayloadDoesNotMarkAFailedRepositorySetLoadAsLoaded(t *testing.T) { + harness := newBootStateTestHarness(t) + workspace := bootActiveWorkspace(t, harness) + + // Drop the table so ListRepositorySets fails the way a real query error would. + if _, err := harness.db.Exec(`DROP TABLE repository_set_items`); err != nil { + t.Fatalf("drop items table: %v", err) + } + if _, err := harness.db.Exec(`DROP TABLE repository_sets`); err != nil { + t.Fatalf("drop sets table: %v", err) + } + + for _, routePath := range []string{"/tasks", "/"} { + t.Run(routePath, func(t *testing.T) { + decoded := decodeBootRepositorySets(t, harness, routePath) + // Boot still succeeds: the failure is non-fatal. + items, present := decoded.InitialState.RepositorySets.ItemsByWorkspaceID[workspace.ID] + if !present || len(items) != 0 { + t.Fatalf("items = %+v, want an empty list", items) + } + if decoded.InitialState.RepositorySets.LoadedByWorkspaceID[workspace.ID] { + t.Fatal("a failed load is marked loaded, so the client will never retry") + } + }) + } +} + +// TestBootPayloadRepositorySetsShapeIsAlwaysPresent covers a workspace with no +// sets. An absent key reads as "not loaded" to the client hook, which then +// refetches on every dialog open even though boot already answered. +func TestBootPayloadRepositorySetsShapeIsAlwaysPresent(t *testing.T) { + harness := newBootStateTestHarness(t) + workspace := bootActiveWorkspace(t, harness) + + decoded := decodeBootRepositorySets(t, harness, "/tasks") + items, present := decoded.InitialState.RepositorySets.ItemsByWorkspaceID[workspace.ID] + if !present { + t.Fatal("repositorySets key is absent for a workspace with no sets") + } + if len(items) != 0 { + t.Fatalf("items = %+v, want empty", items) + } + if !decoded.InitialState.RepositorySets.LoadedByWorkspaceID[workspace.ID] { + t.Fatal("empty workspace is not marked loaded") + } + if decoded.RouteData.TasksPage.RepositorySets == nil { + t.Fatal("routeData.tasksPage.repositorySets serialized as null, want an array") + } +} diff --git a/apps/backend/internal/backendapp/boot_state_routes.go b/apps/backend/internal/backendapp/boot_state_routes.go index dd9273791e..74ae2b8e46 100644 --- a/apps/backend/internal/backendapp/boot_state_routes.go +++ b/apps/backend/internal/backendapp/boot_state_routes.go @@ -61,7 +61,7 @@ func (b bootStateBuilder) tasksPageBootData(ctx context.Context, req *http.Reque state["userSettings"] = mapUserSettingsState(settings, activeWorkspaceID) } if activeWorkspaceID == "" { - return state, map[string]any{"activeWorkspaceId": nil, "workflows": []any{}, "steps": []any{}, "repositories": []any{}, "tasks": []any{}, "total": 0, "tasksListSort": tasksListSort, "tasksListGroup": tasksListGroup} + return state, map[string]any{"activeWorkspaceId": nil, "workflows": []any{}, "steps": []any{}, "repositories": []any{}, "repositorySets": []any{}, "tasks": []any{}, "total": 0, "tasksListSort": tasksListSort, "tasksListGroup": tasksListGroup} } workflows, err := b.p.taskSvc.ListWorkflows(ctx, activeWorkspaceID, false) if err != nil { @@ -75,6 +75,7 @@ func (b bootStateBuilder) tasksPageBootData(ctx context.Context, req *http.Reque state["userSettings"] = mapUserSettingsStateWithWorkflow(settings, activeWorkspaceID, activeWorkflowID) } repositories := b.repositoriesForState(ctx, activeWorkspaceID, state) + repositorySets := b.repositorySetsForState(ctx, activeWorkspaceID, state) steps := b.workflowStepsForWorkspace(ctx, activeWorkspaceID) tasks, total := b.tasksForWorkspace(ctx, activeWorkspaceID, activeWorkflowID, settingsRepositoryID, tasksListSort) routeData := map[string]any{ @@ -82,6 +83,7 @@ func (b bootStateBuilder) tasksPageBootData(ctx context.Context, req *http.Reque "workflows": workflowsToDTOs(workflows), "steps": steps, "repositories": repositories, + "repositorySets": repositorySets, "tasks": tasks, "total": total, "tasksListSort": tasksListSort, @@ -125,7 +127,7 @@ func (b bootStateBuilder) routeContextBootData(ctx context.Context, req *http.Re state["userSettings"] = mapUserSettingsState(settings, activeWorkspaceID) } if activeWorkspaceID == "" { - return state, map[string]any{"activeWorkspaceId": nil, "workflows": []any{}, "steps": []any{}, "repositories": []any{}} + return state, map[string]any{"activeWorkspaceId": nil, "workflows": []any{}, "steps": []any{}, "repositories": []any{}, "repositorySets": []any{}} } workflows, err := b.p.taskSvc.ListWorkflows(ctx, activeWorkspaceID, false) if err != nil { @@ -141,12 +143,14 @@ func (b bootStateBuilder) routeContextBootData(ctx context.Context, req *http.Re state["userSettings"] = mapUserSettingsStateWithWorkflow(settings, activeWorkspaceID, activeWorkflowID) } repositories := b.repositoriesForState(ctx, activeWorkspaceID, state) + repositorySets := b.repositorySetsForState(ctx, activeWorkspaceID, state) steps := b.workflowStepsForWorkspace(ctx, activeWorkspaceID) return state, map[string]any{ "activeWorkspaceId": activeWorkspaceID, "workflows": workflowsToDTOs(workflows), "steps": steps, "repositories": repositories, + "repositorySets": repositorySets, } } @@ -214,6 +218,57 @@ func (b bootStateBuilder) repositoriesForState(ctx context.Context, workspaceID return items } +// repositorySetsForState loads the workspace's repository sets for the boot +// payload, logging and returning empty on failure. The task-create picker offers +// sets on first paint from this, so the hook does not have to fetch. +func (b bootStateBuilder) repositorySetsForState( + ctx context.Context, + workspaceID string, + state map[string]any, +) []taskdto.RepositorySetDTO { + items := repositorySetsToDTOs(nil) + loaded := false + sets, err := b.p.taskSvc.ListRepositorySets(ctx, workspaceID) + if err != nil { + // Non-fatal, like every neighbour here: boot succeeds with an empty list + // and the client fetches instead. Leaving `loaded` false is what makes + // that fetch happen; marking a failed load as loaded would hide the + // workspace's real sets until an explicit refresh. + b.logBootError("list repository sets", err) + } else { + items = repositorySetsToDTOs(sets) + loaded = true + } + state["repositorySets"] = repositorySetsState(workspaceID, items, loaded) + return items +} + +// repositorySetsState is the workspace-keyed slice shape the web store expects. +// An empty workspace still keys an entry: an absent key reads as "not loaded" to +// the client hook, which then refetches on every dialog open. +func repositorySetsState( + workspaceID string, + items []taskdto.RepositorySetDTO, + loaded bool, +) map[string]any { + return map[string]any{ + "itemsByWorkspaceId": map[string]any{workspaceID: items}, + "loadingByWorkspaceId": map[string]any{workspaceID: false}, + "loadedByWorkspaceId": map[string]any{workspaceID: loaded}, + } +} + +// repositorySetsToDTOs maps repository set models to DTOs, always as an array. +func repositorySetsToDTOs(sets []*taskmodels.RepositorySet) []taskdto.RepositorySetDTO { + items := make([]taskdto.RepositorySetDTO, 0, len(sets)) + for _, set := range sets { + if set != nil { + items = append(items, taskdto.FromRepositorySet(set)) + } + } + return items +} + // repositoriesToDTOs maps repository models to DTOs. func repositoriesToDTOs(repositories []*taskmodels.Repository) []taskdto.RepositoryDTO { items := make([]taskdto.RepositoryDTO, 0, len(repositories)) diff --git a/apps/backend/internal/backendapp/e2e_reset.go b/apps/backend/internal/backendapp/e2e_reset.go index e73f8cd15c..b1b878f169 100644 --- a/apps/backend/internal/backendapp/e2e_reset.go +++ b/apps/backend/internal/backendapp/e2e_reset.go @@ -127,6 +127,17 @@ func handleE2EReset( if _, err := repo.DB().ExecContext(ctx, `DELETE FROM runtime_flag_overrides`); err != nil { log.Warn("e2e reset: runtime flag override cleanup failed", zap.Error(err)) } + // Repository sets outlive the tasks a reset removes, so a set seeded by + // one spec would still be offered in the next spec's create dialog. The + // items cascade from the set row. + for _, q := range []string{ + `DELETE FROM repository_set_items WHERE repository_set_id IN (SELECT id FROM repository_sets WHERE workspace_id = ?)`, + `DELETE FROM repository_sets WHERE workspace_id = ?`, + } { + if _, err := repo.DB().ExecContext(ctx, q, workspaceID); err != nil { + log.Warn("e2e reset: repository set cleanup failed", zap.String("sql", q), zap.Error(err)) + } + } if _, err := repo.DB().ExecContext(ctx, `DELETE FROM github_workspace_settings WHERE workspace_id = ?`, workspaceID); err != nil { log.Warn("e2e reset: GitHub workspace settings cleanup failed", zap.Error(err)) } diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index 100ba77845..66ebfc5631 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -1064,6 +1064,7 @@ func registerTaskRoutes(p routeParams, planService *taskservice.PlanService, han }) } taskhandlers.RegisterRepositoryRoutes(p.router, p.gateway.Dispatcher, p.taskSvc, p.log) + taskhandlers.RegisterRepositorySetRoutes(p.router, p.gateway.Dispatcher, p.taskSvc, p.log) taskhandlers.RegisterExecutorRoutes(p.router, p.gateway.Dispatcher, p.taskSvc, p.log) taskhandlers.RegisterExecutorProfileRoutes(p.router, p.gateway.Dispatcher, p.taskSvc, p.agentList, p.log) taskhandlers.RegisterEnvironmentRoutes(p.router, p.gateway.Dispatcher, p.taskSvc, p.log) diff --git a/apps/backend/internal/backendapp/helpers_test.go b/apps/backend/internal/backendapp/helpers_test.go index 4ed20fccd0..7de9eb691f 100644 --- a/apps/backend/internal/backendapp/helpers_test.go +++ b/apps/backend/internal/backendapp/helpers_test.go @@ -1980,6 +1980,7 @@ func newBootStateTestHarness(t *testing.T) bootStateTestHarness { Sessions: taskRepo, GitSnapshots: taskRepo, RepoEntities: taskRepo, + RepositorySets: taskRepo, Executors: taskRepo, Environments: taskRepo, TaskEnvironments: taskRepo, diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index da8b6470d9..7b8023607a 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -103,6 +103,7 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories Sessions: repos.Task, GitSnapshots: repos.Task, RepoEntities: repos.Task, + RepositorySets: repos.Task, RepositoryCleanup: repos.Task, Executors: repos.Task, Environments: repos.Task, diff --git a/apps/backend/internal/events/types.go b/apps/backend/internal/events/types.go index 33593ba7f5..6ea107ed67 100644 --- a/apps/backend/internal/events/types.go +++ b/apps/backend/internal/events/types.go @@ -133,6 +133,13 @@ const ( AzureDevOpsPullRequestWatchMatch = "azure_devops.pull_request_watch.match" ) +// Event types for repository sets +const ( + RepositorySetCreated = "repository_set.created" + RepositorySetUpdated = "repository_set.updated" + RepositorySetDeleted = "repository_set.deleted" +) + // Event types for repository scripts const ( RepositoryScriptCreated = "repository.script.created" diff --git a/apps/backend/internal/gateway/websocket/task_notifications.go b/apps/backend/internal/gateway/websocket/task_notifications.go index 0a6937b41c..235958aeb7 100644 --- a/apps/backend/internal/gateway/websocket/task_notifications.go +++ b/apps/backend/internal/gateway/websocket/task_notifications.go @@ -60,6 +60,9 @@ func RegisterTaskNotifications(ctx context.Context, eventBus bus.EventBus, hub * b.subscribe(eventBus, events.RepositoryCreated, ws.ActionRepositoryCreated) b.subscribe(eventBus, events.RepositoryUpdated, ws.ActionRepositoryUpdated) b.subscribe(eventBus, events.RepositoryDeleted, ws.ActionRepositoryDeleted) + b.subscribe(eventBus, events.RepositorySetCreated, ws.ActionRepositorySetCreated) + b.subscribe(eventBus, events.RepositorySetUpdated, ws.ActionRepositorySetUpdated) + b.subscribe(eventBus, events.RepositorySetDeleted, ws.ActionRepositorySetDeleted) b.subscribe(eventBus, events.RepositoryScriptCreated, ws.ActionRepositoryScriptCreated) b.subscribe(eventBus, events.RepositoryScriptUpdated, ws.ActionRepositoryScriptUpdated) b.subscribe(eventBus, events.RepositoryScriptDeleted, ws.ActionRepositoryScriptDeleted) diff --git a/apps/backend/internal/gateway/websocket/task_notifications_test.go b/apps/backend/internal/gateway/websocket/task_notifications_test.go index ccb3958802..8cfe46bc59 100644 --- a/apps/backend/internal/gateway/websocket/task_notifications_test.go +++ b/apps/backend/internal/gateway/websocket/task_notifications_test.go @@ -111,7 +111,7 @@ func TestTaskEventBroadcaster_NoDuplicateSubscriptions(t *testing.T) { // // Update this number when adding or removing event subscriptions in // RegisterTaskNotifications — it is intentionally exact. - const wantSubscriptions = 64 + const wantSubscriptions = 67 if got := len(b.subscriptions); got != wantSubscriptions { t.Errorf("RegisterTaskNotifications created %d subscriptions, want %d — "+ "did an event get subscribed twice?", got, wantSubscriptions) diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index c172136ed2..2dc5b22544 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -77,6 +77,25 @@ type RepositorySecretBindingDTO struct { SecretID string `json:"secret_id"` } +// RepositorySetDTO is a named group of workspace repositories on the wire. +// Repositories is always an array, never null: the web store indexes it without +// a nil check, and a set whose members were all deleted is legitimately empty. +type RepositorySetDTO struct { + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` + Name string `json:"name"` + Description string `json:"description"` + Repositories []RepositorySetItemDTO `json:"repositories"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// RepositorySetItemDTO is one repository's membership, in apply order. +type RepositorySetItemDTO struct { + RepositoryID string `json:"repository_id"` + Position int `json:"position"` +} + type RepositoryScriptDTO struct { ID string `json:"id"` RepositoryID string `json:"repository_id"` @@ -480,6 +499,11 @@ type ListRepositoriesResponse struct { Total int `json:"total"` } +type ListRepositorySetsResponse struct { + RepositorySets []RepositorySetDTO `json:"repository_sets"` + Total int `json:"total"` +} + type ListRepositoryScriptsResponse struct { Scripts []RepositoryScriptDTO `json:"scripts"` Total int `json:"total"` @@ -634,6 +658,25 @@ func FromRepository(repository *models.Repository) RepositoryDTO { } } +func FromRepositorySet(set *models.RepositorySet) RepositorySetDTO { + items := make([]RepositorySetItemDTO, 0, len(set.Items)) + for _, item := range set.Items { + items = append(items, RepositorySetItemDTO{ + RepositoryID: item.RepositoryID, + Position: item.Position, + }) + } + return RepositorySetDTO{ + ID: set.ID, + WorkspaceID: set.WorkspaceID, + Name: set.Name, + Description: set.Description, + Repositories: items, + CreatedAt: set.CreatedAt, + UpdatedAt: set.UpdatedAt, + } +} + func FromRepositoryScript(script *models.RepositoryScript) RepositoryScriptDTO { return RepositoryScriptDTO{ ID: script.ID, diff --git a/apps/backend/internal/task/dto/repository_set_test.go b/apps/backend/internal/task/dto/repository_set_test.go new file mode 100644 index 0000000000..9210f43c08 --- /dev/null +++ b/apps/backend/internal/task/dto/repository_set_test.go @@ -0,0 +1,69 @@ +package dto + +import ( + "encoding/json" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestFromRepositorySetCarriesOrderedMembership(t *testing.T) { + created := time.Date(2026, 8, 17, 9, 0, 0, 0, time.UTC) + set := &models.RepositorySet{ + ID: "set-1", + WorkspaceID: "ws-1", + Name: "Full-stack", + Description: "web + gateway", + Items: []models.RepositorySetItem{ + {ID: "item-1", RepositorySetID: "set-1", RepositoryID: "repo-web", Position: 0}, + {ID: "item-2", RepositorySetID: "set-1", RepositoryID: "repo-gateway", Position: 1}, + }, + CreatedAt: created, + UpdatedAt: created, + } + + got := FromRepositorySet(set) + + if got.ID != "set-1" || got.WorkspaceID != "ws-1" || got.Name != "Full-stack" { + t.Fatalf("dto = %+v", got) + } + if got.Description != "web + gateway" { + t.Fatalf("description = %q", got.Description) + } + if len(got.Repositories) != 2 { + t.Fatalf("repositories = %+v", got.Repositories) + } + if got.Repositories[0].RepositoryID != "repo-web" || got.Repositories[0].Position != 0 { + t.Fatalf("first member = %+v", got.Repositories[0]) + } + if got.Repositories[1].RepositoryID != "repo-gateway" || got.Repositories[1].Position != 1 { + t.Fatalf("second member = %+v", got.Repositories[1]) + } + if !got.CreatedAt.Equal(created) || !got.UpdatedAt.Equal(created) { + t.Fatalf("timestamps = %v / %v", got.CreatedAt, got.UpdatedAt) + } +} + +func TestFromRepositorySetEmptyMembershipSerializesAsArray(t *testing.T) { + // A set whose repositories were all deleted must serialize as [] rather than + // null: the web store indexes the list without a nil check. + got := FromRepositorySet(&models.RepositorySet{ID: "set-1", WorkspaceID: "ws-1", Name: "Empty"}) + + encoded, err := json.Marshal(got) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded struct { + Repositories []RepositorySetItemDTO `json:"repositories"` + } + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if decoded.Repositories == nil { + t.Fatalf("repositories serialized as null: %s", encoded) + } + if len(decoded.Repositories) != 0 { + t.Fatalf("repositories = %+v", decoded.Repositories) + } +} diff --git a/apps/backend/internal/task/handlers/repository_set_handlers.go b/apps/backend/internal/task/handlers/repository_set_handlers.go new file mode 100644 index 0000000000..144c2dd8a2 --- /dev/null +++ b/apps/backend/internal/task/handlers/repository_set_handlers.go @@ -0,0 +1,365 @@ +package handlers + +import ( + "context" + "errors" + "net/http" + + "github.com/gin-gonic/gin" + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/dto" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" + "github.com/kandev/kandev/internal/task/service" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// RepositorySetHandlers exposes named groups of workspace repositories over HTTP +// and the WebSocket dispatcher. Collection routes are workspace-scoped and item +// routes are flat, matching the sibling repository routes; the workspace param +// must stay `:id` because gin rejects a second wildcard name at that position +// within one group. +type RepositorySetHandlers struct { + service *service.Service + logger *logger.Logger +} + +func NewRepositorySetHandlers(svc *service.Service, log *logger.Logger) *RepositorySetHandlers { + return &RepositorySetHandlers{ + service: svc, + logger: log.WithFields(zap.String("component", "task-repository-set-handlers")), + } +} + +func RegisterRepositorySetRoutes( + router *gin.Engine, + dispatcher *ws.Dispatcher, + svc *service.Service, + log *logger.Logger, +) *RepositorySetHandlers { + handlers := NewRepositorySetHandlers(svc, log) + handlers.registerHTTP(router) + handlers.registerWS(dispatcher) + return handlers +} + +func (h *RepositorySetHandlers) registerHTTP(router *gin.Engine) { + api := router.Group("/api/v1") + api.GET("/workspaces/:id/repository-sets", h.httpListRepositorySets) + api.POST("/workspaces/:id/repository-sets", h.httpCreateRepositorySet) + api.GET("/repository-sets/:id", h.httpGetRepositorySet) + api.PATCH("/repository-sets/:id", h.httpUpdateRepositorySet) + api.DELETE("/repository-sets/:id", h.httpDeleteRepositorySet) +} + +func (h *RepositorySetHandlers) registerWS(dispatcher *ws.Dispatcher) { + dispatcher.RegisterFunc(ws.ActionRepositorySetList, h.wsListRepositorySets) + dispatcher.RegisterFunc(ws.ActionRepositorySetCreate, h.wsCreateRepositorySet) + dispatcher.RegisterFunc(ws.ActionRepositorySetGet, h.wsGetRepositorySet) + dispatcher.RegisterFunc(ws.ActionRepositorySetUpdate, h.wsUpdateRepositorySet) + dispatcher.RegisterFunc(ws.ActionRepositorySetDelete, h.wsDeleteRepositorySet) +} + +// repositorySetsToListResponse keeps the list shape identical on both transports. +func repositorySetsToListResponse(sets []*models.RepositorySet) dto.ListRepositorySetsResponse { + items := make([]dto.RepositorySetDTO, 0, len(sets)) + for _, set := range sets { + if set != nil { + items = append(items, dto.FromRepositorySet(set)) + } + } + return dto.ListRepositorySetsResponse{RepositorySets: items, Total: len(items)} +} + +// repositorySetCreateBody is the create payload. workspace_id is read from the +// path on HTTP and from the payload over WebSocket. +type repositorySetCreateBody struct { + WorkspaceID string `json:"workspace_id"` + Name string `json:"name"` + Description string `json:"description"` + RepositoryIDs []string `json:"repository_ids"` +} + +// repositorySetUpdateBody patches a set. Pointers distinguish "absent" from +// "set to empty": an absent repository_ids leaves membership alone, while an +// explicitly empty one is a rejected request rather than a silent wipe. +type repositorySetUpdateBody struct { + Name *string `json:"name"` + Description *string `json:"description"` + RepositoryIDs *[]string `json:"repository_ids"` +} + +// repositorySetStatus maps a service error to its HTTP status. Categories are +// stable so callers never parse a message. +func repositorySetStatus(err error) int { + switch { + case errors.Is(err, service.ErrInvalidRepositorySet): + return http.StatusBadRequest + case errors.Is(err, service.ErrRepositorySetNameConflict): + return http.StatusConflict + case errors.Is(err, service.ErrUnknownRepositorySetMembers): + return http.StatusUnprocessableEntity + case errors.Is(err, repoerrors.ErrRepositorySetNotFound), + errors.Is(err, repoerrors.ErrWorkspaceNotFound): + return http.StatusNotFound + default: + return http.StatusInternalServerError + } +} + +// abortWithRepositorySetError answers with the mapped status. A 500 is logged +// and given a generic body; every other status carries the service message, +// which names the conflicting set or the offending repository ids. +func (h *RepositorySetHandlers) abortWithRepositorySetError(c *gin.Context, action string, err error) { + status := repositorySetStatus(err) + if status == http.StatusInternalServerError { + h.logger.Error("repository set request failed", zap.String("action", action), zap.Error(err)) + c.JSON(status, gin.H{"error": "failed to " + action}) + return + } + c.JSON(status, gin.H{"error": err.Error()}) +} + +// resolveWritableWorkspace rejects an unknown workspace with 404 and the +// read-only Improve Kandev workspace with 409, before any set is touched. +// Workspace-scoped authorization happens again in the service; this exists so an +// unknown workspace does not read as an empty one. +func (h *RepositorySetHandlers) resolveWritableWorkspace(c *gin.Context, workspaceID string) bool { + workspace, err := h.service.GetWorkspace(c.Request.Context(), workspaceID) + if err != nil { + handleNotFound(c, h.logger, err, "workspace not found") + return false + } + if workspace.IsImproveKandev() { + c.JSON(http.StatusConflict, gin.H{"error": workspaceReadOnlyMsg}) + return false + } + return true +} + +// HTTP handlers + +func (h *RepositorySetHandlers) httpListRepositorySets(c *gin.Context) { + sets, err := h.service.ListRepositorySets(c.Request.Context(), c.Param("id")) + if err != nil { + h.abortWithRepositorySetError(c, "list repository sets", err) + return + } + c.JSON(http.StatusOK, repositorySetsToListResponse(sets)) +} + +func (h *RepositorySetHandlers) httpCreateRepositorySet(c *gin.Context) { + var body repositorySetCreateBody + if err := c.ShouldBindJSON(&body); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"}) + return + } + workspaceID := c.Param("id") + if !h.resolveWritableWorkspace(c, workspaceID) { + return + } + set, err := h.service.CreateRepositorySet(c.Request.Context(), &service.CreateRepositorySetRequest{ + WorkspaceID: workspaceID, + Name: body.Name, + Description: body.Description, + RepositoryIDs: body.RepositoryIDs, + }) + if err != nil { + h.abortWithRepositorySetError(c, "create repository set", err) + return + } + c.JSON(http.StatusCreated, dto.FromRepositorySet(set)) +} + +func (h *RepositorySetHandlers) httpGetRepositorySet(c *gin.Context) { + set, err := h.service.GetRepositorySet(c.Request.Context(), c.Param("id")) + if err != nil { + h.abortWithRepositorySetError(c, "get repository set", err) + return + } + c.JSON(http.StatusOK, dto.FromRepositorySet(set)) +} + +func (h *RepositorySetHandlers) httpUpdateRepositorySet(c *gin.Context) { + var body repositorySetUpdateBody + if err := c.ShouldBindJSON(&body); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"}) + return + } + set, err := h.service.GetRepositorySet(c.Request.Context(), c.Param("id")) + if err != nil { + h.abortWithRepositorySetError(c, "get repository set", err) + return + } + if !h.resolveWritableWorkspace(c, set.WorkspaceID) { + return + } + updated, err := h.service.UpdateRepositorySet(c.Request.Context(), set.ID, &service.UpdateRepositorySetRequest{ + Name: body.Name, + Description: body.Description, + RepositoryIDs: body.RepositoryIDs, + }) + if err != nil { + h.abortWithRepositorySetError(c, "update repository set", err) + return + } + c.JSON(http.StatusOK, dto.FromRepositorySet(updated)) +} + +func (h *RepositorySetHandlers) httpDeleteRepositorySet(c *gin.Context) { + set, err := h.service.GetRepositorySet(c.Request.Context(), c.Param("id")) + if err != nil { + h.abortWithRepositorySetError(c, "get repository set", err) + return + } + if !h.resolveWritableWorkspace(c, set.WorkspaceID) { + return + } + if err := h.service.DeleteRepositorySet(c.Request.Context(), set.ID); err != nil { + h.abortWithRepositorySetError(c, "delete repository set", err) + return + } + c.Status(http.StatusNoContent) +} + +// WebSocket handlers + +// wsRejectReadOnlyWorkspace mirrors the HTTP guard for WebSocket mutations. The +// HTTP path resolves the workspace before writing; without the same check here a +// WebSocket client could create, update, or delete sets in the read-only Improve +// Kandev workspace. +func (h *RepositorySetHandlers) wsRejectReadOnlyWorkspace( + ctx context.Context, + msg *ws.Message, + workspaceID string, +) (*ws.Message, bool) { + workspace, err := h.service.GetWorkspace(ctx, workspaceID) + if err != nil { + errMsg, _ := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "workspace not found", nil) + return errMsg, true + } + if workspace.IsImproveKandev() { + errMsg, _ := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, workspaceReadOnlyMsg, nil) + return errMsg, true + } + return nil, false +} + +// wsRejectReadOnlySet resolves a set's workspace before a mutation keyed by set +// id, so update and delete are guarded exactly like create. +func (h *RepositorySetHandlers) wsRejectReadOnlySet( + ctx context.Context, + msg *ws.Message, + setID string, +) (*ws.Message, bool) { + set, err := h.service.GetRepositorySet(ctx, setID) + if err != nil { + errMsg, _ := h.wsRepositorySetError(msg, "get repository set", err) + return errMsg, true + } + return h.wsRejectReadOnlyWorkspace(ctx, msg, set.WorkspaceID) +} + +// wsRepositorySetError maps a service error onto the WS error codes, mirroring +// the HTTP status mapping so both transports categorize failures the same way. +func (h *RepositorySetHandlers) wsRepositorySetError(msg *ws.Message, action string, err error) (*ws.Message, error) { + switch repositorySetStatus(err) { + case http.StatusBadRequest: + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + case http.StatusConflict, http.StatusUnprocessableEntity: + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + case http.StatusNotFound: + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, err.Error(), nil) + default: + h.logger.Error("repository set request failed", zap.String("action", action), zap.Error(err)) + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to "+action, nil) + } +} + +func (h *RepositorySetHandlers) wsListRepositorySets(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + WorkspaceID string `json:"workspace_id"` + } + if err := msg.ParsePayload(&req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + sets, err := h.service.ListRepositorySets(ctx, req.WorkspaceID) + if err != nil { + return h.wsRepositorySetError(msg, "list repository sets", err) + } + return ws.NewResponse(msg.ID, msg.Action, repositorySetsToListResponse(sets)) +} + +func (h *RepositorySetHandlers) wsCreateRepositorySet(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req repositorySetCreateBody + if err := msg.ParsePayload(&req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + if errMsg, blocked := h.wsRejectReadOnlyWorkspace(ctx, msg, req.WorkspaceID); blocked { + return errMsg, nil + } + set, err := h.service.CreateRepositorySet(ctx, &service.CreateRepositorySetRequest{ + WorkspaceID: req.WorkspaceID, + Name: req.Name, + Description: req.Description, + RepositoryIDs: req.RepositoryIDs, + }) + if err != nil { + return h.wsRepositorySetError(msg, "create repository set", err) + } + return ws.NewResponse(msg.ID, msg.Action, dto.FromRepositorySet(set)) +} + +func (h *RepositorySetHandlers) wsGetRepositorySet(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + ID string `json:"id"` + } + if err := msg.ParsePayload(&req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + set, err := h.service.GetRepositorySet(ctx, req.ID) + if err != nil { + return h.wsRepositorySetError(msg, "get repository set", err) + } + return ws.NewResponse(msg.ID, msg.Action, dto.FromRepositorySet(set)) +} + +func (h *RepositorySetHandlers) wsUpdateRepositorySet(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + ID string `json:"id"` + repositorySetUpdateBody + } + if err := msg.ParsePayload(&req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + if errMsg, blocked := h.wsRejectReadOnlySet(ctx, msg, req.ID); blocked { + return errMsg, nil + } + updated, err := h.service.UpdateRepositorySet(ctx, req.ID, &service.UpdateRepositorySetRequest{ + Name: req.Name, + Description: req.Description, + RepositoryIDs: req.RepositoryIDs, + }) + if err != nil { + return h.wsRepositorySetError(msg, "update repository set", err) + } + return ws.NewResponse(msg.ID, msg.Action, dto.FromRepositorySet(updated)) +} + +func (h *RepositorySetHandlers) wsDeleteRepositorySet(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + ID string `json:"id"` + } + if err := msg.ParsePayload(&req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + if errMsg, blocked := h.wsRejectReadOnlySet(ctx, msg, req.ID); blocked { + return errMsg, nil + } + if err := h.service.DeleteRepositorySet(ctx, req.ID); err != nil { + return h.wsRepositorySetError(msg, "delete repository set", err) + } + return ws.NewResponse(msg.ID, msg.Action, gin.H{"id": req.ID, "deleted": true}) +} diff --git a/apps/backend/internal/task/handlers/repository_set_handlers_test.go b/apps/backend/internal/task/handlers/repository_set_handlers_test.go new file mode 100644 index 0000000000..4d8af7a04e --- /dev/null +++ b/apps/backend/internal/task/handlers/repository_set_handlers_test.go @@ -0,0 +1,278 @@ +package handlers + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/gin-gonic/gin" + "github.com/jmoiron/sqlx" + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/db" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/task/dto" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository" + "github.com/kandev/kandev/internal/task/service" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// newRepositorySetTestRouter builds a real gin router over an in-memory task +// repository seeded with one workspace and three repositories. +func newRepositorySetTestRouter(t *testing.T) (*gin.Engine, *ws.Dispatcher) { + t.Helper() + gin.SetMode(gin.TestMode) + dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "repository-sets.db")) + require.NoError(t, err) + sqlxDB := sqlx.NewDb(dbConn, "sqlite3") + repo, cleanup, err := repository.Provide(sqlxDB, sqlxDB, nil) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, cleanup()) + require.NoError(t, sqlxDB.Close()) + }) + + ctx := context.Background() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"})) + for _, id := range []string{"repo-web", "repo-gateway", "repo-orders"} { + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ + ID: id, WorkspaceID: "ws-1", Name: id, SourceType: "local", LocalPath: t.TempDir(), + })) + } + + log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"}) + require.NoError(t, err) + svc := service.NewService(service.Repos{ + Workspaces: repo, + RepoEntities: repo, + RepositorySets: repo, + }, bus.NewMemoryEventBus(log), log, service.RepositoryDiscoveryConfig{}) + + router := gin.New() + dispatcher := ws.NewDispatcher() + RegisterRepositorySetRoutes(router, dispatcher, svc, log) + return router, dispatcher +} + +func doJSON(t *testing.T, router *gin.Engine, method, path string, body any) *httptest.ResponseRecorder { + t.Helper() + var reader *bytes.Reader + if body == nil { + reader = bytes.NewReader(nil) + } else { + encoded, err := json.Marshal(body) + require.NoError(t, err) + reader = bytes.NewReader(encoded) + } + req := httptest.NewRequest(method, path, reader) + req.Header.Set("Content-Type", "application/json") + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, req) + return recorder +} + +func createSetViaHTTP(t *testing.T, router *gin.Engine, body any) dto.RepositorySetDTO { + t.Helper() + recorder := doJSON(t, router, http.MethodPost, "/api/v1/workspaces/ws-1/repository-sets", body) + require.Equal(t, http.StatusCreated, recorder.Code, recorder.Body.String()) + var created dto.RepositorySetDTO + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &created)) + return created +} + +func TestRegisterRepositorySetRoutesWiresHTTPAndWS(t *testing.T) { + router, dispatcher := newRepositorySetTestRouter(t) + + requireRoutes(t, router, + "GET /api/v1/workspaces/:id/repository-sets", + "POST /api/v1/workspaces/:id/repository-sets", + "GET /api/v1/repository-sets/:id", + "PATCH /api/v1/repository-sets/:id", + "DELETE /api/v1/repository-sets/:id", + ) + requireActions(t, dispatcher, + ws.ActionRepositorySetList, + ws.ActionRepositorySetCreate, + ws.ActionRepositorySetGet, + ws.ActionRepositorySetUpdate, + ws.ActionRepositorySetDelete, + ) +} + +func TestHTTPCreateAndListRepositorySets(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + + created := createSetViaHTTP(t, router, map[string]any{ + "name": "Full-stack", + "description": "web + gateway", + "repository_ids": []string{"repo-web", "repo-gateway"}, + }) + require.NotEmpty(t, created.ID) + require.Equal(t, "ws-1", created.WorkspaceID) + require.Len(t, created.Repositories, 2) + require.Equal(t, "repo-web", created.Repositories[0].RepositoryID) + require.Equal(t, 0, created.Repositories[0].Position) + require.Equal(t, "repo-gateway", created.Repositories[1].RepositoryID) + require.Equal(t, 1, created.Repositories[1].Position) + + recorder := doJSON(t, router, http.MethodGet, "/api/v1/workspaces/ws-1/repository-sets", nil) + require.Equal(t, http.StatusOK, recorder.Code) + var list dto.ListRepositorySetsResponse + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &list)) + require.Equal(t, 1, list.Total) + require.Len(t, list.RepositorySets, 1) + require.Equal(t, created.ID, list.RepositorySets[0].ID) +} + +func TestHTTPListRepositorySetsEmptyWorkspaceReturnsArray(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + + recorder := doJSON(t, router, http.MethodGet, "/api/v1/workspaces/ws-1/repository-sets", nil) + require.Equal(t, http.StatusOK, recorder.Code) + // The client indexes the list without a nil check. + require.Contains(t, recorder.Body.String(), `"repository_sets":[]`) +} + +func TestHTTPGetRepositorySet(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + created := createSetViaHTTP(t, router, map[string]any{ + "name": "Full-stack", + "repository_ids": []string{"repo-web"}, + }) + + recorder := doJSON(t, router, http.MethodGet, "/api/v1/repository-sets/"+created.ID, nil) + require.Equal(t, http.StatusOK, recorder.Code) + var got dto.RepositorySetDTO + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &got)) + require.Equal(t, created.ID, got.ID) + + missing := doJSON(t, router, http.MethodGet, "/api/v1/repository-sets/nope", nil) + require.Equal(t, http.StatusNotFound, missing.Code) +} + +func TestHTTPCreateRepositorySetErrorCodes(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + createSetViaHTTP(t, router, map[string]any{ + "name": "Full-stack", + "repository_ids": []string{"repo-web"}, + }) + + cases := []struct { + name string + body any + want int + }{ + {"blank name", map[string]any{"name": " ", "repository_ids": []string{"repo-web"}}, http.StatusBadRequest}, + {"no members", map[string]any{"name": "Empty"}, http.StatusBadRequest}, + {"repeated member", map[string]any{ + "name": "Twice", "repository_ids": []string{"repo-web", "repo-web"}, + }, http.StatusBadRequest}, + {"duplicate name", map[string]any{ + "name": "full-STACK", "repository_ids": []string{"repo-orders"}, + }, http.StatusConflict}, + {"unknown member", map[string]any{ + "name": "Ghost", "repository_ids": []string{"repo-missing"}, + }, http.StatusUnprocessableEntity}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + recorder := doJSON(t, router, http.MethodPost, + "/api/v1/workspaces/ws-1/repository-sets", testCase.body) + require.Equal(t, testCase.want, recorder.Code, recorder.Body.String()) + }) + } +} + +func TestHTTPCreateRepositorySetUnknownWorkspaceIsNotFound(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + + recorder := doJSON(t, router, http.MethodPost, "/api/v1/workspaces/ws-missing/repository-sets", + map[string]any{"name": "Ghost", "repository_ids": []string{"repo-web"}}) + require.Equal(t, http.StatusNotFound, recorder.Code, recorder.Body.String()) +} + +func TestHTTPUpdateRepositorySetReplacesMembership(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + created := createSetViaHTTP(t, router, map[string]any{ + "name": "Full-stack", + "repository_ids": []string{"repo-web", "repo-gateway"}, + }) + + recorder := doJSON(t, router, http.MethodPatch, "/api/v1/repository-sets/"+created.ID, + map[string]any{"repository_ids": []string{"repo-orders", "repo-web"}}) + require.Equal(t, http.StatusOK, recorder.Code, recorder.Body.String()) + var updated dto.RepositorySetDTO + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &updated)) + require.Len(t, updated.Repositories, 2) + require.Equal(t, "repo-orders", updated.Repositories[0].RepositoryID) + require.Equal(t, "repo-web", updated.Repositories[1].RepositoryID) + require.Equal(t, "Full-stack", updated.Name) +} + +func TestHTTPUpdateRepositorySetOmittedMembershipIsPreserved(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + created := createSetViaHTTP(t, router, map[string]any{ + "name": "Full-stack", + "repository_ids": []string{"repo-web", "repo-gateway"}, + }) + + // A rename must not be read as "remove every member". + recorder := doJSON(t, router, http.MethodPatch, "/api/v1/repository-sets/"+created.ID, + map[string]any{"name": "Renamed"}) + require.Equal(t, http.StatusOK, recorder.Code, recorder.Body.String()) + var updated dto.RepositorySetDTO + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &updated)) + require.Equal(t, "Renamed", updated.Name) + require.Len(t, updated.Repositories, 2) +} + +func TestHTTPDeleteRepositorySet(t *testing.T) { + router, _ := newRepositorySetTestRouter(t) + created := createSetViaHTTP(t, router, map[string]any{ + "name": "Full-stack", + "repository_ids": []string{"repo-web"}, + }) + + recorder := doJSON(t, router, http.MethodDelete, "/api/v1/repository-sets/"+created.ID, nil) + require.Equal(t, http.StatusNoContent, recorder.Code, recorder.Body.String()) + + again := doJSON(t, router, http.MethodDelete, "/api/v1/repository-sets/"+created.ID, nil) + require.Equal(t, http.StatusNotFound, again.Code) +} + +func TestWSRepositorySetListAndCreate(t *testing.T) { + router, dispatcher := newRepositorySetTestRouter(t) + ctx := context.Background() + + createMsg, err := ws.NewRequest("1", ws.ActionRepositorySetCreate, map[string]any{ + "workspace_id": "ws-1", + "name": "Full-stack", + "repository_ids": []string{"repo-web", "repo-gateway"}, + }) + require.NoError(t, err) + response, err := dispatcher.Dispatch(ctx, createMsg) + require.NoError(t, err) + require.NotNil(t, response) + require.NotEqual(t, ws.MessageTypeError, response.Type, "create failed: %s", response.Payload) + + listMsg, err := ws.NewRequest("2", ws.ActionRepositorySetList, map[string]any{"workspace_id": "ws-1"}) + require.NoError(t, err) + listResponse, err := dispatcher.Dispatch(ctx, listMsg) + require.NoError(t, err) + require.NotEqual(t, ws.MessageTypeError, listResponse.Type, "list failed: %s", listResponse.Payload) + + var list dto.ListRepositorySetsResponse + require.NoError(t, json.Unmarshal(listResponse.Payload, &list)) + require.Equal(t, 1, list.Total) + + // The HTTP surface sees the same row: one service, one store. + httpList := doJSON(t, router, http.MethodGet, "/api/v1/workspaces/ws-1/repository-sets", nil) + require.Equal(t, http.StatusOK, httpList.Code) + require.Contains(t, httpList.Body.String(), "Full-stack") +} diff --git a/apps/backend/internal/task/handlers/repository_set_readonly_test.go b/apps/backend/internal/task/handlers/repository_set_readonly_test.go new file mode 100644 index 0000000000..d6cede4c5d --- /dev/null +++ b/apps/backend/internal/task/handlers/repository_set_readonly_test.go @@ -0,0 +1,130 @@ +package handlers + +import ( + "context" + "encoding/json" + "net/http" + "path/filepath" + "testing" + + "github.com/gin-gonic/gin" + "github.com/jmoiron/sqlx" + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/db" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository" + "github.com/kandev/kandev/internal/task/service" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// The HTTP mutations resolve a writable workspace before writing. The WebSocket +// mutations called the service directly, so a WebSocket client could create, +// update, or delete sets in the read-only Improve Kandev workspace. + +// newImproveWorkspaceRouter seeds the read-only Improve workspace with one +// repository and one existing set, so every mutation has a real target. +func newImproveWorkspaceRouter(t *testing.T) (*gin.Engine, *ws.Dispatcher, string) { + t.Helper() + gin.SetMode(gin.TestMode) + dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "repository-sets-readonly.db")) + require.NoError(t, err) + sqlxDB := sqlx.NewDb(dbConn, "sqlite3") + repo, cleanup, err := repository.Provide(sqlxDB, sqlxDB, nil) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, cleanup()) + require.NoError(t, sqlxDB.Close()) + }) + + ctx := context.Background() + improve := &models.Workspace{ID: "ws-improve", Name: models.WorkspaceNameImproveKandev} + require.NoError(t, repo.CreateWorkspace(ctx, improve)) + require.True(t, improve.IsImproveKandev(), "fixture must be the read-only workspace") + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-improve", WorkspaceID: "ws-improve", Name: "improve", + SourceType: "local", LocalPath: t.TempDir(), + })) + existing := &models.RepositorySet{ + WorkspaceID: "ws-improve", + Name: "Existing", + Items: []models.RepositorySetItem{{RepositoryID: "repo-improve"}}, + } + require.NoError(t, repo.CreateRepositorySet(ctx, existing)) + + log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"}) + require.NoError(t, err) + svc := service.NewService(service.Repos{ + Workspaces: repo, + RepoEntities: repo, + RepositorySets: repo, + }, bus.NewMemoryEventBus(log), log, service.RepositoryDiscoveryConfig{}) + + router := gin.New() + dispatcher := ws.NewDispatcher() + RegisterRepositorySetRoutes(router, dispatcher, svc, log) + return router, dispatcher, existing.ID +} + +func dispatchSetAction(t *testing.T, dispatcher *ws.Dispatcher, action string, payload any) *ws.Message { + t.Helper() + msg, err := ws.NewRequest("1", action, payload) + require.NoError(t, err) + response, err := dispatcher.Dispatch(context.Background(), msg) + require.NoError(t, err) + require.NotNil(t, response) + return response +} + +func TestWSRepositorySetMutationsRejectReadOnlyWorkspace(t *testing.T) { + router, dispatcher, setID := newImproveWorkspaceRouter(t) + + cases := []struct { + name string + action string + payload any + }{ + {"create", ws.ActionRepositorySetCreate, map[string]any{ + "workspace_id": "ws-improve", "name": "Blocked", "repository_ids": []string{"repo-improve"}, + }}, + {"update", ws.ActionRepositorySetUpdate, map[string]any{ + "id": setID, "name": "Renamed", + }}, + {"delete", ws.ActionRepositorySetDelete, map[string]any{"id": setID}}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + response := dispatchSetAction(t, dispatcher, testCase.action, testCase.payload) + require.Equal(t, ws.MessageTypeError, response.Type, + "%s must be rejected in the read-only workspace: %s", testCase.name, response.Payload) + }) + } + + // The seeded set is untouched, and no new one was created. + recorder := doJSON(t, router, http.MethodGet, "/api/v1/workspaces/ws-improve/repository-sets", nil) + require.Equal(t, http.StatusOK, recorder.Code) + var list struct { + RepositorySets []struct { + ID string `json:"id"` + Name string `json:"name"` + } `json:"repository_sets"` + } + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &list)) + require.Len(t, list.RepositorySets, 1) + require.Equal(t, setID, list.RepositorySets[0].ID) + require.Equal(t, "Existing", list.RepositorySets[0].Name) +} + +func TestWSRepositorySetReadsAreAllowedInReadOnlyWorkspace(t *testing.T) { + _, dispatcher, setID := newImproveWorkspaceRouter(t) + + // Only mutations are blocked; the workspace stays readable. + list := dispatchSetAction(t, dispatcher, ws.ActionRepositorySetList, + map[string]any{"workspace_id": "ws-improve"}) + require.NotEqual(t, ws.MessageTypeError, list.Type, "list failed: %s", list.Payload) + + get := dispatchSetAction(t, dispatcher, ws.ActionRepositorySetGet, map[string]any{"id": setID}) + require.NotEqual(t, ws.MessageTypeError, get.Type, "get failed: %s", get.Payload) +} diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 7df4d86626..cbbf51478a 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -1464,6 +1464,43 @@ type Repository struct { DeletedAt *time.Time `json:"deleted_at,omitempty"` } +// RepositorySet is a named, reusable group of workspace repositories that fills +// the task-creation repository picker in one action. +// +// A set deliberately stores no branch. Branch choice belongs to a task and is +// already modelled on TaskRepository; a branch cached here would go stale +// against the repository's real refs. +type RepositorySet struct { + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` + Name string `json:"name"` + Description string `json:"description"` + // Items is the ordered membership. Slice order is authoritative on write: + // positions are assigned from it, and reads return items sorted by position. + Items []RepositorySetItem `json:"repositories"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// RepositorySetItem is one repository's membership in a set. +type RepositorySetItem struct { + ID string `json:"id"` + RepositorySetID string `json:"repository_set_id"` + RepositoryID string `json:"repository_id"` + Position int `json:"position"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// RepositoryIDs returns the set's membership in position order. +func (s *RepositorySet) RepositoryIDs() []string { + ids := make([]string, 0, len(s.Items)) + for _, item := range s.Items { + ids = append(ids, item.RepositoryID) + } + return ids +} + // ProviderRepositoryIdentity is the durable provider lookup key. New plugin // providers supply Scope + RepositoryID; legacy built-ins use Host/Owner/Name. type ProviderRepositoryIdentity struct { diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index 7bb5634022..f1d87415c2 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -400,6 +400,27 @@ type RepositoryEntityRepository interface { GetRepositoryByLocalPath(ctx context.Context, workspaceID, localPath string) (*models.Repository, error) } +// RepositorySetRepository stores named, reusable groups of workspace +// repositories. Membership order is authoritative: writes assign contiguous +// positions from the supplied order, and reads return items in that order with +// soft-deleted and out-of-workspace repositories excluded. +type RepositorySetRepository interface { + CreateRepositorySet(ctx context.Context, set *models.RepositorySet) error + GetRepositorySet(ctx context.Context, id string) (*models.RepositorySet, error) + // GetRepositorySetByName compares the name case-insensitively and returns + // nil, nil when it is unused, leaving the conflict decision to the caller. + GetRepositorySetByName(ctx context.Context, workspaceID, name string) (*models.RepositorySet, error) + ListRepositorySets(ctx context.Context, workspaceID string) ([]*models.RepositorySet, error) + // ListRepositorySetIDsByRepository reports which sets hold a repository, so a + // caller can publish their new shape after a deletion prunes membership. + ListRepositorySetIDsByRepository(ctx context.Context, repositoryID string) ([]string, error) + // UpdateRepositorySet writes the set's fields and, when repositoryIDs is + // non-nil, replaces its whole membership in the same transaction so the two + // cannot land apart. A nil repositoryIDs leaves membership untouched. + UpdateRepositorySet(ctx context.Context, set *models.RepositorySet, repositoryIDs *[]string) error + DeleteRepositorySet(ctx context.Context, id string) (bool, error) +} + // RepositorySecretBindingRepository stores normalized repository environment // references. It is optional on RepositoryEntityRepository to keep legacy // adapters source-compatible while the SQLite implementation rolls out. diff --git a/apps/backend/internal/task/repository/repoerrors/errors.go b/apps/backend/internal/task/repository/repoerrors/errors.go index 7f0ae180ea..6c50bac6b4 100644 --- a/apps/backend/internal/task/repository/repoerrors/errors.go +++ b/apps/backend/internal/task/repository/repoerrors/errors.go @@ -18,6 +18,9 @@ var ErrTaskPlanNotFound = errors.New("task plan not found") // ErrRepositoryNotFound reports that no live repository row matched the supplied id. var ErrRepositoryNotFound = errors.New("repository not found") +// ErrRepositorySetNotFound reports that no repository set row matched the supplied id. +var ErrRepositorySetNotFound = errors.New("repository set not found") + // ErrTaskEnvironmentNotFound reports that no task environment row matched the supplied id. var ErrTaskEnvironmentNotFound = errors.New("task environment not found") diff --git a/apps/backend/internal/task/repository/sqlite/base_migrations.go b/apps/backend/internal/task/repository/sqlite/base_migrations.go index 558c95556c..00a5dba837 100644 --- a/apps/backend/internal/task/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/task/repository/sqlite/base_migrations.go @@ -128,6 +128,9 @@ func (r *Repository) runMigrations() error { if err := r.ensureTaskWorkspaceFoldersSchema(); err != nil { return err } + if err := r.ensureRepositorySetsSchema(); err != nil { + return err + } r.migrate.Apply("task_sessions.execution_profile_id", `ALTER TABLE task_sessions ADD COLUMN execution_profile_id TEXT NOT NULL DEFAULT ''`) r.migrate.Apply("executors_running.execution_profile_id", `ALTER TABLE executors_running ADD COLUMN execution_profile_id TEXT NOT NULL DEFAULT ''`) r.migrate.Apply("executors_running.last_message_uuid", `ALTER TABLE executors_running ADD COLUMN last_message_uuid TEXT DEFAULT ''`) @@ -582,6 +585,16 @@ func (r *Repository) ensureTaskWorkspaceFoldersSchema() error { return nil } +// ensureRepositorySetsSchema upgrades databases created before repository sets +// existed. It replays the same DDL as the schema-init step; CREATE TABLE/INDEX +// IF NOT EXISTS is replay-safe on SQLite and Postgres. +func (r *Repository) ensureRepositorySetsSchema() error { + if _, err := r.db.Exec(repositorySetsSchemaDDL); err != nil { + return fmt.Errorf("create repository sets schema: %w", err) + } + return nil +} + // recreateTable checks whether tableName's DDL contains triggerPhrase and, if so, // runs statements inside a transaction with FK enforcement disabled. // This is the standard SQLite pattern for dropping columns or FK constraints, diff --git a/apps/backend/internal/task/repository/sqlite/base_schema.go b/apps/backend/internal/task/repository/sqlite/base_schema.go index 9b8b6ce5c4..99452bee10 100644 --- a/apps/backend/internal/task/repository/sqlite/base_schema.go +++ b/apps/backend/internal/task/repository/sqlite/base_schema.go @@ -16,6 +16,7 @@ import ( func (r *Repository) initSchema() error { steps := []func() error{ r.initCoreSchema, + r.initRepositorySetsSchema, r.initPlansSchema, r.initWalkthroughsSchema, r.initDocumentsSchema, @@ -393,6 +394,56 @@ func (r *Repository) initTaskSchema() error { return err } +// repositorySetsSchemaDDL declares the repository-set tables. It runs after +// initCoreSchema so `workspaces` and `repositories` exist for the foreign keys. +// +// Membership positions are contiguous from zero and carry no branch: branch +// choice belongs to a task (task_repositories), which is exactly what the user +// still decides after applying a set. +const repositorySetsSchemaDDL = ` + CREATE TABLE IF NOT EXISTS repository_sets ( + id TEXT PRIMARY KEY, + workspace_id TEXT NOT NULL, + name TEXT NOT NULL, + description TEXT NOT NULL DEFAULT '', + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + FOREIGN KEY (workspace_id) REFERENCES workspaces(id) ON DELETE CASCADE, + UNIQUE(workspace_id, name) + ); + + CREATE TABLE IF NOT EXISTS repository_set_items ( + id TEXT PRIMARY KEY, + repository_set_id TEXT NOT NULL, + repository_id TEXT NOT NULL, + position INTEGER NOT NULL DEFAULT 0, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + FOREIGN KEY (repository_set_id) REFERENCES repository_sets(id) ON DELETE CASCADE, + FOREIGN KEY (repository_id) REFERENCES repositories(id) ON DELETE CASCADE, + UNIQUE(repository_set_id, repository_id) + ); + + CREATE INDEX IF NOT EXISTS idx_repository_sets_workspace_id + ON repository_sets(workspace_id); + -- Names are compared case-insensitively, so the plain UNIQUE(workspace_id, + -- name) above is not the concurrency backstop the service assumes: two + -- concurrent creates of "Full-stack" and "full-stack" would both pass the + -- service's lookup and both insert. An expression index closes that, and + -- LOWER() is available on both SQLite and Postgres. + CREATE UNIQUE INDEX IF NOT EXISTS uniq_repository_sets_workspace_lower_name + ON repository_sets(workspace_id, LOWER(name)); + CREATE INDEX IF NOT EXISTS idx_repository_set_items_set_position + ON repository_set_items(repository_set_id, position); + CREATE INDEX IF NOT EXISTS idx_repository_set_items_repository_id + ON repository_set_items(repository_id); + ` + +func (r *Repository) initRepositorySetsSchema() error { + _, err := r.db.Exec(repositorySetsSchemaDDL) + return err +} + func (r *Repository) initCoreIndexes() error { _, err := r.db.Exec(` CREATE INDEX IF NOT EXISTS idx_tasks_workflow_id ON tasks(workflow_id); diff --git a/apps/backend/internal/task/repository/sqlite/repository_entity.go b/apps/backend/internal/task/repository/sqlite/repository_entity.go index 8c136cca1c..e9221bdc2b 100644 --- a/apps/backend/internal/task/repository/sqlite/repository_entity.go +++ b/apps/backend/internal/task/repository/sqlite/repository_entity.go @@ -130,6 +130,27 @@ func (r *Repository) UpdateRepositoryWithSecretBindings( return tx.Commit() } +// pruneRepositoryDependents removes the rows that must not outlive a deleted +// repository. Repository deletion is *soft* (deleted_at), so declared foreign +// key cascades never fire for it and these rows have to go explicitly. All three +// delete paths call this, so a fourth dependent table cannot be wired into one +// path and forgotten in the others. +func (r *Repository) pruneRepositoryDependents(ctx context.Context, tx *sqlx.Tx, id string) error { + statements := []string{ + `DELETE FROM repository_secret_bindings WHERE repository_id = ?`, + // A membership row pointing at a deleted repository would offer the user + // a repository they can no longer select. Repository sets keep existing + // with their remaining members. + `DELETE FROM repository_set_items WHERE repository_id = ?`, + } + for _, statement := range statements { + if _, err := tx.ExecContext(ctx, r.db.Rebind(statement), id); err != nil { + return err + } + } + return nil +} + // DeleteRepository soft-deletes a repository by ID func (r *Repository) DeleteRepository(ctx context.Context, id string) error { tx, err := r.db.BeginTxx(ctx, nil) @@ -149,7 +170,7 @@ func (r *Repository) DeleteRepository(ctx context.Context, id string) error { if rows == 0 { return fmt.Errorf("repository not found: %s", id) } - if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM repository_secret_bindings WHERE repository_id = ?`), id); err != nil { + if err := r.pruneRepositoryDependents(ctx, tx, id); err != nil { return err } return tx.Commit() @@ -182,7 +203,7 @@ func (r *Repository) DeleteRepositoryIfUnreferenced(ctx context.Context, id stri return false, err } if rows > 0 { - if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM repository_secret_bindings WHERE repository_id = ?`), id); err != nil { + if err := r.pruneRepositoryDependents(ctx, tx, id); err != nil { return false, err } if err := tx.Commit(); err != nil { @@ -227,7 +248,7 @@ func (r *Repository) DeleteRepositoryIfNoActiveTaskSessions(ctx context.Context, return false, err } if rows > 0 { - if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM repository_secret_bindings WHERE repository_id = ?`), id); err != nil { + if err := r.pruneRepositoryDependents(ctx, tx, id); err != nil { return false, err } if err := tx.Commit(); err != nil { diff --git a/apps/backend/internal/task/repository/sqlite/repository_set.go b/apps/backend/internal/task/repository/sqlite/repository_set.go new file mode 100644 index 0000000000..67036e4d52 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/repository_set.go @@ -0,0 +1,342 @@ +package sqlite + +import ( + "context" + "database/sql" + "errors" + "strings" + "time" + + "github.com/google/uuid" + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" +) + +// Repository sets are a named group of workspace repositories used to fill the +// task-creation picker in one action. +// +// Two contracts drive every query in this file: +// +// - Membership order is authoritative and contiguous. Writes assign positions +// from slice order; reads sort by position. +// - Repositories are soft-deleted (repositories.deleted_at), so the foreign +// key cascade never fires for them. Deleting a repository prunes its +// membership rows (see repository_entity.go), and every read here *also* +// filters soft-deleted and out-of-workspace repositories. Either mechanism +// alone would do; both are present so a set can never surface a repository +// the user cannot select. + +const repositorySetColumns = `id, workspace_id, name, description, created_at, updated_at` + +// CreateRepositorySet inserts a set and its ordered membership in one +// transaction. Positions come from set.Items order; item ids are generated when +// empty. The unique constraints on (workspace_id, name) and +// (repository_set_id, repository_id) mean a duplicate name or a repeated +// repository fails the whole insert, leaving no partial row behind. +func (r *Repository) CreateRepositorySet(ctx context.Context, set *models.RepositorySet) error { + if set.ID == "" { + set.ID = uuid.New().String() + } + now := time.Now().UTC() + set.CreatedAt = now + set.UpdatedAt = now + + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + + _, err = tx.ExecContext(ctx, r.db.Rebind(` + INSERT INTO repository_sets (`+repositorySetColumns+`) + VALUES (?, ?, ?, ?, ?, ?) + `), set.ID, set.WorkspaceID, set.Name, set.Description, set.CreatedAt, set.UpdatedAt) + if err != nil { + return err + } + if err := r.insertRepositorySetItems(ctx, tx, set.ID, set.Items, now); err != nil { + return err + } + return tx.Commit() +} + +// insertRepositorySetItems writes membership rows with contiguous positions, +// filling in each item's generated id, owner, position, and timestamps so the +// caller's model matches what the database now holds without a re-read. +func (r *Repository) insertRepositorySetItems( + ctx context.Context, + tx *sqlx.Tx, + setID string, + items []models.RepositorySetItem, + now time.Time, +) error { + for i := range items { + if items[i].ID == "" { + items[i].ID = uuid.New().String() + } + items[i].RepositorySetID = setID + items[i].Position = i + items[i].CreatedAt = now + items[i].UpdatedAt = now + _, err := tx.ExecContext(ctx, r.db.Rebind(` + INSERT INTO repository_set_items ( + id, repository_set_id, repository_id, position, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?) + `), items[i].ID, setID, items[i].RepositoryID, items[i].Position, now, now) + if err != nil { + return err + } + } + return nil +} + +// GetRepositorySet loads one set with its resolved membership. +func (r *Repository) GetRepositorySet(ctx context.Context, id string) (*models.RepositorySet, error) { + set := &models.RepositorySet{} + err := r.ro.QueryRowContext(ctx, r.ro.Rebind(` + SELECT `+repositorySetColumns+` FROM repository_sets WHERE id = ? + `), id).Scan(&set.ID, &set.WorkspaceID, &set.Name, &set.Description, &set.CreatedAt, &set.UpdatedAt) + if errors.Is(err, sql.ErrNoRows) { + return nil, repoerrors.ErrRepositorySetNotFound + } + if err != nil { + return nil, err + } + itemsBySet, err := r.listRepositorySetItems(ctx, []string{set.ID}) + if err != nil { + return nil, err + } + set.Items = itemsBySet[set.ID] + return set, nil +} + +// GetRepositorySetByName finds a set by workspace and name, comparing the name +// case-insensitively. Returns nil, nil when the name is unused: the caller +// decides whether that is a conflict or a miss. +func (r *Repository) GetRepositorySetByName( + ctx context.Context, + workspaceID string, + name string, +) (*models.RepositorySet, error) { + // LOWER() rather than a NOCASE collation: the same statement has to run on + // Postgres, where NOCASE does not exist. + set := &models.RepositorySet{} + err := r.ro.QueryRowContext(ctx, r.ro.Rebind(` + SELECT `+repositorySetColumns+` FROM repository_sets + WHERE workspace_id = ? AND LOWER(name) = ? + `), workspaceID, strings.ToLower(strings.TrimSpace(name))).Scan( + &set.ID, &set.WorkspaceID, &set.Name, &set.Description, &set.CreatedAt, &set.UpdatedAt) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, err + } + itemsBySet, err := r.listRepositorySetItems(ctx, []string{set.ID}) + if err != nil { + return nil, err + } + set.Items = itemsBySet[set.ID] + return set, nil +} + +// ListRepositorySets returns a workspace's sets, newest name-ordered first, with +// membership resolved in one batched query rather than one query per set. +func (r *Repository) ListRepositorySets(ctx context.Context, workspaceID string) ([]*models.RepositorySet, error) { + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(` + SELECT `+repositorySetColumns+` FROM repository_sets + WHERE workspace_id = ? ORDER BY LOWER(name) + `), workspaceID) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + sets := make([]*models.RepositorySet, 0) + ids := make([]string, 0) + for rows.Next() { + set := &models.RepositorySet{} + if err := rows.Scan(&set.ID, &set.WorkspaceID, &set.Name, &set.Description, + &set.CreatedAt, &set.UpdatedAt); err != nil { + return nil, err + } + sets = append(sets, set) + ids = append(ids, set.ID) + } + if err := rows.Err(); err != nil { + return nil, err + } + itemsBySet, err := r.listRepositorySetItems(ctx, ids) + if err != nil { + return nil, err + } + for _, set := range sets { + set.Items = itemsBySet[set.ID] + } + return sets, nil +} + +// listRepositorySetItems resolves membership for several sets at once, excluding +// members whose repository is soft-deleted or has moved out of the set's +// workspace. +func (r *Repository) listRepositorySetItems( + ctx context.Context, + setIDs []string, +) (map[string][]models.RepositorySetItem, error) { + itemsBySet := make(map[string][]models.RepositorySetItem, len(setIDs)) + if len(setIDs) == 0 { + return itemsBySet, nil + } + query, args, err := sqlx.In(` + SELECT i.id, i.repository_set_id, i.repository_id, i.position, i.created_at, i.updated_at + FROM repository_set_items i + INNER JOIN repository_sets s ON s.id = i.repository_set_id + INNER JOIN repositories rep ON rep.id = i.repository_id + WHERE i.repository_set_id IN (?) + AND rep.deleted_at IS NULL + AND rep.workspace_id = s.workspace_id + ORDER BY i.repository_set_id, i.position + `, setIDs) + if err != nil { + return nil, err + } + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(query), args...) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + for rows.Next() { + item := models.RepositorySetItem{} + if err := rows.Scan(&item.ID, &item.RepositorySetID, &item.RepositoryID, + &item.Position, &item.CreatedAt, &item.UpdatedAt); err != nil { + return nil, err + } + itemsBySet[item.RepositorySetID] = append(itemsBySet[item.RepositorySetID], item) + } + return itemsBySet, rows.Err() +} + +// ListRepositorySetIDsByRepository returns the sets that currently hold a +// repository. Callers use it to capture the affected sets *before* a repository +// deletion prunes their membership, so they can publish the post-delete shape. +func (r *Repository) ListRepositorySetIDsByRepository( + ctx context.Context, + repositoryID string, +) ([]string, error) { + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind( + `SELECT repository_set_id FROM repository_set_items WHERE repository_id = ?`), repositoryID) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + ids := make([]string, 0) + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + ids = append(ids, id) + } + return ids, rows.Err() +} + +// UpdateRepositorySet writes the set's own fields and, when repositoryIDs is +// non-nil, replaces its whole membership in the SAME transaction. +// +// The two must commit together: a name change that lands while the membership +// replacement fails leaves the set renamed but still holding the old +// repositories, with the API reporting failure and publishing nothing. A nil +// repositoryIDs leaves membership untouched, which is how an update that only +// changes name or description is expressed. +func (r *Repository) UpdateRepositorySet( + ctx context.Context, + set *models.RepositorySet, + repositoryIDs *[]string, +) error { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + + set.UpdatedAt = time.Now().UTC() + result, err := tx.ExecContext(ctx, r.db.Rebind(` + UPDATE repository_sets SET name = ?, description = ?, updated_at = ? WHERE id = ? + `), set.Name, set.Description, set.UpdatedAt, set.ID) + if err != nil { + return err + } + affected, err := result.RowsAffected() + if err != nil { + return err + } + // A set deleted between the service's read and this write must not be + // resurrected, nor reported as updated. + if affected == 0 { + return repoerrors.ErrRepositorySetNotFound + } + + if repositoryIDs != nil { + if err := r.replaceItemsTx(ctx, tx, set.ID, *repositoryIDs, set.UpdatedAt); err != nil { + return err + } + } + return tx.Commit() +} + +// replaceItemsTx rewrites a set's whole membership inside an existing +// transaction. Positions come out contiguous from zero in the order supplied, +// which is also how reordering is expressed. +func (r *Repository) replaceItemsTx( + ctx context.Context, + tx *sqlx.Tx, + setID string, + repositoryIDs []string, + now time.Time, +) error { + if _, err := tx.ExecContext(ctx, r.db.Rebind( + `DELETE FROM repository_set_items WHERE repository_set_id = ?`), setID); err != nil { + return err + } + items := make([]models.RepositorySetItem, 0, len(repositoryIDs)) + for _, repositoryID := range repositoryIDs { + items = append(items, models.RepositorySetItem{RepositoryID: repositoryID}) + } + return r.insertRepositorySetItems(ctx, tx, setID, items, now) +} + +// DeleteRepositorySet removes a set and, by cascade, its membership rows. It +// reports whether a row was deleted so a repeat delete is not an error. +// Repositories themselves are never touched. +func (r *Repository) DeleteRepositorySet(ctx context.Context, id string) (bool, error) { + // The items cascade is declared on the table, but a database opened without + // foreign keys enforced would leave orphans; deleting explicitly in the same + // transaction makes the outcome independent of that pragma. + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return false, err + } + defer func() { _ = tx.Rollback() }() + + if _, err := tx.ExecContext(ctx, r.db.Rebind( + `DELETE FROM repository_set_items WHERE repository_set_id = ?`), id); err != nil { + return false, err + } + result, err := tx.ExecContext(ctx, r.db.Rebind( + `DELETE FROM repository_sets WHERE id = ?`), id) + if err != nil { + return false, err + } + affected, err := result.RowsAffected() + if err != nil { + return false, err + } + if err := tx.Commit(); err != nil { + return false, err + } + return affected > 0, nil +} diff --git a/apps/backend/internal/task/repository/sqlite/repository_set_postgres_test.go b/apps/backend/internal/task/repository/sqlite/repository_set_postgres_test.go new file mode 100644 index 0000000000..0c90ee9794 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/repository_set_postgres_test.go @@ -0,0 +1,84 @@ +package sqlite + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/testutil" +) + +// TestPostgresRepositorySetRoundTrip is the Postgres counterpart to the SQLite +// repository-set store tests. The set queries use LOWER(name) ordering and +// sqlx.In expansion rather than SQLite-only constructs, and this pins that they +// really do run on Postgres. Skips unless KANDEV_TEST_POSTGRES_DSN is set. +func TestPostgresRepositorySetRoundTrip(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + ctx := context.Background() + + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-pg-sets", Name: "ws-pg-sets"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + for _, id := range []string{"repo-pg-web", "repo-pg-api"} { + if err := repo.CreateRepository(ctx, &models.Repository{ + ID: id, WorkspaceID: "ws-pg-sets", Name: id, SourceType: "local", LocalPath: "/tmp/" + id, + }); err != nil { + t.Fatalf("CreateRepository %s: %v", id, err) + } + } + + set := &models.RepositorySet{ + WorkspaceID: "ws-pg-sets", + Name: "Full-stack", + Description: "web + api", + Items: []models.RepositorySetItem{ + {RepositoryID: "repo-pg-api"}, + {RepositoryID: "repo-pg-web"}, + }, + } + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-pg-api", "repo-pg-web") + + byName, err := repo.GetRepositorySetByName(ctx, "ws-pg-sets", "FULL-stack") + if err != nil { + t.Fatalf("GetRepositorySetByName: %v", err) + } + if byName == nil || byName.ID != set.ID { + t.Fatalf("GetRepositorySetByName = %+v", byName) + } + + pgMembers := []string{"repo-pg-web"} + if err := repo.UpdateRepositorySet(ctx, set, &pgMembers); err != nil { + t.Fatalf("UpdateRepositorySet: %v", err) + } + sets, err := repo.ListRepositorySets(ctx, "ws-pg-sets") + if err != nil { + t.Fatalf("ListRepositorySets: %v", err) + } + if len(sets) != 1 { + t.Fatalf("ListRepositorySets returned %d sets", len(sets)) + } + assertRepositoryIDs(t, repositoryIDsOf(sets[0]), "repo-pg-web") + + if err := repo.DeleteRepository(ctx, "repo-pg-web"); err != nil { + t.Fatalf("DeleteRepository: %v", err) + } + loaded, err = repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet after repository delete: %v", err) + } + if len(loaded.Items) != 0 { + t.Fatalf("membership survived repository deletion: %+v", loaded.Items) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/repository_set_review_test.go b/apps/backend/internal/task/repository/sqlite/repository_set_review_test.go new file mode 100644 index 0000000000..feb1804bdf --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/repository_set_review_test.go @@ -0,0 +1,155 @@ +package sqlite + +import ( + "context" + "errors" + "testing" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" +) + +// Regressions for PR review findings on repository sets: atomic combined +// updates, not-found on a deleted parent, and case-insensitive name uniqueness. + +func TestUpdateRepositorySetRollsBackMetadataWhenMembershipFails(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + // A membership list naming a repository that does not exist fails the item + // insert on its foreign key, after the metadata UPDATE has already run inside + // the same transaction. + renamed := *set + renamed.Name = "Renamed" + renamed.Description = "changed" + members := []string{"repo-web", "repo-does-not-exist"} + if err := repo.UpdateRepositorySet(ctx, &renamed, &members); err == nil { + t.Fatal("UpdateRepositorySet accepted a member that does not exist") + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + // Neither half may survive: a rename that lands while membership fails leaves + // the set renamed but still holding the old repositories. + if loaded.Name != "Full-stack" { + t.Fatalf("name = %q, want the pre-update value", loaded.Name) + } + if loaded.Description != "web + gateway + orders" { + t.Fatalf("description = %q, want the pre-update value", loaded.Description) + } + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-web", "repo-gateway", "repo-orders") +} + +func TestUpdateRepositorySetAppliesBothHalvesTogether(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + set.Name = "Backend" + members := []string{"repo-orders"} + if err := repo.UpdateRepositorySet(ctx, set, &members); err != nil { + t.Fatalf("UpdateRepositorySet: %v", err) + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + if loaded.Name != "Backend" { + t.Fatalf("name = %q", loaded.Name) + } + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-orders") +} + +func TestUpdateRepositorySetOnDeletedSetReportsNotFound(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + if _, err := repo.DeleteRepositorySet(ctx, set.ID); err != nil { + t.Fatalf("DeleteRepositorySet: %v", err) + } + + // An empty membership list touches no item rows, so without checking the + // metadata UPDATE's RowsAffected this used to commit successfully and let the + // service publish an update for a set that no longer exists. + empty := []string{} + if err := repo.UpdateRepositorySet(ctx, set, &empty); !errors.Is(err, repoerrors.ErrRepositorySetNotFound) { + t.Fatalf("UpdateRepositorySet error = %v, want ErrRepositorySetNotFound", err) + } + if err := repo.UpdateRepositorySet(ctx, set, nil); !errors.Is(err, repoerrors.ErrRepositorySetNotFound) { + t.Fatalf("metadata-only update error = %v, want ErrRepositorySetNotFound", err) + } +} + +func TestCreateRepositorySetRejectsNameDifferingOnlyByCase(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + // The service compares names case-insensitively, so the database has to be + // the backstop for two concurrent creates that differ only in case. + duplicate := &models.RepositorySet{ + WorkspaceID: "ws-sets", + Name: "full-STACK", + Items: []models.RepositorySetItem{{RepositoryID: "repo-web"}}, + } + if err := repo.CreateRepositorySet(ctx, duplicate); err == nil { + t.Fatal("CreateRepositorySet accepted a name differing only by case") + } + + sets, err := repo.ListRepositorySets(ctx, "ws-sets") + if err != nil { + t.Fatalf("ListRepositorySets: %v", err) + } + if len(sets) != 1 { + t.Fatalf("rejected duplicate still wrote a row: %d sets", len(sets)) + } +} + +func TestListRepositorySetIDsByRepository(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + other := &models.RepositorySet{ + WorkspaceID: "ws-sets", + Name: "Orders only", + Items: []models.RepositorySetItem{{RepositoryID: "repo-orders"}}, + } + if err := repo.CreateRepositorySet(ctx, other); err != nil { + t.Fatalf("CreateRepositorySet second: %v", err) + } + + ids, err := repo.ListRepositorySetIDsByRepository(ctx, "repo-orders") + if err != nil { + t.Fatalf("ListRepositorySetIDsByRepository: %v", err) + } + if len(ids) != 2 { + t.Fatalf("sets holding repo-orders = %v, want both", ids) + } + + ids, err = repo.ListRepositorySetIDsByRepository(ctx, "repo-web") + if err != nil { + t.Fatalf("ListRepositorySetIDsByRepository: %v", err) + } + if len(ids) != 1 || ids[0] != set.ID { + t.Fatalf("sets holding repo-web = %v, want only %s", ids, set.ID) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/repository_set_test.go b/apps/backend/internal/task/repository/sqlite/repository_set_test.go new file mode 100644 index 0000000000..d4fdbb1f1b --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/repository_set_test.go @@ -0,0 +1,432 @@ +package sqlite + +import ( + "context" + "errors" + "path/filepath" + "testing" + + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/db" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" +) + +func newRepoForSetTests(t *testing.T) *Repository { + t.Helper() + dbPath := filepath.Join(t.TempDir(), "repository-set-test.db") + dbConn, err := db.OpenSQLite(dbPath) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + sqlxDB := sqlx.NewDb(dbConn, "sqlite3") + repo, err := NewWithDB(sqlxDB, sqlxDB, nil) + if err != nil { + t.Fatalf("new repo: %v", err) + } + t.Cleanup(func() { _ = sqlxDB.Close() }) + return repo +} + +func seedSetRepository(t *testing.T, repo *Repository, workspaceID, id string) { + t.Helper() + err := repo.CreateRepository(context.Background(), &models.Repository{ + ID: id, + WorkspaceID: workspaceID, + Name: id, + SourceType: "local", + LocalPath: "/tmp/" + id, + }) + if err != nil { + t.Fatalf("seed repository %s: %v", id, err) + } +} + +// setFixture seeds one workspace with three repositories and returns a set +// carrying all three in a deliberate non-alphabetical order, so a test that +// asserts ordering cannot pass by accident. +func setFixture(t *testing.T, repo *Repository) *models.RepositorySet { + t.Helper() + seedWorkspace(t, repo, "ws-sets") + seedSetRepository(t, repo, "ws-sets", "repo-web") + seedSetRepository(t, repo, "ws-sets", "repo-gateway") + seedSetRepository(t, repo, "ws-sets", "repo-orders") + return &models.RepositorySet{ + WorkspaceID: "ws-sets", + Name: "Full-stack", + Description: "web + gateway + orders", + Items: []models.RepositorySetItem{ + {RepositoryID: "repo-web"}, + {RepositoryID: "repo-gateway"}, + {RepositoryID: "repo-orders"}, + }, + } +} + +func repositoryIDsOf(set *models.RepositorySet) []string { + ids := make([]string, 0, len(set.Items)) + for _, item := range set.Items { + ids = append(ids, item.RepositoryID) + } + return ids +} + +func assertRepositoryIDs(t *testing.T, got []string, want ...string) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("repository ids = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("repository ids = %v, want %v", got, want) + } + } +} + +func TestCreateRepositorySetPersistsEveryFieldAndOrder(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + if set.ID == "" { + t.Fatal("CreateRepositorySet left the id empty") + } + if set.CreatedAt.IsZero() || set.UpdatedAt.IsZero() { + t.Fatal("CreateRepositorySet left a timestamp zero") + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + if loaded.WorkspaceID != "ws-sets" || loaded.Name != "Full-stack" { + t.Fatalf("loaded set = %+v", loaded) + } + if loaded.Description != "web + gateway + orders" { + t.Fatalf("description = %q", loaded.Description) + } + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-web", "repo-gateway", "repo-orders") + for i, item := range loaded.Items { + if item.Position != i { + t.Fatalf("item %d position = %d", i, item.Position) + } + if item.ID == "" || item.RepositorySetID != set.ID { + t.Fatalf("item %d = %+v", i, item) + } + } +} + +func TestGetRepositorySetMissingReportsNotFound(t *testing.T) { + repo := newRepoForSetTests(t) + if _, err := repo.GetRepositorySet(context.Background(), "nope"); !errors.Is(err, repoerrors.ErrRepositorySetNotFound) { + t.Fatalf("GetRepositorySet error = %v, want ErrRepositorySetNotFound", err) + } +} + +func TestCreateRepositorySetRejectsDuplicateNameInWorkspace(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + duplicate := &models.RepositorySet{ + WorkspaceID: "ws-sets", + Name: "Full-stack", + Items: []models.RepositorySetItem{{RepositoryID: "repo-web"}}, + } + if err := repo.CreateRepositorySet(ctx, duplicate); err == nil { + t.Fatal("CreateRepositorySet accepted a duplicate name") + } + + sets, err := repo.ListRepositorySets(ctx, "ws-sets") + if err != nil { + t.Fatalf("ListRepositorySets: %v", err) + } + if len(sets) != 1 { + t.Fatalf("rejected duplicate still wrote a row: %d sets", len(sets)) + } +} + +func TestCreateRepositorySetAllowsSameNameInAnotherWorkspace(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + seedWorkspace(t, repo, "ws-other") + seedSetRepository(t, repo, "ws-other", "repo-other") + other := &models.RepositorySet{ + WorkspaceID: "ws-other", + Name: "Full-stack", + Items: []models.RepositorySetItem{{RepositoryID: "repo-other"}}, + } + if err := repo.CreateRepositorySet(ctx, other); err != nil { + t.Fatalf("CreateRepositorySet in second workspace: %v", err) + } +} + +func TestCreateRepositorySetRejectsRepeatedRepository(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "ws-sets") + seedSetRepository(t, repo, "ws-sets", "repo-web") + + set := &models.RepositorySet{ + WorkspaceID: "ws-sets", + Name: "Twice", + Items: []models.RepositorySetItem{ + {RepositoryID: "repo-web"}, + {RepositoryID: "repo-web"}, + }, + } + if err := repo.CreateRepositorySet(ctx, set); err == nil { + t.Fatal("CreateRepositorySet accepted the same repository twice") + } + sets, err := repo.ListRepositorySets(ctx, "ws-sets") + if err != nil { + t.Fatalf("ListRepositorySets: %v", err) + } + if len(sets) != 0 { + t.Fatalf("rejected set still wrote a row: %d sets", len(sets)) + } +} + +func TestReplaceRepositorySetItemsRewritesOrderContiguously(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + // Reorder and drop one member in a single replace. + members := []string{"repo-orders", "repo-web"} + if err := repo.UpdateRepositorySet(ctx, set, &members); err != nil { + t.Fatalf("UpdateRepositorySet: %v", err) + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-orders", "repo-web") + for i, item := range loaded.Items { + if item.Position != i { + t.Fatalf("after replace, item %d position = %d", i, item.Position) + } + } +} + +func TestGetRepositorySetExcludesSoftDeletedRepository(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + // Bypass the delete path that also prunes memberships, so this asserts the + // read filter on its own rather than the transaction's cleanup. + _, err := repo.db.Exec(repo.db.Rebind( + `UPDATE repositories SET deleted_at = CURRENT_TIMESTAMP WHERE id = ?`), "repo-gateway") + if err != nil { + t.Fatalf("soft-delete repository: %v", err) + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-web", "repo-orders") +} + +func TestDeleteRepositoryPrunesSetMembership(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + if err := repo.DeleteRepository(ctx, "repo-gateway"); err != nil { + t.Fatalf("DeleteRepository: %v", err) + } + + var count int + err := repo.db.Get(&count, repo.db.Rebind( + `SELECT COUNT(*) FROM repository_set_items WHERE repository_id = ?`), "repo-gateway") + if err != nil { + t.Fatalf("count memberships: %v", err) + } + if count != 0 { + t.Fatalf("membership rows survived repository deletion: %d", count) + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet after repository delete: %v", err) + } + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-web", "repo-orders") +} + +func TestDeleteRepositorySetRemovesItemsAndKeepsRepositories(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + deleted, err := repo.DeleteRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("DeleteRepositorySet: %v", err) + } + if !deleted { + t.Fatal("DeleteRepositorySet reported no deletion") + } + + var items int + if err := repo.db.Get(&items, repo.db.Rebind( + `SELECT COUNT(*) FROM repository_set_items WHERE repository_set_id = ?`), set.ID); err != nil { + t.Fatalf("count items: %v", err) + } + if items != 0 { + t.Fatalf("items survived set deletion: %d", items) + } + if _, err := repo.GetRepository(ctx, "repo-web"); err != nil { + t.Fatalf("set deletion touched a repository: %v", err) + } + + deleted, err = repo.DeleteRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("second DeleteRepositorySet: %v", err) + } + if deleted { + t.Fatal("DeleteRepositorySet reported a second deletion") + } +} + +func TestUpdateRepositorySetChangesNameAndDescription(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + set.Name = "Backend only" + set.Description = "gateway + orders" + if err := repo.UpdateRepositorySet(ctx, set, nil); err != nil { + t.Fatalf("UpdateRepositorySet: %v", err) + } + + loaded, err := repo.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + if loaded.Name != "Backend only" || loaded.Description != "gateway + orders" { + t.Fatalf("loaded set = %+v", loaded) + } + // An update that does not supply members must leave them untouched. + assertRepositoryIDs(t, repositoryIDsOf(loaded), "repo-web", "repo-gateway", "repo-orders") +} + +func TestGetRepositorySetByNameIsCaseInsensitiveWithinWorkspace(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + found, err := repo.GetRepositorySetByName(ctx, "ws-sets", "full-STACK") + if err != nil { + t.Fatalf("GetRepositorySetByName: %v", err) + } + if found == nil || found.ID != set.ID { + t.Fatalf("GetRepositorySetByName = %+v, want set %s", found, set.ID) + } + + missing, err := repo.GetRepositorySetByName(ctx, "ws-sets", "nothing") + if err != nil { + t.Fatalf("GetRepositorySetByName miss: %v", err) + } + if missing != nil { + t.Fatalf("GetRepositorySetByName returned %+v for an unused name", missing) + } +} + +func TestListRepositorySetsScopesToWorkspaceAndResolvesItems(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + second := &models.RepositorySet{ + WorkspaceID: "ws-sets", + Name: "Backend", + Items: []models.RepositorySetItem{ + {RepositoryID: "repo-gateway"}, + {RepositoryID: "repo-orders"}, + }, + } + if err := repo.CreateRepositorySet(ctx, second); err != nil { + t.Fatalf("CreateRepositorySet second: %v", err) + } + seedWorkspace(t, repo, "ws-other") + seedSetRepository(t, repo, "ws-other", "repo-other") + if err := repo.CreateRepositorySet(ctx, &models.RepositorySet{ + WorkspaceID: "ws-other", + Name: "Other", + Items: []models.RepositorySetItem{{RepositoryID: "repo-other"}}, + }); err != nil { + t.Fatalf("CreateRepositorySet other workspace: %v", err) + } + + sets, err := repo.ListRepositorySets(ctx, "ws-sets") + if err != nil { + t.Fatalf("ListRepositorySets: %v", err) + } + if len(sets) != 2 { + t.Fatalf("ListRepositorySets returned %d sets, want 2", len(sets)) + } + byName := map[string][]string{} + for _, entry := range sets { + byName[entry.Name] = repositoryIDsOf(entry) + } + assertRepositoryIDs(t, byName["Full-stack"], "repo-web", "repo-gateway", "repo-orders") + assertRepositoryIDs(t, byName["Backend"], "repo-gateway", "repo-orders") +} + +func TestDeleteWorkspaceCascadesRepositorySets(t *testing.T) { + repo := newRepoForSetTests(t) + ctx := context.Background() + set := setFixture(t, repo) + if err := repo.CreateRepositorySet(ctx, set); err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + + if err := repo.DeleteWorkspace(ctx, "ws-sets"); err != nil { + t.Fatalf("DeleteWorkspace: %v", err) + } + + var sets, items int + if err := repo.db.Get(&sets, `SELECT COUNT(*) FROM repository_sets`); err != nil { + t.Fatalf("count sets: %v", err) + } + if err := repo.db.Get(&items, `SELECT COUNT(*) FROM repository_set_items`); err != nil { + t.Fatalf("count items: %v", err) + } + if sets != 0 || items != 0 { + t.Fatalf("workspace deletion left %d sets and %d items", sets, items) + } +} diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index 93021f1761..a42de0b45e 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -271,6 +271,7 @@ type Repos struct { Sessions repository.SessionRepository GitSnapshots repository.GitSnapshotRepository RepoEntities repository.RepositoryEntityRepository + RepositorySets repository.RepositorySetRepository RepositoryCleanup repository.RepositoryCleanupRepository Executors repository.ExecutorRepository Environments repository.EnvironmentRepository @@ -295,6 +296,7 @@ type Service struct { sessions repository.SessionRepository gitSnapshots repository.GitSnapshotRepository repoEntities repository.RepositoryEntityRepository + repositorySets repository.RepositorySetRepository repositoryCleanup repository.RepositoryCleanupRepository executors repository.ExecutorRepository environments repository.EnvironmentRepository @@ -435,6 +437,7 @@ func NewService(repos Repos, eventBus bus.EventBus, log *logger.Logger, discover sessions: repos.Sessions, gitSnapshots: repos.GitSnapshots, repoEntities: repos.RepoEntities, + repositorySets: repos.RepositorySets, repositoryCleanup: repos.RepositoryCleanup, executors: repos.Executors, environments: repos.Environments, diff --git a/apps/backend/internal/task/service/service_repository_sets.go b/apps/backend/internal/task/service/service_repository_sets.go new file mode 100644 index 0000000000..6a5450f5d9 --- /dev/null +++ b/apps/backend/internal/task/service/service_repository_sets.go @@ -0,0 +1,338 @@ +package service + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" +) + +// Repository sets are named, reusable groups of workspace repositories that fill +// the task-creation picker in one action. They live here rather than in +// service_resources.go, which is already at its size limit. + +const repositorySetNameMaxLength = 100 + +var ( + // ErrInvalidRepositorySet marks a malformed request: a blank or overlong + // name, no members, or a repeated member. Maps to 400. + ErrInvalidRepositorySet = errors.New("invalid repository set") + // ErrRepositorySetNameConflict marks a name already used in the workspace, + // compared case-insensitively. Maps to 409. + ErrRepositorySetNameConflict = errors.New("repository set name already used") + // ErrUnknownRepositorySetMembers marks members that do not resolve to a live + // repository in the set's workspace. Maps to 422. A cross-workspace id is + // reported the same way as a nonexistent one, so membership validation does + // not reveal repositories the caller cannot see. + ErrUnknownRepositorySetMembers = errors.New("unknown repository set members") +) + +// CreateRepositorySetRequest creates a set. RepositoryIDs is ordered and defines +// the apply order; it carries no branch, because branch choice belongs to a task. +type CreateRepositorySetRequest struct { + WorkspaceID string `json:"workspace_id"` + Name string `json:"name"` + Description string `json:"description"` + RepositoryIDs []string `json:"repository_ids"` +} + +// UpdateRepositorySetRequest patches a set. Every field is optional; an absent +// field is left alone. A supplied RepositoryIDs replaces the whole membership +// list, which is also how reordering is expressed. +type UpdateRepositorySetRequest struct { + Name *string `json:"name"` + Description *string `json:"description"` + RepositoryIDs *[]string `json:"repository_ids"` +} + +// CreateRepositorySet validates the whole request, then writes the set and its +// membership together. Nothing is written when any check fails. +func (s *Service) CreateRepositorySet( + ctx context.Context, + req *CreateRepositorySetRequest, +) (*models.RepositorySet, error) { + if err := s.authorizeWorkspaceID(ctx, req.WorkspaceID); err != nil { + return nil, err + } + name, err := validateRepositorySetName(req.Name) + if err != nil { + return nil, err + } + if err := s.assertRepositorySetNameFree(ctx, req.WorkspaceID, name, ""); err != nil { + return nil, err + } + if err := s.validateRepositorySetMembers(ctx, req.WorkspaceID, req.RepositoryIDs); err != nil { + return nil, err + } + set := &models.RepositorySet{ + WorkspaceID: req.WorkspaceID, + Name: name, + Description: strings.TrimSpace(req.Description), + Items: repositorySetItemsFor(req.RepositoryIDs), + } + if err := s.repositorySets.CreateRepositorySet(ctx, set); err != nil { + return nil, err + } + s.publishRepositorySetEvent(ctx, events.RepositorySetCreated, set) + return set, nil +} + +// GetRepositorySet loads one set, authorizing against the workspace that owns it. +func (s *Service) GetRepositorySet(ctx context.Context, id string) (*models.RepositorySet, error) { + set, err := s.repositorySets.GetRepositorySet(ctx, id) + if err != nil { + return nil, err + } + if err := s.authorizeWorkspaceID(ctx, set.WorkspaceID); err != nil { + // A set in a workspace the caller cannot see is indistinguishable from + // one that does not exist. + return nil, repoerrors.ErrRepositorySetNotFound + } + return set, nil +} + +// ListRepositorySets returns a workspace's sets with membership resolved. +func (s *Service) ListRepositorySets(ctx context.Context, workspaceID string) ([]*models.RepositorySet, error) { + if err := s.authorizeWorkspaceID(ctx, workspaceID); err != nil { + return nil, err + } + return s.repositorySets.ListRepositorySets(ctx, workspaceID) +} + +// UpdateRepositorySet applies name, description, and membership changes +// together. It validates everything before the first write, so a rejected +// request leaves the set exactly as it was. +func (s *Service) UpdateRepositorySet( + ctx context.Context, + id string, + req *UpdateRepositorySetRequest, +) (*models.RepositorySet, error) { + set, err := s.GetRepositorySet(ctx, id) + if err != nil { + return nil, err + } + if err := s.prepareRepositorySetUpdate(ctx, set, req); err != nil { + return nil, err + } + // One call, one transaction: a rename that lands while the membership + // replacement fails would leave the set renamed but still holding the old + // repositories, with this method reporting failure and publishing nothing. + if err := s.repositorySets.UpdateRepositorySet(ctx, set, req.RepositoryIDs); err != nil { + return nil, err + } + updated, err := s.repositorySets.GetRepositorySet(ctx, set.ID) + if err != nil { + return nil, err + } + s.publishRepositorySetEvent(ctx, events.RepositorySetUpdated, updated) + return updated, nil +} + +// prepareRepositorySetUpdate validates the patch and folds the accepted values +// into set, leaving the caller to persist them. +func (s *Service) prepareRepositorySetUpdate( + ctx context.Context, + set *models.RepositorySet, + req *UpdateRepositorySetRequest, +) error { + if req.Name != nil { + name, err := validateRepositorySetName(*req.Name) + if err != nil { + return err + } + if err := s.assertRepositorySetNameFree(ctx, set.WorkspaceID, name, set.ID); err != nil { + return err + } + set.Name = name + } + if req.Description != nil { + set.Description = strings.TrimSpace(*req.Description) + } + if req.RepositoryIDs != nil { + if err := s.validateRepositorySetMembers(ctx, set.WorkspaceID, *req.RepositoryIDs); err != nil { + return err + } + } + return nil +} + +// DeleteRepositorySet removes a set and its membership. Repositories are never +// touched. +func (s *Service) DeleteRepositorySet(ctx context.Context, id string) error { + set, err := s.GetRepositorySet(ctx, id) + if err != nil { + return err + } + deleted, err := s.repositorySets.DeleteRepositorySet(ctx, id) + if err != nil { + return err + } + if !deleted { + return repoerrors.ErrRepositorySetNotFound + } + s.publishRepositorySetEvent(ctx, events.RepositorySetDeleted, set) + return nil +} + +// repositorySetIDsHolding reports which sets currently hold a repository. A +// lookup failure is logged and treated as "none": it must not block the +// repository deletion the caller is performing. +func (s *Service) repositorySetIDsHolding(ctx context.Context, repositoryID string) []string { + if s.repositorySets == nil { + return nil + } + ids, err := s.repositorySets.ListRepositorySetIDsByRepository(ctx, repositoryID) + if err != nil { + s.logger.Warn("failed to list repository sets holding repository", + zap.String("repository_id", repositoryID), zap.Error(err)) + return nil + } + return ids +} + +// publishRepositorySetsAfterMembershipChange re-reads each affected set and +// publishes its post-change shape, so clients that only react to +// repository_set.* events converge without a reload. +func (s *Service) publishRepositorySetsAfterMembershipChange(ctx context.Context, setIDs []string) { + for _, setID := range setIDs { + set, err := s.repositorySets.GetRepositorySet(ctx, setID) + if err != nil { + // A set deleted concurrently has its own deleted event; nothing to say. + continue + } + s.publishRepositorySetEvent(ctx, events.RepositorySetUpdated, set) + } +} + +// validateRepositorySetName trims and bounds a set name. +func validateRepositorySetName(raw string) (string, error) { + name := strings.TrimSpace(raw) + if name == "" { + return "", fmt.Errorf("%w: name is required", ErrInvalidRepositorySet) + } + // Count runes, not bytes: a 100-character name of non-ASCII text is still a + // 100-character name to the user typing it. + if len([]rune(name)) > repositorySetNameMaxLength { + return "", fmt.Errorf("%w: name must be at most %d characters", + ErrInvalidRepositorySet, repositorySetNameMaxLength) + } + return name, nil +} + +// assertRepositorySetNameFree rejects a name already used in the workspace, +// naming the holder so the client can point at it. exceptID lets a set keep its +// own name across an update. The database's unique index on +// (workspace_id, LOWER(name)) is the backstop for two concurrent creates racing +// past this check, including two that differ only in case. +func (s *Service) assertRepositorySetNameFree( + ctx context.Context, + workspaceID string, + name string, + exceptID string, +) error { + existing, err := s.repositorySets.GetRepositorySetByName(ctx, workspaceID, name) + if err != nil { + return err + } + if existing == nil || existing.ID == exceptID { + return nil + } + return fmt.Errorf("%w: %q already uses this name", ErrRepositorySetNameConflict, existing.Name) +} + +// validateRepositorySetMembers requires at least one member, no repeats, and +// every id resolving to a live repository in the set's workspace. +func (s *Service) validateRepositorySetMembers( + ctx context.Context, + workspaceID string, + repositoryIDs []string, +) error { + if len(repositoryIDs) == 0 { + return fmt.Errorf("%w: at least one repository is required", ErrInvalidRepositorySet) + } + seen := make(map[string]struct{}, len(repositoryIDs)) + for _, id := range repositoryIDs { + if strings.TrimSpace(id) == "" { + return fmt.Errorf("%w: repository ids must not be blank", ErrInvalidRepositorySet) + } + if _, repeated := seen[id]; repeated { + return fmt.Errorf("%w: repository %q listed more than once", ErrInvalidRepositorySet, id) + } + seen[id] = struct{}{} + } + // One workspace listing rather than a lookup per id, and it already excludes + // soft-deleted rows and other workspaces. + live, err := s.repoEntities.ListRepositories(ctx, workspaceID) + if err != nil { + return err + } + available := make(map[string]struct{}, len(live)) + for _, repository := range live { + if repository != nil { + available[repository.ID] = struct{}{} + } + } + unknown := make([]string, 0) + for _, id := range repositoryIDs { + if _, ok := available[id]; !ok { + unknown = append(unknown, id) + } + } + if len(unknown) > 0 { + return fmt.Errorf("%w: %s", ErrUnknownRepositorySetMembers, strings.Join(unknown, ", ")) + } + return nil +} + +// repositorySetItemsFor turns an ordered id list into membership models. The +// store assigns positions from slice order. +func repositorySetItemsFor(repositoryIDs []string) []models.RepositorySetItem { + items := make([]models.RepositorySetItem, 0, len(repositoryIDs)) + for _, id := range repositoryIDs { + items = append(items, models.RepositorySetItem{RepositoryID: id}) + } + return items +} + +// publishRepositorySetEvent carries id and workspace_id so the gateway's +// existing workspace routing delivers the event without a new routing branch. +func (s *Service) publishRepositorySetEvent( + ctx context.Context, + eventType string, + set *models.RepositorySet, +) { + if s.eventBus == nil || set == nil { + return + } + repositories := make([]map[string]interface{}, 0, len(set.Items)) + for _, item := range set.Items { + repositories = append(repositories, map[string]interface{}{ + "repository_id": item.RepositoryID, + "position": item.Position, + }) + } + data := map[string]interface{}{ + "id": set.ID, + "workspace_id": set.WorkspaceID, + "name": set.Name, + "description": set.Description, + "repositories": repositories, + "created_at": set.CreatedAt.Format(time.RFC3339), + "updated_at": set.UpdatedAt.Format(time.RFC3339), + } + event := bus.NewEvent(eventType, "task-service", data) + if err := s.eventBus.Publish(ctx, eventType, event); err != nil { + s.logger.Error("failed to publish repository set event", + zap.String("event_type", eventType), + zap.String("repository_set_id", set.ID), + zap.Error(err)) + } +} diff --git a/apps/backend/internal/task/service/service_repository_sets_review_test.go b/apps/backend/internal/task/service/service_repository_sets_review_test.go new file mode 100644 index 0000000000..dc80546119 --- /dev/null +++ b/apps/backend/internal/task/service/service_repository_sets_review_test.go @@ -0,0 +1,124 @@ +package service + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +// Regressions for PR review findings on the repository-set service. + +func TestDeletingARepositoryPublishesTheSetsThatHeldIt(t *testing.T) { + svc, eventBus, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + ctx := context.Background() + + fullStack := createFullStackSet(t, svc) + ordersOnly, err := svc.CreateRepositorySet(ctx, &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: "Orders only", + RepositoryIDs: []string{"repo-orders"}, + }) + if err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + eventBus.ClearEvents() + + if err := svc.DeleteRepository(ctx, "repo-gateway"); err != nil { + t.Fatalf("DeleteRepository: %v", err) + } + + // A client that already loaded the workspace only reacts to repository_set.* + // events, so the pruned set has to be republished or it keeps offering the + // deleted member until reload. + var updatedIDs []string + for _, event := range eventBus.GetPublishedEvents() { + if event.Type != "repository_set.updated" { + continue + } + data, ok := event.Data.(map[string]interface{}) + if !ok { + t.Fatalf("event data = %#v", event.Data) + } + id, _ := data["id"].(string) + updatedIDs = append(updatedIDs, id) + } + if len(updatedIDs) != 1 || updatedIDs[0] != fullStack.ID { + t.Fatalf("republished sets = %v, want only %s", updatedIDs, fullStack.ID) + } + + // The set that never held the repository is untouched. + unaffected, err := svc.GetRepositorySet(ctx, ordersOnly.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + if len(unaffected.Items) != 1 { + t.Fatalf("unaffected set membership = %+v", unaffected.Items) + } +} + +func TestDeletingARepositoryInNoSetPublishesNoSetUpdate(t *testing.T) { + svc, eventBus, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + ctx := context.Background() + createFullStackSet(t, svc) + eventBus.ClearEvents() + + // repo-orders is in no set, so nothing about sets changed. + if err := svc.DeleteRepository(ctx, "repo-orders"); err != nil { + t.Fatalf("DeleteRepository: %v", err) + } + for _, event := range eventBus.GetPublishedEvents() { + if event.Type == "repository_set.updated" { + t.Fatalf("published a set update for a repository in no set: %#v", event.Data) + } + } +} + +func TestUpdateRepositorySetRejectsMemberAndLeavesMetadataAlone(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + ctx := context.Background() + set := createFullStackSet(t, svc) + + // Validation runs before the write, so neither half lands. + name := "Renamed" + bad := []string{"repo-web", "repo-missing"} + if _, err := svc.UpdateRepositorySet(ctx, set.ID, &UpdateRepositorySetRequest{ + Name: &name, + RepositoryIDs: &bad, + }); err == nil { + t.Fatal("UpdateRepositorySet accepted an unknown member") + } + + loaded, err := svc.GetRepositorySet(ctx, set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + if loaded.Name != "Full-stack" { + t.Fatalf("name = %q, want the pre-update value", loaded.Name) + } + if len(loaded.Items) != 2 { + t.Fatalf("membership = %+v, want unchanged", loaded.Items) + } +} + +func TestCreateRepositorySetSurvivesAWorkspaceWithNoSetStore(t *testing.T) { + // repositorySetIDsHolding must tolerate a service built without the set + // store, because repository deletion runs on every deployment path. + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-bare", Name: "Bare"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-bare", WorkspaceID: "ws-bare", Name: "bare", SourceType: sourceTypeLocal, + LocalPath: t.TempDir(), + }); err != nil { + t.Fatalf("CreateRepository: %v", err) + } + if err := svc.DeleteRepository(ctx, "repo-bare"); err != nil { + t.Fatalf("DeleteRepository: %v", err) + } +} diff --git a/apps/backend/internal/task/service/service_repository_sets_test.go b/apps/backend/internal/task/service/service_repository_sets_test.go new file mode 100644 index 0000000000..86b26e837e --- /dev/null +++ b/apps/backend/internal/task/service/service_repository_sets_test.go @@ -0,0 +1,373 @@ +package service + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" +) + +// seedSetWorkspace creates one workspace with three repositories, the fixture +// every repository-set service test builds on. +func seedSetWorkspace(t *testing.T, svc *Service, repo interface { + CreateWorkspace(context.Context, *models.Workspace) error + CreateRepository(context.Context, *models.Repository) error +}) { + t.Helper() + _ = svc + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + for _, id := range []string{"repo-web", "repo-gateway", "repo-orders"} { + if err := repo.CreateRepository(ctx, &models.Repository{ + ID: id, WorkspaceID: "ws-1", Name: id, SourceType: sourceTypeLocal, LocalPath: t.TempDir(), + }); err != nil { + t.Fatalf("CreateRepository %s: %v", id, err) + } + } +} + +func createFullStackSet(t *testing.T, svc *Service) *models.RepositorySet { + t.Helper() + set, err := svc.CreateRepositorySet(context.Background(), &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: "Full-stack", + Description: "web + gateway", + RepositoryIDs: []string{"repo-web", "repo-gateway"}, + }) + if err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + return set +} + +func TestCreateRepositorySetPersistsAndPublishes(t *testing.T) { + svc, eventBus, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + + set := createFullStackSet(t, svc) + + if set.ID == "" || set.WorkspaceID != "ws-1" || set.Name != "Full-stack" { + t.Fatalf("created set = %+v", set) + } + if got := set.RepositoryIDs(); len(got) != 2 || got[0] != "repo-web" || got[1] != "repo-gateway" { + t.Fatalf("membership = %v", got) + } + + events := eventBus.GetPublishedEvents() + if len(events) != 1 || events[0].Type != "repository_set.created" { + t.Fatalf("published events = %#v, want one repository_set.created", events) + } + data, ok := events[0].Data.(map[string]interface{}) + if !ok { + t.Fatalf("event data = %#v", events[0].Data) + } + if data["workspace_id"] != "ws-1" || data["id"] != set.ID { + t.Fatalf("event data = %#v, want id + workspace_id for workspace routing", data) + } +} + +func TestCreateRepositorySetTrimsNameAndRejectsBlank(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + ctx := context.Background() + + set, err := svc.CreateRepositorySet(ctx, &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: " Full-stack ", + RepositoryIDs: []string{"repo-web"}, + }) + if err != nil { + t.Fatalf("CreateRepositorySet: %v", err) + } + if set.Name != "Full-stack" { + t.Fatalf("name = %q, want trimmed", set.Name) + } + + _, err = svc.CreateRepositorySet(ctx, &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: " ", + RepositoryIDs: []string{"repo-web"}, + }) + if !errors.Is(err, ErrInvalidRepositorySet) { + t.Fatalf("blank name error = %v, want ErrInvalidRepositorySet", err) + } +} + +func TestCreateRepositorySetRejectsOverlongName(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + + _, err := svc.CreateRepositorySet(context.Background(), &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: strings.Repeat("x", 101), + RepositoryIDs: []string{"repo-web"}, + }) + if !errors.Is(err, ErrInvalidRepositorySet) { + t.Fatalf("overlong name error = %v, want ErrInvalidRepositorySet", err) + } + + // The boundary itself is allowed. + if _, err := svc.CreateRepositorySet(context.Background(), &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: strings.Repeat("x", 100), + RepositoryIDs: []string{"repo-web"}, + }); err != nil { + t.Fatalf("100-character name rejected: %v", err) + } +} + +func TestCreateRepositorySetRequiresAtLeastOneRepository(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + + _, err := svc.CreateRepositorySet(context.Background(), &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: "Empty", + }) + if !errors.Is(err, ErrInvalidRepositorySet) { + t.Fatalf("empty membership error = %v, want ErrInvalidRepositorySet", err) + } +} + +func TestCreateRepositorySetRejectsDuplicateRepositoryID(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + + _, err := svc.CreateRepositorySet(context.Background(), &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: "Twice", + RepositoryIDs: []string{"repo-web", "repo-web"}, + }) + if !errors.Is(err, ErrInvalidRepositorySet) { + t.Fatalf("duplicate id error = %v, want ErrInvalidRepositorySet", err) + } +} + +func TestCreateRepositorySetConflictNamesExistingSet(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + createFullStackSet(t, svc) + + _, err := svc.CreateRepositorySet(context.Background(), &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: "full-STACK", + RepositoryIDs: []string{"repo-orders"}, + }) + if !errors.Is(err, ErrRepositorySetNameConflict) { + t.Fatalf("conflict error = %v, want ErrRepositorySetNameConflict", err) + } + if !strings.Contains(err.Error(), "Full-stack") { + t.Fatalf("conflict error %q does not name the existing set", err) + } +} + +func TestCreateRepositorySetRejectsUnknownAndForeignRepositories(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-2", Name: "Other"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-foreign", WorkspaceID: "ws-2", Name: "foreign", SourceType: sourceTypeLocal, + LocalPath: t.TempDir(), + }); err != nil { + t.Fatalf("CreateRepository: %v", err) + } + + _, err := svc.CreateRepositorySet(ctx, &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: "Mixed", + RepositoryIDs: []string{"repo-web", "repo-foreign", "repo-missing"}, + }) + if !errors.Is(err, ErrUnknownRepositorySetMembers) { + t.Fatalf("member error = %v, want ErrUnknownRepositorySetMembers", err) + } + // Both offenders are named so the client can point at the right rows, and + // the cross-workspace id is not distinguished from a nonexistent one. + if !strings.Contains(err.Error(), "repo-foreign") || !strings.Contains(err.Error(), "repo-missing") { + t.Fatalf("member error %q does not list the offending ids", err) + } + + sets, err := svc.ListRepositorySets(ctx, "ws-1") + if err != nil { + t.Fatalf("ListRepositorySets: %v", err) + } + if len(sets) != 0 { + t.Fatalf("rejected create still wrote a set: %d", len(sets)) + } +} + +func TestCreateRepositorySetRejectsSoftDeletedRepository(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + ctx := context.Background() + if err := repo.DeleteRepository(ctx, "repo-gateway"); err != nil { + t.Fatalf("DeleteRepository: %v", err) + } + + _, err := svc.CreateRepositorySet(ctx, &CreateRepositorySetRequest{ + WorkspaceID: "ws-1", + Name: "Stale", + RepositoryIDs: []string{"repo-web", "repo-gateway"}, + }) + if !errors.Is(err, ErrUnknownRepositorySetMembers) { + t.Fatalf("deleted member error = %v, want ErrUnknownRepositorySetMembers", err) + } +} + +func TestUpdateRepositorySetReplacesMembershipAndPublishes(t *testing.T) { + svc, eventBus, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + set := createFullStackSet(t, svc) + eventBus.ClearEvents() + + name := "Backend" + ids := []string{"repo-orders", "repo-gateway"} + updated, err := svc.UpdateRepositorySet(context.Background(), set.ID, &UpdateRepositorySetRequest{ + Name: &name, + RepositoryIDs: &ids, + }) + if err != nil { + t.Fatalf("UpdateRepositorySet: %v", err) + } + if updated.Name != "Backend" { + t.Fatalf("name = %q", updated.Name) + } + if got := updated.RepositoryIDs(); len(got) != 2 || got[0] != "repo-orders" || got[1] != "repo-gateway" { + t.Fatalf("membership = %v, want the supplied order", got) + } + + events := eventBus.GetPublishedEvents() + if len(events) != 1 || events[0].Type != "repository_set.updated" { + t.Fatalf("published events = %#v, want one repository_set.updated", events) + } +} + +func TestUpdateRepositorySetLeavesOmittedFieldsAlone(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + set := createFullStackSet(t, svc) + + description := "just the description" + updated, err := svc.UpdateRepositorySet(context.Background(), set.ID, &UpdateRepositorySetRequest{ + Description: &description, + }) + if err != nil { + t.Fatalf("UpdateRepositorySet: %v", err) + } + if updated.Name != "Full-stack" { + t.Fatalf("name changed to %q", updated.Name) + } + if updated.Description != "just the description" { + t.Fatalf("description = %q", updated.Description) + } + // An absent RepositoryIDs must not be read as "remove every member". + if got := updated.RepositoryIDs(); len(got) != 2 { + t.Fatalf("membership = %v, want unchanged", got) + } +} + +func TestUpdateRepositorySetRejectsEmptyMembership(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + set := createFullStackSet(t, svc) + + empty := []string{} + _, err := svc.UpdateRepositorySet(context.Background(), set.ID, &UpdateRepositorySetRequest{ + RepositoryIDs: &empty, + }) + if !errors.Is(err, ErrInvalidRepositorySet) { + t.Fatalf("empty membership error = %v, want ErrInvalidRepositorySet", err) + } + loaded, err := svc.GetRepositorySet(context.Background(), set.ID) + if err != nil { + t.Fatalf("GetRepositorySet: %v", err) + } + if len(loaded.Items) != 2 { + t.Fatalf("rejected update changed membership: %+v", loaded.Items) + } +} + +func TestUpdateRepositorySetKeepingItsOwnNameIsNotAConflict(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + set := createFullStackSet(t, svc) + + name := "Full-stack" + description := "same name, new description" + if _, err := svc.UpdateRepositorySet(context.Background(), set.ID, &UpdateRepositorySetRequest{ + Name: &name, + Description: &description, + }); err != nil { + t.Fatalf("UpdateRepositorySet with its own name: %v", err) + } +} + +func TestUpdateAndDeleteMissingRepositorySetReportNotFound(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + ctx := context.Background() + + name := "Nope" + _, err := svc.UpdateRepositorySet(ctx, "missing", &UpdateRepositorySetRequest{Name: &name}) + if !errors.Is(err, repoerrors.ErrRepositorySetNotFound) { + t.Fatalf("update error = %v, want ErrRepositorySetNotFound", err) + } + if err := svc.DeleteRepositorySet(ctx, "missing"); !errors.Is(err, repoerrors.ErrRepositorySetNotFound) { + t.Fatalf("delete error = %v, want ErrRepositorySetNotFound", err) + } + if _, err := svc.GetRepositorySet(ctx, "missing"); !errors.Is(err, repoerrors.ErrRepositorySetNotFound) { + t.Fatalf("get error = %v, want ErrRepositorySetNotFound", err) + } +} + +func TestDeleteRepositorySetPublishesAndKeepsRepositories(t *testing.T) { + svc, eventBus, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + set := createFullStackSet(t, svc) + eventBus.ClearEvents() + ctx := context.Background() + + if err := svc.DeleteRepositorySet(ctx, set.ID); err != nil { + t.Fatalf("DeleteRepositorySet: %v", err) + } + events := eventBus.GetPublishedEvents() + if len(events) != 1 || events[0].Type != "repository_set.deleted" { + t.Fatalf("published events = %#v, want one repository_set.deleted", events) + } + if _, err := repo.GetRepository(ctx, "repo-web"); err != nil { + t.Fatalf("set deletion touched a repository: %v", err) + } +} + +func TestListRepositorySetsIsWorkspaceScoped(t *testing.T) { + svc, _, repo := createTestService(t) + seedSetWorkspace(t, svc, repo) + createFullStackSet(t, svc) + ctx := context.Background() + + sets, err := svc.ListRepositorySets(ctx, "ws-1") + if err != nil { + t.Fatalf("ListRepositorySets: %v", err) + } + if len(sets) != 1 { + t.Fatalf("ListRepositorySets returned %d sets", len(sets)) + } + + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-2", Name: "Other"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + other, err := svc.ListRepositorySets(ctx, "ws-2") + if err != nil { + t.Fatalf("ListRepositorySets other workspace: %v", err) + } + if len(other) != 0 { + t.Fatalf("other workspace saw %d sets", len(other)) + } +} diff --git a/apps/backend/internal/task/service/service_resources.go b/apps/backend/internal/task/service/service_resources.go index e657e3f757..17b941388b 100644 --- a/apps/backend/internal/task/service/service_resources.go +++ b/apps/backend/internal/task/service/service_resources.go @@ -1236,11 +1236,17 @@ func (s *Service) DeleteRepository(ctx context.Context, id string) error { if active { return ErrActiveTaskSessions } + // Captured before the delete, because deleting prunes the membership rows + // that identify the affected sets. + affectedSetIDs := s.repositorySetIDsHolding(ctx, id) if err := s.repoEntities.DeleteRepository(ctx, id); err != nil { s.logger.Error("failed to delete repository", zap.String("repository_id", id), zap.Error(err)) return err } s.publishRepositoryEvent(ctx, events.RepositoryDeleted, repository) + // A client that already loaded the workspace only reacts to repository_set.* + // events, so without this it keeps offering the deleted member until reload. + s.publishRepositorySetsAfterMembershipChange(ctx, affectedSetIDs) s.logger.Info("repository deleted", zap.String("repository_id", id)) return nil } diff --git a/apps/backend/internal/task/service/service_test.go b/apps/backend/internal/task/service/service_test.go index 9acb970ef8..5de6a5f284 100644 --- a/apps/backend/internal/task/service/service_test.go +++ b/apps/backend/internal/task/service/service_test.go @@ -198,6 +198,7 @@ func createTestServiceWithSessionsRepo( Sessions: wrapSessions(repo), GitSnapshots: repo, RepoEntities: repo, + RepositorySets: repo, RepositoryCleanup: repo, Executors: repo, Environments: repo, diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index ec5d390db5..778fbacb62 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -27,6 +27,13 @@ const ( ActionRepositoryUpdate = "repository.update" ActionRepositoryDelete = "repository.delete" + // Repository Set actions + ActionRepositorySetList = "repository_set.list" + ActionRepositorySetCreate = "repository_set.create" + ActionRepositorySetGet = "repository_set.get" + ActionRepositorySetUpdate = "repository_set.update" + ActionRepositorySetDelete = "repository_set.delete" + // Repository Script actions ActionRepositoryScriptList = "repository.script.list" ActionRepositoryScriptCreate = "repository.script.create" @@ -233,6 +240,9 @@ const ( ActionRepositoryCreated = "repository.created" ActionRepositoryUpdated = "repository.updated" ActionRepositoryDeleted = "repository.deleted" + ActionRepositorySetCreated = "repository_set.created" + ActionRepositorySetUpdated = "repository_set.updated" + ActionRepositorySetDeleted = "repository_set.deleted" ActionRepositoryScriptCreated = "repository.script.created" ActionRepositoryScriptUpdated = "repository.script.updated" ActionRepositoryScriptDeleted = "repository.script.deleted" diff --git a/apps/web/app/settings/workspace/use-workspace-repository-sets.ts b/apps/web/app/settings/workspace/use-workspace-repository-sets.ts new file mode 100644 index 0000000000..197d24910d --- /dev/null +++ b/apps/web/app/settings/workspace/use-workspace-repository-sets.ts @@ -0,0 +1,153 @@ +"use client"; + +import { useCallback, useState } from "react"; +import { useTranslation } from "react-i18next"; + +import { createRepositorySet, deleteRepositorySet, updateRepositorySet } from "@/lib/api"; +import { ApiError } from "@/lib/api/client"; +import { useAppStore } from "@/components/state-provider"; +import type { RepositorySet } from "@/lib/types/http"; + +/** + * An in-progress set edit. `setId` is null while creating. + * + * `repositoryIds` is ordered, and that order is what applying the set will use, + * so the editor's selection order is meaningful rather than incidental. + */ +export type RepositorySetDraft = { + setId: string | null; + name: string; + description: string; + repositoryIds: string[]; +}; + +/** + * Create, edit, and delete state for the workspace repository-set section. + * + * Writes go straight to the API and then into the store, so the section reflects + * the change without waiting for the `repository_set.*` event to arrive (which + * still keeps other tabs current). + */ +export function useWorkspaceRepositorySets({ workspaceId }: { workspaceId: string }) { + const { t } = useTranslation(); + const upsertRepositorySet = useAppStore((state) => state.upsertRepositorySet); + const removeRepositorySet = useAppStore((state) => state.removeRepositorySet); + const [draft, setDraft] = useState(null); + const [deleting, setDeleting] = useState(null); + const [error, setError] = useState(null); + const [saving, setSaving] = useState(false); + + const startCreate = useCallback(() => { + setError(null); + setDraft({ setId: null, name: "", description: "", repositoryIds: [] }); + }, []); + + const startEdit = useCallback((set: RepositorySet) => { + setError(null); + setDraft({ + setId: set.id, + name: set.name, + description: set.description, + repositoryIds: set.repositories.map((member) => member.repository_id as string), + }); + }, []); + + const cancelEdit = useCallback(() => { + setDraft(null); + setError(null); + }, []); + + const updateDraft = useCallback((patch: Partial) => { + setDraft((current) => (current ? { ...current, ...patch } : current)); + }, []); + + const submitDraft = useCallback(async () => { + if (!draft || saving) return; + const name = draft.name.trim(); + if (!name || draft.repositoryIds.length === 0) return; + setSaving(true); + setError(null); + try { + // A supplied repository_ids replaces the whole membership list, which is + // also how the editor expresses a reorder. + const saved = draft.setId + ? await updateRepositorySet(draft.setId, { + name, + description: draft.description.trim(), + repositoryIds: draft.repositoryIds, + }) + : await createRepositorySet(workspaceId, { + name, + description: draft.description.trim(), + repositoryIds: draft.repositoryIds, + }); + upsertRepositorySet(workspaceId, saved); + setDraft(null); + } catch (caught) { + setError(editorErrorMessage(caught, t)); + } finally { + setSaving(false); + } + }, [draft, saving, workspaceId, upsertRepositorySet, t]); + + const startDelete = useCallback((set: RepositorySet) => setDeleting(set), []); + const cancelDelete = useCallback(() => setDeleting(null), []); + + const confirmDelete = useCallback(async () => { + if (!deleting) return; + setError(null); + try { + await deleteRepositorySet(deleting.id); + removeRepositorySet(workspaceId, deleting.id); + setDeleting(null); + } catch (caught) { + // The row stays, so silence would read as "the list is stale" and invite a + // retry against a set that may already be gone. Keep the dialog open with + // the reason. + setError(deleteErrorMessage(caught, t)); + } + }, [deleting, workspaceId, removeRepositorySet, t]); + + return { + draft, + deleting, + error, + saving, + startCreate, + startEdit, + cancelEdit, + updateDraft, + submitDraft, + startDelete, + cancelDelete, + confirmDelete, + }; +} + +/** + * A duplicate name is the one failure the user can fix in place, and the backend + * message already names the existing set, so it is shown as-is. + */ +function editorErrorMessage(caught: unknown, t: (key: string) => string): string { + if (caught instanceof ApiError && caught.status === 409) { + return apiErrorBodyMessage(caught) ?? t("workspaces:repositorySetsNameTaken"); + } + if (caught instanceof ApiError && caught.status === 422) { + return apiErrorBodyMessage(caught) ?? t("workspaces:repositorySetsMemberRejected"); + } + return t("workspaces:repositorySetsSaveFailed"); +} + +/** A set already deleted elsewhere is not a failure the user must act on. */ +function deleteErrorMessage(caught: unknown, t: (key: string) => string): string { + if (caught instanceof ApiError && caught.status === 404) { + return t("workspaces:repositorySetsAlreadyDeleted"); + } + return t("workspaces:repositorySetsDeleteFailed"); +} + +function apiErrorBodyMessage(caught: ApiError): string | null { + if (typeof caught.body !== "object" || caught.body === null) return null; + const message = (caught.body as { error?: unknown }).error; + return typeof message === "string" && message !== "" ? message : null; +} diff --git a/apps/web/app/settings/workspace/workspace-add-local-repository-dialog.tsx b/apps/web/app/settings/workspace/workspace-add-local-repository-dialog.tsx new file mode 100644 index 0000000000..cb88559a8c --- /dev/null +++ b/apps/web/app/settings/workspace/workspace-add-local-repository-dialog.tsx @@ -0,0 +1,37 @@ +"use client"; + +import { DiscoverRepoDialog } from "@/app/settings/workspace/workspace-repositories-dialog"; +import type { useWorkspaceRepositoriesPage } from "./workspace-repositories-client"; + +/** + * The discover/add-local-repository dialog for the workspace Repositories page. + * + * It takes the page's whole state object rather than fifteen props: every field it + * reads belongs to that one hook, and passing them individually made the page + * component exceed its length cap once the repository-sets section joined it. + */ +export function AddLocalRepositoryDialog({ + state, +}: { + state: ReturnType; +}) { + return ( + + ); +} diff --git a/apps/web/app/settings/workspace/workspace-repositories-client.tsx b/apps/web/app/settings/workspace/workspace-repositories-client.tsx index ed37e200d9..5a422dd058 100644 --- a/apps/web/app/settings/workspace/workspace-repositories-client.tsx +++ b/apps/web/app/settings/workspace/workspace-repositories-client.tsx @@ -8,6 +8,8 @@ import { IconGitBranch } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; import { SettingsSection } from "@/components/settings/settings-section"; import { RepositoryCard } from "@/components/settings/repository-card"; +import { WorkspaceRepositorySetsSection } from "./workspace-repository-sets-section"; +import { AddLocalRepositoryDialog } from "./workspace-add-local-repository-dialog"; import { generateUUID } from "@/lib/utils"; import { createRepositoryAction, @@ -29,16 +31,14 @@ import { import { useRequest } from "@/lib/http/use-request"; import { useToast } from "@/components/toast-provider"; import { useAppStore } from "@/components/state-provider"; -import { - DiscoverRepoDialog, - type ManualValidation, -} from "@/app/settings/workspace/workspace-repositories-dialog"; +import type { ManualValidation } from "@/app/settings/workspace/workspace-repositories-dialog"; import { WorkspaceNotFoundCard } from "@/app/settings/workspace/workspace-not-found-card"; import { areRepositoryScriptsDirty, cloneRepository, isRepositoryDirty, mergeSavedRepositoryDraft, + persistedRepositoryItems, type RepositoryWithScripts, } from "@/app/settings/workspace/workspace-repositories-dirty"; import { defaultWorktreeBranchTemplate } from "@/lib/worktree-branch-template"; @@ -424,7 +424,7 @@ function useDiscoverDialog( discoveredRepositories, }; } -function useWorkspaceRepositoriesPage( +export function useWorkspaceRepositoriesPage( workspace: Workspace | null, repositories: RepositoryWithScripts[], ) { @@ -527,6 +527,8 @@ export function WorkspaceRepositoriesClient({ }: WorkspaceRepositoriesClientProps) { const { t } = useTranslation(); const state = useWorkspaceRepositoriesPage(workspace, repositories); + // The add-local-repository dialog reads the rest of `state` directly, so only + // what this component renders is destructured here. const { router, repositoryItems, @@ -537,22 +539,7 @@ export function WorkspaceRepositoriesClient({ handleDeleteRepositoryScript, handleSaveRepository, handleDeleteRepository, - localRepoDialogOpen, - setLocalRepoDialogOpen, - filteredRepositories, - repoSearch, - setRepoSearch, - selectedRepoPath, - handleSelectRepoPath, - manualRepoPath, - handleManualRepoPathChange, - manualValidation, - handleValidateManualPath, - isValidating, - isDiscovering, - canSave, openDialog, - handleConfirmLocalRepository, } = state; if (!workspace) @@ -600,25 +587,16 @@ export function WorkspaceRepositoriesClient({ ))} - {!isImproveWorkspace && ( - - )} + {/* Sets group the repositories listed above, so they belong on this page + rather than on a tab of their own. */} + + {!isImproveWorkspace && } ); } diff --git a/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts b/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts index c1ad41dcfa..e6fc2dd42a 100644 --- a/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts +++ b/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts @@ -6,6 +6,7 @@ import { cloneRepository, isRepositoryDirty, mergeSavedRepositoryDraft, + persistedRepositoryItems, type RepositoryWithScripts, } from "./workspace-repositories-dirty"; @@ -99,4 +100,11 @@ describe("repository secret binding draft state", () => { expect(source.scripts[0]!.command).toBe("echo setup"); }); + + it("keeps temporary repository drafts out of persisted repository lists", () => { + const persisted = repository(); + const draft = repository({ id: repositoryId("temp-repo-1") }); + + expect(persistedRepositoryItems([persisted, draft])).toEqual([persisted]); + }); }); diff --git a/apps/web/app/settings/workspace/workspace-repositories-dirty.ts b/apps/web/app/settings/workspace/workspace-repositories-dirty.ts index 7b3c37f6d6..01469ccde4 100644 --- a/apps/web/app/settings/workspace/workspace-repositories-dirty.ts +++ b/apps/web/app/settings/workspace/workspace-repositories-dirty.ts @@ -3,6 +3,18 @@ import { defaultWorktreeBranchTemplate } from "@/lib/worktree-branch-template"; export type RepositoryWithScripts = Repository & { scripts: RepositoryScript[] }; +/** + * Returns the repository rows that exist on the backend. + * + * The settings page keeps unsaved local-repository rows in the same list as + * saved rows. Those temporary IDs must not reach repository-set membership. + */ +export function persistedRepositoryItems( + repositories: RepositoryWithScripts[], +): RepositoryWithScripts[] { + return repositories.filter((repository) => !repository.id.startsWith("temp-repo-")); +} + const repositoryFields: Array = [ "name", "source_type", diff --git a/apps/web/app/settings/workspace/workspace-repository-set-delete.tsx b/apps/web/app/settings/workspace/workspace-repository-set-delete.tsx new file mode 100644 index 0000000000..6e1a2c7b12 --- /dev/null +++ b/apps/web/app/settings/workspace/workspace-repository-set-delete.tsx @@ -0,0 +1,68 @@ +"use client"; + +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@kandev/ui/alert-dialog"; +import { useTranslation } from "react-i18next"; + +import type { RepositorySet } from "@/lib/types/http"; + +type RepositorySetDeleteDialogProps = { + set: RepositorySet | null; + /** Non-null when the last delete attempt failed; keeps the dialog open. */ + error: string | null; + onClose: () => void; + onConfirm: () => void; +}; + +/** + * Confirms deleting a set. The copy states plainly that no repository is + * affected, because "delete set" next to a list of repository names reads as + * though it might remove them. + */ +export function RepositorySetDeleteDialog({ + set, + error, + onClose, + onConfirm, +}: RepositorySetDeleteDialogProps) { + const { t } = useTranslation(); + if (!set) return null; + + return ( + (next ? undefined : onClose())}> + + + + {t("workspaces:repositorySetsDeleteTitle", { name: set.name })} + + + {t("workspaces:repositorySetsDeleteDescription")} + + + {error ? ( +

+ {error} +

+ ) : null} + + {t("common:cancel")} + + {t("workspaces:repositorySetsDelete")} + + + + + ); +} diff --git a/apps/web/app/settings/workspace/workspace-repository-set-editor.tsx b/apps/web/app/settings/workspace/workspace-repository-set-editor.tsx new file mode 100644 index 0000000000..c8ede03258 --- /dev/null +++ b/apps/web/app/settings/workspace/workspace-repository-set-editor.tsx @@ -0,0 +1,229 @@ +"use client"; + +import { IconArrowDown, IconArrowUp } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { Checkbox } from "@kandev/ui/checkbox"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; +import { Input } from "@kandev/ui/input"; +import { Textarea } from "@kandev/ui/textarea"; +import { useTranslation } from "react-i18next"; + +import type { Repository } from "@/lib/types/http"; +import type { RepositorySetDraft } from "./use-workspace-repository-sets"; + +type RepositorySetEditorDialogProps = { + draft: RepositorySetDraft | null; + repositories: Repository[]; + error: string | null; + saving: boolean; + onClose: () => void; + onChange: (patch: Partial) => void; + onSubmit: () => void; +}; + +/** + * Creates or edits one set: name, description, and which repositories belong in + * which order. Member selection offers only this workspace's live repositories, + * so a set can never be given a member the backend would reject. + */ +export function RepositorySetEditorDialog({ + draft, + repositories, + error, + saving, + onClose, + onChange, + onSubmit, +}: RepositorySetEditorDialogProps) { + const { t } = useTranslation(); + if (!draft) return null; + const canSave = draft.name.trim().length > 0 && draft.repositoryIds.length > 0 && !saving; + + return ( + (next ? undefined : onClose())}> + + + + {draft.setId + ? t("workspaces:repositorySetsEditTitle") + : t("workspaces:repositorySetsCreateTitle")} + + {t("workspaces:repositorySetsEditorDescription")} + +
+ +