diff --git a/apps/backend/internal/azuredevops/controller_config_routes_test.go b/apps/backend/internal/azuredevops/controller_config_routes_test.go new file mode 100644 index 0000000000..f17dda18e4 --- /dev/null +++ b/apps/backend/internal/azuredevops/controller_config_routes_test.go @@ -0,0 +1,391 @@ +package azuredevops + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/common/logger" +) + +// newRouteFixture mounts the real router over a real store with a caller +// supplied Azure client, and leaves the workspace unconfigured so the config +// routes themselves can be exercised end to end. +func newRouteFixture(t *testing.T, client Client) (*gin.Engine, *Service) { + t.Helper() + gin.SetMode(gin.TestMode) + db := newTestDB(t) + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("new store: %v", err) + } + service := NewService(store, newFakeSecretStore(), func(*Config, string) Client { return client }, logger.Default()) + router := gin.New() + RegisterRoutes(router, service, logger.Default()) + return router, service +} + +func seedRouteConfig(t *testing.T, service *Service) { + t.Helper() + if _, err := service.SetConfigForWorkspace(t.Context(), "ws-1", &SetConfigRequest{ + OrganizationURL: "https://dev.azure.com/acme", PAT: "pat", + }); err != nil { + t.Fatalf("seed config: %v", err) + } +} + +func decodeJSON[T any](t *testing.T, response *httptest.ResponseRecorder) T { + t.Helper() + var decoded T + if err := json.Unmarshal(response.Body.Bytes(), &decoded); err != nil { + t.Fatalf("decode %d response %q: %v", response.Code, response.Body.String(), err) + } + return decoded +} + +func TestControllerConfigRouteLifecycle(t *testing.T) { + client := &readsClient{auth: &TestConnectionResult{OK: true, ID: "me", DisplayName: "Ada"}} + router, _ := newRouteFixture(t, client) + const query = "?workspace_id=ws-1" + + missing := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/config"+query, nil) + if missing.Code != http.StatusNoContent || missing.Body.Len() != 0 { + t.Fatalf("unconfigured GET /config = %d %q, want 204 with no body", missing.Code, missing.Body.String()) + } + + created := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/config"+query, map[string]any{ + "organizationUrl": "https://dev.azure.com/acme", "pat": "super-secret", + "defaultProjectId": "project-1", "defaultProjectName": "Platform", + }) + if created.Code != http.StatusOK { + t.Fatalf("POST /config = %d: %s", created.Code, created.Body.String()) + } + cfg := decodeJSON[Config](t, created) + if cfg.OrganizationURL != "https://dev.azure.com/acme" || cfg.DefaultProjectID != "project-1" || + cfg.DefaultProjectName != "Platform" || cfg.AuthMethod != AuthMethodPAT || + !cfg.HasSecret || !cfg.LastOK || cfg.LastCheckedAt == nil { + t.Fatalf("created config = %+v", cfg) + } + if raw := created.Body.String(); jsonContainsKey(t, raw, "pat") { + t.Fatalf("config response leaked the PAT: %s", raw) + } + + loaded := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/config"+query, nil) + if loaded.Code != http.StatusOK || decodeJSON[Config](t, loaded).OrganizationURL != cfg.OrganizationURL { + t.Fatalf("GET /config = %d: %s", loaded.Code, loaded.Body.String()) + } + + deleted := performRequest(t, router, http.MethodDelete, "/api/v1/azure-devops/config"+query, nil) + if deleted.Code != http.StatusOK || decodeJSON[map[string]bool](t, deleted)["deleted"] != true { + t.Fatalf("DELETE /config = %d: %s", deleted.Code, deleted.Body.String()) + } + gone := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/config"+query, nil) + if gone.Code != http.StatusNoContent { + t.Fatalf("GET /config after delete = %d: %s", gone.Code, gone.Body.String()) + } +} + +func jsonContainsKey(t *testing.T, raw, key string) bool { + t.Helper() + var decoded map[string]any + if err := json.Unmarshal([]byte(raw), &decoded); err != nil { + t.Fatalf("decode %q: %v", raw, err) + } + _, ok := decoded[key] + return ok +} + +func TestControllerConfigRouteRejectsBadInput(t *testing.T) { + router, _ := newRouteFixture(t, &readsClient{auth: &TestConnectionResult{OK: true, ID: "me"}}) + tests := []struct { + name string + path string + body any + }{ + {"malformed json", "/api/v1/azure-devops/config?workspace_id=ws-1", "not-an-object"}, + {"missing organization", "/api/v1/azure-devops/config?workspace_id=ws-1", map[string]any{"pat": "x"}}, + {"non-canonical organization", "/api/v1/azure-devops/config?workspace_id=ws-1", map[string]any{ + "organizationUrl": "https://dev.azure.com/acme/project", "pat": "x", + }}, + {"unsupported auth method", "/api/v1/azure-devops/config?workspace_id=ws-1", map[string]any{ + "organizationUrl": "https://dev.azure.com/acme", "pat": "x", "authMethod": "oauth", + }}, + {"missing workspace", "/api/v1/azure-devops/config", map[string]any{ + "organizationUrl": "https://dev.azure.com/acme", "pat": "x", + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + response := performRequest(t, router, http.MethodPost, tt.path, tt.body) + if response.Code != http.StatusBadRequest { + t.Fatalf("status = %d: %s", response.Code, response.Body.String()) + } + }) + } + if response := performRequest(t, router, http.MethodDelete, "/api/v1/azure-devops/config", nil); response.Code != http.StatusBadRequest { + t.Fatalf("DELETE without workspace = %d: %s", response.Code, response.Body.String()) + } +} + +func TestControllerTestConfigRouteProbesSubmittedAndStoredCredentials(t *testing.T) { + client := &readsClient{auth: &TestConnectionResult{OK: true, ID: "me", DisplayName: "Ada", Email: "ada@example.com"}} + router, _ := newRouteFixture(t, client) + const query = "?workspace_id=ws-1" + + submitted := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/config/test"+query, map[string]any{ + "organizationUrl": "https://dev.azure.com/acme", "pat": "probe-only", + }) + if submitted.Code != http.StatusOK { + t.Fatalf("POST /config/test = %d: %s", submitted.Code, submitted.Body.String()) + } + result := decodeJSON[TestConnectionResult](t, submitted) + if !result.OK || result.ID != "me" || result.DisplayName != "Ada" || result.Email != "ada@example.com" { + t.Fatalf("probe result = %+v", result) + } + // A probe must not persist anything. + if stored := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/config"+query, nil); stored.Code != http.StatusNoContent { + t.Fatalf("probe persisted a config: %d %s", stored.Code, stored.Body.String()) + } + + // With no body and no stored credentials the failure is reported inside + // the 200 result rather than as an HTTP error. + empty := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/config/test"+query, nil) + if empty.Code != http.StatusOK { + t.Fatalf("empty-body probe = %d: %s", empty.Code, empty.Body.String()) + } + if emptyResult := decodeJSON[TestConnectionResult](t, empty); emptyResult.OK || emptyResult.Error != ErrNotConfigured.Error() { + t.Fatalf("empty-body probe result = %+v", emptyResult) + } + + invalid := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/config/test"+query, "not-an-object") + if invalid.Code != http.StatusBadRequest { + t.Fatalf("malformed probe payload = %d: %s", invalid.Code, invalid.Body.String()) + } +} + +func TestControllerCopyConfigRoute(t *testing.T) { + client := &readsClient{auth: &TestConnectionResult{OK: true, ID: "me"}} + router, service := newRouteFixture(t, client) + if _, err := service.SetConfigForWorkspace(t.Context(), "ws-1", &SetConfigRequest{ + OrganizationURL: "https://dev.azure.com/acme", DefaultProjectID: "project-1", PAT: "pat", + }); err != nil { + t.Fatalf("seed source config: %v", err) + } + + copied := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/config/copy?workspace_id=ws-1", + map[string]any{"targetWorkspaceId": "ws-2"}) + if copied.Code != http.StatusOK { + t.Fatalf("POST /config/copy = %d: %s", copied.Code, copied.Body.String()) + } + target := decodeJSON[Config](t, copied) + if target.WorkspaceID != "ws-2" || target.OrganizationURL != "https://dev.azure.com/acme" || + target.DefaultProjectID != "project-1" || !target.HasSecret { + t.Fatalf("copied config = %+v", target) + } + if target.LastCheckedAt != nil || target.LastOK { + t.Fatalf("copied config inherited health state: %+v", target) + } + + invalid := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/config/copy?workspace_id=ws-1", "nope") + if invalid.Code != http.StatusBadRequest { + t.Fatalf("malformed copy payload = %d: %s", invalid.Code, invalid.Body.String()) + } + blank := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/config/copy?workspace_id=ws-1", + map[string]any{"targetWorkspaceId": " "}) + if blank.Code != http.StatusBadRequest { + t.Fatalf("blank target workspace = %d: %s", blank.Code, blank.Body.String()) + } +} + +func TestControllerWorkspaceSettingsRoutes(t *testing.T) { + router, service := newRouteFixture(t, &readsClient{}) + seedRouteConfig(t, service) + const path = "/api/v1/azure-devops/workspace-settings?workspace_id=ws-1" + + defaults := performRequest(t, router, http.MethodGet, path, nil) + if defaults.Code != http.StatusOK { + t.Fatalf("GET workspace-settings = %d: %s", defaults.Code, defaults.Body.String()) + } + settings := decodeJSON[WorkspaceSettings](t, defaults) + if settings.WorkspaceID != "ws-1" || + len(settings.WorkItemQueries) != len(DefaultWorkItemQueryPresets()) || + len(settings.PullRequestQueries) != len(DefaultPullRequestQueryPresets()) || + len(settings.WorkItemActions) != len(DefaultWorkItemActionPresets()) || + len(settings.PullRequestActions) != len(DefaultPullRequestActionPresets()) { + t.Fatalf("default settings = %+v", settings) + } + + patched := performRequest(t, router, http.MethodPatch, path, map[string]any{ + "workItemActions": []map[string]any{{ + "id": "triage", "label": "Triage", "hint": "Sort it", "icon": "inbox", + "promptTemplate": "Triage {{url}}", + }}, + }) + if patched.Code != http.StatusOK { + t.Fatalf("PATCH workspace-settings = %d: %s", patched.Code, patched.Body.String()) + } + updated := decodeJSON[WorkspaceSettings](t, patched) + if len(updated.WorkItemActions) != 1 || updated.WorkItemActions[0].ID != "triage" || + updated.WorkItemActions[0].PromptTemplate != "Triage {{url}}" { + t.Fatalf("patched work-item actions = %+v", updated.WorkItemActions) + } + if len(updated.PullRequestActions) != len(DefaultPullRequestActionPresets()) { + t.Fatalf("unrelated presets were replaced: %+v", updated.PullRequestActions) + } + + reloaded := decodeJSON[WorkspaceSettings](t, performRequest(t, router, http.MethodGet, path, nil)) + if len(reloaded.WorkItemActions) != 1 || reloaded.WorkItemActions[0].Label != "Triage" { + t.Fatalf("override did not persist: %+v", reloaded.WorkItemActions) + } +} + +func TestControllerWorkspaceSettingsRouteRejectsBadInput(t *testing.T) { + router, service := newRouteFixture(t, &readsClient{}) + seedRouteConfig(t, service) + tests := []struct { + name string + path string + body any + want int + }{ + {"malformed json", "/api/v1/azure-devops/workspace-settings?workspace_id=ws-1", "nope", http.StatusBadRequest}, + {"missing workspace", "/api/v1/azure-devops/workspace-settings", map[string]any{}, http.StatusBadRequest}, + { + name: "preset without a label", path: "/api/v1/azure-devops/workspace-settings?workspace_id=ws-1", + body: map[string]any{"workItemQueries": []map[string]any{{"id": "x", "filters": map[string]any{"wiql": "SELECT"}}}}, + want: http.StatusBadRequest, + }, + { + name: "empty preset list", path: "/api/v1/azure-devops/workspace-settings?workspace_id=ws-1", + body: map[string]any{"pullRequestActions": []map[string]any{}}, + want: http.StatusBadRequest, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + response := performRequest(t, router, http.MethodPatch, tt.path, tt.body) + if response.Code != tt.want { + t.Fatalf("status = %d, want %d: %s", response.Code, tt.want, response.Body.String()) + } + }) + } + if response := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/workspace-settings", nil); response.Code != http.StatusBadRequest { + t.Fatalf("GET without workspace = %d: %s", response.Code, response.Body.String()) + } +} + +func TestControllerRepositoryAndBranchRoutes(t *testing.T) { + client := &readsClient{ + repositories: []Repository{{ + ID: "repo-1", Name: "widgets", ProjectName: "Platform", + DefaultBranch: "main", WebURL: "https://dev.azure.com/acme/Platform/_git/widgets", + }}, + branches: []Branch{{Name: "main"}, {Name: "feature/azure"}}, + } + router, service := newRouteFixture(t, client) + seedRouteConfig(t, service) + + repositories := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/repositories?workspace_id=ws-1&project=project-1", nil) + if repositories.Code != http.StatusOK { + t.Fatalf("GET /repositories = %d: %s", repositories.Code, repositories.Body.String()) + } + repoBody := decodeJSON[struct { + Repositories []Repository `json:"repositories"` + }](t, repositories) + want := Repository{ + ID: "repo-1", Name: "widgets", ProjectID: "project-1", ProjectName: "Platform", + DefaultBranch: "main", WebURL: "https://dev.azure.com/acme/Platform/_git/widgets", + } + if len(repoBody.Repositories) != 1 || repoBody.Repositories[0] != want { + t.Fatalf("repositories = %+v, want [%+v]", repoBody.Repositories, want) + } + if noProject := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/repositories?workspace_id=ws-1", nil); noProject.Code != http.StatusBadRequest { + t.Fatalf("GET /repositories without project = %d: %s", noProject.Code, noProject.Body.String()) + } + + branches := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/branches?workspace_id=ws-1&organization=acme&project=project-1&repository=repo-1", nil) + if branches.Code != http.StatusOK { + t.Fatalf("GET /branches = %d: %s", branches.Code, branches.Body.String()) + } + branchBody := decodeJSON[struct { + Branches []Branch `json:"branches"` + }](t, branches) + if len(branchBody.Branches) != 2 || branchBody.Branches[0].Name != "main" || + branchBody.Branches[1].Name != "feature/azure" { + t.Fatalf("branches = %+v", branchBody.Branches) + } + if client.lastBranchArgs != [2]string{"project-1", "repo-1"} { + t.Fatalf("client received %v", client.lastBranchArgs) + } + + mismatched := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/branches?workspace_id=ws-1&organization=other&project=project-1&repository=repo-1", nil) + if mismatched.Code != http.StatusBadRequest { + t.Fatalf("organization mismatch = %d: %s", mismatched.Code, mismatched.Body.String()) + } +} + +func TestControllerPullRequestReadRoutes(t *testing.T) { + client := &readsClient{ + pr: &PullRequest{ + ID: 42, Title: "Ship it", Status: "active", IsDraft: true, + SourceBranch: "feature/azure", TargetBranch: "main", + ProjectID: "project-1", RepositoryID: "repo-1", + WebURL: "https://dev.azure.com/acme/Platform/_git/widgets/pullrequest/42", + }, + page: &PullRequestPage{Items: []PullRequest{{ID: 42, Title: "Ship it"}}, Count: 1, Skip: 5, Top: 25}, + } + router, service := newRouteFixture(t, client) + seedRouteConfig(t, service) + + detail := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/pull-requests/project-1/repo-1/42?workspace_id=ws-1", nil) + if detail.Code != http.StatusOK { + t.Fatalf("GET pull request = %d: %s", detail.Code, detail.Body.String()) + } + got := decodeJSON[PullRequest](t, detail) + if got.ID != 42 || got.Title != "Ship it" || got.Status != "active" || !got.IsDraft || + got.SourceBranch != "feature/azure" || got.TargetBranch != "main" || + got.ProjectID != "project-1" || got.RepositoryID != "repo-1" || got.WebURL == "" { + t.Fatalf("pull request = %+v", got) + } + + for _, id := range []string{"0", "-3", "abc"} { + invalid := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/pull-requests/project-1/repo-1/"+id+"?workspace_id=ws-1", nil) + if invalid.Code != http.StatusBadRequest { + t.Fatalf("pull request id %q = %d: %s", id, invalid.Code, invalid.Body.String()) + } + feedback := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/pull-requests/project-1/repo-1/"+id+"/feedback?workspace_id=ws-1", nil) + if feedback.Code != http.StatusBadRequest { + t.Fatalf("feedback for pull request id %q = %d: %s", id, feedback.Code, feedback.Body.String()) + } + } + + listed := performRequest(t, router, http.MethodGet, + "/api/v1/azure-devops/pull-requests?workspace_id=ws-1&project=project-1&repository=repo-1"+ + "&status=active&creator=creator-1&reviewer=reviewer-1"+ + "&source_branch=feature/azure&target_branch=main&skip=5&top=25", nil) + if listed.Code != http.StatusOK { + t.Fatalf("GET /pull-requests = %d: %s", listed.Code, listed.Body.String()) + } + page := decodeJSON[PullRequestPage](t, listed) + if page.Count != 1 || len(page.Items) != 1 || page.Skip != 5 || page.Top != 25 { + t.Fatalf("page = %+v", page) + } + wantFilter := PullRequestFilter{ + ProjectID: "project-1", RepositoryID: "repo-1", Status: "active", + CreatorID: "creator-1", ReviewerID: "reviewer-1", + SourceBranch: "feature/azure", TargetBranch: "main", Skip: 5, Top: 25, + } + if client.lastFilter != wantFilter { + t.Fatalf("filter sent to the client = %+v, want %+v", client.lastFilter, wantFilter) + } +} diff --git a/apps/backend/internal/azuredevops/controller_watch_routes_test.go b/apps/backend/internal/azuredevops/controller_watch_routes_test.go new file mode 100644 index 0000000000..966525dfda --- /dev/null +++ b/apps/backend/internal/azuredevops/controller_watch_routes_test.go @@ -0,0 +1,311 @@ +package azuredevops + +import ( + "errors" + "net/http" + "testing" + + "github.com/gin-gonic/gin" +) + +func pullRequestWatchPayload() map[string]any { + return map[string]any{ + "workflowId": "wf-1", "workflowStepId": "step-1", "projectId": "project-1", + "azureRepositoryId": "azure-repo-1", "status": "active", + "repositoryId": "repo-1", "baseBranch": "main", + "agentProfileId": "agent-1", "executorProfileId": "executor-1", + "prompt": "Review {{title}}", "pollIntervalSeconds": 1, + } +} + +func newWatchRouteFixture(t *testing.T) (*gin.Engine, *Service, *Store) { + t.Helper() + client := &watchClient{} + router, service := newRouteFixture(t, client) + seedRouteConfig(t, service) + return router, service, service.Store() +} + +func TestControllerPullRequestWatchRouteLifecycle(t *testing.T) { + router, _, store := newWatchRouteFixture(t) + const path = "/api/v1/azure-devops/watches/pull-requests" + const query = "?workspace_id=ws-1" + + created := performRequest(t, router, http.MethodPost, path+query, pullRequestWatchPayload()) + if created.Code != http.StatusOK { + t.Fatalf("POST watch = %d: %s", created.Code, created.Body.String()) + } + watch := decodeJSON[PullRequestWatch](t, created) + if watch.ID == "" || watch.WorkspaceID != "ws-1" || watch.ProjectID != "project-1" || + watch.AzureRepositoryID != "azure-repo-1" || watch.Status != "active" || + watch.RepositoryID != "repo-1" || watch.BaseBranch != "main" || + !watch.Enabled || watch.PollIntervalSeconds != minWatchPollIntervalSeconds || + watch.CleanupPolicy != CleanupPolicyAuto { + t.Fatalf("created watch = %+v", watch) + } + + listed := performRequest(t, router, http.MethodGet, path+query, nil) + if listed.Code != http.StatusOK { + t.Fatalf("GET watches = %d: %s", listed.Code, listed.Body.String()) + } + list := decodeJSON[struct { + Watches []PullRequestWatch `json:"watches"` + }](t, listed) + if len(list.Watches) != 1 || list.Watches[0].ID != watch.ID { + t.Fatalf("listed watches = %+v", list.Watches) + } + // The list is workspace scoped. + other := performRequest(t, router, http.MethodGet, path+"?workspace_id=ws-2", nil) + if other.Code != http.StatusOK { + t.Fatalf("GET watches for another workspace = %d: %s", other.Code, other.Body.String()) + } + if otherList := decodeJSON[struct { + Watches []PullRequestWatch `json:"watches"` + }](t, other); len(otherList.Watches) != 0 { + t.Fatalf("watch leaked into another workspace: %+v", otherList.Watches) + } + + updated := performRequest(t, router, http.MethodPatch, path+"/"+watch.ID+query, map[string]any{ + "enabled": false, "status": "completed", "prompt": "Fix {{title}}", + }) + if updated.Code != http.StatusOK { + t.Fatalf("PATCH watch = %d: %s", updated.Code, updated.Body.String()) + } + patched := decodeJSON[PullRequestWatch](t, updated) + if patched.Enabled || patched.Status != "completed" || patched.Prompt != "Fix {{title}}" || + patched.AzureRepositoryID != "azure-repo-1" { + t.Fatalf("patched watch = %+v", patched) + } + + triggered := performRequest(t, router, http.MethodPost, path+"/"+watch.ID+"/trigger"+query, nil) + if triggered.Code != http.StatusOK { + t.Fatalf("POST trigger = %d: %s", triggered.Code, triggered.Body.String()) + } + if result := decodeJSON[WatchTriggerResult](t, triggered); result.Matched != 0 { + t.Fatalf("trigger result = %+v, want no matches from the empty client", result) + } + + preview := performRequest(t, router, http.MethodGet, path+"/"+watch.ID+"/reset/preview"+query, nil) + if preview.Code != http.StatusOK || decodeJSON[map[string]int](t, preview)["taskCount"] != 0 { + t.Fatalf("reset preview = %d: %s", preview.Code, preview.Body.String()) + } + + reset := performRequest(t, router, http.MethodPost, path+"/"+watch.ID+"/reset"+query, nil) + if reset.Code != http.StatusOK { + t.Fatalf("POST reset = %d: %s", reset.Code, reset.Body.String()) + } + resetBody := decodeJSON[map[string]int](t, reset) + if resetBody["generation"] != 2 || resetBody["taskCount"] != 0 { + t.Fatalf("reset body = %+v", resetBody) + } + + deleted := performRequest(t, router, http.MethodDelete, path+"/"+watch.ID+query, nil) + if deleted.Code != http.StatusOK || decodeJSON[map[string]bool](t, deleted)["deleted"] != true { + t.Fatalf("DELETE watch = %d: %s", deleted.Code, deleted.Body.String()) + } + if got, err := store.GetPullRequestWatch(t.Context(), watch.ID); err != nil || got != nil { + t.Fatalf("watch after delete = %+v, %v", got, err) + } + if repeat := performRequest(t, router, http.MethodDelete, path+"/"+watch.ID+query, nil); repeat.Code != http.StatusNotFound { + t.Fatalf("repeat DELETE = %d: %s", repeat.Code, repeat.Body.String()) + } +} + +func TestControllerWorkItemWatchDeleteAndTriggerRoutes(t *testing.T) { + router, _, store := newWatchRouteFixture(t) + watch := seedWorkItemWatch(t, store) + const path = "/api/v1/azure-devops/watches/work-items" + const query = "?workspace_id=ws-1" + + triggered := performRequest(t, router, http.MethodPost, path+"/"+watch.ID+"/trigger"+query, nil) + if triggered.Code != http.StatusOK { + t.Fatalf("POST trigger = %d: %s", triggered.Code, triggered.Body.String()) + } + if result := decodeJSON[WatchTriggerResult](t, triggered); result.Matched != 0 { + t.Fatalf("trigger result = %+v", result) + } + + deleted := performRequest(t, router, http.MethodDelete, path+"/"+watch.ID+query, nil) + if deleted.Code != http.StatusOK || decodeJSON[map[string]bool](t, deleted)["deleted"] != true { + t.Fatalf("DELETE watch = %d: %s", deleted.Code, deleted.Body.String()) + } + if got, err := store.GetWorkItemWatch(t.Context(), watch.ID); err != nil || got != nil { + t.Fatalf("watch after delete = %+v, %v", got, err) + } + for _, route := range []struct { + method string + path string + }{ + {http.MethodPost, path + "/" + watch.ID + "/trigger" + query}, + {http.MethodDelete, path + "/" + watch.ID + query}, + {http.MethodGet, path + "/" + watch.ID + "/reset/preview" + query}, + {http.MethodPost, path + "/" + watch.ID + "/reset" + query}, + } { + response := performRequest(t, router, route.method, route.path, nil) + if response.Code != http.StatusNotFound { + t.Fatalf("%s %s after delete = %d: %s", route.method, route.path, response.Code, response.Body.String()) + } + } +} + +func TestControllerWatchRoutesRejectMalformedPayloads(t *testing.T) { + router, _, store := newWatchRouteFixture(t) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + const query = "?workspace_id=ws-1" + tests := []struct { + name string + method string + path string + body any + want int + }{ + {"create work-item watch with malformed json", http.MethodPost, "/api/v1/azure-devops/watches/work-items" + query, "nope", http.StatusBadRequest}, + {"create work-item watch without dependencies", http.MethodPost, "/api/v1/azure-devops/watches/work-items" + query, map[string]any{"projectId": "project-1"}, http.StatusBadRequest}, + {"update work-item watch with malformed json", http.MethodPatch, "/api/v1/azure-devops/watches/work-items/" + work.ID + query, "nope", http.StatusBadRequest}, + {"create pull-request watch with malformed json", http.MethodPost, "/api/v1/azure-devops/watches/pull-requests" + query, "nope", http.StatusBadRequest}, + {"create pull-request watch without dependencies", http.MethodPost, "/api/v1/azure-devops/watches/pull-requests" + query, map[string]any{"projectId": "project-1"}, http.StatusBadRequest}, + {"update pull-request watch with malformed json", http.MethodPatch, "/api/v1/azure-devops/watches/pull-requests/" + pr.ID + query, "nope", http.StatusBadRequest}, + {"create work-item watch without a workspace", http.MethodPost, "/api/v1/azure-devops/watches/work-items", map[string]any{}, http.StatusBadRequest}, + {"list work-item watches without a workspace", http.MethodGet, "/api/v1/azure-devops/watches/work-items", nil, http.StatusBadRequest}, + {"list pull-request watches without a workspace", http.MethodGet, "/api/v1/azure-devops/watches/pull-requests", nil, http.StatusBadRequest}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + response := performRequest(t, router, tt.method, tt.path, tt.body) + if response.Code != tt.want { + t.Fatalf("status = %d, want %d: %s", response.Code, tt.want, response.Body.String()) + } + }) + } + if watches, err := store.ListPullRequestWatches(t.Context(), "ws-1"); err != nil || len(watches) != 1 { + t.Fatalf("rejected payloads created watches: %+v %v", watches, err) + } +} + +func TestControllerTaskWorkItemRoutes(t *testing.T) { + client := &taskWorkItemClient{fakeDetailClient: fakeDetailClient{detail: &WorkItemDetail{WorkItem: WorkItem{ + ID: 101, Title: "Fix build", State: "Active", Type: "Bug", + Project: "project-1", WebURL: "https://dev.azure.com/acme/project/_workitems/edit/101", + }}}} + router, service := newRouteFixture(t, client) + seedRouteConfig(t, service) + if _, err := service.Store().db.Exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, workspace_id TEXT NOT NULL)`); err != nil { + t.Fatalf("create tasks table: %v", err) + } + if _, err := service.Store().db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-1', 'ws-1'), ('task-2', 'ws-other')`); err != nil { + t.Fatalf("seed tasks: %v", err) + } + + associated := performRequest(t, router, http.MethodPost, + "/api/v1/azure-devops/tasks/task-1/work-items?workspace_id=ws-1", + map[string]any{"projectId": "project-1", "workItemId": 101}) + if associated.Code != http.StatusOK { + t.Fatalf("POST task work item = %d: %s", associated.Code, associated.Body.String()) + } + row := decodeJSON[TaskWorkItem](t, associated) + if row.TaskID != "task-1" || row.WorkspaceID != "ws-1" || row.ProjectID != "project-1" || + row.WorkItemID != 101 || row.Title != "Fix build" || row.State != "Active" || + row.Type != "Bug" || row.WorkItemURL == "" { + t.Fatalf("associated row = %+v", row) + } + + listed := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/workspaces/ws-1/task-work-items", nil) + if listed.Code != http.StatusOK { + t.Fatalf("GET task work items = %d: %s", listed.Code, listed.Body.String()) + } + response := decodeJSON[TaskWorkItemsResponse](t, listed) + if len(response.TaskWorkItems["task-1"]) != 1 || response.TaskWorkItems["task-1"][0].WorkItemID != 101 { + t.Fatalf("listed task work items = %+v", response.TaskWorkItems) + } + if empty := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/workspaces/ws-other/task-work-items", nil); empty.Code != http.StatusOK { + t.Fatalf("GET for another workspace = %d: %s", empty.Code, empty.Body.String()) + } +} + +func TestControllerTaskWorkItemRouteRejectsInvalidAssociations(t *testing.T) { + client := &taskWorkItemClient{fakeDetailClient: fakeDetailClient{ + detail: &WorkItemDetail{WorkItem: WorkItem{ID: 101, Project: "project-1"}}, + }} + router, service := newRouteFixture(t, client) + seedRouteConfig(t, service) + if _, err := service.Store().db.Exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, workspace_id TEXT NOT NULL)`); err != nil { + t.Fatalf("create tasks table: %v", err) + } + if _, err := service.Store().db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-1', 'ws-1'), ('task-2', 'ws-other')`); err != nil { + t.Fatalf("seed tasks: %v", err) + } + tests := []struct { + name string + path string + body any + want int + }{ + {"malformed json", "/api/v1/azure-devops/tasks/task-1/work-items?workspace_id=ws-1", "nope", http.StatusBadRequest}, + {"missing project", "/api/v1/azure-devops/tasks/task-1/work-items?workspace_id=ws-1", map[string]any{"workItemId": 101}, http.StatusBadRequest}, + {"non-positive work item", "/api/v1/azure-devops/tasks/task-1/work-items?workspace_id=ws-1", map[string]any{"projectId": "project-1", "workItemId": 0}, http.StatusBadRequest}, + {"task in another workspace", "/api/v1/azure-devops/tasks/task-2/work-items?workspace_id=ws-1", map[string]any{"projectId": "project-1", "workItemId": 101}, http.StatusBadRequest}, + {"work item in another project", "/api/v1/azure-devops/tasks/task-1/work-items?workspace_id=ws-1", map[string]any{"projectId": "project-2", "workItemId": 101}, http.StatusBadRequest}, + {"missing workspace", "/api/v1/azure-devops/tasks/task-1/work-items", map[string]any{"projectId": "project-1", "workItemId": 101}, http.StatusBadRequest}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + response := performRequest(t, router, http.MethodPost, tt.path, tt.body) + if response.Code != tt.want { + t.Fatalf("status = %d, want %d: %s", response.Code, tt.want, response.Body.String()) + } + }) + } + rows, err := service.ListTaskWorkItemsByWorkspace(t.Context(), "ws-1") + if err != nil || len(rows) != 0 { + t.Fatalf("rejected associations persisted rows: %+v %v", rows, err) + } +} + +// TestControllerMapsErrorsToTransportStatuses pins the writeError translation +// table, including the Azure-specific response codes the frontend switches on. +func TestControllerMapsErrorsToTransportStatuses(t *testing.T) { + tests := []struct { + name string + err error + want int + wantCode string + }{ + {name: "invalid config", err: ErrInvalidConfig, want: http.StatusBadRequest}, + {name: "invalid task PR association", err: ErrInvalidTaskPRAssociation, want: http.StatusBadRequest}, + {name: "watch not found", err: ErrWatchNotFound, want: http.StatusNotFound}, + {name: "reservation not found", err: ErrReservationNotFound, want: http.StatusNotFound}, + {name: "watch ownership lost", err: ErrWatchOwnershipLost, want: http.StatusConflict}, + {name: "not configured", err: ErrNotConfigured, want: http.StatusServiceUnavailable, wantCode: notConfiguredCode}, + { + name: "upstream revision conflict", want: http.StatusConflict, + wantCode: "azure_devops_revision_conflict", + err: &APIError{StatusCode: http.StatusConflict, Endpoint: "/_apis/projects", Body: "rev mismatch"}, + }, + {name: "upstream unauthorized", err: &APIError{StatusCode: http.StatusUnauthorized}, want: http.StatusUnauthorized}, + {name: "upstream forbidden", err: &APIError{StatusCode: http.StatusForbidden}, want: http.StatusForbidden}, + {name: "upstream rate limited", err: &APIError{StatusCode: http.StatusTooManyRequests}, want: http.StatusTooManyRequests}, + {name: "upstream server error", err: &APIError{StatusCode: http.StatusInternalServerError}, want: http.StatusBadGateway}, + {name: "upstream bad gateway", err: &APIError{StatusCode: http.StatusBadGateway}, want: http.StatusBadGateway}, + {name: "upstream not found", err: &APIError{StatusCode: http.StatusNotFound}, want: http.StatusBadRequest}, + {name: "wrapped upstream error", err: errors.Join(errors.New("context"), &APIError{StatusCode: http.StatusUnauthorized}), want: http.StatusUnauthorized}, + {name: "unclassified failure", err: errors.New("something broke"), want: http.StatusInternalServerError}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + router, service := newRouteFixture(t, &invalidClient{err: tt.err}) + seedRouteConfig(t, service) + response := performRequest(t, router, http.MethodGet, "/api/v1/azure-devops/projects?workspace_id=ws-1", nil) + if response.Code != tt.want { + t.Fatalf("status = %d, want %d: %s", response.Code, tt.want, response.Body.String()) + } + body := decodeJSON[map[string]string](t, response) + if body["error"] != tt.err.Error() { + t.Fatalf("error body = %q, want %q", body["error"], tt.err.Error()) + } + if body["code"] != tt.wantCode { + t.Fatalf("error code = %q, want %q", body["code"], tt.wantCode) + } + }) + } +} diff --git a/apps/backend/internal/azuredevops/goleak_test.go b/apps/backend/internal/azuredevops/goleak_test.go new file mode 100644 index 0000000000..de97d75706 --- /dev/null +++ b/apps/backend/internal/azuredevops/goleak_test.go @@ -0,0 +1,16 @@ +package azuredevops + +import ( + "testing" + + "go.uber.org/goleak" +) + +// TestMain enforces no goroutine leaks across the azuredevops package. The +// Poller owns an auth-health loop and a watcher loop, both lifecycle-managed +// via Start/Stop; regressions where Stop forgets to cancel either context or +// drain the WaitGroup surface here, as do watcher "initial check" goroutines +// that outlive their service. +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/apps/backend/internal/azuredevops/poller_lifecycle_test.go b/apps/backend/internal/azuredevops/poller_lifecycle_test.go new file mode 100644 index 0000000000..c0c966e545 --- /dev/null +++ b/apps/backend/internal/azuredevops/poller_lifecycle_test.go @@ -0,0 +1,355 @@ +package azuredevops + +import ( + "context" + "errors" + "testing" + "testing/synctest" + "time" + + "github.com/kandev/kandev/internal/common/logger" +) + +func TestWatchDueRespectsTheNormalizedInterval(t *testing.T) { + now := time.Date(2026, 8, 12, 12, 0, 0, 0, time.UTC) + at := func(offset time.Duration) *time.Time { + value := now.Add(offset) + return &value + } + tests := []struct { + name string + last *time.Time + interval int + want bool + }{ + {name: "never polled", want: true}, + {name: "exactly at the interval", last: at(-5 * time.Minute), interval: 300, want: true}, + {name: "one second early", last: at(-299 * time.Second), interval: 300}, + {name: "well past the interval", last: at(-time.Hour), interval: 300, want: true}, + {name: "zero interval falls back to the default", last: at(-4 * time.Minute), interval: 0}, + {name: "sub-minimum interval is clamped up", last: at(-30 * time.Second), interval: 1}, + {name: "sub-minimum interval clears the clamped minimum", last: at(-90 * time.Second), interval: 1, want: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := watchDue(tt.last, tt.interval, now); got != tt.want { + t.Fatalf("watchDue(%v, %d) = %t, want %t", tt.last, tt.interval, got, tt.want) + } + }) + } +} + +func TestNewPollerRequiresAServiceAndToleratesANilLogger(t *testing.T) { + if poller := NewPoller(nil, logger.Default()); poller != nil { + t.Fatalf("NewPoller(nil) = %+v, want nil", poller) + } + service, _ := newWatchService(t, &watchClient{}) + poller := NewPoller(service, nil) + if poller == nil || poller.logger == nil || poller.auth == nil { + t.Fatalf("NewPoller = %+v, want a usable poller with a default logger", poller) + } + + // A nil poller must stay safe to drive: production wiring skips Azure + // entirely when the integration is not constructed. + var absent *Poller + absent.Start(context.Background()) + absent.Stop() +} + +// TestPollerStartRunsChecksImmediatelyAndOnEveryTick drives the real +// timer-owned loop under fake time: the first pass happens on Start and each +// watcherPollTick produces exactly one more pass. +func TestPollerStartRunsChecksImmediatelyAndOnEveryTick(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + client := &watchClient{work: &WorkItemSearchResult{Items: []WorkItem{{ID: 101, WebURL: "https://azure/101"}}}} + service, store := newWatchService(t, client) + watch := seedWorkItemWatch(t, store) + watch.PollIntervalSeconds = minWatchPollIntervalSeconds + if err := store.UpdateWorkItemWatch(t.Context(), watch); err != nil { + t.Fatalf("set poll interval: %v", err) + } + + poller := NewPoller(service, logger.Default()) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(func() { + cancel() + poller.Stop() + }) + poller.Start(ctx) + synctest.Wait() + + rows, err := store.ListWorkItemWatchTasks(t.Context(), watch.ID, watch.Generation) + if err != nil || len(rows) != 1 || rows[0].WorkItemID != 101 { + t.Fatalf("reservations after the start pass = %+v, %v", rows, err) + } + + // A second work item appears upstream; the next tick must import it. + client.mu.Lock() + client.work = &WorkItemSearchResult{Items: []WorkItem{{ID: 101}, {ID: 102, WebURL: "https://azure/102"}}} + client.mu.Unlock() + time.Sleep(watcherPollTick) + synctest.Wait() + + rows, err = store.ListWorkItemWatchTasks(t.Context(), watch.ID, watch.Generation) + if err != nil || len(rows) != 2 { + t.Fatalf("reservations after one tick = %+v, %v", rows, err) + } + if rows[1].WorkItemID != 102 || rows[1].WorkItemURL != "https://azure/102" { + t.Fatalf("second reservation = %+v", rows[1]) + } + }) +} + +// TestPollerStopDrainsAndRestarts pins the Start/Stop contract: Stop waits for +// the loop, repeated calls are no-ops, and a stopped poller can start again. +func TestPollerStopDrainsAndRestarts(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + client := &watchClient{work: &WorkItemSearchResult{Items: []WorkItem{{ID: 101}}}} + service, store := newWatchService(t, client) + watch := seedWorkItemWatch(t, store) + + poller := NewPoller(service, logger.Default()) + ctx, cancel := context.WithCancel(context.Background()) + // Registered up front, before any t.Fatalf path: Stop is idempotent, so + // this stays correct across the explicit Stop and restart below. + t.Cleanup(func() { + cancel() + poller.Stop() + }) + + poller.Start(ctx) + poller.Start(ctx) // second Start must not launch a second loop + synctest.Wait() + poller.Stop() + poller.Stop() // idempotent + + if poller.started || poller.cancel != nil { + t.Fatalf("poller state after Stop = started %t, cancel %v", poller.started, poller.cancel) + } + + // Nothing else runs while stopped, even after a full tick elapses. + before, err := store.ListWorkItemWatchTasks(t.Context(), watch.ID, watch.Generation) + if err != nil || len(before) != 1 { + t.Fatalf("reservations before the quiet period = %+v, %v", before, err) + } + client.mu.Lock() + client.work = &WorkItemSearchResult{Items: []WorkItem{{ID: 101}, {ID: 102}}} + client.mu.Unlock() + // Long enough that the watch is comfortably due again once restarted. + time.Sleep(2 * defaultWatchPollIntervalSeconds * time.Second) + synctest.Wait() + after, err := store.ListWorkItemWatchTasks(t.Context(), watch.ID, watch.Generation) + if err != nil || len(after) != 1 { + t.Fatalf("stopped poller kept polling: %+v, %v", after, err) + } + + poller.Start(ctx) + synctest.Wait() + restarted, err := store.ListWorkItemWatchTasks(t.Context(), watch.ID, watch.Generation) + if err != nil || len(restarted) != 2 { + t.Fatalf("restarted poller reservations = %+v, %v", restarted, err) + } + }) +} + +func TestRunWatchChecksSkipsWatchesThatAreNotDue(t *testing.T) { + client := &watchClient{ + work: &WorkItemSearchResult{Items: []WorkItem{{ID: 101}}}, + page: &PullRequestPage{Items: []PullRequest{{ID: 42, ProjectID: "project-1", RepositoryID: "azure-repo-1"}}}, + } + service, store := newWatchService(t, client) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + polledAt := time.Now().UTC() + if err := store.ClearWorkItemWatchError(t.Context(), work.ID, polledAt); err != nil { + t.Fatalf("mark work-item watch polled: %v", err) + } + if err := store.ClearPullRequestWatchError(t.Context(), pr.ID, polledAt); err != nil { + t.Fatalf("mark pull-request watch polled: %v", err) + } + + poller := NewPoller(service, logger.Default()) + if matched := poller.runWatchChecks(t.Context()); matched != 0 { + t.Fatalf("matched = %d, want 0 for watches inside their interval", matched) + } + if client.lastWIQL != [2]string{} || client.filter().ProjectID != "" { + t.Fatalf("upstream was queried for a watch that was not due: %v / %+v", client.lastWIQL, client.filter()) + } + if rows, _ := store.ListWorkItemWatchTasks(t.Context(), work.ID, work.Generation); len(rows) != 0 { + t.Fatalf("reservations created for a watch that was not due: %+v", rows) + } +} + +// TestRunWatchChecksContinuesPastAFailingWatch proves one broken workspace +// cannot stall every other watcher in the poll pass. +func TestRunWatchChecksContinuesPastAFailingWatch(t *testing.T) { + client := &watchClient{ + work: &WorkItemSearchResult{Items: []WorkItem{{ID: 101}}}, + page: &PullRequestPage{Items: []PullRequest{{ID: 42, ProjectID: "project-1", RepositoryID: "azure-repo-1"}}}, + } + service, store := newWatchService(t, client) + // "ws-broken" has watches but no stored credentials, so its checks fail. + broken := &WorkItemWatch{WorkspaceID: "ws-broken", ProjectID: "project-1", WIQL: "SELECT"} + if err := store.CreateWorkItemWatch(t.Context(), broken); err != nil { + t.Fatalf("create broken work-item watch: %v", err) + } + brokenPR := &PullRequestWatch{WorkspaceID: "ws-broken", ProjectID: "project-1", AzureRepositoryID: "azure-repo-1"} + if err := store.CreatePullRequestWatch(t.Context(), brokenPR); err != nil { + t.Fatalf("create broken pull-request watch: %v", err) + } + healthy := seedWorkItemWatch(t, store) + healthyPR := seedPullRequestWatch(t, store) + + poller := NewPoller(service, logger.Default()) + if matched := poller.runWatchChecks(t.Context()); matched != 2 { + t.Fatalf("matched = %d, want 2 from the healthy watches", matched) + } + if rows, _ := store.ListWorkItemWatchTasks(t.Context(), healthy.ID, healthy.Generation); len(rows) != 1 { + t.Fatalf("healthy work-item watch produced %d reservations", len(rows)) + } + if rows, _ := store.ListPullRequestWatchTasks(t.Context(), healthyPR.ID, healthyPR.Generation); len(rows) != 1 { + t.Fatalf("healthy pull-request watch produced %d reservations", len(rows)) + } + gotBroken, _ := store.GetWorkItemWatch(t.Context(), broken.ID) + if gotBroken.LastError != ErrNotConfigured.Error() { + t.Fatalf("broken work-item watch error = %q", gotBroken.LastError) + } + gotBrokenPR, _ := store.GetPullRequestWatch(t.Context(), brokenPR.ID) + if gotBrokenPR.LastError != ErrNotConfigured.Error() { + t.Fatalf("broken pull-request watch error = %q", gotBrokenPR.LastError) + } +} + +// TestRunWatchChecksRemovesTasksForTerminalItems covers the per-poll cleanup +// hop: items and pull requests that reached a terminal state have their Kandev +// task trees deleted and their dedup rows released. +func TestRunWatchChecksRemovesTasksForTerminalItems(t *testing.T) { + client := &watchClient{ + workItems: map[int]*WorkItem{101: {ID: 101, State: "Closed"}, 102: {ID: 102, State: "Active"}}, + pullRequests: map[int]*PullRequest{42: {ID: 42, Status: "completed"}, 43: {ID: 43, Status: "active"}}, + } + service, store := newWatchService(t, client) + deleter := &recordingDeleter{} + service.SetCascadeTaskDeleter(deleter) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + seedWorkItemReservation(t, store, work, 101, "task-101") + seedWorkItemReservation(t, store, work, 102, "task-102") + seedPullRequestReservation(t, store, pr, 42, "task-42") + seedPullRequestReservation(t, store, pr, 43, "task-43") + + poller := NewPoller(service, logger.Default()) + if matched := poller.runWatchChecks(t.Context()); matched != 0 { + t.Fatalf("matched = %d, want 0 (no new upstream items)", matched) + } + + ids := deleter.ids() + if len(ids) != 2 || ids[0] != "task-101" || ids[1] != "task-42" { + t.Fatalf("cascade-deleted tasks = %v, want the terminal ones only", ids) + } + workRows, _ := store.ListWorkItemWatchTasks(t.Context(), work.ID, work.Generation) + if len(workRows) != 1 || workRows[0].WorkItemID != 102 { + t.Fatalf("work-item reservations after cleanup = %+v", workRows) + } + prRows, _ := store.ListPullRequestWatchTasks(t.Context(), pr.ID, pr.Generation) + if len(prRows) != 1 || prRows[0].PullRequestID != 43 { + t.Fatalf("pull-request reservations after cleanup = %+v", prRows) + } +} + +// TestPollCleanupSkipsWatchesItMustNotTouch guards the early returns in the +// per-poll cleanup helpers. +func TestPollCleanupSkipsWatchesItMustNotTouch(t *testing.T) { + client := &watchClient{ + workItems: map[int]*WorkItem{101: {ID: 101, State: "Closed"}}, + pullRequests: map[int]*PullRequest{42: {ID: 42, Status: "completed"}}, + } + service, store := newWatchService(t, client) + deleter := &recordingDeleter{} + service.SetCascadeTaskDeleter(deleter) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + seedWorkItemReservation(t, store, work, 101, "task-101") + seedPullRequestReservation(t, store, pr, 42, "task-42") + + if cleaned := service.cleanupWorkItemWatchForPoll(t.Context(), "missing-watch"); cleaned != 0 { + t.Fatalf("cleanup for a missing work-item watch = %d", cleaned) + } + if cleaned := service.cleanupPullRequestWatchForPoll(t.Context(), "missing-watch"); cleaned != 0 { + t.Fatalf("cleanup for a missing pull-request watch = %d", cleaned) + } + if err := store.DisableWorkItemWatchWithError(t.Context(), work.ID, "disabled"); err != nil { + t.Fatalf("disable work-item watch: %v", err) + } + if err := store.DisablePullRequestWatchWithError(t.Context(), pr.ID, "disabled"); err != nil { + t.Fatalf("disable pull-request watch: %v", err) + } + if cleaned := service.cleanupWorkItemWatchForPoll(t.Context(), work.ID); cleaned != 0 { + t.Fatalf("cleanup ran for a disabled work-item watch: %d", cleaned) + } + if cleaned := service.cleanupPullRequestWatchForPoll(t.Context(), pr.ID); cleaned != 0 { + t.Fatalf("cleanup ran for a disabled pull-request watch: %d", cleaned) + } + if ids := deleter.ids(); len(ids) != 0 { + t.Fatalf("tasks were deleted for skipped watches: %v", ids) + } + + // Re-enable, then drop the workspace credentials: cleanup must bail out + // rather than run without a client. + work.Enabled = true + if err := store.UpdateWorkItemWatch(t.Context(), work); err != nil { + t.Fatalf("re-enable work-item watch: %v", err) + } + pr.Enabled = true + if err := store.UpdatePullRequestWatch(t.Context(), pr); err != nil { + t.Fatalf("re-enable pull-request watch: %v", err) + } + if err := service.DeleteConfigForWorkspace(t.Context(), "ws-1"); err != nil { + t.Fatalf("delete config: %v", err) + } + if cleaned := service.cleanupWorkItemWatchForPoll(t.Context(), work.ID); cleaned != 0 { + t.Fatalf("cleanup ran without credentials: %d", cleaned) + } + if cleaned := service.cleanupPullRequestWatchForPoll(t.Context(), pr.ID); cleaned != 0 { + t.Fatalf("cleanup ran without credentials: %d", cleaned) + } + if ids := deleter.ids(); len(ids) != 0 { + t.Fatalf("tasks were deleted without credentials: %v", ids) + } +} + +func TestAuthProberReportsConfiguredWorkspacesAndPersistsHealth(t *testing.T) { + client := &fakeClient{result: &TestConnectionResult{OK: false, Error: "401 unauthorized"}} + service, store, _ := newTestService(t, func(*Config, string) Client { return client }) + prober := authProber{service: service} + + configured, err := prober.HasConfig(t.Context()) + if err != nil || configured { + t.Fatalf("HasConfig before any config = %t, %v; want false", configured, err) + } + if _, err := service.SetConfigForWorkspace(t.Context(), "ws-1", &SetConfigRequest{ + OrganizationURL: "https://dev.azure.com/acme", PAT: "pat", + }); err != nil { + t.Fatalf("set config: %v", err) + } + if err := store.ResetAuthHealth(t.Context(), "ws-1"); err != nil { + t.Fatalf("reset health: %v", err) + } + + configured, err = prober.HasConfig(t.Context()) + if err != nil || !configured { + t.Fatalf("HasConfig after config = %t, %v; want true", configured, err) + } + prober.RecordAuthHealth(t.Context()) + cfg, err := service.GetConfigForWorkspace(t.Context(), "ws-1") + if err != nil || cfg.LastCheckedAt == nil || cfg.LastOK || cfg.LastError != "401 unauthorized" { + t.Fatalf("persisted health = %+v, %v", cfg, err) + } + + client.result, client.err = nil, errors.New("network timeout") + prober.RecordAuthHealth(t.Context()) + cfg, _ = service.GetConfigForWorkspace(t.Context(), "ws-1") + if cfg.LastOK || cfg.LastError != "network timeout" { + t.Fatalf("health after a transport failure = %+v", cfg) + } +} diff --git a/apps/backend/internal/azuredevops/provider_routes_test.go b/apps/backend/internal/azuredevops/provider_routes_test.go new file mode 100644 index 0000000000..eccb9365c3 --- /dev/null +++ b/apps/backend/internal/azuredevops/provider_routes_test.go @@ -0,0 +1,244 @@ +package azuredevops + +import ( + "context" + "net/http" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/common/logger" +) + +func TestProvideBuildsAServiceAndOnlyMocksWhenAsked(t *testing.T) { + db := newTestDB(t) + service, cleanup, err := Provide(db, db, newFakeSecretStore(), nil, logger.Default()) + if err != nil || service == nil || cleanup == nil { + t.Fatalf("Provide returned service %v, cleanup present %t, err %v", service, cleanup != nil, err) + } + t.Cleanup(func() { + if err := cleanup(); err != nil { + t.Errorf("cleanup: %v", err) + } + }) + if service.Store() == nil { + t.Fatal("Provide returned a service without a store") + } + if service.MockClient() != nil { + t.Fatalf("Provide wired a mock client without %s", mockEnvVar) + } + + t.Setenv(mockEnvVar, "true") + mocked, mockedCleanup, err := Provide(newTestDB(t), db, newFakeSecretStore(), nil, logger.Default()) + if err != nil { + t.Fatalf("Provide with mock: %v", err) + } + t.Cleanup(func() { _ = mockedCleanup() }) + if mocked.MockClient() == nil { + t.Fatalf("Provide did not wire a mock client with %s=true", mockEnvVar) + } + // The mock client is what the factory hands back for any workspace. + if client := mocked.clientFn(&Config{OrganizationURL: "https://dev.azure.com/acme"}, "pat"); client != mocked.MockClient() { + t.Fatalf("client factory = %T, want the mock client", client) + } +} + +func TestRegisterMockRoutesOnlyMountsWhenAMockIsWired(t *testing.T) { + gin.SetMode(gin.TestMode) + db := newTestDB(t) + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("new store: %v", err) + } + plain := NewService(store, newFakeSecretStore(), func(*Config, string) Client { return &invalidClient{} }, logger.Default()) + plainRouter := gin.New() + RegisterMockRoutes(plainRouter, plain, logger.Default()) + if response := performRequest(t, plainRouter, http.MethodPost, "/api/v1/azure-devops/mock/state", MockState{}); response.Code != http.StatusNotFound { + t.Fatalf("mock route mounted without a mock client: %d", response.Code) + } + RegisterMockRoutes(plainRouter, nil, logger.Default()) + + mock := NewMockClient() + service := NewService(store, newFakeSecretStore(), func(*Config, string) Client { return mock }, logger.Default()) + service.mock = mock + router := gin.New() + RegisterMockRoutes(router, service, logger.Default()) + + seeded := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/mock/state", MockState{ + Authenticated: true, + User: TestConnectionResult{OK: true, ID: "seeded-user", DisplayName: "Seeded"}, + Projects: []Project{{ID: "project-1", Name: "Seeded Platform"}}, + }) + if seeded.Code != http.StatusOK || !decodeJSON[map[string]bool](t, seeded)["seeded"] { + t.Fatalf("POST mock state = %d: %s", seeded.Code, seeded.Body.String()) + } + projects, err := mock.ListProjects(context.Background()) + if err != nil || len(projects) != 1 || projects[0].Name != "Seeded Platform" { + t.Fatalf("seeded projects = %+v, %v", projects, err) + } + + invalid := performRequest(t, router, http.MethodPost, "/api/v1/azure-devops/mock/state", "nope") + if invalid.Code != http.StatusBadRequest { + t.Fatalf("malformed mock state = %d: %s", invalid.Code, invalid.Body.String()) + } + + reset := performRequest(t, router, http.MethodDelete, "/api/v1/azure-devops/mock/state", nil) + if reset.Code != http.StatusOK || !decodeJSON[map[string]bool](t, reset)["reset"] { + t.Fatalf("DELETE mock state = %d: %s", reset.Code, reset.Body.String()) + } + if projects, err := mock.ListProjects(context.Background()); err != nil || len(projects) != 0 { + t.Fatalf("projects after reset = %+v, %v", projects, err) + } + auth, err := mock.TestAuth(context.Background()) + if err != nil || !auth.OK || auth.ID != "mock-user" { + t.Fatalf("auth after reset = %+v, %v", auth, err) + } +} + +// TestDefaultClientFactoryFailsClosedWithoutConfig proves the production +// factory never returns a client that silently succeeds when the workspace +// configuration is missing. +func TestDefaultClientFactoryFailsClosedWithoutConfig(t *testing.T) { + client := DefaultClientFactory(nil, "pat") + ctx := context.Background() + calls := map[string]func() error{ + "TestAuth": func() error { _, err := client.TestAuth(ctx); return err }, + "ListProjects": func() error { _, err := client.ListProjects(ctx); return err }, + "ListRepositories": func() error { _, err := client.ListRepositories(ctx, "p"); return err }, + "QueryWIQL": func() error { _, err := client.QueryWIQL(ctx, "p", "SELECT", 1); return err }, + "GetWorkItem": func() error { _, err := client.GetWorkItem(ctx, "p", 1); return err }, + "ListPullRequests": func() error { _, err := client.ListPullRequests(ctx, PullRequestFilter{}); return err }, + "GetPullRequest": func() error { _, err := client.GetPullRequest(ctx, "p", "r", 1); return err }, + "ListReviewers": func() error { _, err := client.ListReviewers(ctx, "p", "r", 1); return err }, + "ListThreads": func() error { _, err := client.ListThreads(ctx, "p", "r", 1); return err }, + "ListLinkedWorkItems": func() error { _, err := client.ListLinkedWorkItems(ctx, "p", "r", 1); return err }, + "ListPolicyEvaluations": func() error { _, err := client.ListPolicyEvaluations(ctx, "p", 1); return err }, + } + for name, call := range calls { + t.Run(name, func(t *testing.T) { + err := call() + if err == nil || err.Error() != "azure devops: config is required" { + t.Fatalf("error = %v, want the missing-config failure", err) + } + }) + } + + rest, ok := DefaultClientFactory(&Config{OrganizationURL: "https://dev.azure.com/acme"}, "pat").(*RESTClient) + if !ok || rest.organization != "acme" || rest.pat != "pat" || rest.initErr != nil { + t.Fatalf("configured factory = %+v, ok = %t", rest, ok) + } +} + +// TestControllerReadRoutesSurfaceUpstreamFailures walks every workspace-scoped +// read route with a failing Azure client and proves none of them answers 200 +// with empty data. +func TestControllerReadRoutesSurfaceUpstreamFailures(t *testing.T) { + upstream := &APIError{StatusCode: http.StatusInternalServerError, Endpoint: "/_apis", Body: "boom"} + routes := []struct { + name string + method string + path string + body any + }{ + {"projects", http.MethodGet, "/api/v1/azure-devops/projects?workspace_id=ws-1", nil}, + {"teams", http.MethodGet, "/api/v1/azure-devops/teams?workspace_id=ws-1&project=project-1", nil}, + {"boards", http.MethodGet, "/api/v1/azure-devops/boards?workspace_id=ws-1&project=project-1&team=team-1", nil}, + {"board snapshot", http.MethodGet, "/api/v1/azure-devops/boards/board-1?workspace_id=ws-1&project=project-1&team=team-1", nil}, + {"repositories", http.MethodGet, "/api/v1/azure-devops/repositories?workspace_id=ws-1&project=project-1", nil}, + {"branches", http.MethodGet, "/api/v1/azure-devops/branches?workspace_id=ws-1&project=project-1&repository=repo-1", nil}, + {"identity", http.MethodGet, "/api/v1/azure-devops/identity?workspace_id=ws-1", nil}, + {"work item", http.MethodGet, "/api/v1/azure-devops/work-items/101?workspace_id=ws-1&project=project-1", nil}, + {"work item comments", http.MethodGet, "/api/v1/azure-devops/work-items/101/comments?workspace_id=ws-1&project=project-1", nil}, + {"pull request", http.MethodGet, "/api/v1/azure-devops/pull-requests/project-1/repo-1/42?workspace_id=ws-1", nil}, + {"pull request feedback", http.MethodGet, "/api/v1/azure-devops/pull-requests/project-1/repo-1/42/feedback?workspace_id=ws-1", nil}, + {"pull requests", http.MethodGet, "/api/v1/azure-devops/pull-requests?workspace_id=ws-1&project=project-1&repository=repo-1", nil}, + { + "work item search", http.MethodPost, "/api/v1/azure-devops/work-items/search?workspace_id=ws-1", + map[string]any{"project": "project-1", "wiql": "SELECT [System.Id] FROM WorkItems"}, + }, + { + "work item assignment", http.MethodPatch, "/api/v1/azure-devops/work-items/101?workspace_id=ws-1&project=project-1", + map[string]any{"revision": 7, "assigneeAction": unassignAction}, + }, + { + "board work item", http.MethodPatch, + "/api/v1/azure-devops/boards/board-1/work-items/101?workspace_id=ws-1&project=project-1&team=team-1", + map[string]any{"revision": 7, "columnDone": true}, + }, + } + for _, route := range routes { + t.Run(route.name, func(t *testing.T) { + router, service := newRouteFixture(t, &failingAzureClient{err: upstream}) + seedRouteConfig(t, service) + response := performRequest(t, router, route.method, route.path, route.body) + if response.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502: %s", response.Code, response.Body.String()) + } + if body := decodeJSON[map[string]string](t, response); body["error"] != upstream.Error() { + t.Fatalf("error body = %q, want %q", body["error"], upstream.Error()) + } + }) + } +} + +// failingAzureClient implements every optional capability the routes probe for +// and fails all of them, so the tests above exercise the handler error branch +// rather than a "capability unavailable" short circuit. +type failingAzureClient struct { + invalidClient + err error +} + +func (c *failingAzureClient) ListTeams(context.Context, string) ([]Team, error) { return nil, c.err } +func (c *failingAzureClient) ListBoards(context.Context, string, string) ([]BoardReference, error) { + return nil, c.err +} + +func (c *failingAzureClient) GetBoardSnapshot(context.Context, string, string, string) (*BoardSnapshot, error) { + return nil, c.err +} + +func (c *failingAzureClient) UpdateBoardWorkItem(context.Context, string, string, string, int, BoardWorkItemUpdateRequest) (*BoardWorkItem, error) { + return nil, c.err +} + +func (c *failingAzureClient) UpdateWorkItem(context.Context, string, int, WorkItemAssignmentRequest) (*WorkItem, error) { + return nil, c.err +} + +func (c *failingAzureClient) ListBranches(context.Context, string, string) ([]Branch, error) { + return nil, c.err +} + +func (c *failingAzureClient) GetWorkItemDetail(context.Context, string, int) (*WorkItemDetail, error) { + return nil, c.err +} + +func (c *failingAzureClient) ListWorkItemComments(context.Context, string, int, string) (*WorkItemCommentPage, error) { + return nil, c.err +} + +func (c *failingAzureClient) GetCurrentIdentity(context.Context) (*Identity, error) { + return nil, c.err +} + +func (c *failingAzureClient) TestAuth(context.Context) (*TestConnectionResult, error) { + return nil, c.err +} + +func (c *failingAzureClient) ListProjects(context.Context) ([]Project, error) { return nil, c.err } +func (c *failingAzureClient) ListRepositories(context.Context, string) ([]Repository, error) { + return nil, c.err +} + +func (c *failingAzureClient) QueryWIQL(context.Context, string, string, int) (*WorkItemSearchResult, error) { + return nil, c.err +} + +func (c *failingAzureClient) ListPullRequests(context.Context, PullRequestFilter) (*PullRequestPage, error) { + return nil, c.err +} + +func (c *failingAzureClient) GetPullRequest(context.Context, string, string, int) (*PullRequest, error) { + return nil, c.err +} diff --git a/apps/backend/internal/azuredevops/rest_client_errors_test.go b/apps/backend/internal/azuredevops/rest_client_errors_test.go new file mode 100644 index 0000000000..a717866843 --- /dev/null +++ b/apps/backend/internal/azuredevops/rest_client_errors_test.go @@ -0,0 +1,574 @@ +package azuredevops + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +// TestRESTClientSurfacesUpstreamEndpointForEveryRead pins the exact Azure REST +// path each client method targets. The failing server records the path it was +// asked for, and the returned APIError must carry that same path back. +func TestRESTClientSurfacesUpstreamEndpointForEveryRead(t *testing.T) { + const failureBody = `{"message":"upstream is down"}` + var requested string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requested = r.URL.Path + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(failureBody)) + })) + t.Cleanup(server.Close) + + client := newTestRESTClient(t, server, "pat") + ctx := context.Background() + column := "column-active" + assign := assignCurrentUserAction + tests := []struct { + name string + endpoint string + call func() error + }{ + {"TestAuth", "/_apis/connectionData", func() error { + _, err := client.TestAuth(ctx) + return err + }}, + {"ListProjects", "/_apis/projects", func() error { + _, err := client.ListProjects(ctx) + return err + }}, + {"ListRepositories", "/project-1/_apis/git/repositories", func() error { + _, err := client.ListRepositories(ctx, "project-1") + return err + }}, + {"ListBranches", "/project-1/_apis/git/repositories/repo-1/refs", func() error { + _, err := client.ListBranches(ctx, "project-1", "repo-1") + return err + }}, + {"QueryWIQL", "/project-1/_apis/wit/wiql", func() error { + _, err := client.QueryWIQL(ctx, "project-1", "SELECT [System.Id] FROM WorkItems", 0) + return err + }}, + {"GetWorkItem", "/project-1/_apis/wit/workitems/101", func() error { + _, err := client.GetWorkItem(ctx, "project-1", 101) + return err + }}, + {"GetWorkItemDetail", "/project-1/_apis/wit/workitems/101", func() error { + _, err := client.GetWorkItemDetail(ctx, "project-1", 101) + return err + }}, + {"ListWorkItemComments", "/project-1/_apis/wit/workItems/101/comments", func() error { + _, err := client.ListWorkItemComments(ctx, "project-1", 101, "") + return err + }}, + {"GetCurrentIdentity", "/_apis/connectionData", func() error { + _, err := client.GetCurrentIdentity(ctx) + return err + }}, + {"ListTeams", "/_apis/projects/project-1/teams", func() error { + _, err := client.ListTeams(ctx, "project-1") + return err + }}, + {"ListBoards", "/project-1/team-1/_apis/work/backlogs", func() error { + _, err := client.ListBoards(ctx, "project-1", "team-1") + return err + }}, + {"GetBoardSnapshot", "/project-1/team-1/_apis/work/boards/board-1", func() error { + _, err := client.GetBoardSnapshot(ctx, "project-1", "team-1", "board-1") + return err + }}, + {"UpdateBoardWorkItem", "/project-1/team-1/_apis/work/boards/board-1", func() error { + _, err := client.UpdateBoardWorkItem(ctx, "project-1", "team-1", "board-1", 101, + BoardWorkItemUpdateRequest{Revision: 7, ColumnID: &column}) + return err + }}, + {"UpdateWorkItem", "/project-1/_apis/wit/workitems/101", func() error { + _, err := client.UpdateWorkItem(ctx, "project-1", 101, WorkItemAssignmentRequest{ + Revision: 7, AssigneeAction: &assign, + resolvedAssignee: "ada@example.com", hasResolvedAssignee: true, + }) + return err + }}, + {"ListPullRequests", "/project-1/_apis/git/repositories/repo-1/pullrequests", func() error { + _, err := client.ListPullRequests(ctx, PullRequestFilter{ProjectID: "project-1", RepositoryID: "repo-1"}) + return err + }}, + {"ListPullRequestsForProject", "/project-1/_apis/git/pullrequests", func() error { + _, err := client.ListPullRequests(ctx, PullRequestFilter{ProjectID: "project-1"}) + return err + }}, + {"GetPullRequest", "/project-1/_apis/git/repositories/repo-1/pullrequests/42", func() error { + _, err := client.GetPullRequest(ctx, "project-1", "repo-1", 42) + return err + }}, + {"ListReviewers", "/project-1/_apis/git/repositories/repo-1/pullrequests/42/reviewers", func() error { + _, err := client.ListReviewers(ctx, "project-1", "repo-1", 42) + return err + }}, + {"ListThreads", "/project-1/_apis/git/repositories/repo-1/pullrequests/42/threads", func() error { + _, err := client.ListThreads(ctx, "project-1", "repo-1", 42) + return err + }}, + {"ListLinkedWorkItems", "/project-1/_apis/git/repositories/repo-1/pullrequests/42/workitems", func() error { + _, err := client.ListLinkedWorkItems(ctx, "project-1", "repo-1", 42) + return err + }}, + {"ListPolicyEvaluations", "/project-1/_apis/policy/evaluations", func() error { + _, err := client.ListPolicyEvaluations(ctx, "project-1", 42) + return err + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + requested = "" + err := tt.call() + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want *APIError", err) + } + if apiErr.StatusCode != http.StatusInternalServerError { + t.Errorf("status = %d, want 500", apiErr.StatusCode) + } + if apiErr.Endpoint != tt.endpoint { + t.Errorf("APIError.Endpoint = %q, want %q", apiErr.Endpoint, tt.endpoint) + } + if requested != "/acme"+tt.endpoint { + t.Errorf("requested path = %q, want %q", requested, "/acme"+tt.endpoint) + } + if apiErr.Body != failureBody { + t.Errorf("APIError.Body = %q, want %q", apiErr.Body, failureBody) + } + }) + } +} + +func TestAPIErrorMessageIncludesEndpointStatusAndBody(t *testing.T) { + err := &APIError{StatusCode: http.StatusForbidden, Endpoint: "/_apis/projects", Body: "no access"} + want := "azure devops API /_apis/projects returned 403: no access" + if got := err.Error(); got != want { + t.Fatalf("Error() = %q, want %q", got, want) + } + var target *APIError + if !AsAPIError(fmt.Errorf("wrapped: %w", err), &target) || target != err { + t.Fatalf("AsAPIError did not unwrap the API error") + } +} + +// TestRESTClientPropagatesWorkItemHydrationFailure covers the second upstream +// hop: the ID query succeeds and the batch hydration is what fails. +func TestRESTClientPropagatesWorkItemHydrationFailure(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/acme/project-1/_apis/wit/wiql": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"workItems":[{"id":1}]}`)) + case "/acme/project-1/team-1/_apis/work/boards/board-1": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"board-1","fields":{"columnField":{"referenceName":"System.BoardColumn"}}}`)) + case "/acme/project-1/team-1/_apis/work/backlogs/board-1/workItems": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"workItems":[{"target":{"id":1}}]}`)) + default: + w.WriteHeader(http.StatusBadGateway) + _, _ = w.Write([]byte("batch unavailable")) + } + })) + t.Cleanup(server.Close) + + client := newTestRESTClient(t, server, "pat") + for _, tt := range []struct { + name string + call func() error + }{ + {"QueryWIQL", func() error { + _, err := client.QueryWIQL(context.Background(), "project-1", "SELECT", 10) + return err + }}, + {"GetBoardSnapshot", func() error { + _, err := client.GetBoardSnapshot(context.Background(), "project-1", "team-1", "board-1") + return err + }}, + } { + t.Run(tt.name, func(t *testing.T) { + var apiErr *APIError + if err := tt.call(); !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want *APIError", err) + } + if apiErr.Endpoint != "/project-1/_apis/wit/workitemsbatch" || apiErr.StatusCode != http.StatusBadGateway { + t.Fatalf("hydration error = %+v", apiErr) + } + }) + } +} + +// TestRESTClientBoardSnapshotPropagatesBacklogItemFailure covers the board +// read succeeding while the backlog work-item reference read fails. +func TestRESTClientBoardSnapshotPropagatesBacklogItemFailure(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/acme/project-1/team-1/_apis/work/boards/board-1" { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"board-1","name":"Stories"}`)) + return + } + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte("no backlog")) + })) + t.Cleanup(server.Close) + + _, err := newTestRESTClient(t, server, "pat").GetBoardSnapshot(context.Background(), "project-1", "team-1", "board-1") + var apiErr *APIError + if !errors.As(err, &apiErr) || apiErr.Endpoint != "/project-1/team-1/_apis/work/backlogs/board-1/workItems" { + t.Fatalf("error = %v (%+v), want backlog work-item endpoint", err, apiErr) + } +} + +func TestRESTClientWrapsTransportFailure(t *testing.T) { + transportErr := errors.New("dial tcp: connection refused") + client := NewRESTClient("https://dev.azure.com/acme", "pat", &http.Client{ + Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { return nil, transportErr }), + }) + + _, err := client.ListProjects(context.Background()) + if err == nil || !strings.Contains(err.Error(), "azure devops request /_apis/projects") { + t.Fatalf("error = %v, want endpoint-tagged transport failure", err) + } + var apiErr *APIError + if errors.As(err, &apiErr) { + t.Fatalf("transport failure reported as APIError: %+v", apiErr) + } +} + +type failingBody struct{ err error } + +func (b failingBody) Read([]byte) (int, error) { return 0, b.err } +func (b failingBody) Close() error { return nil } + +func TestRESTClientWrapsResponseBodyReadFailure(t *testing.T) { + client := NewRESTClient("https://dev.azure.com/acme", "pat", &http.Client{ + Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{}, + Body: failingBody{err: errors.New("connection reset")}, + }, nil + }), + }) + + _, err := client.ListProjects(context.Background()) + if err == nil || !strings.Contains(err.Error(), "read azure devops response") || + !strings.Contains(err.Error(), "connection reset") { + t.Fatalf("error = %v, want wrapped body read failure", err) + } +} + +func TestRESTClientAcceptsEmptySuccessBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + projects, err := newTestRESTClient(t, server, "pat").ListProjects(context.Background()) + if err != nil || len(projects) != 0 { + t.Fatalf("ListProjects = %+v, %v; want empty result and no error", projects, err) + } +} + +func TestRESTClientRejectsUndecodableSuccessBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"value": not-json}`)) + })) + t.Cleanup(server.Close) + + _, err := newTestRESTClient(t, server, "pat").ListProjects(context.Background()) + if err == nil || !strings.Contains(err.Error(), "decode azure devops response") { + t.Fatalf("error = %v, want decode failure", err) + } +} + +// TestRESTClientNormalizesBranchFiltersIntoRefNames pins the search criteria +// actually sent upstream for branch-scoped pull-request queries. +func TestRESTClientNormalizesBranchFiltersIntoRefNames(t *testing.T) { + var query url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + query = r.URL.Query() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"count":0,"value":[]}`)) + })) + t.Cleanup(server.Close) + + page, err := newTestRESTClient(t, server, "pat").ListPullRequests(context.Background(), PullRequestFilter{ + ProjectID: "project-1", RepositoryID: "repo-1", + SourceBranch: "feature/azure", TargetBranch: "refs/heads/main", + Skip: 25, Top: 500, + }) + if err != nil { + t.Fatalf("ListPullRequests: %v", err) + } + if got := query.Get("searchCriteria.sourceRefName"); got != "refs/heads/feature/azure" { + t.Errorf("sourceRefName = %q, want refs/heads/feature/azure", got) + } + if got := query.Get("searchCriteria.targetRefName"); got != "refs/heads/main" { + t.Errorf("targetRefName = %q, want the already-qualified ref unchanged", got) + } + if query.Get("$skip") != "25" || query.Get("$top") != "50" { + t.Errorf("paging = skip %q top %q, want 25 and the clamped default 50", query.Get("$skip"), query.Get("$top")) + } + if page.Skip != 25 || page.Top != defaultPRPageSize || page.Count != 0 || len(page.Items) != 0 { + t.Errorf("page = %+v", page) + } +} + +// TestRESTClientOmitsUnsetPullRequestSearchCriteria proves empty filters are +// left off the query string rather than sent as empty values. +func TestRESTClientOmitsUnsetPullRequestSearchCriteria(t *testing.T) { + var query url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + query = r.URL.Query() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"count":0,"value":[]}`)) + })) + t.Cleanup(server.Close) + + if _, err := newTestRESTClient(t, server, "pat").ListPullRequests(context.Background(), + PullRequestFilter{ProjectID: "project-1", RepositoryID: "repo-1", Top: 10}); err != nil { + t.Fatalf("ListPullRequests: %v", err) + } + for _, key := range []string{ + "searchCriteria.status", "searchCriteria.creatorId", "searchCriteria.reviewerId", + "searchCriteria.sourceRefName", "searchCriteria.targetRefName", "$skip", + } { + if _, present := query[key]; present { + t.Errorf("query carries unset filter %q = %q", key, query.Get(key)) + } + } + if query.Get("$top") != "10" { + t.Errorf("$top = %q, want the requested 10", query.Get("$top")) + } +} + +func TestPlanningFieldsSkipsAbsentAndBlankValues(t *testing.T) { + fields := map[string]any{ + "Microsoft.VSTS.Scheduling.Effort": 3, + "Microsoft.VSTS.Scheduling.StoryPoints": " ", + "Microsoft.VSTS.Scheduling.Size": nil, + "Microsoft.VSTS.Scheduling.RemainingWork": "2.5", + "Microsoft.VSTS.Scheduling.OriginalEstimate": 0, + "System.Title": "not a planning field", + } + got := planningFields(fields) + want := []PlanningField{ + {ReferenceName: "Microsoft.VSTS.Scheduling.Effort", Label: "Effort", Value: "3"}, + {ReferenceName: "Microsoft.VSTS.Scheduling.RemainingWork", Label: "Remaining work", Value: "2.5"}, + {ReferenceName: "Microsoft.VSTS.Scheduling.OriginalEstimate", Label: "Original estimate", Value: "0"}, + } + if len(got) != len(want) { + t.Fatalf("planningFields = %+v, want %+v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("planningFields[%d] = %+v, want %+v", i, got[i], want[i]) + } + } +} + +func TestBoardItemPlacementReadsStringAndBooleanDoneFields(t *testing.T) { + board := Board{ + Columns: []BoardColumn{{ID: "column-done", Name: "Done"}}, + Fields: BoardFields{ + ColumnField: FieldReference{ReferenceName: "System.BoardColumn"}, + DoneField: FieldReference{ReferenceName: "System.BoardColumnDone"}, + }, + } + tests := []struct { + name string + fields map[string]any + columnID string + done bool + }{ + {"string true", map[string]any{"System.BoardColumn": "Done", "System.BoardColumnDone": "True"}, "column-done", true}, + {"string false", map[string]any{"System.BoardColumn": "Done", "System.BoardColumnDone": "no"}, "column-done", false}, + {"boolean true", map[string]any{"System.BoardColumn": "Done", "System.BoardColumnDone": true}, "column-done", true}, + {"unknown column", map[string]any{"System.BoardColumn": "Triage"}, "", false}, + {"unsupported done type", map[string]any{"System.BoardColumnDone": 1}, "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + columnID, done := boardItemPlacement(board, WorkItem{Fields: tt.fields}) + if columnID != tt.columnID || done != tt.done { + t.Fatalf("boardItemPlacement = (%q, %t), want (%q, %t)", columnID, done, tt.columnID, tt.done) + } + }) + } +} + +func TestWorkItemAssignmentPatchRemovesAssigneeAndRejectsUnresolvedRequests(t *testing.T) { + unassign := unassignAction + patch, err := workItemAssignmentPatch(WorkItemAssignmentRequest{ + Revision: 4, AssigneeAction: &unassign, hasResolvedAssignee: true, + }) + if err != nil { + t.Fatalf("workItemAssignmentPatch: %v", err) + } + if len(patch) != 2 || patch[0]["op"] != "test" || patch[0]["value"] != 4 || + patch[1]["op"] != "remove" || patch[1]["path"] != "/fields/System.AssignedTo" { + t.Fatalf("unassign patch = %#v", patch) + } + + assign := assignCurrentUserAction + if _, err := workItemAssignmentPatch(WorkItemAssignmentRequest{ + Revision: 4, AssigneeAction: &assign, + }); err == nil || !strings.Contains(err.Error(), "resolved server-side") { + t.Fatalf("unresolved assignee error = %v", err) + } + if _, err := workItemAssignmentPatch(WorkItemAssignmentRequest{Revision: 0}); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("invalid revision error = %v, want ErrInvalidConfig", err) + } +} + +type testBoard = struct { + Columns []BoardColumn `json:"columns"` + Fields BoardFields `json:"fields"` +} + +func TestBoardWorkItemPatchRejectsUnusableBoardMetadata(t *testing.T) { + column := "column-active" + done := true + assign := assignCurrentUserAction + withColumn := testBoard{Columns: []BoardColumn{{ID: column, Name: "Active"}}} + tests := []struct { + name string + board testBoard + request BoardWorkItemUpdateRequest + wantErr string + }{ + { + name: "unknown column", + board: testBoard{Columns: []BoardColumn{{ID: "column-todo", Name: "To Do"}}}, + request: BoardWorkItemUpdateRequest{Revision: 7, ColumnID: &column}, + wantErr: "unknown board column", + }, + { + name: "board has no column field", + board: withColumn, + request: BoardWorkItemUpdateRequest{Revision: 7, ColumnID: &column}, + wantErr: "board column field is unavailable", + }, + { + name: "board has no done field", + board: withColumn, + request: BoardWorkItemUpdateRequest{Revision: 7, ColumnDone: &done}, + wantErr: "board done field is unavailable", + }, + { + name: "unresolved assignee action", + board: withColumn, + request: BoardWorkItemUpdateRequest{Revision: 7, AssigneeAction: &assign}, + wantErr: "resolved server-side", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := boardWorkItemPatch(tt.board, tt.request); err == nil || + !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("boardWorkItemPatch error = %v, want %q", err, tt.wantErr) + } + }) + } +} + +func TestBoardWorkItemPatchBuildsColumnAndDoneOperations(t *testing.T) { + column := "column-active" + done := false + board := testBoard{ + Columns: []BoardColumn{{ID: column, Name: "Active"}}, + Fields: BoardFields{ + ColumnField: FieldReference{ReferenceName: "System.BoardColumn"}, + DoneField: FieldReference{ReferenceName: "System.BoardColumnDone"}, + }, + } + patch, err := boardWorkItemPatch(board, BoardWorkItemUpdateRequest{Revision: 9, ColumnID: &column, ColumnDone: &done}) + if err != nil { + t.Fatalf("boardWorkItemPatch: %v", err) + } + if len(patch) != 3 || + patch[1]["op"] != "replace" || patch[1]["path"] != "/fields/System.BoardColumn" || patch[1]["value"] != "Active" || + patch[2]["path"] != "/fields/System.BoardColumnDone" || patch[2]["value"] != false { + t.Fatalf("patch = %#v", patch) + } +} + +// TestRESTClientUpdateBoardWorkItemFallsBackToRequestedPlacement covers the +// branch where Azure echoes no board fields back and the client keeps the +// caller's requested column and done flag. +func TestRESTClientUpdateBoardWorkItemFallsBackToRequestedPlacement(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/acme/project-1/team-1/_apis/work/boards/board-1": + _, _ = w.Write([]byte(`{"fields":{"columnField":{"referenceName":"System.BoardColumn"},"doneField":{"referenceName":"System.BoardColumnDone"}},"columns":[{"id":"column-active","name":"Active"}]}`)) + case "/acme/project-1/_apis/wit/workitems/101": + _, _ = w.Write([]byte(`{"id":101,"rev":9,"fields":{"System.Title":"Moved"}}`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + + column := "column-active" + done := true + item, err := newTestRESTClient(t, server, "pat").UpdateBoardWorkItem( + context.Background(), "project-1", "team-1", "board-1", 101, + BoardWorkItemUpdateRequest{Revision: 8, ColumnID: &column, ColumnDone: &done}, + ) + if err != nil { + t.Fatalf("UpdateBoardWorkItem: %v", err) + } + if item.ID != 101 || item.Revision != 9 || item.Title != "Moved" || + item.ColumnID != column || !item.ColumnDone { + t.Fatalf("item = %+v, want requested placement retained", item) + } +} + +func TestRESTClientRejectsUpdateBoardWorkItemWithoutRevision(t *testing.T) { + called := false + client := NewRESTClient("https://dev.azure.com/acme", "pat", &http.Client{ + Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + called = true + return nil, errors.New("must not be reached") + }), + }) + _, err := client.UpdateBoardWorkItem(context.Background(), "project-1", "team-1", "board-1", 101, + BoardWorkItemUpdateRequest{}) + if !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + if called { + t.Fatal("missing revision reached the HTTP transport") + } +} + +func TestRESTClientListLinkedWorkItemsSkipsNonNumericIDs(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"value":[{"id":"101","url":"https://api/101"},{"id":"vstfs:///artifact","url":"https://api/x"},{"id":"7","url":"https://api/7"}]}`)) + })) + t.Cleanup(server.Close) + + refs, err := newTestRESTClient(t, server, "pat").ListLinkedWorkItems(context.Background(), "project-1", "repo-1", 42) + if err != nil { + t.Fatalf("ListLinkedWorkItems: %v", err) + } + want := []WorkItemRef{{ID: 101, URL: "https://api/101"}, {ID: 7, URL: "https://api/7"}} + if len(refs) != len(want) { + t.Fatalf("refs = %+v, want %+v", refs, want) + } + for i := range want { + if refs[i] != want[i] { + t.Fatalf("refs[%d] = %+v, want %+v", i, refs[i], want[i]) + } + } +} diff --git a/apps/backend/internal/azuredevops/service_reads_test.go b/apps/backend/internal/azuredevops/service_reads_test.go new file mode 100644 index 0000000000..2f66dabb9c --- /dev/null +++ b/apps/backend/internal/azuredevops/service_reads_test.go @@ -0,0 +1,384 @@ +package azuredevops + +import ( + "context" + "errors" + "testing" +) + +// readsClient is a fully programmable Client used by the workspace read +// surface tests: every method can be pointed at a canned result or a failure +// so each error hop can be asserted independently. +type readsClient struct { + invalidClient + auth *TestConnectionResult + authErr error + repositories []Repository + repositoriesErr error + branches []Branch + branchesErr error + lastBranchArgs [2]string + workItem *WorkItem + workItemErr error + search *WorkItemSearchResult + searchErr error + lastSearch [3]any + page *PullRequestPage + pageErr error + lastFilter PullRequestFilter + pr *PullRequest + prErr error + reviewers []Reviewer + reviewersErr error + threads []Thread + threadsErr error + refs []WorkItemRef + refsErr error + policies []PolicyEvaluation + policiesErr error +} + +func (c *readsClient) TestAuth(context.Context) (*TestConnectionResult, error) { + return c.auth, c.authErr +} + +func (c *readsClient) ListRepositories(_ context.Context, projectID string) ([]Repository, error) { + if c.repositoriesErr != nil { + return nil, c.repositoriesErr + } + result := make([]Repository, 0, len(c.repositories)) + for _, repository := range c.repositories { + repository.ProjectID = projectID + result = append(result, repository) + } + return result, nil +} + +func (c *readsClient) ListBranches(_ context.Context, projectID, repositoryID string) ([]Branch, error) { + c.lastBranchArgs = [2]string{projectID, repositoryID} + return c.branches, c.branchesErr +} + +func (c *readsClient) GetWorkItem(context.Context, string, int) (*WorkItem, error) { + return c.workItem, c.workItemErr +} + +func (c *readsClient) QueryWIQL(_ context.Context, projectID, wiql string, top int) (*WorkItemSearchResult, error) { + c.lastSearch = [3]any{projectID, wiql, top} + return c.search, c.searchErr +} + +func (c *readsClient) ListPullRequests(_ context.Context, filter PullRequestFilter) (*PullRequestPage, error) { + c.lastFilter = filter + return c.page, c.pageErr +} + +func (c *readsClient) GetPullRequest(context.Context, string, string, int) (*PullRequest, error) { + return c.pr, c.prErr +} + +func (c *readsClient) ListReviewers(context.Context, string, string, int) ([]Reviewer, error) { + return c.reviewers, c.reviewersErr +} + +func (c *readsClient) ListThreads(context.Context, string, string, int) ([]Thread, error) { + return c.threads, c.threadsErr +} + +func (c *readsClient) ListLinkedWorkItems(context.Context, string, string, int) ([]WorkItemRef, error) { + return c.refs, c.refsErr +} + +func (c *readsClient) ListPolicyEvaluations(context.Context, string, int) ([]PolicyEvaluation, error) { + return c.policies, c.policiesErr +} + +func newReadsService(t *testing.T, client Client) *Service { + t.Helper() + service, _, _ := newTestService(t, func(*Config, string) Client { return client }) + if _, err := service.SetConfigForWorkspace(t.Context(), "ws-1", &SetConfigRequest{ + OrganizationURL: "https://dev.azure.com/acme", PAT: "pat", + }); err != nil { + t.Fatalf("set config: %v", err) + } + return service +} + +func TestListRepositoriesForWorkspaceScopesToProject(t *testing.T) { + client := &readsClient{repositories: []Repository{{ + ID: "repo-1", Name: "widgets", ProjectName: "Platform", + DefaultBranch: "main", WebURL: "https://dev.azure.com/acme/Platform/_git/widgets", + }}} + service := newReadsService(t, client) + + repositories, err := service.ListRepositoriesForWorkspace(t.Context(), "ws-1", "project-1") + if err != nil { + t.Fatalf("ListRepositoriesForWorkspace: %v", err) + } + want := Repository{ + ID: "repo-1", Name: "widgets", ProjectID: "project-1", ProjectName: "Platform", + DefaultBranch: "main", WebURL: "https://dev.azure.com/acme/Platform/_git/widgets", + } + if len(repositories) != 1 || repositories[0] != want { + t.Fatalf("repositories = %+v, want [%+v]", repositories, want) + } +} + +func TestListRepositoriesForWorkspaceRejectsBlankProjectAndPropagatesFailures(t *testing.T) { + upstream := errors.New("repositories unavailable") + client := &readsClient{repositoriesErr: upstream} + service := newReadsService(t, client) + + if _, err := service.ListRepositoriesForWorkspace(t.Context(), "ws-1", " "); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("blank project error = %v, want ErrInvalidConfig", err) + } + if _, err := service.ListRepositoriesForWorkspace(t.Context(), "ws-2", "project-1"); !errors.Is(err, ErrNotConfigured) { + t.Fatalf("unconfigured workspace error = %v, want ErrNotConfigured", err) + } + if _, err := service.ListRepositoriesForWorkspace(t.Context(), "ws-1", "project-1"); !errors.Is(err, upstream) { + t.Fatalf("upstream error = %v, want %v", err, upstream) + } +} + +func TestListBranchesForWorkspaceValidatesInputAndForwardsIdentifiers(t *testing.T) { + client := &readsClient{branches: []Branch{{Name: "main"}, {Name: "feature/azure"}}} + service := newReadsService(t, client) + + for _, tt := range []struct{ name, project, repository string }{ + {"blank project", " ", "repo-1"}, + {"blank repository", "project-1", ""}, + } { + t.Run(tt.name, func(t *testing.T) { + if _, err := service.ListBranchesForWorkspace( + t.Context(), "ws-1", "", tt.project, tt.repository, + ); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + }) + } + + branches, err := service.ListBranchesForWorkspace(t.Context(), "ws-1", "acme", "project-1", "repo-1") + if err != nil { + t.Fatalf("ListBranchesForWorkspace: %v", err) + } + if len(branches) != 2 || branches[0].Name != "main" || branches[1].Name != "feature/azure" { + t.Fatalf("branches = %+v", branches) + } + if client.lastBranchArgs != [2]string{"project-1", "repo-1"} { + t.Fatalf("client received %v, want project-1/repo-1", client.lastBranchArgs) + } +} + +func TestListBranchesForWorkspaceRejectsMalformedOrganizationAndMissingCapability(t *testing.T) { + service := newReadsService(t, &readsClient{}) + if _, err := service.ListBranchesForWorkspace( + t.Context(), "ws-1", "not a valid org name", "project-1", "repo-1", + ); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("malformed organization error = %v, want ErrInvalidConfig", err) + } + + limited := newReadsService(t, &invalidClient{}) + _, err := limited.ListBranchesForWorkspace(t.Context(), "ws-1", "", "project-1", "repo-1") + if err == nil || err.Error() != "azure devops: branch listing is unavailable" { + t.Fatalf("error = %v, want branch listing unavailable", err) + } +} + +func TestSearchAndGetWorkItemForWorkspaceValidateArguments(t *testing.T) { + client := &readsClient{ + search: &WorkItemSearchResult{Items: []WorkItem{{ID: 101, Title: "Fix build"}}, Count: 1}, + workItem: &WorkItem{ID: 101, Revision: 4, Title: "Fix build", State: "Active"}, + } + service := newReadsService(t, client) + + if _, err := service.SearchWorkItemsForWorkspace(t.Context(), "ws-1", "", "SELECT", 10); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("blank project search error = %v, want ErrInvalidConfig", err) + } + if _, err := service.SearchWorkItemsForWorkspace(t.Context(), "ws-1", "project-1", " ", 10); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("blank wiql search error = %v, want ErrInvalidConfig", err) + } + result, err := service.SearchWorkItemsForWorkspace(t.Context(), "ws-1", "project-1", "SELECT [System.Id]", 25) + if err != nil || result.Count != 1 || result.Items[0].ID != 101 { + t.Fatalf("search result = %+v, %v", result, err) + } + if client.lastSearch != [3]any{"project-1", "SELECT [System.Id]", 25} { + t.Fatalf("client received %v", client.lastSearch) + } + + for _, tt := range []struct { + name string + project string + id int + }{ + {"blank project", "", 101}, + {"non-positive id", "project-1", 0}, + } { + t.Run(tt.name, func(t *testing.T) { + if _, err := service.GetWorkItemForWorkspace(t.Context(), "ws-1", tt.project, tt.id); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + }) + } + item, err := service.GetWorkItemForWorkspace(t.Context(), "ws-1", "project-1", 101) + if err != nil || item.ID != 101 || item.Revision != 4 || item.State != "Active" { + t.Fatalf("GetWorkItemForWorkspace = %+v, %v", item, err) + } + if _, err := service.GetWorkItemForWorkspace(t.Context(), "ws-2", "project-1", 101); !errors.Is(err, ErrNotConfigured) { + t.Fatalf("unconfigured workspace error = %v, want ErrNotConfigured", err) + } +} + +func TestListPullRequestsForWorkspaceRequiresProjectAndRepository(t *testing.T) { + service := newReadsService(t, &readsClient{page: &PullRequestPage{}}) + for _, tt := range []struct{ name, project, repository string }{ + {"blank project", " ", "repo-1"}, + {"blank repository", "project-1", ""}, + } { + t.Run(tt.name, func(t *testing.T) { + _, err := service.ListPullRequestsForWorkspace(t.Context(), "ws-1", PullRequestFilter{ + ProjectID: tt.project, RepositoryID: tt.repository, + }) + if !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + }) + } +} + +func TestListPullRequestsForWorkspaceResolvesCurrentUserFilters(t *testing.T) { + client := &readsClient{ + auth: &TestConnectionResult{OK: true, ID: "identity-1", DisplayName: "Ada"}, + page: &PullRequestPage{Items: []PullRequest{{ID: 42}}, Count: 1, Top: 50}, + } + service := newReadsService(t, client) + + page, err := service.ListPullRequestsForWorkspace(t.Context(), "ws-1", PullRequestFilter{ + ProjectID: "project-1", RepositoryID: "repo-1", CreatorID: "@ME", ReviewerID: "@me", + Status: "active", Skip: 10, Top: 20, + }) + if err != nil { + t.Fatalf("ListPullRequestsForWorkspace: %v", err) + } + if page.Count != 1 || len(page.Items) != 1 || page.Items[0].ID != 42 { + t.Fatalf("page = %+v", page) + } + want := PullRequestFilter{ + ProjectID: "project-1", RepositoryID: "repo-1", Status: "active", + CreatorID: "identity-1", ReviewerID: "identity-1", Skip: 10, Top: 20, + } + if client.lastFilter != want { + t.Fatalf("filter sent upstream = %+v, want %+v", client.lastFilter, want) + } +} + +func TestListPullRequestsForWorkspaceKeepsExplicitFiltersUnresolved(t *testing.T) { + client := &readsClient{ + auth: &TestConnectionResult{OK: true, ID: "identity-1"}, + page: &PullRequestPage{}, + } + service := newReadsService(t, client) + + if _, err := service.ListPullRequestsForWorkspace(t.Context(), "ws-1", PullRequestFilter{ + ProjectID: "project-1", RepositoryID: "repo-1", CreatorID: "someone-else", + }); err != nil { + t.Fatalf("ListPullRequestsForWorkspace: %v", err) + } + if client.lastFilter.CreatorID != "someone-else" || client.lastFilter.ReviewerID != "" { + t.Fatalf("filter = %+v, want the explicit creator preserved", client.lastFilter) + } +} + +func TestListPullRequestsForWorkspaceFailsWhenIdentityIsUnavailable(t *testing.T) { + probeErr := errors.New("identity probe failed") + tests := []struct { + name string + client *readsClient + wantErr error + }{ + {"probe error", &readsClient{authErr: probeErr}, probeErr}, + {"nil identity", &readsClient{}, ErrInvalidConfig}, + {"unauthenticated identity", &readsClient{auth: &TestConnectionResult{OK: false, ID: "x"}}, ErrInvalidConfig}, + {"identity without id", &readsClient{auth: &TestConnectionResult{OK: true, ID: " "}}, ErrInvalidConfig}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + service := newReadsService(t, tt.client) + _, err := service.ListPullRequestsForWorkspace(t.Context(), "ws-1", PullRequestFilter{ + ProjectID: "project-1", RepositoryID: "repo-1", ReviewerID: "@me", + }) + if !errors.Is(err, tt.wantErr) { + t.Fatalf("error = %v, want %v", err, tt.wantErr) + } + if tt.client.lastFilter.ProjectID != "" { + t.Fatalf("pull requests were listed despite an unusable identity: %+v", tt.client.lastFilter) + } + }) + } +} + +func TestGetPullRequestForWorkspaceReturnsUpstreamRecord(t *testing.T) { + client := &readsClient{pr: &PullRequest{ + ID: 42, Title: "Ship it", Status: "active", SourceBranch: "feature", TargetBranch: "main", + ProjectID: "project-1", RepositoryID: "repo-1", + }} + service := newReadsService(t, client) + + pr, err := service.GetPullRequestForWorkspace(t.Context(), "ws-1", "project-1", "repo-1", 42) + if err != nil || pr.ID != 42 || pr.Title != "Ship it" || pr.SourceBranch != "feature" { + t.Fatalf("GetPullRequestForWorkspace = %+v, %v", pr, err) + } + if _, err := service.GetPullRequestForWorkspace(t.Context(), "ws-2", "project-1", "repo-1", 42); !errors.Is(err, ErrNotConfigured) { + t.Fatalf("unconfigured workspace error = %v, want ErrNotConfigured", err) + } +} + +// TestGetPullRequestFeedbackPropagatesEveryUpstreamFailure walks each of the +// five upstream calls the aggregation makes and proves a failure at any one of +// them aborts the whole read rather than returning partial feedback. +func TestGetPullRequestFeedbackPropagatesEveryUpstreamFailure(t *testing.T) { + upstream := errors.New("upstream unavailable") + base := func() *readsClient { + return &readsClient{pr: &PullRequest{ID: 42}} + } + tests := []struct { + name string + fail func(*readsClient) + }{ + {"pull request", func(c *readsClient) { c.prErr = upstream }}, + {"reviewers", func(c *readsClient) { c.reviewersErr = upstream }}, + {"threads", func(c *readsClient) { c.threadsErr = upstream }}, + {"linked work items", func(c *readsClient) { c.refsErr = upstream }}, + {"policies", func(c *readsClient) { c.policiesErr = upstream }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := base() + tt.fail(client) + service := newReadsService(t, client) + feedback, err := service.GetPullRequestFeedbackForWorkspace(t.Context(), "ws-1", "project-1", "repo-1", 42) + if !errors.Is(err, upstream) || feedback != nil { + t.Fatalf("feedback = %+v, err = %v; want the upstream failure", feedback, err) + } + }) + } +} + +func TestGetPullRequestFeedbackNormalizesEmptyCollections(t *testing.T) { + client := &readsClient{pr: &PullRequest{ID: 42, Title: "Ship it"}} + service := newReadsService(t, client) + + feedback, err := service.GetPullRequestFeedbackForWorkspace(t.Context(), "ws-1", "project-1", "repo-1", 42) + if err != nil { + t.Fatalf("GetPullRequestFeedbackForWorkspace: %v", err) + } + if feedback.PullRequest.ID != 42 || feedback.ReviewState != "" || feedback.PolicyState != "" { + t.Fatalf("feedback summary = %+v", feedback) + } + if feedback.Reviewers == nil || feedback.Threads == nil || + feedback.LinkedWorkItems == nil || feedback.Policies == nil { + t.Fatalf("nil collections leak into the response: %+v", feedback) + } + if len(feedback.Reviewers)+len(feedback.Threads)+len(feedback.LinkedWorkItems)+len(feedback.Policies) != 0 { + t.Fatalf("unexpected feedback content: %+v", feedback) + } +} diff --git a/apps/backend/internal/azuredevops/service_test.go b/apps/backend/internal/azuredevops/service_test.go index 33c2e091f8..402ccd85a8 100644 --- a/apps/backend/internal/azuredevops/service_test.go +++ b/apps/backend/internal/azuredevops/service_test.go @@ -83,9 +83,26 @@ func newTestService(t *testing.T, factory ClientFactory) (*Service, *Store, *fak t.Fatalf("new store: %v", err) } secrets := newFakeSecretStore() + if factory == nil { + factory = offlineClientFactory() + } return NewService(store, secrets, factory, logger.Default()), store, secrets } +// offlineClientFactory is the stub every test must construct a Service with. +// +// NewService falls back to DefaultClientFactory when clientFn is nil, and +// SetConfigForWorkspace probes auth on every credential change, so a nil +// factory makes the suite issue real HTTPS requests to dev.azure.com. That +// costs a network dependency on an unrelated third party and leaves a live +// transport goroutine behind — which is what goleak_test.go now fails on, so +// a reintroduction cannot pass silently. +func offlineClientFactory() ClientFactory { + return func(*Config, string) Client { + return &fakeClient{result: &TestConnectionResult{OK: true}} + } +} + func TestOrganizationURLValidation(t *testing.T) { valid := []string{ "https://dev.azure.com/acme", @@ -150,7 +167,7 @@ func TestConfigWorkspaceIsolationAndReconstruction(t *testing.T) { t.Fatalf("set %s: %v", tc.workspace, err) } } - reconstructed := NewService(store, secrets, nil, logger.Default()) + reconstructed := NewService(store, secrets, offlineClientFactory(), logger.Default()) for _, tc := range []struct{ workspace, org, pat string }{ {"ws-a", "https://dev.azure.com/acme", "pat-a"}, {"ws-b", "https://dev.azure.com/other", "pat-b"}, diff --git a/apps/backend/internal/azuredevops/watch_service_pull_request_test.go b/apps/backend/internal/azuredevops/watch_service_pull_request_test.go new file mode 100644 index 0000000000..67f63b1e0c --- /dev/null +++ b/apps/backend/internal/azuredevops/watch_service_pull_request_test.go @@ -0,0 +1,518 @@ +package azuredevops + +import ( + "context" + "errors" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" + taskservice "github.com/kandev/kandev/internal/task/service" +) + +// watchClient is the programmable Client used by watcher tests. Matches and +// upstream failures are configurable per call so each error hop of +// TriggerWorkItemWatch / TriggerPullRequestWatch can be asserted. +type watchClient struct { + invalidClient + mu sync.Mutex + work *WorkItemSearchResult + workErr error + page *PullRequestPage + pageErr error + lastWIQL [2]string + lastPRFilter PullRequestFilter + workItems map[int]*WorkItem + pullRequests map[int]*PullRequest +} + +func (c *watchClient) QueryWIQL(_ context.Context, projectID, wiql string, _ int) (*WorkItemSearchResult, error) { + c.mu.Lock() + defer c.mu.Unlock() + c.lastWIQL = [2]string{projectID, wiql} + if c.workErr != nil { + return nil, c.workErr + } + if c.work == nil { + return &WorkItemSearchResult{}, nil + } + return c.work, nil +} + +func (c *watchClient) ListPullRequests(_ context.Context, filter PullRequestFilter) (*PullRequestPage, error) { + c.mu.Lock() + defer c.mu.Unlock() + c.lastPRFilter = filter + if c.pageErr != nil { + return nil, c.pageErr + } + if c.page == nil { + return &PullRequestPage{}, nil + } + return c.page, nil +} + +func (c *watchClient) GetWorkItem(_ context.Context, _ string, id int) (*WorkItem, error) { + c.mu.Lock() + defer c.mu.Unlock() + item, ok := c.workItems[id] + if !ok { + return nil, errors.New("work item not found") + } + return item, nil +} + +func (c *watchClient) GetPullRequest(_ context.Context, _, _ string, id int) (*PullRequest, error) { + c.mu.Lock() + defer c.mu.Unlock() + pr, ok := c.pullRequests[id] + if !ok { + return nil, errors.New("pull request not found") + } + return pr, nil +} + +func (c *watchClient) filter() PullRequestFilter { + c.mu.Lock() + defer c.mu.Unlock() + return c.lastPRFilter +} + +// failingEventBus rejects every publish so the watcher's release-and-record +// compensation can be observed. +type failingEventBus struct{ err error } + +func (b *failingEventBus) Publish(context.Context, string, *bus.Event) error { return b.err } +func (b *failingEventBus) Subscribe(string, bus.EventHandler) (bus.Subscription, error) { + return nil, b.err +} + +func (b *failingEventBus) QueueSubscribe(string, string, bus.EventHandler) (bus.Subscription, error) { + return nil, b.err +} + +func (b *failingEventBus) Request(context.Context, string, *bus.Event, time.Duration) (*bus.Event, error) { + return nil, b.err +} +func (b *failingEventBus) Close() {} +func (b *failingEventBus) IsConnected() bool { return false } + +// recordingDeleter captures the task trees a watcher asks to cascade-delete. +type recordingDeleter struct { + mu sync.Mutex + deleted []string + err error +} + +func (d *recordingDeleter) DeleteTaskTree(_ context.Context, rootID string, cascade bool) (*taskservice.CascadeOutcome, error) { + d.mu.Lock() + defer d.mu.Unlock() + if !cascade { + return nil, errors.New("watcher cleanup must cascade") + } + if d.err != nil { + return nil, d.err + } + d.deleted = append(d.deleted, rootID) + return &taskservice.CascadeOutcome{}, nil +} + +func (d *recordingDeleter) ids() []string { + d.mu.Lock() + defer d.mu.Unlock() + return append([]string(nil), d.deleted...) +} + +func newWatchService(t *testing.T, client Client) (*Service, *Store) { + t.Helper() + service, store, _ := newTestService(t, func(*Config, string) Client { return client }) + if _, err := service.SetConfigForWorkspace(t.Context(), "ws-1", &SetConfigRequest{ + OrganizationURL: "https://dev.azure.com/acme", PAT: "pat", + }); err != nil { + t.Fatalf("set config: %v", err) + } + return service, store +} + +func validPullRequestWatchRequest() *CreatePullRequestWatchRequest { + return &CreatePullRequestWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", + ProjectID: "project-1", AzureRepositoryID: "azure-repo-1", Status: "active", + RepositoryID: "repo-1", BaseBranch: "main", + AgentProfileID: "agent-1", ExecutorProfileID: "executor-1", + Prompt: "Review {{title}}", + } +} + +func TestCreatePullRequestWatchRejectsIncompleteRequests(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + if _, err := service.CreatePullRequestWatch(t.Context(), nil); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("nil request error = %v, want ErrInvalidConfig", err) + } + blank := validPullRequestWatchRequest() + blank.WorkspaceID = " " + if _, err := service.CreatePullRequestWatch(t.Context(), blank); !errors.Is(err, ErrInvalidWorkspaceID) { + t.Fatalf("blank workspace error = %v, want ErrInvalidWorkspaceID", err) + } + for _, field := range []string{"workflow", "step", "agent", "executor", "repository"} { + t.Run("missing "+field, func(t *testing.T) { + req := validPullRequestWatchRequest() + switch field { + case "workflow": + req.WorkflowID = "" + case "step": + req.WorkflowStepID = "" + case "agent": + req.AgentProfileID = "" + case "executor": + req.ExecutorProfileID = "" + case "repository": + req.RepositoryID = " " + } + if _, err := service.CreatePullRequestWatch(t.Context(), req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + }) + } + watches, err := store.ListPullRequestWatches(t.Context(), "ws-1") + if err != nil || len(watches) != 0 { + t.Fatalf("rejected requests persisted watches: %+v %v", watches, err) + } +} + +func TestCreatePullRequestWatchDeniesForeignWorkspaceAccess(t *testing.T) { + service, _ := newWatchService(t, &watchClient{}) + denied := errors.New("workspace denied") + service.SetWorkspaceAuthorizer(func(context.Context, string) error { return denied }) + if _, err := service.CreatePullRequestWatch(t.Context(), validPullRequestWatchRequest()); !errors.Is(err, denied) { + t.Fatalf("error = %v, want the authorizer denial", err) + } + if _, err := service.ListPullRequestWatches(t.Context(), "ws-1"); !errors.Is(err, denied) { + t.Fatalf("list error = %v, want the authorizer denial", err) + } + if _, err := service.ListPullRequestWatches(t.Context(), ""); !errors.Is(err, ErrInvalidWorkspaceID) { + t.Fatalf("blank workspace list error = %v, want ErrInvalidWorkspaceID", err) + } +} + +// TestCreatePullRequestWatchRunsTheInitialCheck proves the create path fires +// its own first poll rather than waiting a full interval for the poller. +// +// CreatePullRequestWatch runs that first check on an untracked goroutine, so +// the test runs inside a synctest bubble and joins it with synctest.Wait +// instead of polling the store or racing a wall-clock deadline. +func TestCreatePullRequestWatchRunsTheInitialCheck(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + client := &watchClient{page: &PullRequestPage{Items: []PullRequest{{ + ID: 42, ProjectID: "project-1", RepositoryID: "azure-repo-1", + WebURL: "https://azure/pr/42", Title: "Ship it", + }}}} + service, store := newWatchService(t, client) + eventBus := bus.NewMemoryEventBus(logger.Default()) + service.SetEventBus(eventBus) + published := make(chan *PullRequestWatchEvent, 1) + if _, err := eventBus.Subscribe(events.AzureDevOpsPullRequestWatchMatch, func(_ context.Context, event *bus.Event) error { + published <- event.Data.(*PullRequestWatchEvent) + return nil + }); err != nil { + t.Fatalf("subscribe: %v", err) + } + + req := validPullRequestWatchRequest() + req.CreatorID = "creator-1" + req.ReviewerID = "reviewer-1" + req.PollIntervalSeconds = 5 + watch, err := service.CreatePullRequestWatch(t.Context(), req) + if err != nil { + t.Fatalf("CreatePullRequestWatch: %v", err) + } + if watch.ID == "" || !watch.Enabled || watch.Generation != 1 || + watch.PollIntervalSeconds != minWatchPollIntervalSeconds || + watch.CleanupPolicy != CleanupPolicyAuto { + t.Fatalf("created watch = %+v", watch) + } + synctest.Wait() + + select { + case event := <-published: + if event.PullRequest.ID != 42 || event.WatchID != watch.ID || + event.AzureRepositoryID != "azure-repo-1" || event.WorkspaceID != "ws-1" || + event.WorkflowStepID != "step-1" || event.RepositoryID != "repo-1" || + event.BaseBranch != "main" || event.Prompt != "Review {{title}}" || + !event.ReservationClaimed { + t.Fatalf("initial watch event = %+v", event) + } + default: + t.Fatal("initial pull-request check never published a match") + } + + filter := client.filter() + if filter.ProjectID != "project-1" || filter.RepositoryID != "azure-repo-1" || + filter.Status != "active" || filter.CreatorID != "creator-1" || + filter.ReviewerID != "reviewer-1" || filter.Top != 100 { + t.Fatalf("upstream filter = %+v", filter) + } + rows, err := store.ListPullRequestWatchTasks(t.Context(), watch.ID, watch.Generation) + if err != nil || len(rows) != 1 || rows[0].PullRequestID != 42 { + t.Fatalf("reservations after the initial check = %+v, %v", rows, err) + } + }) +} + +func TestUpdatePullRequestWatchAppliesEveryRequestedField(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedPullRequestWatch(t, store) + + str := func(value string) *string { return &value } + enabled := false + interval := 1200 + maxInflight := 3 + updated, err := service.UpdatePullRequestWatch(t.Context(), "ws-1", watch.ID, &UpdatePullRequestWatchRequest{ + WorkflowID: str("wf-2"), WorkflowStepID: str("step-2"), ProjectID: str("project-2"), + AzureRepositoryID: str("azure-repo-2"), Status: str("completed"), + CreatorID: str("creator-2"), ReviewerID: str("reviewer-2"), + RepositoryID: str("repo-2"), BaseBranch: str("develop"), + AgentProfileID: str("agent-2"), ExecutorProfileID: str("executor-2"), + Prompt: str("Fix {{title}}"), Enabled: &enabled, + PollIntervalSeconds: &interval, CleanupPolicy: str(CleanupPolicyAlways), + MaxInflightTasks: &maxInflight, + }) + if err != nil { + t.Fatalf("UpdatePullRequestWatch: %v", err) + } + if updated.WorkflowID != "wf-2" || updated.WorkflowStepID != "step-2" || + updated.ProjectID != "project-2" || updated.AzureRepositoryID != "azure-repo-2" || + updated.Status != "completed" || updated.CreatorID != "creator-2" || + updated.ReviewerID != "reviewer-2" || updated.RepositoryID != "repo-2" || + updated.BaseBranch != "develop" || updated.AgentProfileID != "agent-2" || + updated.ExecutorProfileID != "executor-2" || updated.Prompt != "Fix {{title}}" || + updated.Enabled || updated.PollIntervalSeconds != 1200 || + updated.CleanupPolicy != CleanupPolicyAlways || + updated.MaxInflightTasks == nil || *updated.MaxInflightTasks != 3 { + t.Fatalf("updated watch = %+v", updated) + } + + persisted, err := store.GetPullRequestWatch(t.Context(), watch.ID) + if err != nil || persisted.Prompt != "Fix {{title}}" || persisted.Status != "completed" { + t.Fatalf("persisted watch = %+v, %v", persisted, err) + } +} + +func TestUpdatePullRequestWatchClearsUnlimitedInflightAndKeepsUnsetFields(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedPullRequestWatch(t, store) + watch.MaxInflightTasks = intPtr(5) + if err := store.UpdatePullRequestWatch(t.Context(), watch); err != nil { + t.Fatalf("seed max inflight: %v", err) + } + + unlimited := 0 + updated, err := service.UpdatePullRequestWatch(t.Context(), "ws-1", watch.ID, &UpdatePullRequestWatchRequest{ + MaxInflightTasks: &unlimited, + }) + if err != nil { + t.Fatalf("UpdatePullRequestWatch: %v", err) + } + if updated.MaxInflightTasks != nil { + t.Fatalf("max inflight = %v, want nil for unlimited", *updated.MaxInflightTasks) + } + if updated.ProjectID != "project-1" || updated.AzureRepositoryID != "azure-repo-1" || + updated.RepositoryID != "repo-1" || !updated.Enabled { + t.Fatalf("unset fields were mutated: %+v", updated) + } + + // A nil request body is a no-op rather than a wipe. + unchanged, err := service.UpdatePullRequestWatch(t.Context(), "ws-1", watch.ID, nil) + if err != nil || unchanged.ProjectID != "project-1" || !unchanged.Enabled { + t.Fatalf("nil-request update = %+v, %v", unchanged, err) + } +} + +func TestPullRequestWatchOperationsRejectOtherWorkspaces(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedPullRequestWatch(t, store) + if _, err := service.SetConfigForWorkspace(t.Context(), "ws-2", &SetConfigRequest{ + OrganizationURL: "https://dev.azure.com/other", PAT: "pat", + }); err != nil { + t.Fatalf("set second config: %v", err) + } + + operations := map[string]func(id string) error{ + "trigger": func(id string) error { + _, err := service.TriggerPullRequestWatch(t.Context(), "ws-2", id) + return err + }, + "update": func(id string) error { + _, err := service.UpdatePullRequestWatch(t.Context(), "ws-2", id, &UpdatePullRequestWatchRequest{}) + return err + }, + "delete": func(id string) error { return service.DeletePullRequestWatch(t.Context(), "ws-2", id) }, + "preview reset": func(id string) error { + _, err := service.PreviewResetPullRequestWatch(t.Context(), "ws-2", id) + return err + }, + "reset": func(id string) error { + _, err := service.ResetPullRequestWatch(t.Context(), "ws-2", id) + return err + }, + } + for name, operation := range operations { + t.Run(name, func(t *testing.T) { + if err := operation(watch.ID); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("cross-workspace %s error = %v, want ErrWatchNotFound", name, err) + } + if err := operation("no-such-watch"); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("unknown-watch %s error = %v, want ErrWatchNotFound", name, err) + } + if err := operation(""); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("blank-id %s error = %v, want ErrWatchNotFound", name, err) + } + }) + } + if _, err := service.TriggerPullRequestWatch(t.Context(), "", watch.ID); !errors.Is(err, ErrInvalidWorkspaceID) { + t.Fatalf("blank workspace error = %v, want ErrInvalidWorkspaceID", err) + } + if remaining, err := store.GetPullRequestWatch(t.Context(), watch.ID); err != nil || remaining == nil { + t.Fatalf("watch was removed by a foreign-workspace call: %+v %v", remaining, err) + } +} + +func TestDeletePullRequestWatchCascadesItsCreatedTasks(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedPullRequestWatch(t, store) + deleter := &recordingDeleter{} + service.SetCascadeTaskDeleter(deleter) + seedPullRequestReservation(t, store, watch, 42, "task-42") + seedPullRequestReservation(t, store, watch, 43, "") + + if err := service.DeletePullRequestWatch(t.Context(), "ws-1", watch.ID); err != nil { + t.Fatalf("DeletePullRequestWatch: %v", err) + } + if ids := deleter.ids(); len(ids) != 1 || ids[0] != "task-42" { + t.Fatalf("cascade-deleted task ids = %v, want [task-42]", ids) + } + if got, err := store.GetPullRequestWatch(t.Context(), watch.ID); err != nil || got != nil { + t.Fatalf("watch after delete = %+v, %v", got, err) + } +} + +func TestPreviewAndResetPullRequestWatchRotateTheGeneration(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedPullRequestWatch(t, store) + deleter := &recordingDeleter{} + service.SetCascadeTaskDeleter(deleter) + seedPullRequestReservation(t, store, watch, 42, "task-42") + seedPullRequestReservation(t, store, watch, 43, "task-43") + + count, err := service.PreviewResetPullRequestWatch(t.Context(), "ws-1", watch.ID) + if err != nil || count != 2 { + t.Fatalf("PreviewResetPullRequestWatch = %d, %v; want 2", count, err) + } + + result, err := service.ResetPullRequestWatch(t.Context(), "ws-1", watch.ID) + if err != nil { + t.Fatalf("ResetPullRequestWatch: %v", err) + } + if result.Generation != watch.Generation+1 || len(result.TaskIDs) != 2 { + t.Fatalf("reset result = %+v", result) + } + if ids := deleter.ids(); len(ids) != 2 || ids[0] != "task-42" || ids[1] != "task-43" { + t.Fatalf("cascade-deleted task ids = %v", ids) + } + if rows, err := store.ListPullRequestWatchTasks(t.Context(), watch.ID, watch.Generation); err != nil || len(rows) != 0 { + t.Fatalf("old-generation reservations survived the reset: %+v %v", rows, err) + } + if count, err := service.PreviewResetPullRequestWatch(t.Context(), "ws-1", watch.ID); err != nil || count != 0 { + t.Fatalf("preview after reset = %d, %v; want 0", count, err) + } +} + +// TestResetWatchToleratesCascadeDeleteFailures proves a task that cannot be +// deleted does not abort the reset: the generation still rotates so the watch +// can re-import. +func TestResetWatchToleratesCascadeDeleteFailures(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedPullRequestWatch(t, store) + service.SetCascadeTaskDeleter(&recordingDeleter{err: errors.New("task already gone")}) + seedPullRequestReservation(t, store, watch, 42, "task-42") + + result, err := service.ResetPullRequestWatch(t.Context(), "ws-1", watch.ID) + if err != nil || result.Generation != watch.Generation+1 { + t.Fatalf("reset result = %+v, %v", result, err) + } +} + +func seedPullRequestReservation(t *testing.T, store *Store, watch *PullRequestWatch, pullRequestID int, taskID string) { + t.Helper() + reserved, err := store.ReservePullRequestWatchTask( + t.Context(), watch.ID, watch.Generation, watch.ProjectID, + watch.AzureRepositoryID, pullRequestID, "https://azure/pr", + ) + if err != nil || !reserved { + t.Fatalf("reserve pull request %d: reserved=%v err=%v", pullRequestID, reserved, err) + } + if taskID == "" { + return + } + if err := store.AssignPullRequestWatchTaskID( + t.Context(), watch.ID, watch.Generation, watch.ProjectID, + watch.AzureRepositoryID, pullRequestID, taskID, + ); err != nil { + t.Fatalf("assign task %q: %v", taskID, err) + } +} + +// TestServiceReservationSurfaceDelegatesToTheStore covers the orchestrator +// facing reservation methods, which are the seam the shared watcher dispatch +// pipeline calls into. +func TestServiceReservationSurfaceDelegatesToTheStore(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + ctx := t.Context() + + reserved, err := service.ReserveWorkItemWatchTask(ctx, work.ID, work.Generation, "project-1", 101, "https://azure/101") + if err != nil || !reserved { + t.Fatalf("ReserveWorkItemWatchTask = %v, %v", reserved, err) + } + if again, err := service.ReserveWorkItemWatchTask(ctx, work.ID, work.Generation, "project-1", 101, "https://azure/101"); err != nil || again { + t.Fatalf("duplicate work-item reservation = %v, %v", again, err) + } + if err := service.AssignWorkItemWatchTaskID(ctx, work.ID, work.Generation, "project-1", 101, "task-101"); err != nil { + t.Fatalf("AssignWorkItemWatchTaskID: %v", err) + } + if err := service.ReleaseWorkItemWatchTask(ctx, work.ID, work.Generation, "project-1", 101); err != nil { + t.Fatalf("ReleaseWorkItemWatchTask: %v", err) + } + if rows, _ := store.ListWorkItemWatchTasks(ctx, work.ID, work.Generation); len(rows) != 0 { + t.Fatalf("work-item reservation survived release: %+v", rows) + } + + reserved, err = service.ReservePullRequestWatchTask(ctx, pr.ID, pr.Generation, "project-1", "azure-repo-1", 42, "https://azure/pr/42") + if err != nil || !reserved { + t.Fatalf("ReservePullRequestWatchTask = %v, %v", reserved, err) + } + if err := service.AssignPullRequestWatchTaskID(ctx, pr.ID, pr.Generation, "project-1", "azure-repo-1", 42, "task-42"); err != nil { + t.Fatalf("AssignPullRequestWatchTaskID: %v", err) + } + if err := service.ReleasePullRequestWatchTask(ctx, pr.ID, pr.Generation, "project-1", "azure-repo-1", 42); err != nil { + t.Fatalf("ReleasePullRequestWatchTask: %v", err) + } + + if err := service.DisableWorkItemWatchWithError(ctx, work.ID, "workflow step deleted"); err != nil { + t.Fatalf("DisableWorkItemWatchWithError: %v", err) + } + if err := service.DisablePullRequestWatchWithError(ctx, pr.ID, "repository unlinked"); err != nil { + t.Fatalf("DisablePullRequestWatchWithError: %v", err) + } + gotWork, _ := store.GetWorkItemWatch(ctx, work.ID) + gotPR, _ := store.GetPullRequestWatch(ctx, pr.ID) + if gotWork.Enabled || gotWork.LastError != "workflow step deleted" || + gotPR.Enabled || gotPR.LastError != "repository unlinked" { + t.Fatalf("disabled watches = %+v / %+v", gotWork, gotPR) + } +} diff --git a/apps/backend/internal/azuredevops/watch_service_trigger_test.go b/apps/backend/internal/azuredevops/watch_service_trigger_test.go new file mode 100644 index 0000000000..ea755ccf22 --- /dev/null +++ b/apps/backend/internal/azuredevops/watch_service_trigger_test.go @@ -0,0 +1,478 @@ +package azuredevops + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" +) + +func validWorkItemWatchRequest() *CreateWorkItemWatchRequest { + return &CreateWorkItemWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", + ProjectID: "project-1", WIQL: "SELECT [System.Id] FROM WorkItems", + RepositoryID: "repo-1", BaseBranch: "main", + AgentProfileID: "agent-1", ExecutorProfileID: "executor-1", + Prompt: "Fix {{title}}", + } +} + +func TestCreateWorkItemWatchRejectsIncompleteRequests(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + if _, err := service.CreateWorkItemWatch(t.Context(), nil); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("nil request error = %v, want ErrInvalidConfig", err) + } + blank := validWorkItemWatchRequest() + blank.WorkspaceID = "" + if _, err := service.CreateWorkItemWatch(t.Context(), blank); !errors.Is(err, ErrInvalidWorkspaceID) { + t.Fatalf("blank workspace error = %v, want ErrInvalidWorkspaceID", err) + } + noStep := validWorkItemWatchRequest() + noStep.WorkflowStepID = "" + if _, err := service.CreateWorkItemWatch(t.Context(), noStep); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("missing step error = %v, want ErrInvalidConfig", err) + } + noRepository := validWorkItemWatchRequest() + noRepository.RepositoryID = "" + if _, err := service.CreateWorkItemWatch(t.Context(), noRepository); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("missing repository error = %v, want ErrInvalidConfig", err) + } + noWIQL := validWorkItemWatchRequest() + noWIQL.WIQL = "" + if _, err := service.CreateWorkItemWatch(t.Context(), noWIQL); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("missing WIQL error = %v, want ErrInvalidConfig", err) + } + if watches, err := store.ListWorkItemWatches(t.Context(), "ws-1"); err != nil || len(watches) != 0 { + t.Fatalf("rejected requests persisted watches: %+v %v", watches, err) + } +} + +// fakeWatchDependencies fails whichever dependency check the test names, so +// the create/update paths can be pinned to fail closed per reference. +type fakeWatchDependencies struct { + stepOK bool + agentOK bool + executorOK bool + err error +} + +func (f fakeWatchDependencies) WorkflowStepBelongs(context.Context, string, string, string) (bool, error) { + return f.stepOK, f.err +} + +func (f fakeWatchDependencies) AgentProfileBelongs(context.Context, string, string) (bool, error) { + return f.agentOK, f.err +} + +func (f fakeWatchDependencies) ExecutorProfileBelongs(context.Context, string, string) (bool, error) { + return f.executorOK, f.err +} + +type fakeWatchRepositories struct { + workspaceID string + defaultBranch string + ok bool +} + +func (f fakeWatchRepositories) GetRepository(context.Context, string) (string, string, bool) { + return f.workspaceID, f.defaultBranch, f.ok +} + +func TestCreateWorkItemWatchFailsClosedOnCrossWorkspaceReferences(t *testing.T) { + tests := []struct { + name string + validator WatchDependencyValidator + repository WatchRepositoryLookup + }{ + { + name: "workflow step from another workspace", + validator: fakeWatchDependencies{agentOK: true, executorOK: true}, + }, + { + name: "agent profile from another workspace", + validator: fakeWatchDependencies{stepOK: true, executorOK: true}, + }, + { + name: "executor profile from another workspace", + validator: fakeWatchDependencies{stepOK: true, agentOK: true}, + }, + { + name: "repository from another workspace", + validator: fakeWatchDependencies{stepOK: true, agentOK: true, executorOK: true}, + repository: fakeWatchRepositories{workspaceID: "ws-other", ok: true}, + }, + { + name: "repository does not exist", + validator: fakeWatchDependencies{stepOK: true, agentOK: true, executorOK: true}, + repository: fakeWatchRepositories{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + service.SetWatchDependencyValidator(tt.validator) + if tt.repository != nil { + service.SetWatchRepositoryLookup(tt.repository) + } + if _, err := service.CreateWorkItemWatch(t.Context(), validWorkItemWatchRequest()); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + if watches, _ := store.ListWorkItemWatches(t.Context(), "ws-1"); len(watches) != 0 { + t.Fatalf("watch persisted despite an invalid reference: %+v", watches) + } + }) + } +} + +func TestCreateWorkItemWatchSurfacesDependencyLookupErrors(t *testing.T) { + service, _ := newWatchService(t, &watchClient{}) + lookupErr := errors.New("workflow store unavailable") + service.SetWatchDependencyValidator(fakeWatchDependencies{err: lookupErr}) + if _, err := service.CreateWorkItemWatch(t.Context(), validWorkItemWatchRequest()); !errors.Is(err, lookupErr) { + t.Fatalf("error = %v, want the lookup failure", err) + } +} + +func TestCreateWorkItemWatchAdoptsRepositoryDefaultBranch(t *testing.T) { + service, _ := newWatchService(t, &watchClient{}) + service.SetWatchRepositoryLookup(fakeWatchRepositories{workspaceID: "ws-1", defaultBranch: "trunk", ok: true}) + req := validWorkItemWatchRequest() + req.BaseBranch = "" + + watch, err := service.CreateWorkItemWatch(t.Context(), req) + if err != nil { + t.Fatalf("CreateWorkItemWatch: %v", err) + } + if watch.BaseBranch != "trunk" { + t.Fatalf("base branch = %q, want the repository default", watch.BaseBranch) + } + + req = validWorkItemWatchRequest() + req.BaseBranch = "not a ref^" + if _, err := service.CreateWorkItemWatch(t.Context(), req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("invalid base branch error = %v, want ErrInvalidConfig", err) + } +} + +func TestUpdateWorkItemWatchAppliesEveryRequestedField(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedWorkItemWatch(t, store) + + str := func(value string) *string { return &value } + enabled := false + interval := 30 + maxInflight := 2 + updated, err := service.UpdateWorkItemWatch(t.Context(), "ws-1", watch.ID, &UpdateWorkItemWatchRequest{ + WorkflowID: str("wf-2"), WorkflowStepID: str("step-2"), ProjectID: str("project-2"), + WIQL: str("SELECT [System.Id] FROM WorkItemLinks"), RepositoryID: str("repo-2"), + BaseBranch: str("develop"), AgentProfileID: str("agent-2"), + ExecutorProfileID: str("executor-2"), Prompt: str("Investigate {{title}}"), + Enabled: &enabled, PollIntervalSeconds: &interval, + CleanupPolicy: str(CleanupPolicyNever), MaxInflightTasks: &maxInflight, + }) + if err != nil { + t.Fatalf("UpdateWorkItemWatch: %v", err) + } + if updated.WorkflowID != "wf-2" || updated.WorkflowStepID != "step-2" || + updated.ProjectID != "project-2" || updated.WIQL != "SELECT [System.Id] FROM WorkItemLinks" || + updated.RepositoryID != "repo-2" || updated.BaseBranch != "develop" || + updated.AgentProfileID != "agent-2" || updated.ExecutorProfileID != "executor-2" || + updated.Prompt != "Investigate {{title}}" || updated.Enabled || + updated.CleanupPolicy != CleanupPolicyNever || + updated.MaxInflightTasks == nil || *updated.MaxInflightTasks != 2 { + t.Fatalf("updated watch = %+v", updated) + } + // Sub-minimum intervals are clamped rather than accepted verbatim. + if updated.PollIntervalSeconds != minWatchPollIntervalSeconds { + t.Fatalf("poll interval = %d, want the clamped minimum %d", updated.PollIntervalSeconds, minWatchPollIntervalSeconds) + } + + unlimited := 0 + cleared, err := service.UpdateWorkItemWatch(t.Context(), "ws-1", watch.ID, &UpdateWorkItemWatchRequest{ + MaxInflightTasks: &unlimited, + }) + if err != nil || cleared.MaxInflightTasks != nil { + t.Fatalf("cleared watch = %+v, %v", cleared, err) + } + persisted, err := store.GetWorkItemWatch(t.Context(), watch.ID) + if err != nil || persisted.WIQL != "SELECT [System.Id] FROM WorkItemLinks" || persisted.MaxInflightTasks != nil { + t.Fatalf("persisted watch = %+v, %v", persisted, err) + } +} + +func TestWorkItemWatchOperationsRejectOtherWorkspaces(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedWorkItemWatch(t, store) + if _, err := service.SetConfigForWorkspace(t.Context(), "ws-2", &SetConfigRequest{ + OrganizationURL: "https://dev.azure.com/other", PAT: "pat", + }); err != nil { + t.Fatalf("set second config: %v", err) + } + + operations := map[string]func(id string) error{ + "trigger": func(id string) error { + _, err := service.TriggerWorkItemWatch(t.Context(), "ws-2", id) + return err + }, + "update": func(id string) error { + _, err := service.UpdateWorkItemWatch(t.Context(), "ws-2", id, &UpdateWorkItemWatchRequest{}) + return err + }, + "delete": func(id string) error { return service.DeleteWorkItemWatch(t.Context(), "ws-2", id) }, + "preview reset": func(id string) error { + _, err := service.PreviewResetWorkItemWatch(t.Context(), "ws-2", id) + return err + }, + "reset": func(id string) error { + _, err := service.ResetWorkItemWatch(t.Context(), "ws-2", id) + return err + }, + "cleanup": func(id string) error { + _, err := service.CleanupWorkItemWatch(t.Context(), "ws-2", id) + return err + }, + } + for name, operation := range operations { + t.Run(name, func(t *testing.T) { + if err := operation(watch.ID); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("cross-workspace %s error = %v, want ErrWatchNotFound", name, err) + } + }) + } + if remaining, err := store.GetWorkItemWatch(t.Context(), watch.ID); err != nil || remaining == nil { + t.Fatalf("watch was removed by a foreign-workspace call: %+v %v", remaining, err) + } +} + +func TestDeleteWorkItemWatchCascadesItsCreatedTasks(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedWorkItemWatch(t, store) + deleter := &recordingDeleter{} + service.SetCascadeTaskDeleter(deleter) + seedWorkItemReservation(t, store, watch, 101, "task-101") + seedWorkItemReservation(t, store, watch, 102, "") + + if err := service.DeleteWorkItemWatch(t.Context(), "ws-1", watch.ID); err != nil { + t.Fatalf("DeleteWorkItemWatch: %v", err) + } + if ids := deleter.ids(); len(ids) != 1 || ids[0] != "task-101" { + t.Fatalf("cascade-deleted task ids = %v, want [task-101]", ids) + } + if got, err := store.GetWorkItemWatch(t.Context(), watch.ID); err != nil || got != nil { + t.Fatalf("watch after delete = %+v, %v", got, err) + } +} + +// TestDeleteWorkItemWatchWithoutCascadeDeleterStillRemovesTheWatch keeps the +// local-development path (no task service wired) usable. +func TestDeleteWorkItemWatchWithoutCascadeDeleterStillRemovesTheWatch(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + watch := seedWorkItemWatch(t, store) + seedWorkItemReservation(t, store, watch, 101, "task-101") + + if err := service.DeleteWorkItemWatch(t.Context(), "ws-1", watch.ID); err != nil { + t.Fatalf("DeleteWorkItemWatch: %v", err) + } + if got, _ := store.GetWorkItemWatch(t.Context(), watch.ID); got != nil { + t.Fatalf("watch survived deletion: %+v", got) + } +} + +func seedWorkItemReservation(t *testing.T, store *Store, watch *WorkItemWatch, workItemID int, taskID string) { + t.Helper() + reserved, err := store.ReserveWorkItemWatchTask( + t.Context(), watch.ID, watch.Generation, watch.ProjectID, workItemID, "https://azure/item", + ) + if err != nil || !reserved { + t.Fatalf("reserve work item %d: reserved=%v err=%v", workItemID, reserved, err) + } + if taskID == "" { + return + } + if err := store.AssignWorkItemWatchTaskID( + t.Context(), watch.ID, watch.Generation, watch.ProjectID, workItemID, taskID, + ); err != nil { + t.Fatalf("assign task %q: %v", taskID, err) + } +} + +// TestTriggerWatchRecordsUpstreamFailureOnTheWatchRow proves a failing check +// is surfaced to the user through last_error rather than silently retried. +func TestTriggerWatchRecordsUpstreamFailureOnTheWatchRow(t *testing.T) { + upstream := errors.New("401 unauthorized") + client := &watchClient{workErr: upstream, pageErr: upstream} + service, store := newWatchService(t, client) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + + if _, err := service.TriggerWorkItemWatch(t.Context(), "ws-1", work.ID); !errors.Is(err, upstream) { + t.Fatalf("work-item trigger error = %v, want the upstream failure", err) + } + if _, err := service.TriggerPullRequestWatch(t.Context(), "ws-1", pr.ID); !errors.Is(err, upstream) { + t.Fatalf("pull-request trigger error = %v, want the upstream failure", err) + } + gotWork, _ := store.GetWorkItemWatch(t.Context(), work.ID) + if gotWork.LastError != "401 unauthorized" || gotWork.LastErrorAt == nil || !gotWork.Enabled { + t.Fatalf("work-item watch after failure = %+v", gotWork) + } + gotPR, _ := store.GetPullRequestWatch(t.Context(), pr.ID) + if gotPR.LastError != "401 unauthorized" || gotPR.LastErrorAt == nil { + t.Fatalf("pull-request watch after failure = %+v", gotPR) + } +} + +// TestTriggerWatchRecordsMissingCredentials covers the credential-resolution +// hop, which happens before any upstream call. +func TestTriggerWatchRecordsMissingCredentials(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + if err := service.DeleteConfigForWorkspace(t.Context(), "ws-1"); err != nil { + t.Fatalf("delete config: %v", err) + } + + if _, err := service.TriggerWorkItemWatch(t.Context(), "ws-1", work.ID); !errors.Is(err, ErrNotConfigured) { + t.Fatalf("work-item trigger error = %v, want ErrNotConfigured", err) + } + if _, err := service.TriggerPullRequestWatch(t.Context(), "ws-1", pr.ID); !errors.Is(err, ErrNotConfigured) { + t.Fatalf("pull-request trigger error = %v, want ErrNotConfigured", err) + } + gotWork, _ := store.GetWorkItemWatch(t.Context(), work.ID) + gotPR, _ := store.GetPullRequestWatch(t.Context(), pr.ID) + if gotWork.LastError != ErrNotConfigured.Error() || gotPR.LastError != ErrNotConfigured.Error() { + t.Fatalf("watch errors = %q / %q", gotWork.LastError, gotPR.LastError) + } +} + +// TestTriggerWatchReleasesReservationWhenPublishFails is the retry-safety +// property: a match whose event could not be published must not stay claimed, +// otherwise the item is silently never imported. +func TestTriggerWatchReleasesReservationWhenPublishFails(t *testing.T) { + client := &watchClient{ + work: &WorkItemSearchResult{Items: []WorkItem{{ID: 101, WebURL: "https://azure/item/101"}}}, + page: &PullRequestPage{Items: []PullRequest{{ID: 42, ProjectID: "project-1", RepositoryID: "azure-repo-1"}}}, + } + service, store := newWatchService(t, client) + service.SetEventBus(&failingEventBus{err: errors.New("bus unavailable")}) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + + result, err := service.TriggerWorkItemWatch(t.Context(), "ws-1", work.ID) + if err != nil || result.Matched != 0 { + t.Fatalf("work-item trigger = %+v, %v; want zero matches without a hard error", result, err) + } + if rows, err := store.ListWorkItemWatchTasks(t.Context(), work.ID, work.Generation); err != nil || len(rows) != 0 { + t.Fatalf("work-item reservation stayed claimed after a publish failure: %+v %v", rows, err) + } + gotWork, _ := store.GetWorkItemWatch(t.Context(), work.ID) + if gotWork.LastError != "one or more watcher matches could not be published" { + t.Fatalf("work-item watch error = %q", gotWork.LastError) + } + + prResult, err := service.TriggerPullRequestWatch(t.Context(), "ws-1", pr.ID) + if err != nil || prResult.Matched != 0 { + t.Fatalf("pull-request trigger = %+v, %v", prResult, err) + } + if rows, err := store.ListPullRequestWatchTasks(t.Context(), pr.ID, pr.Generation); err != nil || len(rows) != 0 { + t.Fatalf("pull-request reservation stayed claimed after a publish failure: %+v %v", rows, err) + } + gotPR, _ := store.GetPullRequestWatch(t.Context(), pr.ID) + if gotPR.LastError != "one or more watcher matches could not be published" { + t.Fatalf("pull-request watch error = %q", gotPR.LastError) + } +} + +// TestTriggerWatchClearsPreviousErrorOnSuccess proves a recovered watch stops +// reporting a stale failure. +func TestTriggerWatchClearsPreviousErrorOnSuccess(t *testing.T) { + client := &watchClient{work: &WorkItemSearchResult{Items: []WorkItem{{ID: 101, Title: "Fix build"}}}} + service, store := newWatchService(t, client) + eventBus := bus.NewMemoryEventBus(logger.Default()) + service.SetEventBus(eventBus) + seen := make(chan *WorkItemWatchEvent, 1) + if _, err := eventBus.Subscribe(events.AzureDevOpsWorkItemWatchMatch, func(_ context.Context, event *bus.Event) error { + seen <- event.Data.(*WorkItemWatchEvent) + return nil + }); err != nil { + t.Fatalf("subscribe: %v", err) + } + watch := seedWorkItemWatch(t, store) + if err := store.SetWorkItemWatchError(t.Context(), watch.ID, "stale failure", time.Now().UTC()); err != nil { + t.Fatalf("seed stale failure: %v", err) + } + + result, err := service.TriggerWorkItemWatch(t.Context(), "ws-1", watch.ID) + if err != nil || result.Matched != 1 { + t.Fatalf("trigger = %+v, %v", result, err) + } + if client.lastWIQL != [2]string{"project-1", "SELECT [System.Id] FROM WorkItems"} { + t.Fatalf("query sent upstream = %v", client.lastWIQL) + } + select { + case event := <-seen: + if event.WorkItem.ID != 101 || event.WatchID != watch.ID || !event.ReservationClaimed { + t.Fatalf("published event = %+v", event) + } + default: + t.Fatal("no watch event was published for the match") + } + got, _ := store.GetWorkItemWatch(t.Context(), watch.ID) + if got.LastError != "" || got.LastErrorAt != nil || got.LastPolledAt == nil { + t.Fatalf("watch after a successful check = %+v", got) + } +} + +// TestTriggerWatchWithoutEventBusStillClaimsMatches keeps the nil-bus local +// path working: reservations are made even though nothing is published. +func TestTriggerWatchWithoutEventBusStillClaimsMatches(t *testing.T) { + client := &watchClient{work: &WorkItemSearchResult{Items: []WorkItem{{ID: 101}, {ID: 102}}}} + service, store := newWatchService(t, client) + watch := seedWorkItemWatch(t, store) + + result, err := service.TriggerWorkItemWatch(t.Context(), "ws-1", watch.ID) + if err != nil || result.Matched != 2 { + t.Fatalf("trigger = %+v, %v; want both items matched", result, err) + } + rows, err := store.ListWorkItemWatchTasks(t.Context(), watch.ID, watch.Generation) + if err != nil || len(rows) != 2 { + t.Fatalf("reservations = %+v, %v", rows, err) + } + repeat, err := service.TriggerWorkItemWatch(t.Context(), "ws-1", watch.ID) + if err != nil || repeat.Matched != 0 { + t.Fatalf("repeat trigger = %+v, %v; want the dedup to hold", repeat, err) + } +} + +// TestTriggerWatchSkipsDeletingWatches covers the soft-delete guard in the +// authorization helpers. +func TestTriggerWatchSkipsDeletingWatches(t *testing.T) { + service, store := newWatchService(t, &watchClient{}) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + marks := []struct { + query string + id string + }{ + {`UPDATE azure_devops_work_item_watches SET deleting = 1 WHERE id = ?`, work.ID}, + {`UPDATE azure_devops_pull_request_watches SET deleting = 1 WHERE id = ?`, pr.ID}, + } + for _, mark := range marks { + if _, err := store.db.ExecContext(t.Context(), mark.query, mark.id); err != nil { + t.Fatalf("mark deleting: %v", err) + } + } + + if _, err := service.TriggerWorkItemWatch(t.Context(), "ws-1", work.ID); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("work-item trigger error = %v, want ErrWatchNotFound", err) + } + if _, err := service.TriggerPullRequestWatch(t.Context(), "ws-1", pr.ID); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("pull-request trigger error = %v, want ErrWatchNotFound", err) + } + if watches, err := store.ListWorkItemWatches(t.Context(), "ws-1"); err != nil || len(watches) != 0 { + t.Fatalf("deleting watch is still listed: %+v %v", watches, err) + } +} diff --git a/apps/backend/internal/azuredevops/watch_store_mutation_test.go b/apps/backend/internal/azuredevops/watch_store_mutation_test.go new file mode 100644 index 0000000000..c185121890 --- /dev/null +++ b/apps/backend/internal/azuredevops/watch_store_mutation_test.go @@ -0,0 +1,439 @@ +package azuredevops + +import ( + "errors" + "testing" + "time" +) + +func newWatchStore(t *testing.T) *Store { + t.Helper() + db := newTestDB(t) + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("new store: %v", err) + } + return store +} + +func seedWorkItemWatch(t *testing.T, store *Store) *WorkItemWatch { + t.Helper() + watch := &WorkItemWatch{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", + ProjectID: "project-1", WIQL: "SELECT [System.Id] FROM WorkItems", + RepositoryID: "repo-1", BaseBranch: "main", + AgentProfileID: "agent-1", ExecutorProfileID: "executor-1", + } + if err := store.CreateWorkItemWatch(t.Context(), watch); err != nil { + t.Fatalf("create work-item watch: %v", err) + } + return watch +} + +func seedPullRequestWatch(t *testing.T, store *Store) *PullRequestWatch { + t.Helper() + watch := &PullRequestWatch{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", + ProjectID: "project-1", AzureRepositoryID: "azure-repo-1", + RepositoryID: "repo-1", BaseBranch: "main", + AgentProfileID: "agent-1", ExecutorProfileID: "executor-1", + } + if err := store.CreatePullRequestWatch(t.Context(), watch); err != nil { + t.Fatalf("create pull-request watch: %v", err) + } + return watch +} + +func TestStoreUpdatePullRequestWatchPersistsEveryMutableField(t *testing.T) { + store := newWatchStore(t) + watch := seedPullRequestWatch(t, store) + + watch.WorkflowID = "wf-2" + watch.WorkflowStepID = "step-2" + watch.ProjectID = "project-2" + watch.AzureRepositoryID = "azure-repo-2" + watch.Status = "completed" + watch.CreatorID = "creator-1" + watch.ReviewerID = "reviewer-1" + watch.RepositoryID = "repo-2" + watch.BaseBranch = "develop" + watch.AgentProfileID = "agent-2" + watch.ExecutorProfileID = "executor-2" + watch.Prompt = "Review {{title}}" + watch.Enabled = false + watch.PollIntervalSeconds = 900 + watch.CleanupPolicy = CleanupPolicyNever + watch.MaxInflightTasks = intPtr(4) + if err := store.UpdatePullRequestWatch(t.Context(), watch); err != nil { + t.Fatalf("update pull-request watch: %v", err) + } + + got, err := store.GetPullRequestWatch(t.Context(), watch.ID) + if err != nil || got == nil { + t.Fatalf("get pull-request watch: %+v %v", got, err) + } + if got.WorkflowID != "wf-2" || got.WorkflowStepID != "step-2" || got.ProjectID != "project-2" || + got.AzureRepositoryID != "azure-repo-2" || got.Status != "completed" || + got.CreatorID != "creator-1" || got.ReviewerID != "reviewer-1" || + got.RepositoryID != "repo-2" || got.BaseBranch != "develop" || + got.AgentProfileID != "agent-2" || got.ExecutorProfileID != "executor-2" || + got.Prompt != "Review {{title}}" || got.Enabled || + got.PollIntervalSeconds != 900 || got.CleanupPolicy != CleanupPolicyNever || + got.MaxInflightTasks == nil || *got.MaxInflightTasks != 4 { + t.Fatalf("persisted watch = %+v", got) + } + // The update must stamp a fresh updated_at; equal timestamps would mean the + // store persisted the row without advancing it. + if !got.UpdatedAt.After(got.CreatedAt) { + t.Fatalf("updated_at = %v, want it advanced past created_at = %v", got.UpdatedAt, got.CreatedAt) + } + // The disabled watch is still listed for its workspace; only deletion hides it. + list, err := store.ListPullRequestWatches(t.Context(), "ws-1") + if err != nil || len(list) != 1 || list[0].ID != watch.ID { + t.Fatalf("workspace list = %+v, %v", list, err) + } + if enabled, err := store.ListEnabledPullRequestWatches(t.Context()); err != nil || len(enabled) != 0 { + t.Fatalf("enabled list = %+v, %v; want the disabled watch excluded", enabled, err) + } +} + +func TestStoreWatchUpdatesReportMissingRows(t *testing.T) { + store := newWatchStore(t) + work := &WorkItemWatch{ID: "missing", WorkspaceID: "ws-1", ProjectID: "project-1", WIQL: "SELECT"} + if err := store.UpdateWorkItemWatch(t.Context(), work); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("UpdateWorkItemWatch error = %v, want ErrWatchNotFound", err) + } + pr := &PullRequestWatch{ID: "missing", WorkspaceID: "ws-1", ProjectID: "project-1"} + if err := store.UpdatePullRequestWatch(t.Context(), pr); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("UpdatePullRequestWatch error = %v, want ErrWatchNotFound", err) + } + if got, err := store.GetWorkItemWatch(t.Context(), "missing"); err != nil || got != nil { + t.Fatalf("GetWorkItemWatch = %+v, %v; want nil without error", got, err) + } + if got, err := store.GetPullRequestWatch(t.Context(), "missing"); err != nil || got != nil { + t.Fatalf("GetPullRequestWatch = %+v, %v; want nil without error", got, err) + } +} + +func TestStoreWatchNormalizationRejectsInvalidInput(t *testing.T) { + store := newWatchStore(t) + negative := -1 + workItemCases := []struct { + name string + watch *WorkItemWatch + }{ + {"nil watch", nil}, + {"missing workspace", &WorkItemWatch{ProjectID: "project-1", WIQL: "SELECT"}}, + {"missing project", &WorkItemWatch{WorkspaceID: "ws-1", WIQL: "SELECT"}}, + {"missing wiql", &WorkItemWatch{WorkspaceID: "ws-1", ProjectID: "project-1"}}, + {"invalid cleanup policy", &WorkItemWatch{WorkspaceID: "ws-1", ProjectID: "project-1", WIQL: "SELECT", CleanupPolicy: "sometimes"}}, + {"negative max inflight", &WorkItemWatch{WorkspaceID: "ws-1", ProjectID: "project-1", WIQL: "SELECT", MaxInflightTasks: &negative}}, + } + for _, tt := range workItemCases { + t.Run("work item/"+tt.name, func(t *testing.T) { + if err := store.CreateWorkItemWatch(t.Context(), tt.watch); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + }) + } + pullRequestCases := []struct { + name string + watch *PullRequestWatch + }{ + {"nil watch", nil}, + {"missing workspace", &PullRequestWatch{ProjectID: "project-1"}}, + {"missing project", &PullRequestWatch{WorkspaceID: "ws-1"}}, + {"invalid cleanup policy", &PullRequestWatch{WorkspaceID: "ws-1", ProjectID: "project-1", CleanupPolicy: "sometimes"}}, + {"negative max inflight", &PullRequestWatch{WorkspaceID: "ws-1", ProjectID: "project-1", MaxInflightTasks: &negative}}, + } + for _, tt := range pullRequestCases { + t.Run("pull request/"+tt.name, func(t *testing.T) { + if err := store.CreatePullRequestWatch(t.Context(), tt.watch); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("error = %v, want ErrInvalidConfig", err) + } + }) + } +} + +func TestStoreCreatePullRequestWatchDefaultsStatusAndInterval(t *testing.T) { + store := newWatchStore(t) + watch := &PullRequestWatch{WorkspaceID: "ws-1", ProjectID: "project-1"} + if err := store.CreatePullRequestWatch(t.Context(), watch); err != nil { + t.Fatalf("create pull-request watch: %v", err) + } + if watch.ID == "" || watch.Status != activePullRequestState || !watch.Enabled || + watch.Generation != 1 || watch.PollIntervalSeconds != defaultWatchPollIntervalSeconds || + watch.CleanupPolicy != CleanupPolicyAuto { + t.Fatalf("normalized watch = %+v", watch) + } +} + +func TestStoreWatchErrorLifecycle(t *testing.T) { + store := newWatchStore(t) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + failedAt := time.Date(2026, 8, 12, 9, 30, 0, 0, time.UTC) + + if err := store.SetWorkItemWatchError(t.Context(), work.ID, "401 unauthorized", failedAt); err != nil { + t.Fatalf("set work-item error: %v", err) + } + if err := store.SetPullRequestWatchError(t.Context(), pr.ID, "403 forbidden", failedAt); err != nil { + t.Fatalf("set pull-request error: %v", err) + } + gotWork, err := store.GetWorkItemWatch(t.Context(), work.ID) + if err != nil || gotWork.LastError != "401 unauthorized" || + gotWork.LastErrorAt == nil || !gotWork.LastErrorAt.Equal(failedAt) || + gotWork.LastPolledAt == nil || !gotWork.LastPolledAt.Equal(failedAt) { + t.Fatalf("work-item watch after failure = %+v, %v", gotWork, err) + } + gotPR, err := store.GetPullRequestWatch(t.Context(), pr.ID) + if err != nil || gotPR.LastError != "403 forbidden" || gotPR.LastErrorAt == nil { + t.Fatalf("pull-request watch after failure = %+v, %v", gotPR, err) + } + + polledAt := failedAt.Add(time.Minute) + if err := store.ClearWorkItemWatchError(t.Context(), work.ID, polledAt); err != nil { + t.Fatalf("clear work-item error: %v", err) + } + if err := store.ClearPullRequestWatchError(t.Context(), pr.ID, polledAt); err != nil { + t.Fatalf("clear pull-request error: %v", err) + } + gotWork, _ = store.GetWorkItemWatch(t.Context(), work.ID) + if gotWork.LastError != "" || gotWork.LastErrorAt != nil || + gotWork.LastPolledAt == nil || !gotWork.LastPolledAt.Equal(polledAt) { + t.Fatalf("work-item watch after clear = %+v", gotWork) + } + gotPR, _ = store.GetPullRequestWatch(t.Context(), pr.ID) + if gotPR.LastError != "" || gotPR.LastErrorAt != nil { + t.Fatalf("pull-request watch after clear = %+v", gotPR) + } +} + +func TestStoreDisableWatchWithErrorStopsPolling(t *testing.T) { + store := newWatchStore(t) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + + if err := store.DisableWorkItemWatchWithError(t.Context(), work.ID, "workflow step deleted"); err != nil { + t.Fatalf("disable work-item watch: %v", err) + } + if err := store.DisablePullRequestWatchWithError(t.Context(), pr.ID, "repository unlinked"); err != nil { + t.Fatalf("disable pull-request watch: %v", err) + } + gotWork, _ := store.GetWorkItemWatch(t.Context(), work.ID) + if gotWork.Enabled || gotWork.LastError != "workflow step deleted" || gotWork.LastErrorAt == nil { + t.Fatalf("disabled work-item watch = %+v", gotWork) + } + gotPR, _ := store.GetPullRequestWatch(t.Context(), pr.ID) + if gotPR.Enabled || gotPR.LastError != "repository unlinked" { + t.Fatalf("disabled pull-request watch = %+v", gotPR) + } + enabled, err := store.ListEnabledWorkItemWatches(t.Context()) + if err != nil || len(enabled) != 0 { + t.Fatalf("enabled work-item watches = %+v, %v", enabled, err) + } + // A disabled watch also refuses new reservations. + if reserved, err := store.ReserveWorkItemWatchTask( + t.Context(), work.ID, work.Generation, "project-1", 101, "https://azure/101", + ); err != nil || reserved { + t.Fatalf("reservation on a disabled watch: reserved=%v err=%v", reserved, err) + } + + if err := store.DisableWorkItemWatchWithError(t.Context(), "missing", "gone"); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("disable missing work-item watch error = %v, want ErrWatchNotFound", err) + } + if err := store.DisablePullRequestWatchWithError(t.Context(), "missing", "gone"); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("disable missing pull-request watch error = %v, want ErrWatchNotFound", err) + } +} + +func TestStoreDeleteWatchRemovesReservationsAtomically(t *testing.T) { + store := newWatchStore(t) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + if ok, err := store.ReserveWorkItemWatchTask(t.Context(), work.ID, work.Generation, "project-1", 101, "https://azure/101"); err != nil || !ok { + t.Fatalf("reserve work item: ok=%v err=%v", ok, err) + } + if ok, err := store.ReservePullRequestWatchTask(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 42, "https://azure/pr/42"); err != nil || !ok { + t.Fatalf("reserve pull request: ok=%v err=%v", ok, err) + } + + if err := store.DeleteWorkItemWatch(t.Context(), work.ID); err != nil { + t.Fatalf("delete work-item watch: %v", err) + } + if err := store.DeletePullRequestWatch(t.Context(), pr.ID); err != nil { + t.Fatalf("delete pull-request watch: %v", err) + } + if got, err := store.GetWorkItemWatch(t.Context(), work.ID); err != nil || got != nil { + t.Fatalf("work-item watch after delete = %+v, %v", got, err) + } + rows, err := store.ListWorkItemWatchTasks(t.Context(), work.ID, work.Generation) + if err != nil || len(rows) != 0 { + t.Fatalf("work-item reservations after delete = %+v, %v", rows, err) + } + prRows, err := store.ListPullRequestWatchTasks(t.Context(), pr.ID, pr.Generation) + if err != nil || len(prRows) != 0 { + t.Fatalf("pull-request reservations after delete = %+v, %v", prRows, err) + } + + if err := store.DeleteWorkItemWatch(t.Context(), work.ID); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("repeat delete error = %v, want ErrWatchNotFound", err) + } + if err := store.DeletePullRequestWatch(t.Context(), "missing"); !errors.Is(err, ErrWatchNotFound) { + t.Fatalf("delete missing pull-request watch error = %v, want ErrWatchNotFound", err) + } +} + +func TestStoreReleaseWatchTaskFreesTheReservationKey(t *testing.T) { + store := newWatchStore(t) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + if ok, err := store.ReserveWorkItemWatchTask(t.Context(), work.ID, work.Generation, "project-1", 101, "https://azure/101"); err != nil || !ok { + t.Fatalf("reserve work item: ok=%v err=%v", ok, err) + } + if ok, err := store.ReservePullRequestWatchTask(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 42, "https://azure/pr/42"); err != nil || !ok { + t.Fatalf("reserve pull request: ok=%v err=%v", ok, err) + } + + if err := store.ReleaseWorkItemWatchTask(t.Context(), work.ID, work.Generation, "project-1", 101); err != nil { + t.Fatalf("release work-item reservation: %v", err) + } + if err := store.ReleasePullRequestWatchTask(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 42); err != nil { + t.Fatalf("release pull-request reservation: %v", err) + } + // Releasing must make the same identity claimable again. + if ok, err := store.ReserveWorkItemWatchTask(t.Context(), work.ID, work.Generation, "project-1", 101, "https://azure/101"); err != nil || !ok { + t.Fatalf("re-reservation after release: ok=%v err=%v", ok, err) + } + + if err := store.ReleaseWorkItemWatchTask(t.Context(), work.ID, work.Generation, "project-1", 999); !errors.Is(err, ErrReservationNotFound) { + t.Fatalf("release unknown work item error = %v, want ErrReservationNotFound", err) + } + if err := store.ReleasePullRequestWatchTask(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 999); !errors.Is(err, ErrReservationNotFound) { + t.Fatalf("release unknown pull request error = %v, want ErrReservationNotFound", err) + } +} + +func TestStoreDeleteWatchTaskRowsByID(t *testing.T) { + store := newWatchStore(t) + work := seedWorkItemWatch(t, store) + pr := seedPullRequestWatch(t, store) + if ok, err := store.ReserveWorkItemWatchTask(t.Context(), work.ID, work.Generation, "project-1", 101, "https://azure/101"); err != nil || !ok { + t.Fatalf("reserve work item: ok=%v err=%v", ok, err) + } + if ok, err := store.ReservePullRequestWatchTask(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 42, "https://azure/pr/42"); err != nil || !ok { + t.Fatalf("reserve pull request: ok=%v err=%v", ok, err) + } + workRows, err := store.ListWorkItemWatchTasks(t.Context(), work.ID, work.Generation) + if err != nil || len(workRows) != 1 || workRows[0].WorkItemID != 101 || + workRows[0].WorkItemURL != "https://azure/101" || workRows[0].TaskID != "" { + t.Fatalf("work reservations = %+v, %v", workRows, err) + } + prRows, err := store.ListPullRequestWatchTasks(t.Context(), pr.ID, pr.Generation) + if err != nil || len(prRows) != 1 || prRows[0].PullRequestID != 42 || + prRows[0].AzureRepositoryID != "azure-repo-1" { + t.Fatalf("PR reservations = %+v, %v", prRows, err) + } + + if err := store.DeleteWorkItemWatchTask(t.Context(), workRows[0].ID); err != nil { + t.Fatalf("delete work-item reservation row: %v", err) + } + if err := store.DeletePullRequestWatchTask(t.Context(), prRows[0].ID); err != nil { + t.Fatalf("delete pull-request reservation row: %v", err) + } + if rows, _ := store.ListWorkItemWatchTasks(t.Context(), work.ID, work.Generation); len(rows) != 0 { + t.Fatalf("work reservations after row delete = %+v", rows) + } + if rows, _ := store.ListPullRequestWatchTasks(t.Context(), pr.ID, pr.Generation); len(rows) != 0 { + t.Fatalf("PR reservations after row delete = %+v", rows) + } +} + +// TestStoreAssignWatchTaskIDDistinguishesOwnershipLossFromMissingReservation +// pins the two failure modes apart: a stale generation means the watch moved +// on (ownership lost), while a live generation with no row means the specific +// reservation is gone. +func TestStoreAssignWatchTaskIDDistinguishesOwnershipLossFromMissingReservation(t *testing.T) { + store := newWatchStore(t) + work := seedWorkItemWatch(t, store) + if ok, err := store.ReserveWorkItemWatchTask(t.Context(), work.ID, work.Generation, "project-1", 101, "https://azure/101"); err != nil || !ok { + t.Fatalf("reserve work item: ok=%v err=%v", ok, err) + } + if err := store.AssignWorkItemWatchTaskID(t.Context(), work.ID, work.Generation, "project-1", 101, "task-1"); err != nil { + t.Fatalf("assign task id: %v", err) + } + rows, _ := store.ListWorkItemWatchTasks(t.Context(), work.ID, work.Generation) + if len(rows) != 1 || rows[0].TaskID != "task-1" { + t.Fatalf("reservation after assignment = %+v", rows) + } + if err := store.AssignWorkItemWatchTaskID(t.Context(), work.ID, work.Generation, "project-1", 999, "task-2"); !errors.Is(err, ErrReservationNotFound) { + t.Fatalf("assign to unreserved item error = %v, want ErrReservationNotFound", err) + } + + reset, err := store.BeginWorkItemWatchReset(t.Context(), work.ID) + if err != nil { + t.Fatalf("begin reset: %v", err) + } + if err := store.FinishWorkItemWatchReset(t.Context(), work.ID, reset.Generation); err != nil { + t.Fatalf("finish reset: %v", err) + } + if err := store.AssignWorkItemWatchTaskID(t.Context(), work.ID, work.Generation, "project-1", 101, "task-3"); !errors.Is(err, ErrWatchOwnershipLost) { + t.Fatalf("assign at stale generation error = %v, want ErrWatchOwnershipLost", err) + } + if err := store.AssignWorkItemWatchTaskID(t.Context(), "unknown-watch", reset.Generation, "project-1", 101, "task-4"); !errors.Is(err, ErrWatchOwnershipLost) { + t.Fatalf("assign for unknown watch error = %v, want ErrWatchOwnershipLost", err) + } +} + +// TestStoreAssignPullRequestWatchTaskIDResolvesGenerationFromTheSecondTable +// covers watchGeneration falling through the work-item lookup to the +// pull-request table. +func TestStoreAssignPullRequestWatchTaskIDResolvesGenerationFromTheSecondTable(t *testing.T) { + store := newWatchStore(t) + pr := seedPullRequestWatch(t, store) + if ok, err := store.ReservePullRequestWatchTask(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 42, "https://azure/pr/42"); err != nil || !ok { + t.Fatalf("reserve pull request: ok=%v err=%v", ok, err) + } + if err := store.AssignPullRequestWatchTaskID(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 42, "task-42"); err != nil { + t.Fatalf("assign task id: %v", err) + } + if err := store.AssignPullRequestWatchTaskID(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 99, "task-99"); !errors.Is(err, ErrReservationNotFound) { + t.Fatalf("assign to unreserved PR error = %v, want ErrReservationNotFound", err) + } + if err := store.DisablePullRequestWatchWithError(t.Context(), pr.ID, "disabled"); err != nil { + t.Fatalf("disable watch: %v", err) + } + if err := store.AssignPullRequestWatchTaskID(t.Context(), pr.ID, pr.Generation, "project-1", "azure-repo-1", 99, "task-99"); !errors.Is(err, ErrWatchOwnershipLost) { + t.Fatalf("assign on disabled watch error = %v, want ErrWatchOwnershipLost", err) + } +} + +func TestStoreEnabledWatchListsSkipUnpolledAndDeletedRows(t *testing.T) { + store := newWatchStore(t) + first := seedWorkItemWatch(t, store) + second := seedWorkItemWatch(t, store) + polledAt := time.Date(2026, 8, 12, 8, 0, 0, 0, time.UTC) + if err := store.ClearWorkItemWatchError(t.Context(), first.ID, polledAt); err != nil { + t.Fatalf("mark first watch polled: %v", err) + } + + watches, err := store.ListEnabledWorkItemWatches(t.Context()) + if err != nil || len(watches) != 2 { + t.Fatalf("enabled watches = %+v, %v", watches, err) + } + // Never-polled watches sort first so a new watch is not starved. + if watches[0].ID != second.ID || watches[0].LastPolledAt != nil { + t.Fatalf("enabled ordering = %+v, want the never-polled watch first", watches) + } + if watches[1].ID != first.ID || watches[1].LastPolledAt == nil { + t.Fatalf("enabled ordering = %+v", watches) + } + + if err := store.DeleteWorkItemWatch(t.Context(), second.ID); err != nil { + t.Fatalf("delete watch: %v", err) + } + watches, err = store.ListEnabledWorkItemWatches(t.Context()) + if err != nil || len(watches) != 1 || watches[0].ID != first.ID { + t.Fatalf("enabled watches after delete = %+v, %v", watches, err) + } +}