diff --git a/apps/backend/internal/gitlab/controller_config_routes_test.go b/apps/backend/internal/gitlab/controller_config_routes_test.go new file mode 100644 index 0000000000..71da8683a3 --- /dev/null +++ b/apps/backend/internal/gitlab/controller_config_routes_test.go @@ -0,0 +1,252 @@ +package gitlab + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" +) + +// newConfigRouteFixture wires the config HTTP routes against a store, a fake +// workspace secret store, and a stub GitLab whose /api/v4/user accepts exactly +// one token. The returned server URL is the host to configure. +func newConfigRouteFixture(t *testing.T, validToken string) (*Store, *configTestSecrets, *gin.Engine, string) { + t.Helper() + gin.SetMode(gin.TestMode) + gitlabServer := httptest.NewServer(userHandler(func(token string) bool { return token == validToken })) + t.Cleanup(gitlabServer.Close) + + store := newTestStore(t) + seedWorkspace(t, store, "ws-1") + seedWorkspace(t, store, "ws-2") + secrets := &configTestSecrets{values: make(map[string]string)} + svc := NewService(DefaultHost, NewNoopClient(DefaultHost), AuthMethodNone, nil, newTestLogger(t)) + svc.SetStore(store) + svc.SetWorkspaceSecretStore(secrets) + router := gin.New() + NewController(svc, newTestLogger(t)).RegisterHTTPRoutes(router) + return store, secrets, router, gitlabServer.URL +} + +// configureWorkspace saves a PAT connection through the real PUT /config route +// so the follow-up delete/copy tests operate on state the API itself produced. +func configureWorkspace(t *testing.T, router *gin.Engine, workspaceID, host, token string) { + t.Helper() + recorder := watchScopeRequest(t, router, http.MethodPut, + "/api/v1/gitlab/config?workspace_id="+workspaceID, + map[string]string{"host": host, "auth_method": AuthMethodPAT, "token": token}) + if recorder.Code != http.StatusOK { + t.Fatalf("configure %s: status = %d, body = %s", workspaceID, recorder.Code, recorder.Body.String()) + } +} + +// --- DELETE /config --- + +func TestHTTPDeleteConfigRemovesRowAndSecret(t *testing.T) { + store, secrets, router, host := newConfigRouteFixture(t, "good-token") + configureWorkspace(t, router, "ws-1", host, "good-token") + if _, ok := secrets.values[SecretKeyForWorkspace("ws-1")]; !ok { + t.Fatal("fixture did not store a workspace secret to begin with") + } + + var body map[string]bool + recorder := httpJSON(t, router, http.MethodDelete, + "/api/v1/gitlab/config?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if !body[respKeyDeleted] { + t.Errorf("payload = %v, want {\"deleted\":true}", body) + } + cfg, err := store.GetConfigForWorkspace(context.Background(), "ws-1") + if err != nil { + t.Fatalf("reload config: %v", err) + } + if cfg != nil { + t.Errorf("config = %+v, want the row removed", cfg) + } + if _, ok := secrets.values[SecretKeyForWorkspace("ws-1")]; ok { + t.Error("the workspace token secret survived the connection delete") + } +} + +// TestHTTPDeleteConfigRequiresWorkspaceQuery covers writeConfigMutationError's +// ErrWorkspaceRequired branch, which must be a 400 rather than the default 500. +func TestHTTPDeleteConfigRequiresWorkspaceQuery(t *testing.T) { + _, _, router, _ := newConfigRouteFixture(t, "good-token") + + recorder := watchScopeRequest(t, router, http.MethodDelete, "/api/v1/gitlab/config", nil) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "workspace_id query parameter required" { + t.Errorf("error = %q, want the workspace_id requirement", got) + } +} + +// --- POST /config/test --- + +func TestHTTPTestConfigReportsSuccessWithoutPersisting(t *testing.T) { + store, secrets, router, host := newConfigRouteFixture(t, "good-token") + + var result TestConnectionResult + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/config/test?workspace_id=ws-1", + map[string]string{"host": host, "auth_method": AuthMethodPAT, "token": "good-token"}, &result) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if result.Error != "" { + t.Fatalf("error = %q, want a successful probe", result.Error) + } + if result.Username != "alice" { + t.Errorf("username = %q, want alice from the stub", result.Username) + } + cfg, err := store.GetConfigForWorkspace(context.Background(), "ws-1") + if err != nil { + t.Fatalf("reload config: %v", err) + } + if cfg != nil { + t.Errorf("config = %+v, want nothing persisted by a test probe", cfg) + } + if len(secrets.values) != 0 { + t.Errorf("secrets = %v, want nothing persisted by a test probe", secrets.values) + } +} + +// TestHTTPTestConfigReportsFailureAsA200WithError pins the contract the +// settings UI depends on: a rejected credential is a *result*, not an HTTP +// error, so the form can render the message inline. +func TestHTTPTestConfigReportsFailureAsA200WithError(t *testing.T) { + _, _, router, host := newConfigRouteFixture(t, "good-token") + + var result TestConnectionResult + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/config/test?workspace_id=ws-1", + map[string]string{"host": host, "auth_method": AuthMethodPAT, "token": "wrong-token"}, &result) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 even for a failed probe (body %s)", + recorder.Code, recorder.Body.String()) + } + if result.Error == "" { + t.Error("error is empty; a rejected token must be reported in the result") + } + if result.Username != "" { + t.Errorf("username = %q, want empty for a failed probe", result.Username) + } +} + +func TestHTTPTestConfigRejectsInvalidPayload(t *testing.T) { + _, _, router, _ := newConfigRouteFixture(t, "good-token") + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/config/test?workspace_id=ws-1", []string{"not", "an", "object"}) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "invalid payload" { + t.Errorf("error = %q, want \"invalid payload\"", got) + } +} + +// --- POST /config/copy --- + +func TestHTTPCopyConfigDuplicatesConnectionToTargetWorkspace(t *testing.T) { + store, secrets, router, host := newConfigRouteFixture(t, "good-token") + configureWorkspace(t, router, "ws-1", host, "good-token") + + var copied GitLabConfig + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/config/copy?workspace_id=ws-1", + map[string]string{"targetWorkspaceId": "ws-2"}, &copied) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + target, err := store.GetConfigForWorkspace(context.Background(), "ws-2") + if err != nil || target == nil { + t.Fatalf("target config = %+v, err = %v", target, err) + } + if target.Host != host { + t.Errorf("target host = %q, want %q", target.Host, host) + } + if secrets.values[SecretKeyForWorkspace("ws-2")] != "good-token" { + t.Errorf("target secret = %q, want the source token copied under the target's own key", + secrets.values[SecretKeyForWorkspace("ws-2")]) + } + if secrets.values[SecretKeyForWorkspace("ws-1")] != "good-token" { + t.Error("the source workspace's secret was disturbed by the copy") + } +} + +func TestHTTPCopyConfigRequiresTargetWorkspace(t *testing.T) { + _, _, router, host := newConfigRouteFixture(t, "good-token") + configureWorkspace(t, router, "ws-1", host, "good-token") + + for name, body := range map[string]interface{}{ + "absent": map[string]string{}, + "blank": map[string]string{"targetWorkspaceId": " "}, + } { + t.Run(name, func(t *testing.T) { + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/config/copy?workspace_id=ws-1", body) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "targetWorkspaceId required" { + t.Errorf("error = %q, want \"targetWorkspaceId required\"", got) + } + }) + } +} + +// TestHTTPCopyConfigRejectsCopyingAWorkspaceOntoItself covers the +// ErrSameWorkspace branch of writeConfigMutationError. +func TestHTTPCopyConfigRejectsCopyingAWorkspaceOntoItself(t *testing.T) { + _, _, router, host := newConfigRouteFixture(t, "good-token") + configureWorkspace(t, router, "ws-1", host, "good-token") + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/config/copy?workspace_id=ws-1", + map[string]string{"targetWorkspaceId": "ws-1"}) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "source and target workspace must differ" { + t.Errorf("error = %q, want the same-workspace rejection", got) + } +} + +// TestHTTPCopyConfigRejectsUnconfiguredSource covers the ErrNothingToCopy +// branch: copying from a workspace with no connection is a client error, and +// the target must be left untouched. +func TestHTTPCopyConfigRejectsUnconfiguredSource(t *testing.T) { + store, _, router, _ := newConfigRouteFixture(t, "good-token") + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/config/copy?workspace_id=ws-1", + map[string]string{"targetWorkspaceId": "ws-2"}) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "source workspace has no GitLab connection" { + t.Errorf("error = %q, want the nothing-to-copy rejection", got) + } + target, err := store.GetConfigForWorkspace(context.Background(), "ws-2") + if err != nil { + t.Fatalf("reload target config: %v", err) + } + if target != nil { + t.Errorf("target config = %+v, want it left unconfigured", target) + } +} diff --git a/apps/backend/internal/gitlab/controller_http_surface_test.go b/apps/backend/internal/gitlab/controller_http_surface_test.go new file mode 100644 index 0000000000..e0d5c962bb --- /dev/null +++ b/apps/backend/internal/gitlab/controller_http_surface_test.go @@ -0,0 +1,430 @@ +package gitlab + +import ( + "context" + "net/http" + "testing" +) + +// --- Task MR listing routes --- + +func TestHTTPListWorkspaceTaskMRsGroupsByTask(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + ctx := context.Background() + seedTask(t, store, "task-a", "ws-1") + seedTask(t, store, "task-b", "ws-1") + seedTask(t, store, "task-foreign", "ws-2") + for _, mr := range []*TaskMR{ + newTestMR("task-a", "repo-1", "group/a", 11), + newTestMR("task-b", "repo-1", "group/b", 22), + newTestMR("task-foreign", "repo-1", "group/c", 33), + } { + if err := store.UpsertTaskMR(ctx, mr); err != nil { + t.Fatalf("seed %s MR: %v", mr.TaskID, err) + } + } + + var body TaskMRsResponse + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/workspaces/ws-1/task-mrs", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.TaskMRs) != 2 { + t.Fatalf("grouped tasks = %d, want 2 (%v)", len(body.TaskMRs), body.TaskMRs) + } + if _, leaked := body.TaskMRs["task-foreign"]; leaked { + t.Error("the ws-2 task leaked into the ws-1 listing") + } + if got := body.TaskMRs["task-a"]; len(got) != 1 || got[0].MRIID != 11 { + t.Errorf("task-a group = %+v, want the single !11 row", got) + } +} + +func TestHTTPListTaskMRsReturnsRowsForTask(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + ctx := context.Background() + seedTask(t, store, "task-a", "ws-1") + if err := store.UpsertTaskMR(ctx, newTestMR("task-a", "repo-1", "group/a", 11)); err != nil { + t.Fatalf("seed MR: %v", err) + } + + var body struct { + TaskMRs []*TaskMR `json:"task_mrs"` + } + recorder := httpJSON(t, router, http.MethodGet, "/api/v1/gitlab/tasks/task-a/mrs", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.TaskMRs) != 1 || body.TaskMRs[0].ProjectPath != "group/a" { + t.Fatalf("task_mrs = %+v, want the single group/a row", body.TaskMRs) + } +} + +func TestHTTPListTaskMRsReturnsEmptyListForUnknownTask(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + var body struct { + TaskMRs []*TaskMR `json:"task_mrs"` + } + recorder := httpJSON(t, router, http.MethodGet, "/api/v1/gitlab/tasks/nope/mrs", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.TaskMRs) != 0 { + t.Errorf("task_mrs = %+v, want an empty list", body.TaskMRs) + } +} + +// --- Watch reset routes --- + +func TestHTTPPreviewResetReviewWatchCountsTasks(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + ctx := context.Background() + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + if _, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url"); err != nil { + t.Fatalf("reserve review MR task: %v", err) + } + if err := store.AssignReviewMRTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil { + t.Fatalf("assign review MR task: %v", err) + } + + var body struct { + TaskCount int `json:"taskCount"` + } + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/watches/review/"+watch.ID+"/reset/preview?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if body.TaskCount != 1 { + t.Errorf("taskCount = %d, want 1", body.TaskCount) + } +} + +func TestHTTPResetReviewWatchDeletesTaskTrees(t *testing.T) { + svc, store, router := newWatchHTTPFixture(t) + ctx := context.Background() + deleter := &recordingCascadeDeleter{} + svc.SetCascadeTaskDeleter(deleter) + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + if _, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url"); err != nil { + t.Fatalf("reserve review MR task: %v", err) + } + if err := store.AssignReviewMRTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil { + t.Fatalf("assign review MR task: %v", err) + } + + var body struct { + TasksDeleted int `json:"tasksDeleted"` + } + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/watches/review/"+watch.ID+"/reset?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if body.TasksDeleted != 1 { + t.Errorf("tasksDeleted = %d, want 1", body.TasksDeleted) + } + if len(deleter.ids) != 1 || deleter.ids[0] != "task-1" { + t.Errorf("deleted task trees = %v, want [task-1]", deleter.ids) + } +} + +// TestHTTPResetReviewWatchWithoutCascadeDeleterFails covers writeWatchResetError: +// an unwired cascade deleter is a server-side misconfiguration, and the reply +// must be a 500 whose message names the operation, not the internal cause. +func TestHTTPResetReviewWatchWithoutCascadeDeleterFails(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(t.Context(), watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/watches/review/"+watch.ID+"/reset?workspace_id=ws-1", nil) + + if recorder.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "reset review watch failed" { + t.Errorf("error = %q, want \"reset review watch failed\"", got) + } +} + +func TestHTTPResetReviewWatchRejectsForeignWatch(t *testing.T) { + svc, store, router := newWatchHTTPFixture(t) + deleter := &recordingCascadeDeleter{} + svc.SetCascadeTaskDeleter(deleter) + foreign := validReviewWatchRow() + foreign.WorkspaceID = "ws-2" + if err := store.CreateReviewWatch(t.Context(), foreign); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/watches/review/"+foreign.ID+"/reset?workspace_id=ws-1", nil) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(deleter.ids) != 0 { + t.Errorf("a cross-workspace reset deleted task trees %v", deleter.ids) + } +} + +func TestHTTPPreviewResetIssueWatchCountsTasks(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + ctx := context.Background() + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil { + t.Fatalf("reserve issue task: %v", err) + } + if err := store.AssignIssueWatchTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil { + t.Fatalf("assign issue task: %v", err) + } + + var body struct { + TaskCount int `json:"taskCount"` + } + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/watches/issue/"+watch.ID+"/reset/preview?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if body.TaskCount != 1 { + t.Errorf("taskCount = %d, want 1", body.TaskCount) + } +} + +func TestHTTPResetIssueWatchDeletesTaskTrees(t *testing.T) { + svc, store, router := newWatchHTTPFixture(t) + ctx := context.Background() + deleter := &recordingCascadeDeleter{} + svc.SetCascadeTaskDeleter(deleter) + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil { + t.Fatalf("reserve issue task: %v", err) + } + if err := store.AssignIssueWatchTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil { + t.Fatalf("assign issue task: %v", err) + } + + var body struct { + TasksDeleted int `json:"tasksDeleted"` + } + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/watches/issue/"+watch.ID+"/reset?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if body.TasksDeleted != 1 { + t.Errorf("tasksDeleted = %d, want 1", body.TasksDeleted) + } + if len(deleter.ids) != 1 || deleter.ids[0] != "task-1" { + t.Errorf("deleted task trees = %v, want [task-1]", deleter.ids) + } +} + +func TestHTTPPreviewResetIssueWatchRejectsForeignWatch(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + foreign := validIssueWatchRow() + foreign.WorkspaceID = "ws-2" + if err := store.CreateIssueWatch(t.Context(), foreign); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodGet, + "/api/v1/gitlab/watches/issue/"+foreign.ID+"/reset/preview?workspace_id=ws-1", nil) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "issue watch not found" { + t.Errorf("error = %q, want \"issue watch not found\"", got) + } +} + +// --- Action presets over HTTP --- + +func TestHTTPActionPresetsRoundTrip(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + + var defaults ActionPresets + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/action-presets?workspace_id=ws-1", nil, &defaults) + if recorder.Code != http.StatusOK { + t.Fatalf("get status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(defaults.MR) == 0 { + t.Fatal("mr presets are empty; the built-in defaults must be substituted") + } + + var updated ActionPresets + recorder = httpJSON(t, router, http.MethodPut, + "/api/v1/gitlab/action-presets?workspace_id=ws-1", + UpdateActionPresetsRequest{ + MR: &[]ActionPreset{{ID: "ship", Label: "Ship it", PromptTemplate: "ship {{mr}}"}}, + }, &updated) + if recorder.Code != http.StatusOK { + t.Fatalf("put status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(updated.MR) != 1 || updated.MR[0].ID != "ship" { + t.Fatalf("mr presets = %+v, want only the custom ship preset", updated.MR) + } + if updated.WorkspaceID != "ws-1" { + t.Errorf("workspace = %q, want ws-1 from the query parameter", updated.WorkspaceID) + } + + var reset ActionPresets + recorder = httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/action-presets/reset?workspace_id=ws-1", nil, &reset) + if recorder.Code != http.StatusOK { + t.Fatalf("reset status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + for _, preset := range reset.MR { + if preset.ID == "ship" { + t.Fatalf("mr presets = %+v, want the custom preset gone after reset", reset.MR) + } + } + stored, err := store.GetActionPresets(t.Context(), "ws-1") + if err != nil { + t.Fatalf("reload presets: %v", err) + } + if len(stored.MR) != 0 { + t.Errorf("stored mr presets = %+v, want the row deleted", stored.MR) + } +} + +func TestHTTPUpdateActionPresetsRejectsInvalidPayload(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + recorder := watchScopeRequest(t, router, http.MethodPut, + "/api/v1/gitlab/action-presets?workspace_id=ws-1", []string{"not", "an", "object"}) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "invalid payload" { + t.Errorf("error = %q, want \"invalid payload\"", got) + } +} + +// --- Project routes --- + +func TestHTTPListUserProjectsReturnsProjects(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + var body struct { + Projects []Project `json:"projects"` + } + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/projects?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.Projects) != 1 || body.Projects[0].PathWithNamespace != "kandev/sample" { + t.Fatalf("projects = %+v, want the single kandev/sample project", body.Projects) + } +} + +func TestHTTPSearchProjectsFiltersByQuery(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + tests := []struct { + name string + query string + want int + }{ + {"matching", "sample", 1}, + {"non-matching", "no-such-project", 0}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var body struct { + Projects []Project `json:"projects"` + } + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/projects/search?workspace_id=ws-1&query="+test.query, nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.Projects) != test.want { + t.Errorf("projects = %d, want %d (%+v)", len(body.Projects), test.want, body.Projects) + } + }) + } +} + +func TestHTTPProjectRoutesRequireProjectQuery(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + for _, path := range []string{"/projects/branches", "/projects/merge-methods"} { + t.Run(path, func(t *testing.T) { + recorder := watchScopeRequest(t, router, http.MethodGet, + "/api/v1/gitlab"+path+"?workspace_id=ws-1", nil) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "project required" { + t.Errorf("error = %q, want \"project required\"", got) + } + }) + } +} + +func TestHTTPGetProjectMergeMethodsReturnsAllowedMethods(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + var methods ProjectMergeMethods + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/projects/merge-methods?workspace_id=ws-1&project=group%2Fp", nil, &methods) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if !methods.Merge || !methods.AllowSquash { + t.Errorf("methods = %+v, want the mock's merge+squash defaults", methods) + } +} + +func TestHTTPListProjectBranchesReturnsBranches(t *testing.T) { + svc, _, router := newWatchHTTPFixture(t) + svc.workspaceClients["ws-1"].(*MockClient).SeedBranches("group/p", + []RepoBranch{{Name: "main"}, {Name: "dev"}}) + + var body struct { + Branches []RepoBranch `json:"branches"` + } + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/projects/branches?workspace_id=ws-1&project=group%2Fp", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.Branches) != 2 || body.Branches[0].Name != "main" { + t.Fatalf("branches = %+v, want [main dev]", body.Branches) + } +} diff --git a/apps/backend/internal/gitlab/controller_watches_http_test.go b/apps/backend/internal/gitlab/controller_watches_http_test.go new file mode 100644 index 0000000000..e35678d081 --- /dev/null +++ b/apps/backend/internal/gitlab/controller_watches_http_test.go @@ -0,0 +1,571 @@ +package gitlab + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" +) + +// newWatchHTTPFixture builds the full v1 HTTP surface over a store seeded with +// two workspaces, the dependency validators that accept the "ws-1" fixture +// identifiers, and a per-workspace mock client. Watch CRUD over HTTP is +// workspace-scoped through the ?workspace_id= query parameter, so every test +// here can also assert the cross-workspace rejection. +func newWatchHTTPFixture(t *testing.T) (*Service, *Store, *gin.Engine) { + t.Helper() + gin.SetMode(gin.TestMode) + svc, store := newDependencyTestService(t) + seedWorkspace(t, store, "ws-1") + seedWorkspace(t, store, "ws-2") + svc.workspaceClients = map[string]Client{ + "ws-1": NewMockClient(DefaultHost), + "ws-2": NewMockClient(DefaultHost), + } + router := gin.New() + NewController(svc, newTestLogger(t)).RegisterHTTPRoutes(router) + return svc, store, router +} + +// httpJSON issues a request against the in-process router and decodes the JSON +// body, returning the recorder so the caller can assert the status code too. +func httpJSON(t *testing.T, router *gin.Engine, method, target string, body interface{}, out interface{}) *httptest.ResponseRecorder { + t.Helper() + recorder := watchScopeRequest(t, router, method, target, body) + if out != nil && recorder.Body.Len() > 0 { + if err := json.Unmarshal(recorder.Body.Bytes(), out); err != nil { + t.Fatalf("decode %s %s body %s: %v", method, target, recorder.Body.String(), err) + } + } + return recorder +} + +// httpErrorMessage decodes the {"error": "..."} envelope every failure path in +// this controller writes. +func httpErrorMessage(t *testing.T, recorder *httptest.ResponseRecorder) string { + t.Helper() + var body struct { + Error string `json:"error"` + } + if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil { + t.Fatalf("decode error body %s: %v", recorder.Body.String(), err) + } + return body.Error +} + +// --- Workspace guard --- + +// TestWatchRoutesRequireWorkspaceQuery covers the requireWorkspaceQuery +// middleware: every route under it must abort with 400 before its handler runs, +// so an unscoped caller can never reach workspace data. +func TestWatchRoutesRequireWorkspaceQuery(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + routes := []struct { + method string + path string + }{ + {http.MethodGet, "/api/v1/gitlab/watches/review"}, + {http.MethodGet, "/api/v1/gitlab/watches/issue"}, + {http.MethodGet, "/api/v1/gitlab/watches/mr"}, + {http.MethodPost, "/api/v1/gitlab/watches/review/trigger-all"}, + {http.MethodPost, "/api/v1/gitlab/watches/issue/trigger-all"}, + {http.MethodGet, "/api/v1/gitlab/stats"}, + {http.MethodGet, "/api/v1/gitlab/action-presets"}, + {http.MethodGet, "/api/v1/gitlab/projects"}, + } + for _, route := range routes { + t.Run(route.method+" "+route.path, func(t *testing.T) { + recorder := watchScopeRequest(t, router, route.method, route.path, nil) + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 without workspace_id (body %s)", + recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "workspace_id query parameter required" { + t.Errorf("error = %q, want the workspace_id requirement", got) + } + }) + } +} + +// --- Review watches over HTTP --- + +func TestHTTPCreateReviewWatchPersistsUnderQueryWorkspace(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + // WorkspaceID is deliberately wrong in the body: the controller must + // overwrite it with the query parameter, which is the scoped value. + body := validReviewWatchRequest() + body.WorkspaceID = "ws-2" + + var created ReviewWatch + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/watches/review?workspace_id=ws-1", body, &created) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if created.WorkspaceID != "ws-1" { + t.Errorf("workspace = %q, want ws-1 from the query parameter, not the body", created.WorkspaceID) + } + stored, err := store.GetReviewWatch(t.Context(), created.ID) + if err != nil || stored == nil { + t.Fatalf("watch was not persisted: row=%v err=%v", stored, err) + } + if stored.WorkspaceID != "ws-1" { + t.Errorf("stored workspace = %q, want ws-1", stored.WorkspaceID) + } +} + +func TestHTTPCreateReviewWatchRejectsInvalidPayload(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + + req := httptest.NewRequest(http.MethodPost, + "/api/v1/gitlab/watches/review?workspace_id=ws-1", strings.NewReader("{not json")) + req.Header.Set("Content-Type", "application/json") + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, req) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "invalid payload" { + t.Errorf("error = %q, want \"invalid payload\"", got) + } +} + +// TestHTTPCreateReviewWatchMapsInvalidConfigTo400 pins the httpRespondError +// mapping: a dependency that belongs to another workspace is a client error, +// and the response must not disclose which dependency failed. +func TestHTTPCreateReviewWatchMapsInvalidConfigTo400(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + body := validReviewWatchRequest() + body.RepositoryID = "repo-other" // belongs to ws-2 + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/watches/review?workspace_id=ws-1", body) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "invalid watch configuration" { + t.Errorf("error = %q, want the generic configuration message", got) + } + if body := recorder.Body.String(); strings.Contains(body, "repo-other") { + t.Errorf("response disclosed the rejected dependency id: %s", body) + } +} + +func TestHTTPListReviewWatchesScopesToQueryWorkspace(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + mine := validReviewWatchRow() + foreign := validReviewWatchRow() + foreign.WorkspaceID = "ws-2" + for _, watch := range []*ReviewWatch{mine, foreign} { + if err := store.CreateReviewWatch(t.Context(), watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + } + + var body struct { + Watches []*ReviewWatch `json:"watches"` + } + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/watches/review?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.Watches) != 1 || body.Watches[0].ID != mine.ID { + t.Fatalf("watches = %+v, want only the ws-1 watch", body.Watches) + } +} + +func TestHTTPUpdateReviewWatchAppliesPatchAndReturnsRow(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(t.Context(), watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + var updated ReviewWatch + recorder := httpJSON(t, router, http.MethodPatch, + "/api/v1/gitlab/watches/review/"+watch.ID+"?workspace_id=ws-1", + map[string]interface{}{"enabled": false}, &updated) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if updated.ID != watch.ID || updated.Enabled { + t.Errorf("returned watch = %+v, want the disabled row echoed back", updated) + } + stored, err := store.GetReviewWatch(t.Context(), watch.ID) + if err != nil { + t.Fatalf("reload watch: %v", err) + } + if stored.Enabled { + t.Error("watch is still enabled; the patch did not reach the store") + } +} + +// TestHTTPUpdateReviewWatchRejectsForeignWatch is the cross-workspace guard: +// requireReviewWatchInWorkspace must 404 before the patch is applied, and the +// foreign row must be untouched. +func TestHTTPUpdateReviewWatchRejectsForeignWatch(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + foreign := validReviewWatchRow() + foreign.WorkspaceID = "ws-2" + if err := store.CreateReviewWatch(t.Context(), foreign); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodPatch, + "/api/v1/gitlab/watches/review/"+foreign.ID+"?workspace_id=ws-1", + map[string]interface{}{"enabled": false}) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "review watch not found" { + t.Errorf("error = %q, want \"review watch not found\"", got) + } + stored, err := store.GetReviewWatch(t.Context(), foreign.ID) + if err != nil { + t.Fatalf("reload foreign watch: %v", err) + } + if !stored.Enabled { + t.Error("the foreign watch was modified by a cross-workspace request") + } +} + +func TestHTTPDeleteReviewWatchRemovesRow(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(t.Context(), watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + var body map[string]bool + recorder := httpJSON(t, router, http.MethodDelete, + "/api/v1/gitlab/watches/review/"+watch.ID+"?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if !body[respKeyDeleted] { + t.Errorf("payload = %v, want {\"deleted\":true}", body) + } + remaining, err := store.ListReviewWatches(t.Context(), "ws-1") + if err != nil { + t.Fatalf("list review watches: %v", err) + } + if len(remaining) != 0 { + t.Errorf("watches remaining = %d, want 0", len(remaining)) + } +} + +func TestHTTPDeleteReviewWatchRejectsForeignWatch(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + foreign := validReviewWatchRow() + foreign.WorkspaceID = "ws-2" + if err := store.CreateReviewWatch(t.Context(), foreign); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodDelete, + "/api/v1/gitlab/watches/review/"+foreign.ID+"?workspace_id=ws-1", nil) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + remaining, err := store.ListReviewWatches(t.Context(), "ws-2") + if err != nil || len(remaining) != 1 { + t.Fatalf("foreign watch was deleted: rows=%d err=%v", len(remaining), err) + } +} + +func TestHTTPTriggerReviewWatchReturnsMRsAndCount(t *testing.T) { + svc, store, router := newWatchHTTPFixture(t) + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(t.Context(), watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + client := svc.workspaceClients["ws-1"].(*MockClient) + client.SetUser("kandev-tester") + client.SeedMR("group/p", &MR{IID: 1, Title: "needs review", WebURL: "https://x/1", + Reviewers: []MRReviewer{{Username: "kandev-tester"}}}) + + var body struct { + MRs []*MR `json:"mrs"` + Count int `json:"count"` + } + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/watches/review/"+watch.ID+"/trigger?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.MRs) != 1 { + t.Fatalf("mrs = %+v, want the single seeded MR to reach the client", body.MRs) + } + if body.MRs[0].IID != 1 { + t.Errorf("mr iid = %d, want 1", body.MRs[0].IID) + } + if body.Count != len(body.MRs) { + t.Errorf("count = %d but mrs = %d; the two must agree", body.Count, len(body.MRs)) + } +} + +func TestHTTPTriggerReviewWatchRejectsForeignWatch(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + foreign := validReviewWatchRow() + foreign.WorkspaceID = "ws-2" + if err := store.CreateReviewWatch(t.Context(), foreign); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/watches/review/"+foreign.ID+"/trigger?workspace_id=ws-1", nil) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } +} + +// --- Issue watches over HTTP --- + +func TestHTTPCreateIssueWatchPersistsUnderQueryWorkspace(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + body := validIssueWatchRequest() + body.WorkspaceID = "ws-2" + + var created IssueWatch + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/watches/issue?workspace_id=ws-1", body, &created) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if created.WorkspaceID != "ws-1" { + t.Errorf("workspace = %q, want ws-1 from the query parameter", created.WorkspaceID) + } + stored, err := store.GetIssueWatch(t.Context(), created.ID) + if err != nil || stored == nil { + t.Fatalf("watch was not persisted: row=%v err=%v", stored, err) + } +} + +func TestHTTPCreateIssueWatchMapsInvalidConfigTo400(t *testing.T) { + _, _, router := newWatchHTTPFixture(t) + body := validIssueWatchRequest() + body.ExecutorProfileID = "executor-missing" + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/watches/issue?workspace_id=ws-1", body) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "invalid watch configuration" { + t.Errorf("error = %q, want the generic configuration message", got) + } +} + +func TestHTTPListIssueWatchesScopesToQueryWorkspace(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + mine := validIssueWatchRow() + foreign := validIssueWatchRow() + foreign.WorkspaceID = "ws-2" + for _, watch := range []*IssueWatch{mine, foreign} { + if err := store.CreateIssueWatch(t.Context(), watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + } + + var body struct { + Watches []*IssueWatch `json:"watches"` + } + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/watches/issue?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.Watches) != 1 || body.Watches[0].ID != mine.ID { + t.Fatalf("watches = %+v, want only the ws-1 watch", body.Watches) + } +} + +func TestHTTPUpdateIssueWatchAppliesPatchAndReturnsRow(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(t.Context(), watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + + var updated IssueWatch + recorder := httpJSON(t, router, http.MethodPut, + "/api/v1/gitlab/watches/issue/"+watch.ID+"?workspace_id=ws-1", + map[string]interface{}{"enabled": false}, &updated) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if updated.ID != watch.ID || updated.Enabled { + t.Errorf("returned watch = %+v, want the disabled row echoed back", updated) + } + stored, err := store.GetIssueWatch(t.Context(), watch.ID) + if err != nil { + t.Fatalf("reload watch: %v", err) + } + if stored.Enabled { + t.Error("watch is still enabled; the patch did not reach the store") + } +} + +func TestHTTPUpdateIssueWatchRejectsForeignWatch(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + foreign := validIssueWatchRow() + foreign.WorkspaceID = "ws-2" + if err := store.CreateIssueWatch(t.Context(), foreign); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodPut, + "/api/v1/gitlab/watches/issue/"+foreign.ID+"?workspace_id=ws-1", + map[string]interface{}{"enabled": false}) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := httpErrorMessage(t, recorder); got != "issue watch not found" { + t.Errorf("error = %q, want \"issue watch not found\"", got) + } + stored, err := store.GetIssueWatch(t.Context(), foreign.ID) + if err != nil { + t.Fatalf("reload foreign watch: %v", err) + } + if !stored.Enabled { + t.Error("the foreign watch was modified by a cross-workspace request") + } +} + +func TestHTTPDeleteIssueWatchRemovesRow(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(t.Context(), watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + + var body map[string]bool + recorder := httpJSON(t, router, http.MethodDelete, + "/api/v1/gitlab/watches/issue/"+watch.ID+"?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if !body[respKeyDeleted] { + t.Errorf("payload = %v, want {\"deleted\":true}", body) + } + remaining, err := store.ListIssueWatches(t.Context(), "ws-1") + if err != nil { + t.Fatalf("list issue watches: %v", err) + } + if len(remaining) != 0 { + t.Errorf("watches remaining = %d, want 0", len(remaining)) + } +} + +func TestHTTPTriggerIssueWatchRejectsForeignWatch(t *testing.T) { + _, store, router := newWatchHTTPFixture(t) + foreign := validIssueWatchRow() + foreign.WorkspaceID = "ws-2" + if err := store.CreateIssueWatch(t.Context(), foreign); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + + recorder := watchScopeRequest(t, router, http.MethodPost, + "/api/v1/gitlab/watches/issue/"+foreign.ID+"/trigger?workspace_id=ws-1", nil) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } +} + +func TestHTTPTriggerIssueWatchReturnsIssuesAndCount(t *testing.T) { + svc, store, router := newWatchHTTPFixture(t) + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(t.Context(), watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + svc.workspaceClients["ws-1"].(*MockClient).SeedIssue("group/p", + &Issue{IID: 3, Title: "bug", WebURL: "https://x/3"}) + + var body struct { + Issues []*Issue `json:"issues"` + Count int `json:"count"` + } + recorder := httpJSON(t, router, http.MethodPost, + "/api/v1/gitlab/watches/issue/"+watch.ID+"/trigger?workspace_id=ws-1", nil, &body) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if len(body.Issues) != 1 { + t.Fatalf("issues = %+v, want the single seeded issue to reach the client", body.Issues) + } + if body.Issues[0].IID != 3 { + t.Errorf("issue iid = %d, want 3", body.Issues[0].IID) + } + if body.Count != len(body.Issues) { + t.Errorf("count = %d but issues = %d; the two must agree", body.Count, len(body.Issues)) + } +} + +// --- Stats --- + +func TestHTTPGetStatsReturnsWorkspaceCounts(t *testing.T) { + svc, _, router := newWatchHTTPFixture(t) + client := svc.workspaceClients["ws-1"].(*MockClient) + client.SetUser("alice") + client.SeedMR("group/p", &MR{IID: 1, Title: "one", State: "opened"}) + client.SeedIssue("group/p", &Issue{IID: 2, Title: "bug", State: "opened"}) + + var stats Stats + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/stats?workspace_id=ws-1", nil, &stats) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if stats.OpenMRs != 1 { + t.Errorf("open_mrs = %d, want 1", stats.OpenMRs) + } + if stats.OpenIssuesAssignedMe != 1 { + t.Errorf("open_issues_assigned_me = %d, want 1", stats.OpenIssuesAssignedMe) + } +} + +// TestHTTPGetStatsIsScopedToItsWorkspace proves the stats route reads the +// workspace's own client rather than a shared one: ws-2 has nothing seeded, so +// its counts must be zero while ws-1's are not. +func TestHTTPGetStatsIsScopedToItsWorkspace(t *testing.T) { + svc, _, router := newWatchHTTPFixture(t) + client := svc.workspaceClients["ws-1"].(*MockClient) + client.SetUser("alice") + client.SeedMR("group/p", &MR{IID: 1, Title: "one", State: "opened"}) + svc.workspaceClients["ws-2"].(*MockClient).SetUser("bob") + + var stats Stats + recorder := httpJSON(t, router, http.MethodGet, + "/api/v1/gitlab/stats?workspace_id=ws-2", nil, &stats) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if stats.OpenMRs != 0 { + t.Errorf("open_mrs = %d for ws-2, want 0 — ws-1's MRs must not leak", stats.OpenMRs) + } +} diff --git a/apps/backend/internal/gitlab/handlers_registration_test.go b/apps/backend/internal/gitlab/handlers_registration_test.go new file mode 100644 index 0000000000..c4da8b5466 --- /dev/null +++ b/apps/backend/internal/gitlab/handlers_registration_test.go @@ -0,0 +1,200 @@ +package gitlab + +import ( + "context" + "testing" + + "github.com/gin-gonic/gin" + + ws "github.com/kandev/kandev/pkg/websocket" +) + +// gitLabWSActions is every action registerWSHandlers wires. Kept explicit so a +// handler added to handlers.go without a corresponding entry here fails +// TestRegisterWSHandlersCoversEveryGitLabAction rather than going untested. +var gitLabWSActions = []string{ + ws.ActionGitLabStatus, + ws.ActionGitLabTaskMRsList, + ws.ActionGitLabTaskMRGet, + ws.ActionGitLabMRFeedbackGet, + ws.ActionGitLabReviewWatchesList, + ws.ActionGitLabReviewWatchCreate, + ws.ActionGitLabReviewWatchUpdate, + ws.ActionGitLabReviewWatchDelete, + ws.ActionGitLabReviewTrigger, + ws.ActionGitLabReviewTriggerAll, + ws.ActionGitLabMRWatchesList, + ws.ActionGitLabMRWatchDelete, + ws.ActionGitLabMRFilesGet, + ws.ActionGitLabMRCommitsGet, + ws.ActionGitLabTaskMRSync, + ws.ActionGitLabStats, + ws.ActionGitLabMRMerge, + ws.ActionGitLabMRApprove, + ws.ActionGitLabMRUnapprove, + ws.ActionGitLabMRSetLabels, + ws.ActionGitLabMRSetAssignees, + ws.ActionGitLabMRDiscussionNew, + ws.ActionGitLabMRDiscussionResolve, + ws.ActionGitLabProjectMergeMethodsGet, + ws.ActionGitLabIssueWatchesList, + ws.ActionGitLabIssueWatchCreate, + ws.ActionGitLabIssueWatchUpdate, + ws.ActionGitLabIssueWatchDelete, + ws.ActionGitLabIssueTrigger, + ws.ActionGitLabIssueTriggerAll, + ws.ActionGitLabActionPresetsList, + ws.ActionGitLabActionPresetsUpdate, + ws.ActionGitLabActionPresetsReset, + ws.ActionGitLabListUserProjects, + ws.ActionGitLabSearchProjects, + ws.ActionGitLabProjectBranches, + ws.ActionGitLabCleanupReviewTasks, + ws.ActionGitLabCleanupIssueTasks, +} + +func TestRegisterWSHandlersCoversEveryGitLabAction(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + dispatcher := ws.NewDispatcher() + + registerWSHandlers(dispatcher, svc, log) + + for _, action := range gitLabWSActions { + if !dispatcher.HasHandler(action) { + t.Errorf("action %q has no handler registered", action) + } + } + if got := dispatcher.HandlerCount(); got != len(gitLabWSActions) { + t.Errorf("handler count = %d, want %d; a handler was added or removed "+ + "without updating gitLabWSActions", got, len(gitLabWSActions)) + } +} + +// TestRegisterRoutesWithDispatcherLeavesWSSurfaceUnmounted pins the +// authorization boundary documented in handlers.go: the legacy GitLab WS +// handlers are workspace-unscoped, so RegisterRoutesWithDispatcher must mount +// the HTTP surface only and leave the dispatcher untouched. Wiring +// registerWSHandlers back in here would silently expose every workspace's +// watches over a connection that carries no workspace scope. +func TestRegisterRoutesWithDispatcherLeavesWSSurfaceUnmounted(t *testing.T) { + gin.SetMode(gin.TestMode) + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + dispatcher := ws.NewDispatcher() + router := gin.New() + + RegisterRoutesWithDispatcher(router, dispatcher, svc, log) + + if got := dispatcher.HandlerCount(); got != 0 { + t.Fatalf("dispatcher has %d handlers, want 0 — the GitLab WS surface is "+ + "deliberately unmounted because it cannot carry the workspace "+ + "authorization boundary the HTTP routes enforce", got) + } + for _, action := range gitLabWSActions { + if dispatcher.HasHandler(action) { + t.Errorf("action %q was mounted on the dispatcher", action) + } + } +} + +func TestRegisterRoutesWithDispatcherMountsHTTPRoutes(t *testing.T) { + gin.SetMode(gin.TestMode) + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + router := gin.New() + + RegisterRoutesWithDispatcher(router, ws.NewDispatcher(), svc, log) + + want := map[string]string{ + "GET": "/api/v1/gitlab/status", + "POST": "/api/v1/gitlab/watches/review", + "DELETE": "/api/v1/gitlab/watches/review/:id", + "PUT": "/api/v1/gitlab/action-presets", + } + routes := map[string]bool{} + for _, route := range router.Routes() { + routes[route.Method+" "+route.Path] = true + } + for method, path := range want { + if !routes[method+" "+path] { + t.Errorf("route %s %s is not registered", method, path) + } + } +} + +// TestRegisterRoutesMatchesDispatcherVariant guards the package-level +// RegisterRoutes entrypoint against drifting from the dispatcher-aware sibling: +// both must mount exactly the same HTTP surface. +func TestRegisterRoutesMatchesDispatcherVariant(t *testing.T) { + gin.SetMode(gin.TestMode) + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + + plain := gin.New() + RegisterRoutes(plain, svc, log) + withDispatcher := gin.New() + RegisterRoutesWithDispatcher(withDispatcher, ws.NewDispatcher(), svc, log) + + collect := func(router *gin.Engine) map[string]bool { + out := map[string]bool{} + for _, route := range router.Routes() { + out[route.Method+" "+route.Path] = true + } + return out + } + plainRoutes, dispatcherRoutes := collect(plain), collect(withDispatcher) + if len(plainRoutes) == 0 { + t.Fatal("RegisterRoutes mounted no routes at all") + } + for route := range plainRoutes { + if !dispatcherRoutes[route] { + t.Errorf("route %q is missing from RegisterRoutesWithDispatcher", route) + } + } + for route := range dispatcherRoutes { + if !plainRoutes[route] { + t.Errorf("route %q is missing from RegisterRoutes", route) + } + } +} + +func TestRegisterMockRoutesIsNoOpForNonMockClient(t *testing.T) { + gin.SetMode(gin.TestMode) + svc, _, log := newWSFixture(t, NewNoopClient(DefaultHost)) + router := gin.New() + + RegisterMockRoutes(router, svc, log) + + if got := len(router.Routes()); got != 0 { + t.Errorf("mock routes = %d, want 0 for a non-mock client (%v)", got, router.Routes()) + } +} + +func TestRegisterMockRoutesMountsControlEndpointsForMockClient(t *testing.T) { + gin.SetMode(gin.TestMode) + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + router := gin.New() + + RegisterMockRoutes(router, svc, log) + + if len(router.Routes()) == 0 { + t.Fatal("no mock control endpoints were registered for a MockClient") + } +} + +// TestWSHandlersDispatchThroughDispatcher proves the registered closures are +// reachable by action name and return their own action in the reply, which is +// what lets the frontend correlate a response with the request that caused it. +func TestWSHandlersDispatchThroughDispatcher(t *testing.T) { + client := NewMockClient(DefaultHost) + client.SetUser("alice") + svc, _, log := newWSFixture(t, client) + dispatcher := ws.NewDispatcher() + registerWSHandlers(dispatcher, svc, log) + + resp, err := dispatcher.Dispatch(context.Background(), + wsRequest(t, ws.ActionGitLabStatus, nil)) + + var status Status + wsOK(t, resp, err, ws.ActionGitLabStatus, &status) + if status.Username != "alice" { + t.Errorf("username = %q, want alice", status.Username) + } +} diff --git a/apps/backend/internal/gitlab/handlers_ws_actions_test.go b/apps/backend/internal/gitlab/handlers_ws_actions_test.go new file mode 100644 index 0000000000..063524ac97 --- /dev/null +++ b/apps/backend/internal/gitlab/handlers_ws_actions_test.go @@ -0,0 +1,701 @@ +package gitlab + +import ( + "context" + "errors" + "testing" + + "github.com/kandev/kandev/internal/common/logger" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// actionRecordingClient captures the arguments each write handler forwards to +// the GitLab client, so the tests can assert the exact upstream call rather +// than only the response envelope. Any method left unset delegates to the +// embedded MockClient. +type actionRecordingClient struct { + *MockClient + + labelCalls []labelCall + assigneeCalls []assigneeCall + noteCalls []noteCall + resolveCalls []resolveCall + mergeMethods *ProjectMergeMethods + mergeErr error +} + +type labelCall struct { + Project string + IID int + Labels []string +} + +type assigneeCall struct { + Project string + IID int + IDs []int +} + +type noteCall struct { + Project string + IID int + DiscussionID string + Body string +} + +type resolveCall struct { + Project string + IID int + DiscussionID string +} + +func (c *actionRecordingClient) SetMRLabels(_ context.Context, project string, iid int, labels []string) error { + c.labelCalls = append(c.labelCalls, labelCall{Project: project, IID: iid, Labels: labels}) + return nil +} + +func (c *actionRecordingClient) SetMRAssignees(_ context.Context, project string, iid int, ids []int) error { + c.assigneeCalls = append(c.assigneeCalls, assigneeCall{Project: project, IID: iid, IDs: ids}) + return nil +} + +func (c *actionRecordingClient) CreateMRDiscussionNote(_ context.Context, project string, iid int, discussionID, body string) (*MRNote, error) { + c.noteCalls = append(c.noteCalls, noteCall{Project: project, IID: iid, DiscussionID: discussionID, Body: body}) + return &MRNote{ID: 99, Body: body}, nil +} + +func (c *actionRecordingClient) ResolveMRDiscussion(_ context.Context, project string, iid int, discussionID string) error { + c.resolveCalls = append(c.resolveCalls, resolveCall{Project: project, IID: iid, DiscussionID: discussionID}) + return nil +} + +func (c *actionRecordingClient) GetProjectMergeMethods(ctx context.Context, project string) (*ProjectMergeMethods, error) { + if c.mergeErr != nil { + return nil, c.mergeErr + } + if c.mergeMethods != nil { + return c.mergeMethods, nil + } + return c.MockClient.GetProjectMergeMethods(ctx, project) +} + +func newActionFixture(t *testing.T) (*Service, *actionRecordingClient, *logger.Logger) { + t.Helper() + client := &actionRecordingClient{MockClient: NewMockClient(DefaultHost)} + client.SetUser("alice") + svc, _, log := newWSFixture(t, client) + return svc, client, log +} + +// projectAndIIDValidationCases is shared by every handler whose guard is the +// same "project and iid required" triple. +func projectAndIIDValidationCases(t *testing.T, action string, handler ws.HandlerFunc) { + t.Helper() + cases := map[string]*ws.Message{ + "missing project": wsRequest(t, action, map[string]interface{}{"iid": 1}), + "missing iid": wsRequest(t, action, map[string]interface{}{"project": "g/p"}), + "malformed": wsRawRequest(action, `{"iid":"one"}`), + } + for name, msg := range cases { + t.Run(name, func(t *testing.T) { + resp, err := handler(context.Background(), msg) + wsErr(t, resp, err, action, ws.ErrorCodeBadRequest, "project and iid required") + }) + } +} + +// --- Merge --- + +func TestWSMergeMRMergesAndReturnsMR(t *testing.T) { + svc, client, log := newActionFixture(t) + iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"}) + + resp, err := wsMergeMR(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRMerge, map[string]interface{}{ + "project": "group/p", "iid": iid, "method": "merge", + })) + + var mr MR + wsOK(t, resp, err, ws.ActionGitLabMRMerge, &mr) + if mr.State != "merged" { + t.Errorf("state = %q, want merged", mr.State) + } +} + +// TestWSMergeMRSurfacesDisallowedMethod pins that method validation happens +// before the merge call: the project below allows only merge commits, so a +// fast-forward request must fail rather than silently merging some other way. +func TestWSMergeMRSurfacesDisallowedMethod(t *testing.T) { + svc, client, log := newActionFixture(t) + client.mergeMethods = &ProjectMergeMethods{Merge: true} + iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"}) + + resp, err := wsMergeMR(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRMerge, map[string]interface{}{ + "project": "group/p", "iid": iid, "method": "ff", + })) + + payload := wsErr(t, resp, err, ws.ActionGitLabMRMerge, ws.ErrorCodeInternalError, "") + if payload.Message != "project does not allow fast-forward merge" { + t.Errorf("message = %q, want the fast-forward rejection", payload.Message) + } + mr, getErr := client.GetMR(context.Background(), "group/p", iid) + if getErr != nil { + t.Fatalf("reload MR: %v", getErr) + } + if mr.State == "merged" { + t.Error("MR was merged despite the disallowed method") + } +} + +func TestWSMergeMRValidatesProjectAndIID(t *testing.T) { + svc, _, log := newActionFixture(t) + projectAndIIDValidationCases(t, ws.ActionGitLabMRMerge, wsMergeMR(svc, log)) +} + +// --- Approve / unapprove --- + +func TestWSApproveMRRecordsApproval(t *testing.T) { + svc, client, log := newActionFixture(t) + iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"}) + + resp, err := wsApproveMR(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRApprove, map[string]interface{}{"project": "group/p", "iid": iid})) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabMRApprove, &body) + if !body["approved"] { + t.Errorf("payload = %v, want {\"approved\":true}", body) + } + approvals, err := client.ListMRApprovals(context.Background(), "group/p", iid) + if err != nil { + t.Fatalf("list approvals: %v", err) + } + if len(approvals) != 1 || approvals[0].Username != "alice" { + t.Errorf("approvals = %+v, want alice's single approval", approvals) + } +} + +func TestWSApproveMRSurfacesUpstreamFailure(t *testing.T) { + svc, _, log := newActionFixture(t) + + resp, err := wsApproveMR(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRApprove, map[string]interface{}{"project": "group/p", "iid": 404})) + + wsErr(t, resp, err, ws.ActionGitLabMRApprove, ws.ErrorCodeInternalError, "") +} + +func TestWSApproveMRValidatesProjectAndIID(t *testing.T) { + svc, _, log := newActionFixture(t) + projectAndIIDValidationCases(t, ws.ActionGitLabMRApprove, wsApproveMR(svc, log)) +} + +func TestWSUnapproveMRRemovesApproval(t *testing.T) { + svc, client, log := newActionFixture(t) + iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"}) + client.SeedApprovals("group/p", iid, []MRApproval{{Username: "alice"}}, 1) + + resp, err := wsUnapproveMR(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRUnapprove, map[string]interface{}{"project": "group/p", "iid": iid})) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabMRUnapprove, &body) + if !body["unapproved"] { + t.Errorf("payload = %v, want {\"unapproved\":true}", body) + } + approvals, err := client.ListMRApprovals(context.Background(), "group/p", iid) + if err != nil { + t.Fatalf("list approvals: %v", err) + } + if len(approvals) != 0 { + t.Errorf("approvals = %+v, want alice's approval revoked", approvals) + } +} + +func TestWSUnapproveMRValidatesProjectAndIID(t *testing.T) { + svc, _, log := newActionFixture(t) + projectAndIIDValidationCases(t, ws.ActionGitLabMRUnapprove, wsUnapproveMR(svc, log)) +} + +// --- Labels / assignees --- + +func TestWSSetMRLabelsForwardsExactLabels(t *testing.T) { + svc, client, log := newActionFixture(t) + + resp, err := wsSetMRLabels(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRSetLabels, map[string]interface{}{ + "project": "group/p", "iid": 5, "labels": []string{"bug", "p1"}, + })) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabMRSetLabels, &body) + if !body["updated"] { + t.Errorf("payload = %v, want {\"updated\":true}", body) + } + if len(client.labelCalls) != 1 { + t.Fatalf("label calls = %d, want exactly 1", len(client.labelCalls)) + } + call := client.labelCalls[0] + if call.Project != "group/p" || call.IID != 5 { + t.Errorf("call target = %s!%d, want group/p!5", call.Project, call.IID) + } + if len(call.Labels) != 2 || call.Labels[0] != "bug" || call.Labels[1] != "p1" { + t.Errorf("labels = %v, want [bug p1] in order", call.Labels) + } +} + +// TestWSSetMRLabelsForwardsEmptyListToClearLabels guards the "remove every +// label" case: an empty list must still reach the client, because dropping the +// call would silently leave the old labels in place. +func TestWSSetMRLabelsForwardsEmptyListToClearLabels(t *testing.T) { + svc, client, log := newActionFixture(t) + + resp, err := wsSetMRLabels(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRSetLabels, map[string]interface{}{ + "project": "group/p", "iid": 5, "labels": []string{}, + })) + + wsOK(t, resp, err, ws.ActionGitLabMRSetLabels, nil) + if len(client.labelCalls) != 1 { + t.Fatalf("label calls = %d, want 1 even for an empty list", len(client.labelCalls)) + } + if len(client.labelCalls[0].Labels) != 0 { + t.Errorf("labels = %v, want an empty list", client.labelCalls[0].Labels) + } +} + +func TestWSSetMRLabelsValidatesProjectAndIID(t *testing.T) { + svc, _, log := newActionFixture(t) + projectAndIIDValidationCases(t, ws.ActionGitLabMRSetLabels, wsSetMRLabels(svc, log)) +} + +func TestWSSetMRAssigneesForwardsExactIDs(t *testing.T) { + svc, client, log := newActionFixture(t) + + resp, err := wsSetMRAssignees(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRSetAssignees, map[string]interface{}{ + "project": "group/p", "iid": 6, "assignee_ids": []int{7, 8}, + })) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabMRSetAssignees, &body) + if !body["updated"] { + t.Errorf("payload = %v, want {\"updated\":true}", body) + } + if len(client.assigneeCalls) != 1 { + t.Fatalf("assignee calls = %d, want exactly 1", len(client.assigneeCalls)) + } + call := client.assigneeCalls[0] + if call.Project != "group/p" || call.IID != 6 { + t.Errorf("call target = %s!%d, want group/p!6", call.Project, call.IID) + } + if len(call.IDs) != 2 || call.IDs[0] != 7 || call.IDs[1] != 8 { + t.Errorf("assignee ids = %v, want [7 8]", call.IDs) + } +} + +func TestWSSetMRAssigneesValidatesProjectAndIID(t *testing.T) { + svc, _, log := newActionFixture(t) + projectAndIIDValidationCases(t, ws.ActionGitLabMRSetAssignees, wsSetMRAssignees(svc, log)) +} + +// --- Discussions --- + +func TestWSNewDiscussionNoteForwardsBody(t *testing.T) { + svc, client, log := newActionFixture(t) + + resp, err := wsNewDiscussionNote(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRDiscussionNew, map[string]interface{}{ + "project": "group/p", "iid": 9, "discussion_id": "disc-1", "body": "looks good", + })) + + var note MRNote + wsOK(t, resp, err, ws.ActionGitLabMRDiscussionNew, ¬e) + if note.Body != "looks good" { + t.Errorf("note body = %q, want the posted body echoed back", note.Body) + } + if len(client.noteCalls) != 1 { + t.Fatalf("note calls = %d, want exactly 1", len(client.noteCalls)) + } + if client.noteCalls[0] != (noteCall{Project: "group/p", IID: 9, DiscussionID: "disc-1", Body: "looks good"}) { + t.Errorf("note call = %+v, want the exact request arguments", client.noteCalls[0]) + } +} + +func TestWSNewDiscussionNoteRequiresProjectIIDAndDiscussion(t *testing.T) { + svc, _, log := newActionFixture(t) + handler := wsNewDiscussionNote(svc, log) + + cases := map[string]interface{}{ + "missing project": map[string]interface{}{"iid": 1, "discussion_id": "d", "body": "b"}, + "missing iid": map[string]interface{}{"project": "g/p", "discussion_id": "d", "body": "b"}, + "missing discussion": map[string]interface{}{"project": "g/p", "iid": 1, "body": "b"}, + } + for name, payload := range cases { + t.Run(name, func(t *testing.T) { + resp, err := handler(context.Background(), wsRequest(t, ws.ActionGitLabMRDiscussionNew, payload)) + wsErr(t, resp, err, ws.ActionGitLabMRDiscussionNew, ws.ErrorCodeBadRequest, + "project, iid, discussion_id required") + }) + } +} + +// TestWSNewDiscussionNoteRejectsBlankBody covers the separate whitespace guard: +// a body of only spaces is rejected with its own reason and never reaches the +// client, so no empty comment is posted upstream. +func TestWSNewDiscussionNoteRejectsBlankBody(t *testing.T) { + svc, client, log := newActionFixture(t) + + resp, err := wsNewDiscussionNote(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRDiscussionNew, map[string]interface{}{ + "project": "group/p", "iid": 9, "discussion_id": "disc-1", "body": " \t\n ", + })) + + wsErr(t, resp, err, ws.ActionGitLabMRDiscussionNew, ws.ErrorCodeBadRequest, "body required") + if len(client.noteCalls) != 0 { + t.Errorf("note calls = %+v, want none for a blank body", client.noteCalls) + } +} + +func TestWSResolveDiscussionForwardsDiscussionID(t *testing.T) { + svc, client, log := newActionFixture(t) + + resp, err := wsResolveDiscussion(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRDiscussionResolve, map[string]interface{}{ + "project": "group/p", "iid": 9, "discussion_id": "disc-1", + })) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabMRDiscussionResolve, &body) + if !body["resolved"] { + t.Errorf("payload = %v, want {\"resolved\":true}", body) + } + if len(client.resolveCalls) != 1 || + client.resolveCalls[0] != (resolveCall{Project: "group/p", IID: 9, DiscussionID: "disc-1"}) { + t.Errorf("resolve calls = %+v, want the exact request arguments", client.resolveCalls) + } +} + +func TestWSResolveDiscussionRequiresDiscussionID(t *testing.T) { + svc, _, log := newActionFixture(t) + + resp, err := wsResolveDiscussion(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRDiscussionResolve, map[string]interface{}{ + "project": "group/p", "iid": 9, + })) + + wsErr(t, resp, err, ws.ActionGitLabMRDiscussionResolve, ws.ErrorCodeBadRequest, + "project, iid, discussion_id required") +} + +// --- Merge methods --- + +func TestWSGetProjectMergeMethodsReturnsAllowedMethods(t *testing.T) { + svc, client, log := newActionFixture(t) + client.mergeMethods = &ProjectMergeMethods{Merge: true, AllowSquash: true} + + resp, err := wsGetProjectMergeMethods(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabProjectMergeMethodsGet, map[string]string{"project": "group/p"})) + + var methods ProjectMergeMethods + wsOK(t, resp, err, ws.ActionGitLabProjectMergeMethodsGet, &methods) + if !methods.Merge || !methods.AllowSquash || methods.FastForward { + t.Errorf("methods = %+v, want merge+squash only", methods) + } +} + +func TestWSGetProjectMergeMethodsRequiresProject(t *testing.T) { + svc, _, log := newActionFixture(t) + + resp, err := wsGetProjectMergeMethods(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabProjectMergeMethodsGet, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabProjectMergeMethodsGet, ws.ErrorCodeBadRequest, "project required") +} + +func TestWSGetProjectMergeMethodsSurfacesUpstreamFailure(t *testing.T) { + svc, client, log := newActionFixture(t) + client.mergeErr = errors.New("gitlab unreachable") + + resp, err := wsGetProjectMergeMethods(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabProjectMergeMethodsGet, map[string]string{"project": "group/p"})) + + wsErr(t, resp, err, ws.ActionGitLabProjectMergeMethodsGet, ws.ErrorCodeInternalError, "gitlab unreachable") +} + +// --- Projects --- + +func TestWSListUserProjectsReturnsProjects(t *testing.T) { + svc, _, log := newActionFixture(t) + + resp, err := wsListUserProjects(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabListUserProjects, nil)) + + var body struct { + Projects []Project `json:"projects"` + } + wsOK(t, resp, err, ws.ActionGitLabListUserProjects, &body) + if len(body.Projects) != 1 || body.Projects[0].PathWithNamespace != "kandev/sample" { + t.Fatalf("projects = %+v, want the single kandev/sample project", body.Projects) + } +} + +func TestWSSearchProjectsFiltersByQuery(t *testing.T) { + svc, _, log := newActionFixture(t) + handler := wsSearchProjects(svc, log) + + tests := []struct { + name string + query string + want int + }{ + {"matching query", "sample", 1}, + {"non-matching query", "nothing-like-this", 0}, + {"empty query returns all", "", 1}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resp, err := handler(context.Background(), + wsRequest(t, ws.ActionGitLabSearchProjects, map[string]interface{}{ + "query": test.query, "limit": 10, + })) + + var body struct { + Projects []Project `json:"projects"` + } + wsOK(t, resp, err, ws.ActionGitLabSearchProjects, &body) + if len(body.Projects) != test.want { + t.Errorf("projects = %d, want %d (%+v)", len(body.Projects), test.want, body.Projects) + } + }) + } +} + +func TestWSSearchProjectsRejectsMalformedPayload(t *testing.T) { + svc, _, log := newActionFixture(t) + + resp, err := wsSearchProjects(svc, log)(context.Background(), + wsRawRequest(ws.ActionGitLabSearchProjects, `{"limit":"ten"}`)) + + wsErr(t, resp, err, ws.ActionGitLabSearchProjects, ws.ErrorCodeBadRequest, errMsgInvalidPayload) +} + +func TestWSListProjectBranchesReturnsBranches(t *testing.T) { + svc, client, log := newActionFixture(t) + client.SeedBranches("group/p", []RepoBranch{{Name: "main"}, {Name: "dev"}}) + + resp, err := wsListProjectBranches(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabProjectBranches, map[string]string{"project": "group/p"})) + + var body struct { + Branches []RepoBranch `json:"branches"` + } + wsOK(t, resp, err, ws.ActionGitLabProjectBranches, &body) + if len(body.Branches) != 2 || body.Branches[0].Name != "main" { + t.Fatalf("branches = %+v, want [main dev]", body.Branches) + } +} + +func TestWSListProjectBranchesRequiresProject(t *testing.T) { + svc, _, log := newActionFixture(t) + + resp, err := wsListProjectBranches(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabProjectBranches, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabProjectBranches, ws.ErrorCodeBadRequest, "project required") +} + +// --- Action presets --- + +func TestWSListActionPresetsReturnsDefaultsForNewWorkspace(t *testing.T) { + svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsListActionPresets(svc)(context.Background(), + wsRequest(t, ws.ActionGitLabActionPresetsList, map[string]string{"workspace_id": "ws-1"})) + + var presets ActionPresets + wsOK(t, resp, err, ws.ActionGitLabActionPresetsList, &presets) + if len(presets.MR) == 0 || len(presets.Issue) == 0 { + t.Fatalf("presets = %+v, want the built-in defaults substituted", presets) + } +} + +func TestWSListActionPresetsRequiresWorkspaceID(t *testing.T) { + svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsListActionPresets(svc)(context.Background(), + wsRequest(t, ws.ActionGitLabActionPresetsList, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabActionPresetsList, ws.ErrorCodeBadRequest, "workspace_id required") +} + +func TestWSUpdateActionPresetsPersistsMRList(t *testing.T) { + svc, store, _ := newWSFixture(t, NewMockClient(DefaultHost)) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + + resp, err := wsUpdateActionPresets(svc)(ctx, + wsRequest(t, ws.ActionGitLabActionPresetsUpdate, UpdateActionPresetsRequest{ + WorkspaceID: "ws-1", + MR: &[]ActionPreset{{ID: "ship", Label: "Ship it", PromptTemplate: "ship {{mr}}"}}, + })) + + var presets ActionPresets + wsOK(t, resp, err, ws.ActionGitLabActionPresetsUpdate, &presets) + if len(presets.MR) != 1 || presets.MR[0].ID != "ship" { + t.Fatalf("mr presets = %+v, want only the custom ship preset", presets.MR) + } + stored, err := store.GetActionPresets(ctx, "ws-1") + if err != nil { + t.Fatalf("reload presets: %v", err) + } + if len(stored.MR) != 1 || stored.MR[0].Label != "Ship it" { + t.Errorf("stored mr presets = %+v, want the custom preset persisted", stored.MR) + } +} + +func TestWSUpdateActionPresetsRequiresWorkspaceID(t *testing.T) { + svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsUpdateActionPresets(svc)(context.Background(), + wsRequest(t, ws.ActionGitLabActionPresetsUpdate, UpdateActionPresetsRequest{})) + + wsErr(t, resp, err, ws.ActionGitLabActionPresetsUpdate, ws.ErrorCodeBadRequest, "workspace_id required") +} + +func TestWSResetActionPresetsClearsStoredOverrides(t *testing.T) { + svc, store, _ := newWSFixture(t, NewMockClient(DefaultHost)) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + if err := store.UpsertActionPresets(ctx, &ActionPresets{ + WorkspaceID: "ws-1", + MR: []ActionPreset{{ID: "ship", Label: "Ship it", PromptTemplate: "ship"}}, + }); err != nil { + t.Fatalf("seed presets: %v", err) + } + + resp, err := wsResetActionPresets(svc)(ctx, + wsRequest(t, ws.ActionGitLabActionPresetsReset, map[string]string{"workspace_id": "ws-1"})) + + var presets ActionPresets + wsOK(t, resp, err, ws.ActionGitLabActionPresetsReset, &presets) + for _, preset := range presets.MR { + if preset.ID == "ship" { + t.Fatalf("mr presets = %+v, want the custom preset gone after reset", presets.MR) + } + } + stored, err := store.GetActionPresets(ctx, "ws-1") + if err != nil { + t.Fatalf("reload presets: %v", err) + } + if len(stored.MR) != 0 { + t.Errorf("stored mr presets = %+v, want the row deleted", stored.MR) + } +} + +func TestWSResetActionPresetsRequiresWorkspaceID(t *testing.T) { + svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsResetActionPresets(svc)(context.Background(), + wsRequest(t, ws.ActionGitLabActionPresetsReset, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabActionPresetsReset, ws.ErrorCodeBadRequest, "workspace_id required") +} + +// --- Cleanup --- + +// cleanupTaskDeleter records the task IDs a cleanup sweep deletes. +type cleanupTaskDeleter struct{ ids []string } + +func (d *cleanupTaskDeleter) DeleteTask(_ context.Context, taskID string) error { + d.ids = append(d.ids, taskID) + return nil +} + +func TestWSCleanupReviewTasksDeletesTasksForMergedMRs(t *testing.T) { + client := NewMockClient(DefaultHost) + svc, store, _ := newWSFixture(t, client) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + deleter := &cleanupTaskDeleter{} + svc.SetTaskDeleter(deleter) + + const project, iid = "team/repo", 7 + watch := &ReviewWatch{WorkspaceID: "ws-1", Enabled: true, CleanupPolicy: CleanupPolicyAlways} + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("create review watch: %v", err) + } + if _, err := store.ReserveReviewMRTask(ctx, watch.ID, project, iid, "url"); err != nil { + t.Fatalf("reserve review MR task: %v", err) + } + if err := store.AssignReviewMRTaskID(ctx, watch.ID, project, iid, "task-merged"); err != nil { + t.Fatalf("assign review MR task: %v", err) + } + client.SeedMR(project, &MR{IID: iid, State: gitlabStateMerged}) + + resp, err := wsCleanupReviewTasks(svc)(ctx, + wsRequest(t, ws.ActionGitLabCleanupReviewTasks, nil)) + + var body map[string]int + wsOK(t, resp, err, ws.ActionGitLabCleanupReviewTasks, &body) + if body[respKeyDeleted] != 1 { + t.Errorf("payload = %v, want {%q:1}", body, respKeyDeleted) + } + if len(deleter.ids) != 1 || deleter.ids[0] != "task-merged" { + t.Errorf("deleted task ids = %v, want [task-merged]", deleter.ids) + } +} + +func TestWSCleanupReviewTasksReportsMissingDeleterAsInternalError(t *testing.T) { + svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsCleanupReviewTasks(svc)(context.Background(), + wsRequest(t, ws.ActionGitLabCleanupReviewTasks, nil)) + + wsErr(t, resp, err, ws.ActionGitLabCleanupReviewTasks, ws.ErrorCodeInternalError, + "task deleter not configured") +} + +func TestWSCleanupIssueTasksDeletesTasksForClosedIssues(t *testing.T) { + client := NewMockClient(DefaultHost) + svc, store, _ := newWSFixture(t, client) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + deleter := &cleanupTaskDeleter{} + svc.SetTaskDeleter(deleter) + + const project, iid = "team/repo", 5 + watch := &IssueWatch{WorkspaceID: "ws-1", Enabled: true, CleanupPolicy: CleanupPolicyAlways} + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("create issue watch: %v", err) + } + if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, project, iid, "url"); err != nil { + t.Fatalf("reserve issue task: %v", err) + } + if err := store.AssignIssueWatchTaskID(ctx, watch.ID, project, iid, "task-closed"); err != nil { + t.Fatalf("assign issue task: %v", err) + } + client.SeedIssue(project, &Issue{IID: iid, State: gitlabStateClosed}) + + resp, err := wsCleanupIssueTasks(svc)(ctx, + wsRequest(t, ws.ActionGitLabCleanupIssueTasks, nil)) + + var body map[string]int + wsOK(t, resp, err, ws.ActionGitLabCleanupIssueTasks, &body) + if body[respKeyDeleted] != 1 { + t.Errorf("payload = %v, want {%q:1}", body, respKeyDeleted) + } + if len(deleter.ids) != 1 || deleter.ids[0] != "task-closed" { + t.Errorf("deleted task ids = %v, want [task-closed]", deleter.ids) + } +} + +func TestWSCleanupIssueTasksReportsMissingDeleterAsInternalError(t *testing.T) { + svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsCleanupIssueTasks(svc)(context.Background(), + wsRequest(t, ws.ActionGitLabCleanupIssueTasks, nil)) + + wsErr(t, resp, err, ws.ActionGitLabCleanupIssueTasks, ws.ErrorCodeInternalError, + "task deleter not configured") +} diff --git a/apps/backend/internal/gitlab/handlers_ws_test.go b/apps/backend/internal/gitlab/handlers_ws_test.go new file mode 100644 index 0000000000..c51ab4e620 --- /dev/null +++ b/apps/backend/internal/gitlab/handlers_ws_test.go @@ -0,0 +1,484 @@ +package gitlab + +import ( + "context" + "encoding/json" + "errors" + "testing" + + "github.com/kandev/kandev/internal/common/logger" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// --- Shared WS handler fixture --- +// +// The WS surface in handlers.go is the legacy, workspace-unscoped GitLab +// action set: every handler closes over the package Service and translates a +// ws.Message into one service call. These helpers drive a handler directly +// and decode the envelope it returns, so the tests can assert the wire +// contract (error code, reason string, payload keys) rather than merely +// executing the closure. + +func newWSFixture(t *testing.T, client Client) (*Service, *Store, *logger.Logger) { + t.Helper() + log := newTestLogger(t) + store := newTestStore(t) + svc := NewService(DefaultHost, client, AuthMethodPAT, nil, log) + svc.SetStore(store) + return svc, store, log +} + +// wsRequest builds a request envelope with payload marshalled as JSON. +func wsRequest(t *testing.T, action string, payload interface{}) *ws.Message { + t.Helper() + msg, err := ws.NewRequest("req-42", action, payload) + if err != nil { + t.Fatalf("build %s request: %v", action, err) + } + return msg +} + +// wsRawRequest builds a request envelope carrying a verbatim payload, used to +// exercise the malformed-payload branches that a marshalled Go value cannot +// produce. +func wsRawRequest(action, rawPayload string) *ws.Message { + return &ws.Message{ + ID: "req-42", + Type: ws.MessageTypeRequest, + Action: action, + Payload: json.RawMessage(rawPayload), + } +} + +// wsOK asserts a success envelope that echoes the request's correlation ID and +// action, then decodes the payload into out. +func wsOK(t *testing.T, resp *ws.Message, err error, action string, out interface{}) { + t.Helper() + if err != nil { + t.Fatalf("handler returned a transport error: %v", err) + } + if resp == nil { + t.Fatal("handler returned a nil message") + } + if resp.Type != ws.MessageTypeResponse { + t.Fatalf("type = %q, want %q (payload: %s)", resp.Type, ws.MessageTypeResponse, resp.Payload) + } + if resp.ID != "req-42" { + t.Errorf("id = %q, want the request id echoed back", resp.ID) + } + if resp.Action != action { + t.Errorf("action = %q, want %q", resp.Action, action) + } + if out == nil { + return + } + if decodeErr := json.Unmarshal(resp.Payload, out); decodeErr != nil { + t.Fatalf("decode payload %s: %v", resp.Payload, decodeErr) + } +} + +// wsErr asserts an error envelope with the expected code and reason and +// returns the decoded error payload. +func wsErr(t *testing.T, resp *ws.Message, err error, action, code, reason string) ws.ErrorPayload { + t.Helper() + if err != nil { + t.Fatalf("handler returned a transport error: %v", err) + } + if resp == nil { + t.Fatal("handler returned a nil message") + } + if resp.Type != ws.MessageTypeError { + t.Fatalf("type = %q, want %q (payload: %s)", resp.Type, ws.MessageTypeError, resp.Payload) + } + if resp.ID != "req-42" { + t.Errorf("id = %q, want the request id echoed back", resp.ID) + } + if resp.Action != action { + t.Errorf("action = %q, want %q", resp.Action, action) + } + var payload ws.ErrorPayload + if decodeErr := json.Unmarshal(resp.Payload, &payload); decodeErr != nil { + t.Fatalf("decode error payload %s: %v", resp.Payload, decodeErr) + } + if payload.Code != code { + t.Errorf("error code = %q, want %q", payload.Code, code) + } + if reason != "" && payload.Message != reason { + t.Errorf("error message = %q, want %q", payload.Message, reason) + } + return payload +} + +// failingSecretProvider makes Service.GetStatus fail at its findTokenSecret +// step, which is the only internal-error path wsStatus has. +type failingSecretProvider struct{ err error } + +func (f *failingSecretProvider) List(context.Context) ([]*SecretListItem, error) { + return nil, f.err +} + +func (f *failingSecretProvider) Reveal(context.Context, string) (string, error) { + return "", f.err +} + +// --- Status --- + +func TestWSStatusReturnsServiceStatus(t *testing.T) { + client := NewMockClient(DefaultHost) + client.SetUser("alice") + svc, _, log := newWSFixture(t, client) + + resp, err := wsStatus(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabStatus, nil)) + + var status Status + wsOK(t, resp, err, ws.ActionGitLabStatus, &status) + if !status.Authenticated { + t.Error("authenticated = false, want true for the mock client") + } + if status.Username != "alice" { + t.Errorf("username = %q, want alice", status.Username) + } + if status.AuthMethod != AuthMethodPAT { + t.Errorf("auth_method = %q, want %q", status.AuthMethod, AuthMethodPAT) + } + if status.Host != DefaultHost { + t.Errorf("host = %q, want %q", status.Host, DefaultHost) + } +} + +func TestWSStatusMapsSecretLookupFailureToInternalError(t *testing.T) { + log := newTestLogger(t) + svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, + &failingSecretProvider{err: errors.New("vault offline")}, log) + + resp, err := wsStatus(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabStatus, nil)) + + payload := wsErr(t, resp, err, ws.ActionGitLabStatus, ws.ErrorCodeInternalError, "") + if payload.Message == "" { + t.Error("internal error message is empty; the cause must reach the client") + } +} + +// --- Task MR listing --- + +func TestWSListTaskMRsByTaskReturnsOnlyThatTasksRows(t *testing.T) { + svc, store, log := newWSFixture(t, NewMockClient(DefaultHost)) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedTask(t, store, "task-a", "ws-1") + seedTask(t, store, "task-b", "ws-1") + if err := store.UpsertTaskMR(ctx, newTestMR("task-a", "repo-1", "group/a", 11)); err != nil { + t.Fatalf("seed task-a MR: %v", err) + } + if err := store.UpsertTaskMR(ctx, newTestMR("task-b", "repo-1", "group/b", 22)); err != nil { + t.Fatalf("seed task-b MR: %v", err) + } + + resp, err := wsListTaskMRs(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabTaskMRsList, map[string]string{"task_id": "task-a"})) + + var body struct { + TaskMRs []*TaskMR `json:"task_mrs"` + } + wsOK(t, resp, err, ws.ActionGitLabTaskMRsList, &body) + if len(body.TaskMRs) != 1 { + t.Fatalf("task_mrs = %d rows, want exactly task-a's single row", len(body.TaskMRs)) + } + if body.TaskMRs[0].ProjectPath != "group/a" || body.TaskMRs[0].MRIID != 11 { + t.Errorf("row = %s!%d, want group/a!11", body.TaskMRs[0].ProjectPath, body.TaskMRs[0].MRIID) + } +} + +func TestWSListTaskMRsByWorkspaceGroupsRowsByTask(t *testing.T) { + svc, store, log := newWSFixture(t, NewMockClient(DefaultHost)) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedTask(t, store, "task-a", "ws-1") + seedTask(t, store, "task-b", "ws-1") + for _, mr := range []*TaskMR{ + newTestMR("task-a", "repo-1", "group/a", 11), + newTestMR("task-b", "repo-1", "group/b", 22), + } { + if err := store.UpsertTaskMR(ctx, mr); err != nil { + t.Fatalf("seed %s MR: %v", mr.TaskID, err) + } + } + + resp, err := wsListTaskMRs(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabTaskMRsList, map[string]string{"workspace_id": "ws-1"})) + + var body TaskMRsResponse + wsOK(t, resp, err, ws.ActionGitLabTaskMRsList, &body) + if len(body.TaskMRs) != 2 { + t.Fatalf("grouped tasks = %d, want 2 (%v)", len(body.TaskMRs), body.TaskMRs) + } + if got := body.TaskMRs["task-a"]; len(got) != 1 || got[0].MRIID != 11 { + t.Errorf("task-a group = %+v, want the single !11 row", got) + } +} + +// TestWSListTaskMRsRejectsMalformedPayload pins the comment in handlers.go: a +// payload that fails to parse must not fall through to the "list everything in +// the workspace" branch, which would return more rows than the caller asked +// for. +func TestWSListTaskMRsRejectsMalformedPayload(t *testing.T) { + svc, store, log := newWSFixture(t, NewMockClient(DefaultHost)) + seedWorkspace(t, store, "ws-1") + + resp, err := wsListTaskMRs(svc, log)(context.Background(), + wsRawRequest(ws.ActionGitLabTaskMRsList, `{"task_id":12345}`)) + + wsErr(t, resp, err, ws.ActionGitLabTaskMRsList, ws.ErrorCodeBadRequest, errMsgInvalidPayload) +} + +func TestWSListTaskMRsRequiresWorkspaceOrTask(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsListTaskMRs(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabTaskMRsList, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabTaskMRsList, ws.ErrorCodeBadRequest, + "workspace_id or task_id required") +} + +func TestWSGetTaskMRRequiresTaskID(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + + for name, msg := range map[string]*ws.Message{ + "absent": wsRequest(t, ws.ActionGitLabTaskMRGet, map[string]string{}), + "malformed": wsRawRequest(ws.ActionGitLabTaskMRGet, `{"task_id":[]}`), + } { + t.Run(name, func(t *testing.T) { + resp, err := wsGetTaskMR(svc, log)(context.Background(), msg) + wsErr(t, resp, err, ws.ActionGitLabTaskMRGet, ws.ErrorCodeBadRequest, "task_id required") + }) + } +} + +func TestWSGetTaskMRReturnsRowsForTask(t *testing.T) { + svc, store, log := newWSFixture(t, NewMockClient(DefaultHost)) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedTask(t, store, "task-a", "ws-1") + if err := store.UpsertTaskMR(ctx, newTestMR("task-a", "repo-1", "group/a", 7)); err != nil { + t.Fatalf("seed MR: %v", err) + } + + resp, err := wsGetTaskMR(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabTaskMRGet, map[string]string{"task_id": "task-a"})) + + var body struct { + TaskMRs []*TaskMR `json:"task_mrs"` + } + wsOK(t, resp, err, ws.ActionGitLabTaskMRGet, &body) + if len(body.TaskMRs) != 1 || body.TaskMRs[0].MRIID != 7 { + t.Fatalf("task_mrs = %+v, want the single !7 row", body.TaskMRs) + } +} + +// --- MR feedback / files / commits --- + +func TestWSGetMRFeedbackValidatesProjectAndIID(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + handler := wsGetMRFeedback(svc, log) + + cases := map[string]*ws.Message{ + "missing project": wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"iid": 3}), + "zero iid": wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"project": "g/p", "iid": 0}), + "negative iid": wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"project": "g/p", "iid": -1}), + "malformed": wsRawRequest(ws.ActionGitLabMRFeedbackGet, `{"project":true}`), + } + for name, msg := range cases { + t.Run(name, func(t *testing.T) { + resp, err := handler(context.Background(), msg) + wsErr(t, resp, err, ws.ActionGitLabMRFeedbackGet, ws.ErrorCodeBadRequest, "project and iid required") + }) + } +} + +func TestWSGetMRFeedbackReturnsClientFeedback(t *testing.T) { + client := NewMockClient(DefaultHost) + client.SetUser("alice") + iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"}) + client.SeedApprovals("group/p", iid, []MRApproval{{Username: "bob"}}, 1) + svc, _, log := newWSFixture(t, client) + + resp, err := wsGetMRFeedback(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"project": "group/p", "iid": iid})) + + var feedback MRFeedback + wsOK(t, resp, err, ws.ActionGitLabMRFeedbackGet, &feedback) + if len(feedback.Approvals) != 1 || feedback.Approvals[0].Username != "bob" { + t.Errorf("approvals = %+v, want bob's single approval", feedback.Approvals) + } +} + +func TestWSGetMRFilesReturnsSeededFiles(t *testing.T) { + client := NewMockClient(DefaultHost) + iid := client.SeedMR("group/p", &MR{Title: "feat: x"}) + client.SeedFiles("group/p", iid, []MRFile{{Filename: "a.go", Additions: 3}}) + svc, _, log := newWSFixture(t, client) + + resp, err := wsGetMRFiles(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRFilesGet, map[string]interface{}{"project": "group/p", "iid": iid})) + + var body struct { + Files []MRFile `json:"files"` + } + wsOK(t, resp, err, ws.ActionGitLabMRFilesGet, &body) + if len(body.Files) != 1 || body.Files[0].Filename != "a.go" { + t.Fatalf("files = %+v, want the single a.go entry", body.Files) + } +} + +func TestWSGetMRFilesValidatesProjectAndIID(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsGetMRFiles(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRFilesGet, map[string]interface{}{"project": "", "iid": 1})) + + wsErr(t, resp, err, ws.ActionGitLabMRFilesGet, ws.ErrorCodeBadRequest, "project and iid required") +} + +func TestWSGetMRCommitsReturnsSeededCommits(t *testing.T) { + client := NewMockClient(DefaultHost) + iid := client.SeedMR("group/p", &MR{Title: "feat: x"}) + client.SeedCommits("group/p", iid, []MRCommitInfo{{SHA: "abc123", Message: "feat: x"}}) + svc, _, log := newWSFixture(t, client) + + resp, err := wsGetMRCommits(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRCommitsGet, map[string]interface{}{"project": "group/p", "iid": iid})) + + var body struct { + Commits []MRCommitInfo `json:"commits"` + } + wsOK(t, resp, err, ws.ActionGitLabMRCommitsGet, &body) + if len(body.Commits) != 1 || body.Commits[0].SHA != "abc123" { + t.Fatalf("commits = %+v, want the single abc123 entry", body.Commits) + } +} + +func TestWSGetMRCommitsValidatesProjectAndIID(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsGetMRCommits(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRCommitsGet, map[string]interface{}{"project": "g/p"})) + + wsErr(t, resp, err, ws.ActionGitLabMRCommitsGet, ws.ErrorCodeBadRequest, "project and iid required") +} + +// --- Sync --- + +func TestWSSyncTaskMRRequiresTaskID(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsSyncTaskMR(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabTaskMRSync, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabTaskMRSync, ws.ErrorCodeBadRequest, "task_id required") +} + +func TestWSSyncTaskMRReportsMissingStoreAsInternalError(t *testing.T) { + log := newTestLogger(t) + svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, log) + + resp, err := wsSyncTaskMR(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabTaskMRSync, map[string]string{"task_id": "task-a"})) + + wsErr(t, resp, err, ws.ActionGitLabTaskMRSync, ws.ErrorCodeInternalError, + "gitlab store not configured") +} + +func TestWSSyncTaskMRReturnsRefreshedRows(t *testing.T) { + client := NewMockClient(DefaultHost) + iid := client.SeedMR("group/p", &MR{Title: "feat: refreshed", State: "opened"}) + svc, store, log := newWSFixture(t, client) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedTask(t, store, "task-a", "ws-1") + row := newTestMR("task-a", "repo-1", "group/p", iid) + row.MRTitle = "stale title" + if err := store.UpsertTaskMR(ctx, row); err != nil { + t.Fatalf("seed MR row: %v", err) + } + + resp, err := wsSyncTaskMR(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabTaskMRSync, map[string]string{"task_id": "task-a"})) + + var body struct { + TaskMRs []*TaskMR `json:"task_mrs"` + } + wsOK(t, resp, err, ws.ActionGitLabTaskMRSync, &body) + if len(body.TaskMRs) != 1 { + t.Fatalf("task_mrs = %d rows, want 1", len(body.TaskMRs)) + } + if body.TaskMRs[0].MRTitle != "feat: refreshed" { + t.Errorf("title = %q, want the upstream title to have replaced the stale one", + body.TaskMRs[0].MRTitle) + } +} + +// --- Stats --- + +// statsRecordingClient records the filters GetStats sends upstream so the test +// can assert the three distinct queries, not just the aggregate numbers. +type statsRecordingClient struct { + *MockClient + mrFilters []string +} + +func (c *statsRecordingClient) SearchMRsPaged(ctx context.Context, filter, customQuery string, page, perPage int) (*MRSearchPage, error) { + c.mrFilters = append(c.mrFilters, filter) + return c.MockClient.SearchMRsPaged(ctx, filter, customQuery, page, perPage) +} + +func TestWSGetStatsReturnsCountsAndQueriesEachScope(t *testing.T) { + client := &statsRecordingClient{MockClient: NewMockClient(DefaultHost)} + client.SetUser("alice") + client.SeedMR("group/p", &MR{Title: "one", State: "opened"}) + client.SeedMR("group/q", &MR{Title: "two", State: "opened"}) + client.SeedIssue("group/p", &Issue{IID: 1, Title: "bug", State: "opened"}) + svc, _, log := newWSFixture(t, client) + + resp, err := wsGetStats(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabStats, nil)) + + var stats Stats + wsOK(t, resp, err, ws.ActionGitLabStats, &stats) + if stats.OpenMRs != 2 { + t.Errorf("open_mrs = %d, want 2", stats.OpenMRs) + } + if stats.MRsAwaitingMyReview != 2 { + t.Errorf("mrs_awaiting_my_review = %d, want 2", stats.MRsAwaitingMyReview) + } + if stats.OpenIssuesAssignedMe != 1 { + t.Errorf("open_issues_assigned_me = %d, want 1", stats.OpenIssuesAssignedMe) + } + want := []string{"scope=created_by_me&state=opened", "reviewer_username=alice&state=opened"} + if len(client.mrFilters) != len(want) { + t.Fatalf("MR search filters = %q, want %q", client.mrFilters, want) + } + for i, filter := range want { + if client.mrFilters[i] != filter { + t.Errorf("MR search filter[%d] = %q, want %q", i, client.mrFilters[i], filter) + } + } +} + +// TestWSGetStatsDegradesToZeroWithoutAuthenticatedUser pins the documented +// behavior of Service.GetStats: an unresolvable user yields empty counts +// rather than an error envelope, so the dashboard still renders. +func TestWSGetStatsDegradesToZeroWithoutAuthenticatedUser(t *testing.T) { + svc, _, log := newWSFixture(t, NewMockClient(DefaultHost)) + + resp, err := wsGetStats(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabStats, nil)) + + var stats Stats + wsOK(t, resp, err, ws.ActionGitLabStats, &stats) + if stats.OpenMRs != 0 || stats.OpenIssuesAssignedMe != 0 { + t.Errorf("stats = %+v, want zeroed counts when no user resolves", stats) + } +} diff --git a/apps/backend/internal/gitlab/handlers_ws_watches_test.go b/apps/backend/internal/gitlab/handlers_ws_watches_test.go new file mode 100644 index 0000000000..0507c361d0 --- /dev/null +++ b/apps/backend/internal/gitlab/handlers_ws_watches_test.go @@ -0,0 +1,563 @@ +package gitlab + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/common/logger" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// newWatchWSFixture builds a service whose watch-dependency validators accept +// the "ws-1" fixture identifiers, so the WS create/update handlers exercise the +// real validation path rather than a stub that waves everything through. +func newWatchWSFixture(t *testing.T) (*Service, *Store, *logger.Logger) { + t.Helper() + svc, store := newDependencyTestService(t) + seedWorkspace(t, store, "ws-1") + seedWorkspace(t, store, "ws-2") + return svc, store, newTestLogger(t) +} + +// --- Review watches --- + +func TestWSListReviewWatchesScopesToWorkspaceWhenGiven(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + mine := validReviewWatchRow() + other := validReviewWatchRow() + other.WorkspaceID = "ws-2" + for _, w := range []*ReviewWatch{mine, other} { + if err := store.CreateReviewWatch(ctx, w); err != nil { + t.Fatalf("seed review watch: %v", err) + } + } + + resp, err := wsListReviewWatches(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabReviewWatchesList, map[string]string{"workspace_id": "ws-1"})) + + var body struct { + Watches []*ReviewWatch `json:"watches"` + } + wsOK(t, resp, err, ws.ActionGitLabReviewWatchesList, &body) + if len(body.Watches) != 1 { + t.Fatalf("watches = %d, want only the ws-1 watch", len(body.Watches)) + } + if body.Watches[0].WorkspaceID != "ws-1" { + t.Errorf("watch workspace = %q, want ws-1", body.Watches[0].WorkspaceID) + } +} + +// TestWSListReviewWatchesWithoutWorkspaceListsAll pins the deliberate fallback +// in wsListReviewWatches: an absent workspace_id lists every workspace's +// watches. If that ever needs to become a rejection, this test is the one to +// change. +func TestWSListReviewWatchesWithoutWorkspaceListsAll(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + other := validReviewWatchRow() + other.WorkspaceID = "ws-2" + for _, w := range []*ReviewWatch{validReviewWatchRow(), other} { + if err := store.CreateReviewWatch(ctx, w); err != nil { + t.Fatalf("seed review watch: %v", err) + } + } + + resp, err := wsListReviewWatches(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabReviewWatchesList, map[string]string{})) + + var body struct { + Watches []*ReviewWatch `json:"watches"` + } + wsOK(t, resp, err, ws.ActionGitLabReviewWatchesList, &body) + if len(body.Watches) != 2 { + t.Fatalf("watches = %d, want both workspaces' watches", len(body.Watches)) + } +} + +func TestWSListReviewWatchesRejectsMalformedPayload(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsListReviewWatches(svc, log)(context.Background(), + wsRawRequest(ws.ActionGitLabReviewWatchesList, `{"workspace_id":7}`)) + + wsErr(t, resp, err, ws.ActionGitLabReviewWatchesList, ws.ErrorCodeBadRequest, errMsgInvalidPayload) +} + +func TestWSCreateReviewWatchPersistsAndReturnsRow(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + + resp, err := wsCreateReviewWatch(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabReviewWatchCreate, validReviewWatchRequest())) + + var created ReviewWatch + wsOK(t, resp, err, ws.ActionGitLabReviewWatchCreate, &created) + if created.ID == "" { + t.Fatal("created watch has no id") + } + if created.WorkspaceID != "ws-1" || created.RepositoryID != "repo-valid" { + t.Errorf("created watch = %+v, want the requested workspace/repository", created) + } + stored, err := store.GetReviewWatch(ctx, created.ID) + if err != nil || stored == nil { + t.Fatalf("watch was not persisted: row=%v err=%v", stored, err) + } +} + +func TestWSCreateReviewWatchRejectsInvalidDependencies(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + req := validReviewWatchRequest() + req.RepositoryID = "repo-other" // belongs to ws-2 + + resp, err := wsCreateReviewWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabReviewWatchCreate, req)) + + wsErr(t, resp, err, ws.ActionGitLabReviewWatchCreate, ws.ErrorCodeInternalError, "") +} + +func TestWSCreateReviewWatchRejectsMalformedPayload(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsCreateReviewWatch(svc, log)(context.Background(), + wsRawRequest(ws.ActionGitLabReviewWatchCreate, `{"workspace_id":[]}`)) + + wsErr(t, resp, err, ws.ActionGitLabReviewWatchCreate, ws.ErrorCodeBadRequest, errMsgInvalidPayload) +} + +func TestWSUpdateReviewWatchAppliesPatch(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + resp, err := wsUpdateReviewWatch(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabReviewWatchUpdate, map[string]interface{}{ + "id": watch.ID, + "enabled": false, + })) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabReviewWatchUpdate, &body) + if !body["updated"] { + t.Errorf("payload = %v, want {\"updated\":true}", body) + } + stored, err := store.GetReviewWatch(ctx, watch.ID) + if err != nil { + t.Fatalf("reload watch: %v", err) + } + if stored.Enabled { + t.Error("watch is still enabled; the patch did not reach the store") + } +} + +func TestWSUpdateReviewWatchRequiresID(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsUpdateReviewWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabReviewWatchUpdate, map[string]interface{}{"enabled": false})) + + wsErr(t, resp, err, ws.ActionGitLabReviewWatchUpdate, ws.ErrorCodeBadRequest, errMsgIDRequired) +} + +func TestWSUpdateReviewWatchReportsUnknownIDAsInternalError(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsUpdateReviewWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabReviewWatchUpdate, map[string]interface{}{ + "id": "does-not-exist", "enabled": false, + })) + + wsErr(t, resp, err, ws.ActionGitLabReviewWatchUpdate, ws.ErrorCodeInternalError, "") +} + +func TestWSDeleteReviewWatchRemovesRow(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + + resp, err := wsDeleteReviewWatch(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabReviewWatchDelete, map[string]string{"id": watch.ID})) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabReviewWatchDelete, &body) + if !body[respKeyDeleted] { + t.Errorf("payload = %v, want {\"deleted\":true}", body) + } + remaining, err := store.ListReviewWatches(ctx, "ws-1") + if err != nil { + t.Fatalf("list review watches: %v", err) + } + if len(remaining) != 0 { + t.Errorf("watches remaining = %d, want 0", len(remaining)) + } +} + +func TestWSDeleteReviewWatchRequiresID(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsDeleteReviewWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabReviewWatchDelete, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabReviewWatchDelete, ws.ErrorCodeBadRequest, errMsgIDRequired) +} + +func TestWSTriggerReviewWatchRequiresID(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsTriggerReviewWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabReviewTrigger, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabReviewTrigger, ws.ErrorCodeBadRequest, errMsgIDRequired) +} + +func TestWSTriggerReviewWatchReportsUnknownWatch(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsTriggerReviewWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabReviewTrigger, map[string]string{"id": "nope"})) + + wsErr(t, resp, err, ws.ActionGitLabReviewTrigger, ws.ErrorCodeInternalError, "") +} + +// TestWSTriggerAllReviewChecksCountsOnlyEnabledWatches seeds one enabled and +// one disabled watch over the same MR. The count is the number of MRs found +// across enabled watches, so a handler that ignored the enabled flag would +// report 2 and one that always returned 0 would report 0 — only the correct +// filter yields 1. +func TestWSTriggerAllReviewChecksCountsOnlyEnabledWatches(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + client := NewMockClient(DefaultHost) + client.SetUser("kandev-tester") + client.SeedMR("group/p", &MR{IID: 1, Title: "needs review", WebURL: "https://x/1", + Reviewers: []MRReviewer{{Username: "kandev-tester"}}}) + svc.workspaceClients["ws-1"] = client + + disabled := validReviewWatchRow() + disabled.Enabled = false + for _, watch := range []*ReviewWatch{validReviewWatchRow(), disabled} { + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("seed review watch: %v", err) + } + } + + resp, err := wsTriggerAllReviewChecks(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabReviewTriggerAll, nil)) + + var body map[string]int + wsOK(t, resp, err, ws.ActionGitLabReviewTriggerAll, &body) + if body["count"] != 1 { + t.Errorf("count = %d, want 1 — the enabled watch contributes its MR and "+ + "the disabled one contributes nothing", body["count"]) + } +} + +// --- MR watches --- + +func TestWSListMRWatchesSelectsBranchBySessionThenTaskThenAll(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + seedTask(t, store, "task-a", "ws-1") + seedTask(t, store, "task-b", "ws-1") + if _, err := svc.CreateMRWatch(ctx, "sess-a", "task-a", "repo-1", "group/a", 1, "feat/a"); err != nil { + t.Fatalf("seed watch a: %v", err) + } + if _, err := svc.CreateMRWatch(ctx, "sess-b", "task-b", "repo-1", "group/b", 2, "feat/b"); err != nil { + t.Fatalf("seed watch b: %v", err) + } + + tests := []struct { + name string + payload map[string]string + want []string + }{ + {"by session", map[string]string{"session_id": "sess-a"}, []string{"group/a"}}, + {"by task", map[string]string{"task_id": "task-b"}, []string{"group/b"}}, + {"all active", map[string]string{}, []string{"group/a", "group/b"}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resp, err := wsListMRWatches(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabMRWatchesList, test.payload)) + + var body struct { + Watches []*MRWatch `json:"watches"` + } + wsOK(t, resp, err, ws.ActionGitLabMRWatchesList, &body) + if len(body.Watches) != len(test.want) { + t.Fatalf("watches = %d, want %d (%v)", len(body.Watches), len(test.want), body.Watches) + } + got := map[string]bool{} + for _, w := range body.Watches { + got[w.ProjectPath] = true + } + for _, project := range test.want { + if !got[project] { + t.Errorf("watch for %q missing from %v", project, got) + } + } + }) + } +} + +func TestWSListMRWatchesRejectsMalformedPayload(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsListMRWatches(svc, log)(context.Background(), + wsRawRequest(ws.ActionGitLabMRWatchesList, `{"session_id":{}}`)) + + wsErr(t, resp, err, ws.ActionGitLabMRWatchesList, ws.ErrorCodeBadRequest, errMsgInvalidPayload) +} + +func TestWSDeleteMRWatchRemovesRow(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + seedTask(t, store, "task-a", "ws-1") + watch, err := svc.CreateMRWatch(ctx, "sess-a", "task-a", "repo-1", "group/a", 1, "feat/a") + if err != nil { + t.Fatalf("seed watch: %v", err) + } + + resp, err := wsDeleteMRWatch(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabMRWatchDelete, map[string]string{"id": watch.ID})) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabMRWatchDelete, &body) + if !body[respKeyDeleted] { + t.Errorf("payload = %v, want {\"deleted\":true}", body) + } + remaining, err := svc.ListActiveMRWatches(ctx) + if err != nil { + t.Fatalf("list active watches: %v", err) + } + if len(remaining) != 0 { + t.Errorf("watches remaining = %d, want 0", len(remaining)) + } +} + +func TestWSDeleteMRWatchRequiresID(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsDeleteMRWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabMRWatchDelete, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabMRWatchDelete, ws.ErrorCodeBadRequest, errMsgIDRequired) +} + +// --- Issue watches --- + +func TestWSListIssueWatchesScopesToWorkspaceWhenGiven(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + other := validIssueWatchRow() + other.WorkspaceID = "ws-2" + for _, w := range []*IssueWatch{validIssueWatchRow(), other} { + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + } + + resp, err := wsListIssueWatches(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabIssueWatchesList, map[string]string{"workspace_id": "ws-2"})) + + var body struct { + Watches []*IssueWatch `json:"watches"` + } + wsOK(t, resp, err, ws.ActionGitLabIssueWatchesList, &body) + if len(body.Watches) != 1 || body.Watches[0].WorkspaceID != "ws-2" { + t.Fatalf("watches = %+v, want only the ws-2 watch", body.Watches) + } +} + +func TestWSListIssueWatchesWithoutWorkspaceListsAll(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + other := validIssueWatchRow() + other.WorkspaceID = "ws-2" + for _, w := range []*IssueWatch{validIssueWatchRow(), other} { + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + } + + resp, err := wsListIssueWatches(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabIssueWatchesList, map[string]string{})) + + var body struct { + Watches []*IssueWatch `json:"watches"` + } + wsOK(t, resp, err, ws.ActionGitLabIssueWatchesList, &body) + if len(body.Watches) != 2 { + t.Fatalf("watches = %d, want both workspaces' watches", len(body.Watches)) + } +} + +func TestWSListIssueWatchesRejectsMalformedPayload(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsListIssueWatches(svc, log)(context.Background(), + wsRawRequest(ws.ActionGitLabIssueWatchesList, `{"workspace_id":false}`)) + + wsErr(t, resp, err, ws.ActionGitLabIssueWatchesList, ws.ErrorCodeBadRequest, errMsgInvalidPayload) +} + +func TestWSCreateIssueWatchPersistsAndReturnsRow(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + + resp, err := wsCreateIssueWatch(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabIssueWatchCreate, validIssueWatchRequest())) + + var created IssueWatch + wsOK(t, resp, err, ws.ActionGitLabIssueWatchCreate, &created) + if created.ID == "" { + t.Fatal("created watch has no id") + } + stored, err := store.GetIssueWatch(ctx, created.ID) + if err != nil || stored == nil { + t.Fatalf("watch was not persisted: row=%v err=%v", stored, err) + } +} + +func TestWSCreateIssueWatchRejectsInvalidDependencies(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + req := validIssueWatchRequest() + req.AgentProfileID = "agent-missing" + + resp, err := wsCreateIssueWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabIssueWatchCreate, req)) + + wsErr(t, resp, err, ws.ActionGitLabIssueWatchCreate, ws.ErrorCodeInternalError, "") +} + +func TestWSCreateIssueWatchRejectsMalformedPayload(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsCreateIssueWatch(svc, log)(context.Background(), + wsRawRequest(ws.ActionGitLabIssueWatchCreate, `"a string, not an object"`)) + + wsErr(t, resp, err, ws.ActionGitLabIssueWatchCreate, ws.ErrorCodeBadRequest, errMsgInvalidPayload) +} + +func TestWSUpdateIssueWatchAppliesPatch(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + + resp, err := wsUpdateIssueWatch(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabIssueWatchUpdate, map[string]interface{}{ + "id": watch.ID, "enabled": false, + })) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabIssueWatchUpdate, &body) + if !body["updated"] { + t.Errorf("payload = %v, want {\"updated\":true}", body) + } + stored, err := store.GetIssueWatch(ctx, watch.ID) + if err != nil { + t.Fatalf("reload watch: %v", err) + } + if stored.Enabled { + t.Error("watch is still enabled; the patch did not reach the store") + } +} + +func TestWSUpdateIssueWatchRequiresID(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsUpdateIssueWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabIssueWatchUpdate, map[string]interface{}{"enabled": true})) + + wsErr(t, resp, err, ws.ActionGitLabIssueWatchUpdate, ws.ErrorCodeBadRequest, errMsgIDRequired) +} + +func TestWSDeleteIssueWatchRemovesRow(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + + resp, err := wsDeleteIssueWatch(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabIssueWatchDelete, map[string]string{"id": watch.ID})) + + var body map[string]bool + wsOK(t, resp, err, ws.ActionGitLabIssueWatchDelete, &body) + if !body[respKeyDeleted] { + t.Errorf("payload = %v, want {\"deleted\":true}", body) + } + remaining, err := store.ListIssueWatches(ctx, "ws-1") + if err != nil { + t.Fatalf("list issue watches: %v", err) + } + if len(remaining) != 0 { + t.Errorf("watches remaining = %d, want 0", len(remaining)) + } +} + +func TestWSDeleteIssueWatchRequiresID(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsDeleteIssueWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabIssueWatchDelete, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabIssueWatchDelete, ws.ErrorCodeBadRequest, errMsgIDRequired) +} + +func TestWSTriggerIssueWatchRequiresID(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsTriggerIssueWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabIssueTrigger, map[string]string{})) + + wsErr(t, resp, err, ws.ActionGitLabIssueTrigger, ws.ErrorCodeBadRequest, errMsgIDRequired) +} + +func TestWSTriggerIssueWatchReportsUnknownWatch(t *testing.T) { + svc, _, log := newWatchWSFixture(t) + + resp, err := wsTriggerIssueWatch(svc, log)(context.Background(), + wsRequest(t, ws.ActionGitLabIssueTrigger, map[string]string{"id": "nope"})) + + wsErr(t, resp, err, ws.ActionGitLabIssueTrigger, ws.ErrorCodeInternalError, "") +} + +// TestWSTriggerAllIssueChecksCountsOnlyEnabledWatches is the issue mirror of +// the review test above, and discriminates the same three outcomes. +func TestWSTriggerAllIssueChecksCountsOnlyEnabledWatches(t *testing.T) { + svc, store, log := newWatchWSFixture(t) + ctx := context.Background() + client := NewMockClient(DefaultHost) + client.SetUser("kandev-tester") + client.SeedIssue("group/p", &Issue{IID: 3, Title: "bug", WebURL: "https://x/3"}) + svc.workspaceClients["ws-1"] = client + + disabled := validIssueWatchRow() + disabled.Enabled = false + for _, watch := range []*IssueWatch{validIssueWatchRow(), disabled} { + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("seed issue watch: %v", err) + } + } + + resp, err := wsTriggerAllIssueChecks(svc, log)(ctx, + wsRequest(t, ws.ActionGitLabIssueTriggerAll, nil)) + + var body map[string]int + wsOK(t, resp, err, ws.ActionGitLabIssueTriggerAll, &body) + if body["count"] != 1 { + t.Errorf("count = %d, want 1 — the enabled watch contributes its issue and "+ + "the disabled one contributes nothing", body["count"]) + } +} diff --git a/apps/backend/internal/gitlab/noop_client_contract_test.go b/apps/backend/internal/gitlab/noop_client_contract_test.go new file mode 100644 index 0000000000..82ca5243fc --- /dev/null +++ b/apps/backend/internal/gitlab/noop_client_contract_test.go @@ -0,0 +1,139 @@ +package gitlab + +import ( + "context" + "errors" + "reflect" + "testing" +) + +// noopClientNonErroringMethods are the only NoopClient methods that must NOT +// report ErrNoClient. They answer questions that have a meaningful answer even +// with no GitLab configured, and callers branch on those answers rather than on +// an error. +// +// GetProtectedBranch is the subtle one: (nil, nil) is not "no data", it is the +// affirmative answer "this branch is not protected". PATClient returns exactly +// that for a 404 (see pat_client_actions.go), so NoopClient reporting +// ErrNoClient here would make the unconfigured path *more* restrictive than a +// real GitLab that simply has no protection rule. +var noopClientNonErroringMethods = map[string]struct{}{ + "Host": {}, + "IsAuthenticated": {}, + "GetAuthenticatedUser": {}, + "GetProtectedBranch": {}, +} + +// TestNoopClientDegradesUniformly is the contract for the unconfigured-GitLab +// path: every data method must return ErrNoClient rather than a nil error with +// a nil result, because callers that see (nil, nil) dereference it. Driving the +// method set by reflection rather than a hand-written list means a method added +// to the Client interface cannot quietly skip this guarantee, and it also +// proves no method panics on zero-valued arguments. +func TestNoopClientDegradesUniformly(t *testing.T) { + client := NewNoopClient("") + clientType := reflect.TypeOf(client) + ctxValue := reflect.ValueOf(context.Background()) + + exercised := 0 + for i := range clientType.NumMethod() { + method := clientType.Method(i) + if !method.IsExported() { + continue + } + t.Run(method.Name, func(t *testing.T) { + results := method.Func.Call(noopCallArgs(reflect.ValueOf(client), method.Type, ctxValue)) + err := trailingError(results) + + if _, exempt := noopClientNonErroringMethods[method.Name]; exempt { + if err != nil { + t.Fatalf("%s returned %v, want no error", method.Name, err) + } + return + } + if !errors.Is(err, ErrNoClient) { + t.Fatalf("%s returned err = %v, want ErrNoClient", method.Name, err) + } + // A method that reports ErrNoClient must not also hand back a + // half-built value the caller might use. + for j := range len(results) - 1 { + if result := results[j]; result.IsValid() && !result.IsZero() { + t.Errorf("%s returned a non-zero value %v alongside ErrNoClient", + method.Name, result.Interface()) + } + } + }) + exercised++ + } + if exercised == 0 { + t.Fatal("no exported methods were exercised; the reflection walk is broken") + } +} + +// noopCallArgs builds the receiver plus zero-valued arguments for one method, +// substituting the real context for the first context.Context parameter. +func noopCallArgs(receiver reflect.Value, methodType reflect.Type, ctxValue reflect.Value) []reflect.Value { + ctxType := reflect.TypeOf((*context.Context)(nil)).Elem() + args := make([]reflect.Value, 0, methodType.NumIn()) + args = append(args, receiver) + for i := 1; i < methodType.NumIn(); i++ { + paramType := methodType.In(i) + if paramType == ctxType { + args = append(args, ctxValue) + continue + } + args = append(args, reflect.Zero(paramType)) + } + return args +} + +// trailingError extracts the error from a call's results, or nil when the +// method does not return one. +func trailingError(results []reflect.Value) error { + if len(results) == 0 { + return nil + } + last := results[len(results)-1] + if last.Type() != reflect.TypeOf((*error)(nil)).Elem() { + return nil + } + if last.IsNil() { + return nil + } + return last.Interface().(error) +} + +// TestNoopClientAuthAnswersAreExplicitlyNegative pins the values behind the +// three exemptions above: "not authenticated, no username" is the answer the +// status endpoint renders, and it must be a definite negative rather than an +// error the caller has to interpret. +func TestNoopClientAuthAnswersAreExplicitlyNegative(t *testing.T) { + client := NewNoopClient("https://gitlab.acme.test") + ctx := context.Background() + + authenticated, err := client.IsAuthenticated(ctx) + if err != nil || authenticated { + t.Errorf("IsAuthenticated = (%v, %v), want (false, nil)", authenticated, err) + } + username, err := client.GetAuthenticatedUser(ctx) + if err != nil || username != "" { + t.Errorf("GetAuthenticatedUser = (%q, %v), want (\"\", nil)", username, err) + } + if host := client.Host(); host != "https://gitlab.acme.test" { + t.Errorf("Host = %q, want the configured host reported even while unauthenticated", host) + } +} + +// TestNoopClientReportsBranchesAsUnprotected pins the exemption above: the +// unconfigured client must give the same "not protected" answer a real GitLab +// gives for a branch with no protection rule, not an error. +func TestNoopClientReportsBranchesAsUnprotected(t *testing.T) { + branch, err := NewNoopClient("").GetProtectedBranch(context.Background(), "group/p", "main") + + if err != nil { + t.Fatalf("GetProtectedBranch err = %v, want nil", err) + } + if branch != nil { + t.Errorf("protected branch = %+v, want nil (not protected)", branch) + } +} diff --git a/apps/backend/internal/gitlab/store_watches_scope_test.go b/apps/backend/internal/gitlab/store_watches_scope_test.go new file mode 100644 index 0000000000..c397050216 --- /dev/null +++ b/apps/backend/internal/gitlab/store_watches_scope_test.go @@ -0,0 +1,449 @@ +package gitlab + +import ( + "context" + "errors" + "testing" +) + +// newMRWatchScopeStore seeds two workspaces, one task in each, and one MR +// watch per task, returning the store plus the two watch IDs. The +// *ForWorkspace lookups reach the watch through its owning task, so this is +// the minimum fixture that can tell a scoped lookup from an unscoped one. +func newMRWatchScopeStore(t *testing.T) (*Store, *MRWatch, *MRWatch) { + t.Helper() + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedWorkspace(t, store, "ws-2") + seedTask(t, store, "task-1", "ws-1") + seedTask(t, store, "task-2", "ws-2") + mine := &MRWatch{SessionID: "sess-1", TaskID: "task-1", RepositoryID: "repo-1", ProjectPath: "group/a", MRIID: 1} + foreign := &MRWatch{SessionID: "sess-2", TaskID: "task-2", RepositoryID: "repo-2", ProjectPath: "group/b", MRIID: 2} + for _, watch := range []*MRWatch{mine, foreign} { + if err := store.CreateMRWatch(ctx, watch); err != nil { + t.Fatalf("seed MR watch: %v", err) + } + } + return store, mine, foreign +} + +func TestStoreListMRWatchesBySessionForWorkspaceExcludesForeignSessions(t *testing.T) { + store, mine, foreign := newMRWatchScopeStore(t) + ctx := context.Background() + + own, err := store.ListMRWatchesBySessionForWorkspace(ctx, "ws-1", mine.SessionID) + if err != nil { + t.Fatalf("list own session watches: %v", err) + } + if len(own) != 1 || own[0].ID != mine.ID { + t.Fatalf("watches = %+v, want only the ws-1 watch", own) + } + + // The same session id queried from the other workspace must return + // nothing: the scope comes from the owning task's workspace, not from + // the caller-supplied session id. + crossed, err := store.ListMRWatchesBySessionForWorkspace(ctx, "ws-2", mine.SessionID) + if err != nil { + t.Fatalf("list cross-workspace session watches: %v", err) + } + if len(crossed) != 0 { + t.Errorf("cross-workspace lookup returned %+v, want nothing", crossed) + } + + foreignScoped, err := store.ListMRWatchesBySessionForWorkspace(ctx, "ws-2", foreign.SessionID) + if err != nil { + t.Fatalf("list ws-2 session watches: %v", err) + } + if len(foreignScoped) != 1 || foreignScoped[0].ID != foreign.ID { + t.Errorf("ws-2 watches = %+v, want its own watch", foreignScoped) + } +} + +func TestStoreListMRWatchesByTaskForWorkspaceExcludesForeignTasks(t *testing.T) { + store, mine, _ := newMRWatchScopeStore(t) + ctx := context.Background() + + own, err := store.ListMRWatchesByTaskForWorkspace(ctx, "ws-1", "task-1") + if err != nil { + t.Fatalf("list own task watches: %v", err) + } + if len(own) != 1 || own[0].ID != mine.ID { + t.Fatalf("watches = %+v, want only the ws-1 watch", own) + } + + crossed, err := store.ListMRWatchesByTaskForWorkspace(ctx, "ws-1", "task-2") + if err != nil { + t.Fatalf("list cross-workspace task watches: %v", err) + } + if len(crossed) != 0 { + t.Errorf("cross-workspace lookup returned %+v, want nothing", crossed) + } +} + +// TestStoreListMRWatchesForWorkspaceHidesOrphanedRows pins the documented +// behavior: a watch whose task row is gone has no resolvable workspace, so the +// workspace-facing lookups must not surface it even though the unscoped +// listing still does. +func TestStoreListMRWatchesForWorkspaceHidesOrphanedRows(t *testing.T) { + store, mine, _ := newMRWatchScopeStore(t) + ctx := context.Background() + if _, err := store.db.Exec(`DELETE FROM tasks WHERE id = ?`, "task-1"); err != nil { + t.Fatalf("delete owning task: %v", err) + } + + scoped, err := store.ListMRWatchesByTaskForWorkspace(ctx, "ws-1", "task-1") + if err != nil { + t.Fatalf("list orphaned task watches: %v", err) + } + if len(scoped) != 0 { + t.Errorf("workspace lookup surfaced the orphaned watch %+v", scoped) + } + + unscoped, err := store.ListMRWatchesByTask(ctx, "task-1") + if err != nil { + t.Fatalf("list unscoped task watches: %v", err) + } + if len(unscoped) != 1 || unscoped[0].ID != mine.ID { + t.Errorf("unscoped listing = %+v, want the orphaned row still present", unscoped) + } +} + +func TestStoreDeleteMRWatchesByTaskIDRemovesOnlyThatTasksWatches(t *testing.T) { + store, _, foreign := newMRWatchScopeStore(t) + ctx := context.Background() + + deleted, err := store.DeleteMRWatchesByTaskID(ctx, "task-1") + if err != nil { + t.Fatalf("delete MR watches by task: %v", err) + } + if deleted != 1 { + t.Errorf("rows affected = %d, want 1", deleted) + } + remaining, err := store.ListActiveMRWatches(ctx) + if err != nil { + t.Fatalf("list active watches: %v", err) + } + if len(remaining) != 1 || remaining[0].ID != foreign.ID { + t.Fatalf("remaining watches = %+v, want only task-2's watch", remaining) + } +} + +func TestStoreDeleteMRWatchesByTaskIDIsZeroForUnknownTask(t *testing.T) { + store, _, _ := newMRWatchScopeStore(t) + + deleted, err := store.DeleteMRWatchesByTaskID(context.Background(), "no-such-task") + if err != nil { + t.Fatalf("delete MR watches by task: %v", err) + } + if deleted != 0 { + t.Errorf("rows affected = %d, want 0", deleted) + } +} + +// --- Service-level session lookups --- + +func TestServiceGetMRWatchBySessionAndRepoDiscriminatesRepositories(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedTask(t, store, "task-1", "ws-1") + svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, newTestLogger(t)) + svc.SetStore(store) + for _, repo := range []struct { + id string + project string + iid int + }{{"repo-a", "group/a", 1}, {"repo-b", "group/b", 2}} { + if _, err := svc.CreateMRWatch(ctx, "sess-1", "task-1", repo.id, repo.project, repo.iid, "feat/x"); err != nil { + t.Fatalf("seed watch for %s: %v", repo.id, err) + } + } + + watch, err := svc.GetMRWatchBySessionAndRepo(ctx, "sess-1", "repo-b") + if err != nil { + t.Fatalf("get watch by session and repo: %v", err) + } + if watch == nil || watch.ProjectPath != "group/b" { + t.Fatalf("watch = %+v, want repo-b's group/b watch", watch) + } + + missing, err := svc.GetMRWatchBySessionAndRepo(ctx, "sess-1", "repo-missing") + if err != nil { + t.Fatalf("get watch for unknown repo: %v", err) + } + if missing != nil { + t.Errorf("watch = %+v, want nil for an unknown repository", missing) + } +} + +func TestServiceMRWatchLookupsRequireAStore(t *testing.T) { + svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, newTestLogger(t)) + ctx := context.Background() + + if _, err := svc.GetMRWatchBySession(ctx, "sess-1"); !errors.Is(err, errStoreUnavailable) { + t.Errorf("GetMRWatchBySession error = %v, want errStoreUnavailable", err) + } + if _, err := svc.GetMRWatchBySessionAndRepo(ctx, "sess-1", "repo-a"); !errors.Is(err, errStoreUnavailable) { + t.Errorf("GetMRWatchBySessionAndRepo error = %v, want errStoreUnavailable", err) + } +} + +// TestServiceGetMRWatchBySessionOnlyMatchesTheLegacyRepositorylessRow pins the +// documented meaning of GetMRWatchBySession: it is the legacy single-repo +// accessor, keyed on an empty repository_id. A repo-keyed watch on the same +// session must not answer it, otherwise a multi-repo session would silently +// resolve to an arbitrary one of its repositories. +func TestServiceGetMRWatchBySessionOnlyMatchesTheLegacyRepositorylessRow(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedTask(t, store, "task-1", "ws-1") + svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, newTestLogger(t)) + svc.SetStore(store) + if _, err := svc.CreateMRWatch(ctx, "sess-1", "task-1", "repo-a", "group/a", 1, "feat/x"); err != nil { + t.Fatalf("seed repo-keyed watch: %v", err) + } + + watch, err := svc.GetMRWatchBySession(ctx, "sess-1") + if err != nil { + t.Fatalf("get watch by session: %v", err) + } + if watch != nil { + t.Fatalf("watch = %+v, want nil — a repo-keyed watch is not the legacy row", watch) + } + + legacy, err := svc.CreateMRWatch(ctx, "sess-1", "task-1", "", "group/legacy", 2, "feat/y") + if err != nil { + t.Fatalf("seed legacy watch: %v", err) + } + watch, err = svc.GetMRWatchBySession(ctx, "sess-1") + if err != nil { + t.Fatalf("get watch by session: %v", err) + } + if watch == nil || watch.ID != legacy.ID { + t.Fatalf("watch = %+v, want the legacy repository-less watch %s", watch, legacy.ID) + } +} + +// --- Reservation release --- + +func TestStoreReleaseReviewMRTaskAllowsARetryOnTheSameMR(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("create review watch: %v", err) + } + + reserved, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("first reserve: %v", err) + } + if !reserved { + t.Fatal("first reservation was refused") + } + again, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("second reserve: %v", err) + } + if again { + t.Fatal("the same MR was reserved twice without a release") + } + + if err := store.ReleaseReviewMRTask(ctx, watch.ID, "group/p", 1); err != nil { + t.Fatalf("release reservation: %v", err) + } + + retried, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("reserve after release: %v", err) + } + if !retried { + t.Error("the MR could not be reserved again after its reservation was released") + } +} + +// TestStoreReleaseReviewMRTaskForGenerationIgnoresStaleGenerations guards the +// ownership check: a release carrying a generation the reservation no longer +// belongs to must leave the row alone, so a slow poller from before a watch +// reset cannot free a reservation the current generation owns. +func TestStoreReleaseReviewMRTaskForGenerationIgnoresStaleGenerations(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + watch := validReviewWatchRow() + if err := store.CreateReviewWatch(ctx, watch); err != nil { + t.Fatalf("create review watch: %v", err) + } + if _, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url"); err != nil { + t.Fatalf("reserve: %v", err) + } + + if err := store.ReleaseReviewMRTaskForGeneration(ctx, watch.ID, 999, "group/p", 1); err != nil { + t.Fatalf("stale-generation release: %v", err) + } + stillHeld, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("reserve after stale release: %v", err) + } + if stillHeld { + t.Fatal("a stale-generation release freed the reservation") + } + + generation, err := store.reviewWatchGeneration(ctx, watch.ID) + if err != nil { + t.Fatalf("load watch generation: %v", err) + } + if err := store.ReleaseReviewMRTaskForGeneration(ctx, watch.ID, generation, "group/p", 1); err != nil { + t.Fatalf("current-generation release: %v", err) + } + freed, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("reserve after current release: %v", err) + } + if !freed { + t.Error("a current-generation release did not free the reservation") + } +} + +func TestStoreReleaseIssueWatchTaskAllowsARetryOnTheSameIssue(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("create issue watch: %v", err) + } + if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil { + t.Fatalf("reserve: %v", err) + } + + if err := store.ReleaseIssueWatchTask(ctx, watch.ID, "group/p", 1); err != nil { + t.Fatalf("release reservation: %v", err) + } + + retried, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("reserve after release: %v", err) + } + if !retried { + t.Error("the issue could not be reserved again after its reservation was released") + } +} + +func TestStoreReleaseIssueWatchTaskForGenerationIgnoresStaleGenerations(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + watch := validIssueWatchRow() + if err := store.CreateIssueWatch(ctx, watch); err != nil { + t.Fatalf("create issue watch: %v", err) + } + if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil { + t.Fatalf("reserve: %v", err) + } + + if err := store.ReleaseIssueWatchTaskForGeneration(ctx, watch.ID, 999, "group/p", 1); err != nil { + t.Fatalf("stale-generation release: %v", err) + } + stillHeld, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("reserve after stale release: %v", err) + } + if stillHeld { + t.Fatal("a stale-generation release freed the reservation") + } + + generation, err := store.issueWatchGeneration(ctx, watch.ID) + if err != nil { + t.Fatalf("load watch generation: %v", err) + } + if err := store.ReleaseIssueWatchTaskForGeneration(ctx, watch.ID, generation, "group/p", 1); err != nil { + t.Fatalf("current-generation release: %v", err) + } + freed, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url") + if err != nil { + t.Fatalf("reserve after current release: %v", err) + } + if !freed { + t.Error("a current-generation release did not free the reservation") + } +} + +// TestStoreIssueWatchGenerationDefaultsForUnknownWatch covers the sql.ErrNoRows +// branch: a missing watch reports generation 1 rather than an error, so a +// release racing a watch deletion degrades instead of failing the poll. +func TestStoreIssueWatchGenerationDefaultsForUnknownWatch(t *testing.T) { + store := newTestStore(t) + + generation, err := store.issueWatchGeneration(context.Background(), "no-such-watch") + if err != nil { + t.Fatalf("issue watch generation: %v", err) + } + if generation != 1 { + t.Errorf("generation = %d, want 1 for an unknown watch", generation) + } +} + +// --- Repository provider identity --- + +// TestValidateTaskMRRepositoryIdentityFailsClosedOnEmptyProviderHost encodes +// the invariant from apps/backend/CLAUDE.md: a provider-backed repository is +// keyed by workspace, provider, normalized provider_host origin, owner and +// name. A legacy row that carries a gitlab provider and a matching +// owner/name but no provider_host has unknown identity, so it must fail +// closed rather than being inferred from owner/name alone. +func TestValidateTaskMRRepositoryIdentityFailsClosedOnEmptyProviderHost(t *testing.T) { + const host = "https://gitlab.self-managed.test" + _, store, _ := newTaskMRLinkService(t, host) + seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1") + // Provider and owner/name match the MR exactly; only the host is blank. + if _, err := store.db.Exec(`UPDATE repositories + SET provider = 'gitlab', provider_host = '', provider_owner = 'acme', provider_name = 'api' + WHERE id = ?`, "repo-1"); err != nil { + t.Fatalf("seed hostless repository identity: %v", err) + } + + err := store.ValidateTaskMRRepositoryIdentity( + context.Background(), "ws-1", "task-1", "repo-1", host, "acme/api") + + if !errors.Is(err, ErrTaskMRRepositoryMismatch) { + t.Fatalf("error = %v, want ErrTaskMRRepositoryMismatch — an owner/name match "+ + "without a provider_host must not establish identity", err) + } +} + +// TestValidateTaskMRRepositoryIdentityRejectsSameProjectOnAnotherHost is the +// other half of the same invariant: identical owner/name on a different +// GitLab installation is a different repository. +func TestValidateTaskMRRepositoryIdentityRejectsSameProjectOnAnotherHost(t *testing.T) { + const host = "https://gitlab.self-managed.test" + _, store, _ := newTaskMRLinkService(t, host) + seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1") + setTaskMRRepositoryIdentity(t, store, "repo-1", "https://gitlab.other.test", "acme/api") + + err := store.ValidateTaskMRRepositoryIdentity( + context.Background(), "ws-1", "task-1", "repo-1", host, "acme/api") + + if !errors.Is(err, ErrTaskMRRepositoryMismatch) { + t.Fatalf("error = %v, want ErrTaskMRRepositoryMismatch for the same project "+ + "on a different host", err) + } +} + +// TestValidateTaskMRRepositoryIdentityAcceptsMatchingHostAndProject is the +// positive control for the two rejections above: with the host present and +// matching, the same owner/name is accepted. +func TestValidateTaskMRRepositoryIdentityAcceptsMatchingHostAndProject(t *testing.T) { + const host = "https://gitlab.self-managed.test" + _, store, _ := newTaskMRLinkService(t, host) + seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1") + setTaskMRRepositoryIdentity(t, store, "repo-1", host, "acme/api") + + if err := store.ValidateTaskMRRepositoryIdentity( + context.Background(), "ws-1", "task-1", "repo-1", host, "acme/api"); err != nil { + t.Fatalf("identity check rejected an exact host+project match: %v", err) + } +}