test(gitlab): cover WS handlers, watch HTTP routes, and provider identity (#2585)

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

apps/backend/internal/gitlab/controller_config_routes_test.go

@@ -0,0 +1,252 @@
+package gitlab
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+)
+
+// newConfigRouteFixture wires the config HTTP routes against a store, a fake
+// workspace secret store, and a stub GitLab whose /api/v4/user accepts exactly
+// one token. The returned server URL is the host to configure.
+func newConfigRouteFixture(t *testing.T, validToken string) (*Store, *configTestSecrets, *gin.Engine, string) {
+	t.Helper()
+	gin.SetMode(gin.TestMode)
+	gitlabServer := httptest.NewServer(userHandler(func(token string) bool { return token == validToken }))
+	t.Cleanup(gitlabServer.Close)
+
+	store := newTestStore(t)
+	seedWorkspace(t, store, "ws-1")
+	seedWorkspace(t, store, "ws-2")
+	secrets := &configTestSecrets{values: make(map[string]string)}
+	svc := NewService(DefaultHost, NewNoopClient(DefaultHost), AuthMethodNone, nil, newTestLogger(t))
+	svc.SetStore(store)
+	svc.SetWorkspaceSecretStore(secrets)
+	router := gin.New()
+	NewController(svc, newTestLogger(t)).RegisterHTTPRoutes(router)
+	return store, secrets, router, gitlabServer.URL
+}
+
+// configureWorkspace saves a PAT connection through the real PUT /config route
+// so the follow-up delete/copy tests operate on state the API itself produced.
+func configureWorkspace(t *testing.T, router *gin.Engine, workspaceID, host, token string) {
+	t.Helper()
+	recorder := watchScopeRequest(t, router, http.MethodPut,
+		"/api/v1/gitlab/config?workspace_id="+workspaceID,
+		map[string]string{"host": host, "auth_method": AuthMethodPAT, "token": token})
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("configure %s: status = %d, body = %s", workspaceID, recorder.Code, recorder.Body.String())
+	}
+}
+
+// --- DELETE /config ---
+
+func TestHTTPDeleteConfigRemovesRowAndSecret(t *testing.T) {
+	store, secrets, router, host := newConfigRouteFixture(t, "good-token")
+	configureWorkspace(t, router, "ws-1", host, "good-token")
+	if _, ok := secrets.values[SecretKeyForWorkspace("ws-1")]; !ok {
+		t.Fatal("fixture did not store a workspace secret to begin with")
+	}
+
+	var body map[string]bool
+	recorder := httpJSON(t, router, http.MethodDelete,
+		"/api/v1/gitlab/config?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if !body[respKeyDeleted] {
+		t.Errorf("payload = %v, want {\"deleted\":true}", body)
+	}
+	cfg, err := store.GetConfigForWorkspace(context.Background(), "ws-1")
+	if err != nil {
+		t.Fatalf("reload config: %v", err)
+	}
+	if cfg != nil {
+		t.Errorf("config = %+v, want the row removed", cfg)
+	}
+	if _, ok := secrets.values[SecretKeyForWorkspace("ws-1")]; ok {
+		t.Error("the workspace token secret survived the connection delete")
+	}
+}
+
+// TestHTTPDeleteConfigRequiresWorkspaceQuery covers writeConfigMutationError's
+// ErrWorkspaceRequired branch, which must be a 400 rather than the default 500.
+func TestHTTPDeleteConfigRequiresWorkspaceQuery(t *testing.T) {
+	_, _, router, _ := newConfigRouteFixture(t, "good-token")
+
+	recorder := watchScopeRequest(t, router, http.MethodDelete, "/api/v1/gitlab/config", nil)
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "workspace_id query parameter required" {
+		t.Errorf("error = %q, want the workspace_id requirement", got)
+	}
+}
+
+// --- POST /config/test ---
+
+func TestHTTPTestConfigReportsSuccessWithoutPersisting(t *testing.T) {
+	store, secrets, router, host := newConfigRouteFixture(t, "good-token")
+
+	var result TestConnectionResult
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/config/test?workspace_id=ws-1",
+		map[string]string{"host": host, "auth_method": AuthMethodPAT, "token": "good-token"}, &result)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if result.Error != "" {
+		t.Fatalf("error = %q, want a successful probe", result.Error)
+	}
+	if result.Username != "alice" {
+		t.Errorf("username = %q, want alice from the stub", result.Username)
+	}
+	cfg, err := store.GetConfigForWorkspace(context.Background(), "ws-1")
+	if err != nil {
+		t.Fatalf("reload config: %v", err)
+	}
+	if cfg != nil {
+		t.Errorf("config = %+v, want nothing persisted by a test probe", cfg)
+	}
+	if len(secrets.values) != 0 {
+		t.Errorf("secrets = %v, want nothing persisted by a test probe", secrets.values)
+	}
+}
+
+// TestHTTPTestConfigReportsFailureAsA200WithError pins the contract the
+// settings UI depends on: a rejected credential is a *result*, not an HTTP
+// error, so the form can render the message inline.
+func TestHTTPTestConfigReportsFailureAsA200WithError(t *testing.T) {
+	_, _, router, host := newConfigRouteFixture(t, "good-token")
+
+	var result TestConnectionResult
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/config/test?workspace_id=ws-1",
+		map[string]string{"host": host, "auth_method": AuthMethodPAT, "token": "wrong-token"}, &result)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 even for a failed probe (body %s)",
+			recorder.Code, recorder.Body.String())
+	}
+	if result.Error == "" {
+		t.Error("error is empty; a rejected token must be reported in the result")
+	}
+	if result.Username != "" {
+		t.Errorf("username = %q, want empty for a failed probe", result.Username)
+	}
+}
+
+func TestHTTPTestConfigRejectsInvalidPayload(t *testing.T) {
+	_, _, router, _ := newConfigRouteFixture(t, "good-token")
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/config/test?workspace_id=ws-1", []string{"not", "an", "object"})
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "invalid payload" {
+		t.Errorf("error = %q, want \"invalid payload\"", got)
+	}
+}
+
+// --- POST /config/copy ---
+
+func TestHTTPCopyConfigDuplicatesConnectionToTargetWorkspace(t *testing.T) {
+	store, secrets, router, host := newConfigRouteFixture(t, "good-token")
+	configureWorkspace(t, router, "ws-1", host, "good-token")
+
+	var copied GitLabConfig
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/config/copy?workspace_id=ws-1",
+		map[string]string{"targetWorkspaceId": "ws-2"}, &copied)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	target, err := store.GetConfigForWorkspace(context.Background(), "ws-2")
+	if err != nil || target == nil {
+		t.Fatalf("target config = %+v, err = %v", target, err)
+	}
+	if target.Host != host {
+		t.Errorf("target host = %q, want %q", target.Host, host)
+	}
+	if secrets.values[SecretKeyForWorkspace("ws-2")] != "good-token" {
+		t.Errorf("target secret = %q, want the source token copied under the target's own key",
+			secrets.values[SecretKeyForWorkspace("ws-2")])
+	}
+	if secrets.values[SecretKeyForWorkspace("ws-1")] != "good-token" {
+		t.Error("the source workspace's secret was disturbed by the copy")
+	}
+}
+
+func TestHTTPCopyConfigRequiresTargetWorkspace(t *testing.T) {
+	_, _, router, host := newConfigRouteFixture(t, "good-token")
+	configureWorkspace(t, router, "ws-1", host, "good-token")
+
+	for name, body := range map[string]interface{}{
+		"absent": map[string]string{},
+		"blank":  map[string]string{"targetWorkspaceId": "   "},
+	} {
+		t.Run(name, func(t *testing.T) {
+			recorder := watchScopeRequest(t, router, http.MethodPost,
+				"/api/v1/gitlab/config/copy?workspace_id=ws-1", body)
+
+			if recorder.Code != http.StatusBadRequest {
+				t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+			}
+			if got := httpErrorMessage(t, recorder); got != "targetWorkspaceId required" {
+				t.Errorf("error = %q, want \"targetWorkspaceId required\"", got)
+			}
+		})
+	}
+}
+
+// TestHTTPCopyConfigRejectsCopyingAWorkspaceOntoItself covers the
+// ErrSameWorkspace branch of writeConfigMutationError.
+func TestHTTPCopyConfigRejectsCopyingAWorkspaceOntoItself(t *testing.T) {
+	_, _, router, host := newConfigRouteFixture(t, "good-token")
+	configureWorkspace(t, router, "ws-1", host, "good-token")
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/config/copy?workspace_id=ws-1",
+		map[string]string{"targetWorkspaceId": "ws-1"})
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "source and target workspace must differ" {
+		t.Errorf("error = %q, want the same-workspace rejection", got)
+	}
+}
+
+// TestHTTPCopyConfigRejectsUnconfiguredSource covers the ErrNothingToCopy
+// branch: copying from a workspace with no connection is a client error, and
+// the target must be left untouched.
+func TestHTTPCopyConfigRejectsUnconfiguredSource(t *testing.T) {
+	store, _, router, _ := newConfigRouteFixture(t, "good-token")
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/config/copy?workspace_id=ws-1",
+		map[string]string{"targetWorkspaceId": "ws-2"})
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "source workspace has no GitLab connection" {
+		t.Errorf("error = %q, want the nothing-to-copy rejection", got)
+	}
+	target, err := store.GetConfigForWorkspace(context.Background(), "ws-2")
+	if err != nil {
+		t.Fatalf("reload target config: %v", err)
+	}
+	if target != nil {
+		t.Errorf("target config = %+v, want it left unconfigured", target)
+	}
+}

apps/backend/internal/gitlab/controller_http_surface_test.go

@@ -0,0 +1,430 @@
+package gitlab
+
+import (
+	"context"
+	"net/http"
+	"testing"
+)
+
+// --- Task MR listing routes ---
+
+func TestHTTPListWorkspaceTaskMRsGroupsByTask(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	ctx := context.Background()
+	seedTask(t, store, "task-a", "ws-1")
+	seedTask(t, store, "task-b", "ws-1")
+	seedTask(t, store, "task-foreign", "ws-2")
+	for _, mr := range []*TaskMR{
+		newTestMR("task-a", "repo-1", "group/a", 11),
+		newTestMR("task-b", "repo-1", "group/b", 22),
+		newTestMR("task-foreign", "repo-1", "group/c", 33),
+	} {
+		if err := store.UpsertTaskMR(ctx, mr); err != nil {
+			t.Fatalf("seed %s MR: %v", mr.TaskID, err)
+		}
+	}
+
+	var body TaskMRsResponse
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/workspaces/ws-1/task-mrs", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.TaskMRs) != 2 {
+		t.Fatalf("grouped tasks = %d, want 2 (%v)", len(body.TaskMRs), body.TaskMRs)
+	}
+	if _, leaked := body.TaskMRs["task-foreign"]; leaked {
+		t.Error("the ws-2 task leaked into the ws-1 listing")
+	}
+	if got := body.TaskMRs["task-a"]; len(got) != 1 || got[0].MRIID != 11 {
+		t.Errorf("task-a group = %+v, want the single !11 row", got)
+	}
+}
+
+func TestHTTPListTaskMRsReturnsRowsForTask(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	ctx := context.Background()
+	seedTask(t, store, "task-a", "ws-1")
+	if err := store.UpsertTaskMR(ctx, newTestMR("task-a", "repo-1", "group/a", 11)); err != nil {
+		t.Fatalf("seed MR: %v", err)
+	}
+
+	var body struct {
+		TaskMRs []*TaskMR `json:"task_mrs"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet, "/api/v1/gitlab/tasks/task-a/mrs", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.TaskMRs) != 1 || body.TaskMRs[0].ProjectPath != "group/a" {
+		t.Fatalf("task_mrs = %+v, want the single group/a row", body.TaskMRs)
+	}
+}
+
+func TestHTTPListTaskMRsReturnsEmptyListForUnknownTask(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	var body struct {
+		TaskMRs []*TaskMR `json:"task_mrs"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet, "/api/v1/gitlab/tasks/nope/mrs", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.TaskMRs) != 0 {
+		t.Errorf("task_mrs = %+v, want an empty list", body.TaskMRs)
+	}
+}
+
+// --- Watch reset routes ---
+
+func TestHTTPPreviewResetReviewWatchCountsTasks(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	ctx := context.Background()
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+	if _, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url"); err != nil {
+		t.Fatalf("reserve review MR task: %v", err)
+	}
+	if err := store.AssignReviewMRTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil {
+		t.Fatalf("assign review MR task: %v", err)
+	}
+
+	var body struct {
+		TaskCount int `json:"taskCount"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/watches/review/"+watch.ID+"/reset/preview?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if body.TaskCount != 1 {
+		t.Errorf("taskCount = %d, want 1", body.TaskCount)
+	}
+}
+
+func TestHTTPResetReviewWatchDeletesTaskTrees(t *testing.T) {
+	svc, store, router := newWatchHTTPFixture(t)
+	ctx := context.Background()
+	deleter := &recordingCascadeDeleter{}
+	svc.SetCascadeTaskDeleter(deleter)
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+	if _, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url"); err != nil {
+		t.Fatalf("reserve review MR task: %v", err)
+	}
+	if err := store.AssignReviewMRTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil {
+		t.Fatalf("assign review MR task: %v", err)
+	}
+
+	var body struct {
+		TasksDeleted int `json:"tasksDeleted"`
+	}
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/review/"+watch.ID+"/reset?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if body.TasksDeleted != 1 {
+		t.Errorf("tasksDeleted = %d, want 1", body.TasksDeleted)
+	}
+	if len(deleter.ids) != 1 || deleter.ids[0] != "task-1" {
+		t.Errorf("deleted task trees = %v, want [task-1]", deleter.ids)
+	}
+}
+
+// TestHTTPResetReviewWatchWithoutCascadeDeleterFails covers writeWatchResetError:
+// an unwired cascade deleter is a server-side misconfiguration, and the reply
+// must be a 500 whose message names the operation, not the internal cause.
+func TestHTTPResetReviewWatchWithoutCascadeDeleterFails(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(t.Context(), watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/review/"+watch.ID+"/reset?workspace_id=ws-1", nil)
+
+	if recorder.Code != http.StatusInternalServerError {
+		t.Fatalf("status = %d, want 500 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "reset review watch failed" {
+		t.Errorf("error = %q, want \"reset review watch failed\"", got)
+	}
+}
+
+func TestHTTPResetReviewWatchRejectsForeignWatch(t *testing.T) {
+	svc, store, router := newWatchHTTPFixture(t)
+	deleter := &recordingCascadeDeleter{}
+	svc.SetCascadeTaskDeleter(deleter)
+	foreign := validReviewWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	if err := store.CreateReviewWatch(t.Context(), foreign); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/review/"+foreign.ID+"/reset?workspace_id=ws-1", nil)
+
+	if recorder.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(deleter.ids) != 0 {
+		t.Errorf("a cross-workspace reset deleted task trees %v", deleter.ids)
+	}
+}
+
+func TestHTTPPreviewResetIssueWatchCountsTasks(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	ctx := context.Background()
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(ctx, watch); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+	if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil {
+		t.Fatalf("reserve issue task: %v", err)
+	}
+	if err := store.AssignIssueWatchTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil {
+		t.Fatalf("assign issue task: %v", err)
+	}
+
+	var body struct {
+		TaskCount int `json:"taskCount"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/watches/issue/"+watch.ID+"/reset/preview?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if body.TaskCount != 1 {
+		t.Errorf("taskCount = %d, want 1", body.TaskCount)
+	}
+}
+
+func TestHTTPResetIssueWatchDeletesTaskTrees(t *testing.T) {
+	svc, store, router := newWatchHTTPFixture(t)
+	ctx := context.Background()
+	deleter := &recordingCascadeDeleter{}
+	svc.SetCascadeTaskDeleter(deleter)
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(ctx, watch); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+	if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil {
+		t.Fatalf("reserve issue task: %v", err)
+	}
+	if err := store.AssignIssueWatchTaskID(ctx, watch.ID, "group/p", 1, "task-1"); err != nil {
+		t.Fatalf("assign issue task: %v", err)
+	}
+
+	var body struct {
+		TasksDeleted int `json:"tasksDeleted"`
+	}
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/issue/"+watch.ID+"/reset?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if body.TasksDeleted != 1 {
+		t.Errorf("tasksDeleted = %d, want 1", body.TasksDeleted)
+	}
+	if len(deleter.ids) != 1 || deleter.ids[0] != "task-1" {
+		t.Errorf("deleted task trees = %v, want [task-1]", deleter.ids)
+	}
+}
+
+func TestHTTPPreviewResetIssueWatchRejectsForeignWatch(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	foreign := validIssueWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	if err := store.CreateIssueWatch(t.Context(), foreign); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodGet,
+		"/api/v1/gitlab/watches/issue/"+foreign.ID+"/reset/preview?workspace_id=ws-1", nil)
+
+	if recorder.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "issue watch not found" {
+		t.Errorf("error = %q, want \"issue watch not found\"", got)
+	}
+}
+
+// --- Action presets over HTTP ---
+
+func TestHTTPActionPresetsRoundTrip(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+
+	var defaults ActionPresets
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/action-presets?workspace_id=ws-1", nil, &defaults)
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("get status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(defaults.MR) == 0 {
+		t.Fatal("mr presets are empty; the built-in defaults must be substituted")
+	}
+
+	var updated ActionPresets
+	recorder = httpJSON(t, router, http.MethodPut,
+		"/api/v1/gitlab/action-presets?workspace_id=ws-1",
+		UpdateActionPresetsRequest{
+			MR: &[]ActionPreset{{ID: "ship", Label: "Ship it", PromptTemplate: "ship {{mr}}"}},
+		}, &updated)
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("put status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(updated.MR) != 1 || updated.MR[0].ID != "ship" {
+		t.Fatalf("mr presets = %+v, want only the custom ship preset", updated.MR)
+	}
+	if updated.WorkspaceID != "ws-1" {
+		t.Errorf("workspace = %q, want ws-1 from the query parameter", updated.WorkspaceID)
+	}
+
+	var reset ActionPresets
+	recorder = httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/action-presets/reset?workspace_id=ws-1", nil, &reset)
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("reset status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	for _, preset := range reset.MR {
+		if preset.ID == "ship" {
+			t.Fatalf("mr presets = %+v, want the custom preset gone after reset", reset.MR)
+		}
+	}
+	stored, err := store.GetActionPresets(t.Context(), "ws-1")
+	if err != nil {
+		t.Fatalf("reload presets: %v", err)
+	}
+	if len(stored.MR) != 0 {
+		t.Errorf("stored mr presets = %+v, want the row deleted", stored.MR)
+	}
+}
+
+func TestHTTPUpdateActionPresetsRejectsInvalidPayload(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	recorder := watchScopeRequest(t, router, http.MethodPut,
+		"/api/v1/gitlab/action-presets?workspace_id=ws-1", []string{"not", "an", "object"})
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "invalid payload" {
+		t.Errorf("error = %q, want \"invalid payload\"", got)
+	}
+}
+
+// --- Project routes ---
+
+func TestHTTPListUserProjectsReturnsProjects(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	var body struct {
+		Projects []Project `json:"projects"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/projects?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.Projects) != 1 || body.Projects[0].PathWithNamespace != "kandev/sample" {
+		t.Fatalf("projects = %+v, want the single kandev/sample project", body.Projects)
+	}
+}
+
+func TestHTTPSearchProjectsFiltersByQuery(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	tests := []struct {
+		name  string
+		query string
+		want  int
+	}{
+		{"matching", "sample", 1},
+		{"non-matching", "no-such-project", 0},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var body struct {
+				Projects []Project `json:"projects"`
+			}
+			recorder := httpJSON(t, router, http.MethodGet,
+				"/api/v1/gitlab/projects/search?workspace_id=ws-1&query="+test.query, nil, &body)
+
+			if recorder.Code != http.StatusOK {
+				t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+			}
+			if len(body.Projects) != test.want {
+				t.Errorf("projects = %d, want %d (%+v)", len(body.Projects), test.want, body.Projects)
+			}
+		})
+	}
+}
+
+func TestHTTPProjectRoutesRequireProjectQuery(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	for _, path := range []string{"/projects/branches", "/projects/merge-methods"} {
+		t.Run(path, func(t *testing.T) {
+			recorder := watchScopeRequest(t, router, http.MethodGet,
+				"/api/v1/gitlab"+path+"?workspace_id=ws-1", nil)
+
+			if recorder.Code != http.StatusBadRequest {
+				t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+			}
+			if got := httpErrorMessage(t, recorder); got != "project required" {
+				t.Errorf("error = %q, want \"project required\"", got)
+			}
+		})
+	}
+}
+
+func TestHTTPGetProjectMergeMethodsReturnsAllowedMethods(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	var methods ProjectMergeMethods
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/projects/merge-methods?workspace_id=ws-1&project=group%2Fp", nil, &methods)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if !methods.Merge || !methods.AllowSquash {
+		t.Errorf("methods = %+v, want the mock's merge+squash defaults", methods)
+	}
+}
+
+func TestHTTPListProjectBranchesReturnsBranches(t *testing.T) {
+	svc, _, router := newWatchHTTPFixture(t)
+	svc.workspaceClients["ws-1"].(*MockClient).SeedBranches("group/p",
+		[]RepoBranch{{Name: "main"}, {Name: "dev"}})
+
+	var body struct {
+		Branches []RepoBranch `json:"branches"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/projects/branches?workspace_id=ws-1&project=group%2Fp", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.Branches) != 2 || body.Branches[0].Name != "main" {
+		t.Fatalf("branches = %+v, want [main dev]", body.Branches)
+	}
+}

apps/backend/internal/gitlab/controller_watches_http_test.go

@@ -0,0 +1,571 @@
+package gitlab
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+)
+
+// newWatchHTTPFixture builds the full v1 HTTP surface over a store seeded with
+// two workspaces, the dependency validators that accept the "ws-1" fixture
+// identifiers, and a per-workspace mock client. Watch CRUD over HTTP is
+// workspace-scoped through the ?workspace_id= query parameter, so every test
+// here can also assert the cross-workspace rejection.
+func newWatchHTTPFixture(t *testing.T) (*Service, *Store, *gin.Engine) {
+	t.Helper()
+	gin.SetMode(gin.TestMode)
+	svc, store := newDependencyTestService(t)
+	seedWorkspace(t, store, "ws-1")
+	seedWorkspace(t, store, "ws-2")
+	svc.workspaceClients = map[string]Client{
+		"ws-1": NewMockClient(DefaultHost),
+		"ws-2": NewMockClient(DefaultHost),
+	}
+	router := gin.New()
+	NewController(svc, newTestLogger(t)).RegisterHTTPRoutes(router)
+	return svc, store, router
+}
+
+// httpJSON issues a request against the in-process router and decodes the JSON
+// body, returning the recorder so the caller can assert the status code too.
+func httpJSON(t *testing.T, router *gin.Engine, method, target string, body interface{}, out interface{}) *httptest.ResponseRecorder {
+	t.Helper()
+	recorder := watchScopeRequest(t, router, method, target, body)
+	if out != nil && recorder.Body.Len() > 0 {
+		if err := json.Unmarshal(recorder.Body.Bytes(), out); err != nil {
+			t.Fatalf("decode %s %s body %s: %v", method, target, recorder.Body.String(), err)
+		}
+	}
+	return recorder
+}
+
+// httpErrorMessage decodes the {"error": "..."} envelope every failure path in
+// this controller writes.
+func httpErrorMessage(t *testing.T, recorder *httptest.ResponseRecorder) string {
+	t.Helper()
+	var body struct {
+		Error string `json:"error"`
+	}
+	if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil {
+		t.Fatalf("decode error body %s: %v", recorder.Body.String(), err)
+	}
+	return body.Error
+}
+
+// --- Workspace guard ---
+
+// TestWatchRoutesRequireWorkspaceQuery covers the requireWorkspaceQuery
+// middleware: every route under it must abort with 400 before its handler runs,
+// so an unscoped caller can never reach workspace data.
+func TestWatchRoutesRequireWorkspaceQuery(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	routes := []struct {
+		method string
+		path   string
+	}{
+		{http.MethodGet, "/api/v1/gitlab/watches/review"},
+		{http.MethodGet, "/api/v1/gitlab/watches/issue"},
+		{http.MethodGet, "/api/v1/gitlab/watches/mr"},
+		{http.MethodPost, "/api/v1/gitlab/watches/review/trigger-all"},
+		{http.MethodPost, "/api/v1/gitlab/watches/issue/trigger-all"},
+		{http.MethodGet, "/api/v1/gitlab/stats"},
+		{http.MethodGet, "/api/v1/gitlab/action-presets"},
+		{http.MethodGet, "/api/v1/gitlab/projects"},
+	}
+	for _, route := range routes {
+		t.Run(route.method+" "+route.path, func(t *testing.T) {
+			recorder := watchScopeRequest(t, router, route.method, route.path, nil)
+			if recorder.Code != http.StatusBadRequest {
+				t.Fatalf("status = %d, want 400 without workspace_id (body %s)",
+					recorder.Code, recorder.Body.String())
+			}
+			if got := httpErrorMessage(t, recorder); got != "workspace_id query parameter required" {
+				t.Errorf("error = %q, want the workspace_id requirement", got)
+			}
+		})
+	}
+}
+
+// --- Review watches over HTTP ---
+
+func TestHTTPCreateReviewWatchPersistsUnderQueryWorkspace(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	// WorkspaceID is deliberately wrong in the body: the controller must
+	// overwrite it with the query parameter, which is the scoped value.
+	body := validReviewWatchRequest()
+	body.WorkspaceID = "ws-2"
+
+	var created ReviewWatch
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/review?workspace_id=ws-1", body, &created)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if created.WorkspaceID != "ws-1" {
+		t.Errorf("workspace = %q, want ws-1 from the query parameter, not the body", created.WorkspaceID)
+	}
+	stored, err := store.GetReviewWatch(t.Context(), created.ID)
+	if err != nil || stored == nil {
+		t.Fatalf("watch was not persisted: row=%v err=%v", stored, err)
+	}
+	if stored.WorkspaceID != "ws-1" {
+		t.Errorf("stored workspace = %q, want ws-1", stored.WorkspaceID)
+	}
+}
+
+func TestHTTPCreateReviewWatchRejectsInvalidPayload(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+
+	req := httptest.NewRequest(http.MethodPost,
+		"/api/v1/gitlab/watches/review?workspace_id=ws-1", strings.NewReader("{not json"))
+	req.Header.Set("Content-Type", "application/json")
+	recorder := httptest.NewRecorder()
+	router.ServeHTTP(recorder, req)
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "invalid payload" {
+		t.Errorf("error = %q, want \"invalid payload\"", got)
+	}
+}
+
+// TestHTTPCreateReviewWatchMapsInvalidConfigTo400 pins the httpRespondError
+// mapping: a dependency that belongs to another workspace is a client error,
+// and the response must not disclose which dependency failed.
+func TestHTTPCreateReviewWatchMapsInvalidConfigTo400(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+	body := validReviewWatchRequest()
+	body.RepositoryID = "repo-other" // belongs to ws-2
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/review?workspace_id=ws-1", body)
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "invalid watch configuration" {
+		t.Errorf("error = %q, want the generic configuration message", got)
+	}
+	if body := recorder.Body.String(); strings.Contains(body, "repo-other") {
+		t.Errorf("response disclosed the rejected dependency id: %s", body)
+	}
+}
+
+func TestHTTPListReviewWatchesScopesToQueryWorkspace(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	mine := validReviewWatchRow()
+	foreign := validReviewWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	for _, watch := range []*ReviewWatch{mine, foreign} {
+		if err := store.CreateReviewWatch(t.Context(), watch); err != nil {
+			t.Fatalf("seed review watch: %v", err)
+		}
+	}
+
+	var body struct {
+		Watches []*ReviewWatch `json:"watches"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/watches/review?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.Watches) != 1 || body.Watches[0].ID != mine.ID {
+		t.Fatalf("watches = %+v, want only the ws-1 watch", body.Watches)
+	}
+}
+
+func TestHTTPUpdateReviewWatchAppliesPatchAndReturnsRow(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(t.Context(), watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	var updated ReviewWatch
+	recorder := httpJSON(t, router, http.MethodPatch,
+		"/api/v1/gitlab/watches/review/"+watch.ID+"?workspace_id=ws-1",
+		map[string]interface{}{"enabled": false}, &updated)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if updated.ID != watch.ID || updated.Enabled {
+		t.Errorf("returned watch = %+v, want the disabled row echoed back", updated)
+	}
+	stored, err := store.GetReviewWatch(t.Context(), watch.ID)
+	if err != nil {
+		t.Fatalf("reload watch: %v", err)
+	}
+	if stored.Enabled {
+		t.Error("watch is still enabled; the patch did not reach the store")
+	}
+}
+
+// TestHTTPUpdateReviewWatchRejectsForeignWatch is the cross-workspace guard:
+// requireReviewWatchInWorkspace must 404 before the patch is applied, and the
+// foreign row must be untouched.
+func TestHTTPUpdateReviewWatchRejectsForeignWatch(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	foreign := validReviewWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	if err := store.CreateReviewWatch(t.Context(), foreign); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodPatch,
+		"/api/v1/gitlab/watches/review/"+foreign.ID+"?workspace_id=ws-1",
+		map[string]interface{}{"enabled": false})
+
+	if recorder.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "review watch not found" {
+		t.Errorf("error = %q, want \"review watch not found\"", got)
+	}
+	stored, err := store.GetReviewWatch(t.Context(), foreign.ID)
+	if err != nil {
+		t.Fatalf("reload foreign watch: %v", err)
+	}
+	if !stored.Enabled {
+		t.Error("the foreign watch was modified by a cross-workspace request")
+	}
+}
+
+func TestHTTPDeleteReviewWatchRemovesRow(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(t.Context(), watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	var body map[string]bool
+	recorder := httpJSON(t, router, http.MethodDelete,
+		"/api/v1/gitlab/watches/review/"+watch.ID+"?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if !body[respKeyDeleted] {
+		t.Errorf("payload = %v, want {\"deleted\":true}", body)
+	}
+	remaining, err := store.ListReviewWatches(t.Context(), "ws-1")
+	if err != nil {
+		t.Fatalf("list review watches: %v", err)
+	}
+	if len(remaining) != 0 {
+		t.Errorf("watches remaining = %d, want 0", len(remaining))
+	}
+}
+
+func TestHTTPDeleteReviewWatchRejectsForeignWatch(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	foreign := validReviewWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	if err := store.CreateReviewWatch(t.Context(), foreign); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodDelete,
+		"/api/v1/gitlab/watches/review/"+foreign.ID+"?workspace_id=ws-1", nil)
+
+	if recorder.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	remaining, err := store.ListReviewWatches(t.Context(), "ws-2")
+	if err != nil || len(remaining) != 1 {
+		t.Fatalf("foreign watch was deleted: rows=%d err=%v", len(remaining), err)
+	}
+}
+
+func TestHTTPTriggerReviewWatchReturnsMRsAndCount(t *testing.T) {
+	svc, store, router := newWatchHTTPFixture(t)
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(t.Context(), watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+	client := svc.workspaceClients["ws-1"].(*MockClient)
+	client.SetUser("kandev-tester")
+	client.SeedMR("group/p", &MR{IID: 1, Title: "needs review", WebURL: "https://x/1",
+		Reviewers: []MRReviewer{{Username: "kandev-tester"}}})
+
+	var body struct {
+		MRs   []*MR `json:"mrs"`
+		Count int   `json:"count"`
+	}
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/review/"+watch.ID+"/trigger?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.MRs) != 1 {
+		t.Fatalf("mrs = %+v, want the single seeded MR to reach the client", body.MRs)
+	}
+	if body.MRs[0].IID != 1 {
+		t.Errorf("mr iid = %d, want 1", body.MRs[0].IID)
+	}
+	if body.Count != len(body.MRs) {
+		t.Errorf("count = %d but mrs = %d; the two must agree", body.Count, len(body.MRs))
+	}
+}
+
+func TestHTTPTriggerReviewWatchRejectsForeignWatch(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	foreign := validReviewWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	if err := store.CreateReviewWatch(t.Context(), foreign); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/review/"+foreign.ID+"/trigger?workspace_id=ws-1", nil)
+
+	if recorder.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String())
+	}
+}
+
+// --- Issue watches over HTTP ---
+
+func TestHTTPCreateIssueWatchPersistsUnderQueryWorkspace(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	body := validIssueWatchRequest()
+	body.WorkspaceID = "ws-2"
+
+	var created IssueWatch
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/issue?workspace_id=ws-1", body, &created)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if created.WorkspaceID != "ws-1" {
+		t.Errorf("workspace = %q, want ws-1 from the query parameter", created.WorkspaceID)
+	}
+	stored, err := store.GetIssueWatch(t.Context(), created.ID)
+	if err != nil || stored == nil {
+		t.Fatalf("watch was not persisted: row=%v err=%v", stored, err)
+	}
+}
+
+func TestHTTPCreateIssueWatchMapsInvalidConfigTo400(t *testing.T) {
+	_, _, router := newWatchHTTPFixture(t)
+	body := validIssueWatchRequest()
+	body.ExecutorProfileID = "executor-missing"
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/issue?workspace_id=ws-1", body)
+
+	if recorder.Code != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "invalid watch configuration" {
+		t.Errorf("error = %q, want the generic configuration message", got)
+	}
+}
+
+func TestHTTPListIssueWatchesScopesToQueryWorkspace(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	mine := validIssueWatchRow()
+	foreign := validIssueWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	for _, watch := range []*IssueWatch{mine, foreign} {
+		if err := store.CreateIssueWatch(t.Context(), watch); err != nil {
+			t.Fatalf("seed issue watch: %v", err)
+		}
+	}
+
+	var body struct {
+		Watches []*IssueWatch `json:"watches"`
+	}
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/watches/issue?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.Watches) != 1 || body.Watches[0].ID != mine.ID {
+		t.Fatalf("watches = %+v, want only the ws-1 watch", body.Watches)
+	}
+}
+
+func TestHTTPUpdateIssueWatchAppliesPatchAndReturnsRow(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(t.Context(), watch); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+
+	var updated IssueWatch
+	recorder := httpJSON(t, router, http.MethodPut,
+		"/api/v1/gitlab/watches/issue/"+watch.ID+"?workspace_id=ws-1",
+		map[string]interface{}{"enabled": false}, &updated)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if updated.ID != watch.ID || updated.Enabled {
+		t.Errorf("returned watch = %+v, want the disabled row echoed back", updated)
+	}
+	stored, err := store.GetIssueWatch(t.Context(), watch.ID)
+	if err != nil {
+		t.Fatalf("reload watch: %v", err)
+	}
+	if stored.Enabled {
+		t.Error("watch is still enabled; the patch did not reach the store")
+	}
+}
+
+func TestHTTPUpdateIssueWatchRejectsForeignWatch(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	foreign := validIssueWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	if err := store.CreateIssueWatch(t.Context(), foreign); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodPut,
+		"/api/v1/gitlab/watches/issue/"+foreign.ID+"?workspace_id=ws-1",
+		map[string]interface{}{"enabled": false})
+
+	if recorder.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if got := httpErrorMessage(t, recorder); got != "issue watch not found" {
+		t.Errorf("error = %q, want \"issue watch not found\"", got)
+	}
+	stored, err := store.GetIssueWatch(t.Context(), foreign.ID)
+	if err != nil {
+		t.Fatalf("reload foreign watch: %v", err)
+	}
+	if !stored.Enabled {
+		t.Error("the foreign watch was modified by a cross-workspace request")
+	}
+}
+
+func TestHTTPDeleteIssueWatchRemovesRow(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(t.Context(), watch); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+
+	var body map[string]bool
+	recorder := httpJSON(t, router, http.MethodDelete,
+		"/api/v1/gitlab/watches/issue/"+watch.ID+"?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if !body[respKeyDeleted] {
+		t.Errorf("payload = %v, want {\"deleted\":true}", body)
+	}
+	remaining, err := store.ListIssueWatches(t.Context(), "ws-1")
+	if err != nil {
+		t.Fatalf("list issue watches: %v", err)
+	}
+	if len(remaining) != 0 {
+		t.Errorf("watches remaining = %d, want 0", len(remaining))
+	}
+}
+
+func TestHTTPTriggerIssueWatchRejectsForeignWatch(t *testing.T) {
+	_, store, router := newWatchHTTPFixture(t)
+	foreign := validIssueWatchRow()
+	foreign.WorkspaceID = "ws-2"
+	if err := store.CreateIssueWatch(t.Context(), foreign); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+
+	recorder := watchScopeRequest(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/issue/"+foreign.ID+"/trigger?workspace_id=ws-1", nil)
+
+	if recorder.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String())
+	}
+}
+
+func TestHTTPTriggerIssueWatchReturnsIssuesAndCount(t *testing.T) {
+	svc, store, router := newWatchHTTPFixture(t)
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(t.Context(), watch); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+	svc.workspaceClients["ws-1"].(*MockClient).SeedIssue("group/p",
+		&Issue{IID: 3, Title: "bug", WebURL: "https://x/3"})
+
+	var body struct {
+		Issues []*Issue `json:"issues"`
+		Count  int      `json:"count"`
+	}
+	recorder := httpJSON(t, router, http.MethodPost,
+		"/api/v1/gitlab/watches/issue/"+watch.ID+"/trigger?workspace_id=ws-1", nil, &body)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if len(body.Issues) != 1 {
+		t.Fatalf("issues = %+v, want the single seeded issue to reach the client", body.Issues)
+	}
+	if body.Issues[0].IID != 3 {
+		t.Errorf("issue iid = %d, want 3", body.Issues[0].IID)
+	}
+	if body.Count != len(body.Issues) {
+		t.Errorf("count = %d but issues = %d; the two must agree", body.Count, len(body.Issues))
+	}
+}
+
+// --- Stats ---
+
+func TestHTTPGetStatsReturnsWorkspaceCounts(t *testing.T) {
+	svc, _, router := newWatchHTTPFixture(t)
+	client := svc.workspaceClients["ws-1"].(*MockClient)
+	client.SetUser("alice")
+	client.SeedMR("group/p", &MR{IID: 1, Title: "one", State: "opened"})
+	client.SeedIssue("group/p", &Issue{IID: 2, Title: "bug", State: "opened"})
+
+	var stats Stats
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/stats?workspace_id=ws-1", nil, &stats)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if stats.OpenMRs != 1 {
+		t.Errorf("open_mrs = %d, want 1", stats.OpenMRs)
+	}
+	if stats.OpenIssuesAssignedMe != 1 {
+		t.Errorf("open_issues_assigned_me = %d, want 1", stats.OpenIssuesAssignedMe)
+	}
+}
+
+// TestHTTPGetStatsIsScopedToItsWorkspace proves the stats route reads the
+// workspace's own client rather than a shared one: ws-2 has nothing seeded, so
+// its counts must be zero while ws-1's are not.
+func TestHTTPGetStatsIsScopedToItsWorkspace(t *testing.T) {
+	svc, _, router := newWatchHTTPFixture(t)
+	client := svc.workspaceClients["ws-1"].(*MockClient)
+	client.SetUser("alice")
+	client.SeedMR("group/p", &MR{IID: 1, Title: "one", State: "opened"})
+	svc.workspaceClients["ws-2"].(*MockClient).SetUser("bob")
+
+	var stats Stats
+	recorder := httpJSON(t, router, http.MethodGet,
+		"/api/v1/gitlab/stats?workspace_id=ws-2", nil, &stats)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String())
+	}
+	if stats.OpenMRs != 0 {
+		t.Errorf("open_mrs = %d for ws-2, want 0 — ws-1's MRs must not leak", stats.OpenMRs)
+	}
+}

apps/backend/internal/gitlab/handlers_registration_test.go

@@ -0,0 +1,200 @@
+package gitlab
+
+import (
+	"context"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+
+	ws "github.com/kandev/kandev/pkg/websocket"
+)
+
+// gitLabWSActions is every action registerWSHandlers wires. Kept explicit so a
+// handler added to handlers.go without a corresponding entry here fails
+// TestRegisterWSHandlersCoversEveryGitLabAction rather than going untested.
+var gitLabWSActions = []string{
+	ws.ActionGitLabStatus,
+	ws.ActionGitLabTaskMRsList,
+	ws.ActionGitLabTaskMRGet,
+	ws.ActionGitLabMRFeedbackGet,
+	ws.ActionGitLabReviewWatchesList,
+	ws.ActionGitLabReviewWatchCreate,
+	ws.ActionGitLabReviewWatchUpdate,
+	ws.ActionGitLabReviewWatchDelete,
+	ws.ActionGitLabReviewTrigger,
+	ws.ActionGitLabReviewTriggerAll,
+	ws.ActionGitLabMRWatchesList,
+	ws.ActionGitLabMRWatchDelete,
+	ws.ActionGitLabMRFilesGet,
+	ws.ActionGitLabMRCommitsGet,
+	ws.ActionGitLabTaskMRSync,
+	ws.ActionGitLabStats,
+	ws.ActionGitLabMRMerge,
+	ws.ActionGitLabMRApprove,
+	ws.ActionGitLabMRUnapprove,
+	ws.ActionGitLabMRSetLabels,
+	ws.ActionGitLabMRSetAssignees,
+	ws.ActionGitLabMRDiscussionNew,
+	ws.ActionGitLabMRDiscussionResolve,
+	ws.ActionGitLabProjectMergeMethodsGet,
+	ws.ActionGitLabIssueWatchesList,
+	ws.ActionGitLabIssueWatchCreate,
+	ws.ActionGitLabIssueWatchUpdate,
+	ws.ActionGitLabIssueWatchDelete,
+	ws.ActionGitLabIssueTrigger,
+	ws.ActionGitLabIssueTriggerAll,
+	ws.ActionGitLabActionPresetsList,
+	ws.ActionGitLabActionPresetsUpdate,
+	ws.ActionGitLabActionPresetsReset,
+	ws.ActionGitLabListUserProjects,
+	ws.ActionGitLabSearchProjects,
+	ws.ActionGitLabProjectBranches,
+	ws.ActionGitLabCleanupReviewTasks,
+	ws.ActionGitLabCleanupIssueTasks,
+}
+
+func TestRegisterWSHandlersCoversEveryGitLabAction(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+	dispatcher := ws.NewDispatcher()
+
+	registerWSHandlers(dispatcher, svc, log)
+
+	for _, action := range gitLabWSActions {
+		if !dispatcher.HasHandler(action) {
+			t.Errorf("action %q has no handler registered", action)
+		}
+	}
+	if got := dispatcher.HandlerCount(); got != len(gitLabWSActions) {
+		t.Errorf("handler count = %d, want %d; a handler was added or removed "+
+			"without updating gitLabWSActions", got, len(gitLabWSActions))
+	}
+}
+
+// TestRegisterRoutesWithDispatcherLeavesWSSurfaceUnmounted pins the
+// authorization boundary documented in handlers.go: the legacy GitLab WS
+// handlers are workspace-unscoped, so RegisterRoutesWithDispatcher must mount
+// the HTTP surface only and leave the dispatcher untouched. Wiring
+// registerWSHandlers back in here would silently expose every workspace's
+// watches over a connection that carries no workspace scope.
+func TestRegisterRoutesWithDispatcherLeavesWSSurfaceUnmounted(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+	dispatcher := ws.NewDispatcher()
+	router := gin.New()
+
+	RegisterRoutesWithDispatcher(router, dispatcher, svc, log)
+
+	if got := dispatcher.HandlerCount(); got != 0 {
+		t.Fatalf("dispatcher has %d handlers, want 0 — the GitLab WS surface is "+
+			"deliberately unmounted because it cannot carry the workspace "+
+			"authorization boundary the HTTP routes enforce", got)
+	}
+	for _, action := range gitLabWSActions {
+		if dispatcher.HasHandler(action) {
+			t.Errorf("action %q was mounted on the dispatcher", action)
+		}
+	}
+}
+
+func TestRegisterRoutesWithDispatcherMountsHTTPRoutes(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+	router := gin.New()
+
+	RegisterRoutesWithDispatcher(router, ws.NewDispatcher(), svc, log)
+
+	want := map[string]string{
+		"GET":    "/api/v1/gitlab/status",
+		"POST":   "/api/v1/gitlab/watches/review",
+		"DELETE": "/api/v1/gitlab/watches/review/:id",
+		"PUT":    "/api/v1/gitlab/action-presets",
+	}
+	routes := map[string]bool{}
+	for _, route := range router.Routes() {
+		routes[route.Method+" "+route.Path] = true
+	}
+	for method, path := range want {
+		if !routes[method+" "+path] {
+			t.Errorf("route %s %s is not registered", method, path)
+		}
+	}
+}
+
+// TestRegisterRoutesMatchesDispatcherVariant guards the package-level
+// RegisterRoutes entrypoint against drifting from the dispatcher-aware sibling:
+// both must mount exactly the same HTTP surface.
+func TestRegisterRoutesMatchesDispatcherVariant(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+
+	plain := gin.New()
+	RegisterRoutes(plain, svc, log)
+	withDispatcher := gin.New()
+	RegisterRoutesWithDispatcher(withDispatcher, ws.NewDispatcher(), svc, log)
+
+	collect := func(router *gin.Engine) map[string]bool {
+		out := map[string]bool{}
+		for _, route := range router.Routes() {
+			out[route.Method+" "+route.Path] = true
+		}
+		return out
+	}
+	plainRoutes, dispatcherRoutes := collect(plain), collect(withDispatcher)
+	if len(plainRoutes) == 0 {
+		t.Fatal("RegisterRoutes mounted no routes at all")
+	}
+	for route := range plainRoutes {
+		if !dispatcherRoutes[route] {
+			t.Errorf("route %q is missing from RegisterRoutesWithDispatcher", route)
+		}
+	}
+	for route := range dispatcherRoutes {
+		if !plainRoutes[route] {
+			t.Errorf("route %q is missing from RegisterRoutes", route)
+		}
+	}
+}
+
+func TestRegisterMockRoutesIsNoOpForNonMockClient(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	svc, _, log := newWSFixture(t, NewNoopClient(DefaultHost))
+	router := gin.New()
+
+	RegisterMockRoutes(router, svc, log)
+
+	if got := len(router.Routes()); got != 0 {
+		t.Errorf("mock routes = %d, want 0 for a non-mock client (%v)", got, router.Routes())
+	}
+}
+
+func TestRegisterMockRoutesMountsControlEndpointsForMockClient(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+	router := gin.New()
+
+	RegisterMockRoutes(router, svc, log)
+
+	if len(router.Routes()) == 0 {
+		t.Fatal("no mock control endpoints were registered for a MockClient")
+	}
+}
+
+// TestWSHandlersDispatchThroughDispatcher proves the registered closures are
+// reachable by action name and return their own action in the reply, which is
+// what lets the frontend correlate a response with the request that caused it.
+func TestWSHandlersDispatchThroughDispatcher(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	client.SetUser("alice")
+	svc, _, log := newWSFixture(t, client)
+	dispatcher := ws.NewDispatcher()
+	registerWSHandlers(dispatcher, svc, log)
+
+	resp, err := dispatcher.Dispatch(context.Background(),
+		wsRequest(t, ws.ActionGitLabStatus, nil))
+
+	var status Status
+	wsOK(t, resp, err, ws.ActionGitLabStatus, &status)
+	if status.Username != "alice" {
+		t.Errorf("username = %q, want alice", status.Username)
+	}
+}

apps/backend/internal/gitlab/handlers_ws_actions_test.go

@@ -0,0 +1,701 @@
+package gitlab
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/kandev/kandev/internal/common/logger"
+	ws "github.com/kandev/kandev/pkg/websocket"
+)
+
+// actionRecordingClient captures the arguments each write handler forwards to
+// the GitLab client, so the tests can assert the exact upstream call rather
+// than only the response envelope. Any method left unset delegates to the
+// embedded MockClient.
+type actionRecordingClient struct {
+	*MockClient
+
+	labelCalls    []labelCall
+	assigneeCalls []assigneeCall
+	noteCalls     []noteCall
+	resolveCalls  []resolveCall
+	mergeMethods  *ProjectMergeMethods
+	mergeErr      error
+}
+
+type labelCall struct {
+	Project string
+	IID     int
+	Labels  []string
+}
+
+type assigneeCall struct {
+	Project string
+	IID     int
+	IDs     []int
+}
+
+type noteCall struct {
+	Project      string
+	IID          int
+	DiscussionID string
+	Body         string
+}
+
+type resolveCall struct {
+	Project      string
+	IID          int
+	DiscussionID string
+}
+
+func (c *actionRecordingClient) SetMRLabels(_ context.Context, project string, iid int, labels []string) error {
+	c.labelCalls = append(c.labelCalls, labelCall{Project: project, IID: iid, Labels: labels})
+	return nil
+}
+
+func (c *actionRecordingClient) SetMRAssignees(_ context.Context, project string, iid int, ids []int) error {
+	c.assigneeCalls = append(c.assigneeCalls, assigneeCall{Project: project, IID: iid, IDs: ids})
+	return nil
+}
+
+func (c *actionRecordingClient) CreateMRDiscussionNote(_ context.Context, project string, iid int, discussionID, body string) (*MRNote, error) {
+	c.noteCalls = append(c.noteCalls, noteCall{Project: project, IID: iid, DiscussionID: discussionID, Body: body})
+	return &MRNote{ID: 99, Body: body}, nil
+}
+
+func (c *actionRecordingClient) ResolveMRDiscussion(_ context.Context, project string, iid int, discussionID string) error {
+	c.resolveCalls = append(c.resolveCalls, resolveCall{Project: project, IID: iid, DiscussionID: discussionID})
+	return nil
+}
+
+func (c *actionRecordingClient) GetProjectMergeMethods(ctx context.Context, project string) (*ProjectMergeMethods, error) {
+	if c.mergeErr != nil {
+		return nil, c.mergeErr
+	}
+	if c.mergeMethods != nil {
+		return c.mergeMethods, nil
+	}
+	return c.MockClient.GetProjectMergeMethods(ctx, project)
+}
+
+func newActionFixture(t *testing.T) (*Service, *actionRecordingClient, *logger.Logger) {
+	t.Helper()
+	client := &actionRecordingClient{MockClient: NewMockClient(DefaultHost)}
+	client.SetUser("alice")
+	svc, _, log := newWSFixture(t, client)
+	return svc, client, log
+}
+
+// projectAndIIDValidationCases is shared by every handler whose guard is the
+// same "project and iid required" triple.
+func projectAndIIDValidationCases(t *testing.T, action string, handler ws.HandlerFunc) {
+	t.Helper()
+	cases := map[string]*ws.Message{
+		"missing project": wsRequest(t, action, map[string]interface{}{"iid": 1}),
+		"missing iid":     wsRequest(t, action, map[string]interface{}{"project": "g/p"}),
+		"malformed":       wsRawRequest(action, `{"iid":"one"}`),
+	}
+	for name, msg := range cases {
+		t.Run(name, func(t *testing.T) {
+			resp, err := handler(context.Background(), msg)
+			wsErr(t, resp, err, action, ws.ErrorCodeBadRequest, "project and iid required")
+		})
+	}
+}
+
+// --- Merge ---
+
+func TestWSMergeMRMergesAndReturnsMR(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+	iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"})
+
+	resp, err := wsMergeMR(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRMerge, map[string]interface{}{
+			"project": "group/p", "iid": iid, "method": "merge",
+		}))
+
+	var mr MR
+	wsOK(t, resp, err, ws.ActionGitLabMRMerge, &mr)
+	if mr.State != "merged" {
+		t.Errorf("state = %q, want merged", mr.State)
+	}
+}
+
+// TestWSMergeMRSurfacesDisallowedMethod pins that method validation happens
+// before the merge call: the project below allows only merge commits, so a
+// fast-forward request must fail rather than silently merging some other way.
+func TestWSMergeMRSurfacesDisallowedMethod(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+	client.mergeMethods = &ProjectMergeMethods{Merge: true}
+	iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"})
+
+	resp, err := wsMergeMR(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRMerge, map[string]interface{}{
+			"project": "group/p", "iid": iid, "method": "ff",
+		}))
+
+	payload := wsErr(t, resp, err, ws.ActionGitLabMRMerge, ws.ErrorCodeInternalError, "")
+	if payload.Message != "project does not allow fast-forward merge" {
+		t.Errorf("message = %q, want the fast-forward rejection", payload.Message)
+	}
+	mr, getErr := client.GetMR(context.Background(), "group/p", iid)
+	if getErr != nil {
+		t.Fatalf("reload MR: %v", getErr)
+	}
+	if mr.State == "merged" {
+		t.Error("MR was merged despite the disallowed method")
+	}
+}
+
+func TestWSMergeMRValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+	projectAndIIDValidationCases(t, ws.ActionGitLabMRMerge, wsMergeMR(svc, log))
+}
+
+// --- Approve / unapprove ---
+
+func TestWSApproveMRRecordsApproval(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+	iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"})
+
+	resp, err := wsApproveMR(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRApprove, map[string]interface{}{"project": "group/p", "iid": iid}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabMRApprove, &body)
+	if !body["approved"] {
+		t.Errorf("payload = %v, want {\"approved\":true}", body)
+	}
+	approvals, err := client.ListMRApprovals(context.Background(), "group/p", iid)
+	if err != nil {
+		t.Fatalf("list approvals: %v", err)
+	}
+	if len(approvals) != 1 || approvals[0].Username != "alice" {
+		t.Errorf("approvals = %+v, want alice's single approval", approvals)
+	}
+}
+
+func TestWSApproveMRSurfacesUpstreamFailure(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+
+	resp, err := wsApproveMR(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRApprove, map[string]interface{}{"project": "group/p", "iid": 404}))
+
+	wsErr(t, resp, err, ws.ActionGitLabMRApprove, ws.ErrorCodeInternalError, "")
+}
+
+func TestWSApproveMRValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+	projectAndIIDValidationCases(t, ws.ActionGitLabMRApprove, wsApproveMR(svc, log))
+}
+
+func TestWSUnapproveMRRemovesApproval(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+	iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"})
+	client.SeedApprovals("group/p", iid, []MRApproval{{Username: "alice"}}, 1)
+
+	resp, err := wsUnapproveMR(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRUnapprove, map[string]interface{}{"project": "group/p", "iid": iid}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabMRUnapprove, &body)
+	if !body["unapproved"] {
+		t.Errorf("payload = %v, want {\"unapproved\":true}", body)
+	}
+	approvals, err := client.ListMRApprovals(context.Background(), "group/p", iid)
+	if err != nil {
+		t.Fatalf("list approvals: %v", err)
+	}
+	if len(approvals) != 0 {
+		t.Errorf("approvals = %+v, want alice's approval revoked", approvals)
+	}
+}
+
+func TestWSUnapproveMRValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+	projectAndIIDValidationCases(t, ws.ActionGitLabMRUnapprove, wsUnapproveMR(svc, log))
+}
+
+// --- Labels / assignees ---
+
+func TestWSSetMRLabelsForwardsExactLabels(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+
+	resp, err := wsSetMRLabels(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRSetLabels, map[string]interface{}{
+			"project": "group/p", "iid": 5, "labels": []string{"bug", "p1"},
+		}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabMRSetLabels, &body)
+	if !body["updated"] {
+		t.Errorf("payload = %v, want {\"updated\":true}", body)
+	}
+	if len(client.labelCalls) != 1 {
+		t.Fatalf("label calls = %d, want exactly 1", len(client.labelCalls))
+	}
+	call := client.labelCalls[0]
+	if call.Project != "group/p" || call.IID != 5 {
+		t.Errorf("call target = %s!%d, want group/p!5", call.Project, call.IID)
+	}
+	if len(call.Labels) != 2 || call.Labels[0] != "bug" || call.Labels[1] != "p1" {
+		t.Errorf("labels = %v, want [bug p1] in order", call.Labels)
+	}
+}
+
+// TestWSSetMRLabelsForwardsEmptyListToClearLabels guards the "remove every
+// label" case: an empty list must still reach the client, because dropping the
+// call would silently leave the old labels in place.
+func TestWSSetMRLabelsForwardsEmptyListToClearLabels(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+
+	resp, err := wsSetMRLabels(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRSetLabels, map[string]interface{}{
+			"project": "group/p", "iid": 5, "labels": []string{},
+		}))
+
+	wsOK(t, resp, err, ws.ActionGitLabMRSetLabels, nil)
+	if len(client.labelCalls) != 1 {
+		t.Fatalf("label calls = %d, want 1 even for an empty list", len(client.labelCalls))
+	}
+	if len(client.labelCalls[0].Labels) != 0 {
+		t.Errorf("labels = %v, want an empty list", client.labelCalls[0].Labels)
+	}
+}
+
+func TestWSSetMRLabelsValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+	projectAndIIDValidationCases(t, ws.ActionGitLabMRSetLabels, wsSetMRLabels(svc, log))
+}
+
+func TestWSSetMRAssigneesForwardsExactIDs(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+
+	resp, err := wsSetMRAssignees(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRSetAssignees, map[string]interface{}{
+			"project": "group/p", "iid": 6, "assignee_ids": []int{7, 8},
+		}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabMRSetAssignees, &body)
+	if !body["updated"] {
+		t.Errorf("payload = %v, want {\"updated\":true}", body)
+	}
+	if len(client.assigneeCalls) != 1 {
+		t.Fatalf("assignee calls = %d, want exactly 1", len(client.assigneeCalls))
+	}
+	call := client.assigneeCalls[0]
+	if call.Project != "group/p" || call.IID != 6 {
+		t.Errorf("call target = %s!%d, want group/p!6", call.Project, call.IID)
+	}
+	if len(call.IDs) != 2 || call.IDs[0] != 7 || call.IDs[1] != 8 {
+		t.Errorf("assignee ids = %v, want [7 8]", call.IDs)
+	}
+}
+
+func TestWSSetMRAssigneesValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+	projectAndIIDValidationCases(t, ws.ActionGitLabMRSetAssignees, wsSetMRAssignees(svc, log))
+}
+
+// --- Discussions ---
+
+func TestWSNewDiscussionNoteForwardsBody(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+
+	resp, err := wsNewDiscussionNote(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRDiscussionNew, map[string]interface{}{
+			"project": "group/p", "iid": 9, "discussion_id": "disc-1", "body": "looks good",
+		}))
+
+	var note MRNote
+	wsOK(t, resp, err, ws.ActionGitLabMRDiscussionNew, &note)
+	if note.Body != "looks good" {
+		t.Errorf("note body = %q, want the posted body echoed back", note.Body)
+	}
+	if len(client.noteCalls) != 1 {
+		t.Fatalf("note calls = %d, want exactly 1", len(client.noteCalls))
+	}
+	if client.noteCalls[0] != (noteCall{Project: "group/p", IID: 9, DiscussionID: "disc-1", Body: "looks good"}) {
+		t.Errorf("note call = %+v, want the exact request arguments", client.noteCalls[0])
+	}
+}
+
+func TestWSNewDiscussionNoteRequiresProjectIIDAndDiscussion(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+	handler := wsNewDiscussionNote(svc, log)
+
+	cases := map[string]interface{}{
+		"missing project":    map[string]interface{}{"iid": 1, "discussion_id": "d", "body": "b"},
+		"missing iid":        map[string]interface{}{"project": "g/p", "discussion_id": "d", "body": "b"},
+		"missing discussion": map[string]interface{}{"project": "g/p", "iid": 1, "body": "b"},
+	}
+	for name, payload := range cases {
+		t.Run(name, func(t *testing.T) {
+			resp, err := handler(context.Background(), wsRequest(t, ws.ActionGitLabMRDiscussionNew, payload))
+			wsErr(t, resp, err, ws.ActionGitLabMRDiscussionNew, ws.ErrorCodeBadRequest,
+				"project, iid, discussion_id required")
+		})
+	}
+}
+
+// TestWSNewDiscussionNoteRejectsBlankBody covers the separate whitespace guard:
+// a body of only spaces is rejected with its own reason and never reaches the
+// client, so no empty comment is posted upstream.
+func TestWSNewDiscussionNoteRejectsBlankBody(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+
+	resp, err := wsNewDiscussionNote(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRDiscussionNew, map[string]interface{}{
+			"project": "group/p", "iid": 9, "discussion_id": "disc-1", "body": "   \t\n ",
+		}))
+
+	wsErr(t, resp, err, ws.ActionGitLabMRDiscussionNew, ws.ErrorCodeBadRequest, "body required")
+	if len(client.noteCalls) != 0 {
+		t.Errorf("note calls = %+v, want none for a blank body", client.noteCalls)
+	}
+}
+
+func TestWSResolveDiscussionForwardsDiscussionID(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+
+	resp, err := wsResolveDiscussion(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRDiscussionResolve, map[string]interface{}{
+			"project": "group/p", "iid": 9, "discussion_id": "disc-1",
+		}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabMRDiscussionResolve, &body)
+	if !body["resolved"] {
+		t.Errorf("payload = %v, want {\"resolved\":true}", body)
+	}
+	if len(client.resolveCalls) != 1 ||
+		client.resolveCalls[0] != (resolveCall{Project: "group/p", IID: 9, DiscussionID: "disc-1"}) {
+		t.Errorf("resolve calls = %+v, want the exact request arguments", client.resolveCalls)
+	}
+}
+
+func TestWSResolveDiscussionRequiresDiscussionID(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+
+	resp, err := wsResolveDiscussion(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRDiscussionResolve, map[string]interface{}{
+			"project": "group/p", "iid": 9,
+		}))
+
+	wsErr(t, resp, err, ws.ActionGitLabMRDiscussionResolve, ws.ErrorCodeBadRequest,
+		"project, iid, discussion_id required")
+}
+
+// --- Merge methods ---
+
+func TestWSGetProjectMergeMethodsReturnsAllowedMethods(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+	client.mergeMethods = &ProjectMergeMethods{Merge: true, AllowSquash: true}
+
+	resp, err := wsGetProjectMergeMethods(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabProjectMergeMethodsGet, map[string]string{"project": "group/p"}))
+
+	var methods ProjectMergeMethods
+	wsOK(t, resp, err, ws.ActionGitLabProjectMergeMethodsGet, &methods)
+	if !methods.Merge || !methods.AllowSquash || methods.FastForward {
+		t.Errorf("methods = %+v, want merge+squash only", methods)
+	}
+}
+
+func TestWSGetProjectMergeMethodsRequiresProject(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+
+	resp, err := wsGetProjectMergeMethods(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabProjectMergeMethodsGet, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabProjectMergeMethodsGet, ws.ErrorCodeBadRequest, "project required")
+}
+
+func TestWSGetProjectMergeMethodsSurfacesUpstreamFailure(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+	client.mergeErr = errors.New("gitlab unreachable")
+
+	resp, err := wsGetProjectMergeMethods(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabProjectMergeMethodsGet, map[string]string{"project": "group/p"}))
+
+	wsErr(t, resp, err, ws.ActionGitLabProjectMergeMethodsGet, ws.ErrorCodeInternalError, "gitlab unreachable")
+}
+
+// --- Projects ---
+
+func TestWSListUserProjectsReturnsProjects(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+
+	resp, err := wsListUserProjects(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabListUserProjects, nil))
+
+	var body struct {
+		Projects []Project `json:"projects"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabListUserProjects, &body)
+	if len(body.Projects) != 1 || body.Projects[0].PathWithNamespace != "kandev/sample" {
+		t.Fatalf("projects = %+v, want the single kandev/sample project", body.Projects)
+	}
+}
+
+func TestWSSearchProjectsFiltersByQuery(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+	handler := wsSearchProjects(svc, log)
+
+	tests := []struct {
+		name  string
+		query string
+		want  int
+	}{
+		{"matching query", "sample", 1},
+		{"non-matching query", "nothing-like-this", 0},
+		{"empty query returns all", "", 1},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			resp, err := handler(context.Background(),
+				wsRequest(t, ws.ActionGitLabSearchProjects, map[string]interface{}{
+					"query": test.query, "limit": 10,
+				}))
+
+			var body struct {
+				Projects []Project `json:"projects"`
+			}
+			wsOK(t, resp, err, ws.ActionGitLabSearchProjects, &body)
+			if len(body.Projects) != test.want {
+				t.Errorf("projects = %d, want %d (%+v)", len(body.Projects), test.want, body.Projects)
+			}
+		})
+	}
+}
+
+func TestWSSearchProjectsRejectsMalformedPayload(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+
+	resp, err := wsSearchProjects(svc, log)(context.Background(),
+		wsRawRequest(ws.ActionGitLabSearchProjects, `{"limit":"ten"}`))
+
+	wsErr(t, resp, err, ws.ActionGitLabSearchProjects, ws.ErrorCodeBadRequest, errMsgInvalidPayload)
+}
+
+func TestWSListProjectBranchesReturnsBranches(t *testing.T) {
+	svc, client, log := newActionFixture(t)
+	client.SeedBranches("group/p", []RepoBranch{{Name: "main"}, {Name: "dev"}})
+
+	resp, err := wsListProjectBranches(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabProjectBranches, map[string]string{"project": "group/p"}))
+
+	var body struct {
+		Branches []RepoBranch `json:"branches"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabProjectBranches, &body)
+	if len(body.Branches) != 2 || body.Branches[0].Name != "main" {
+		t.Fatalf("branches = %+v, want [main dev]", body.Branches)
+	}
+}
+
+func TestWSListProjectBranchesRequiresProject(t *testing.T) {
+	svc, _, log := newActionFixture(t)
+
+	resp, err := wsListProjectBranches(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabProjectBranches, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabProjectBranches, ws.ErrorCodeBadRequest, "project required")
+}
+
+// --- Action presets ---
+
+func TestWSListActionPresetsReturnsDefaultsForNewWorkspace(t *testing.T) {
+	svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsListActionPresets(svc)(context.Background(),
+		wsRequest(t, ws.ActionGitLabActionPresetsList, map[string]string{"workspace_id": "ws-1"}))
+
+	var presets ActionPresets
+	wsOK(t, resp, err, ws.ActionGitLabActionPresetsList, &presets)
+	if len(presets.MR) == 0 || len(presets.Issue) == 0 {
+		t.Fatalf("presets = %+v, want the built-in defaults substituted", presets)
+	}
+}
+
+func TestWSListActionPresetsRequiresWorkspaceID(t *testing.T) {
+	svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsListActionPresets(svc)(context.Background(),
+		wsRequest(t, ws.ActionGitLabActionPresetsList, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabActionPresetsList, ws.ErrorCodeBadRequest, "workspace_id required")
+}
+
+func TestWSUpdateActionPresetsPersistsMRList(t *testing.T) {
+	svc, store, _ := newWSFixture(t, NewMockClient(DefaultHost))
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+
+	resp, err := wsUpdateActionPresets(svc)(ctx,
+		wsRequest(t, ws.ActionGitLabActionPresetsUpdate, UpdateActionPresetsRequest{
+			WorkspaceID: "ws-1",
+			MR:          &[]ActionPreset{{ID: "ship", Label: "Ship it", PromptTemplate: "ship {{mr}}"}},
+		}))
+
+	var presets ActionPresets
+	wsOK(t, resp, err, ws.ActionGitLabActionPresetsUpdate, &presets)
+	if len(presets.MR) != 1 || presets.MR[0].ID != "ship" {
+		t.Fatalf("mr presets = %+v, want only the custom ship preset", presets.MR)
+	}
+	stored, err := store.GetActionPresets(ctx, "ws-1")
+	if err != nil {
+		t.Fatalf("reload presets: %v", err)
+	}
+	if len(stored.MR) != 1 || stored.MR[0].Label != "Ship it" {
+		t.Errorf("stored mr presets = %+v, want the custom preset persisted", stored.MR)
+	}
+}
+
+func TestWSUpdateActionPresetsRequiresWorkspaceID(t *testing.T) {
+	svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsUpdateActionPresets(svc)(context.Background(),
+		wsRequest(t, ws.ActionGitLabActionPresetsUpdate, UpdateActionPresetsRequest{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabActionPresetsUpdate, ws.ErrorCodeBadRequest, "workspace_id required")
+}
+
+func TestWSResetActionPresetsClearsStoredOverrides(t *testing.T) {
+	svc, store, _ := newWSFixture(t, NewMockClient(DefaultHost))
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	if err := store.UpsertActionPresets(ctx, &ActionPresets{
+		WorkspaceID: "ws-1",
+		MR:          []ActionPreset{{ID: "ship", Label: "Ship it", PromptTemplate: "ship"}},
+	}); err != nil {
+		t.Fatalf("seed presets: %v", err)
+	}
+
+	resp, err := wsResetActionPresets(svc)(ctx,
+		wsRequest(t, ws.ActionGitLabActionPresetsReset, map[string]string{"workspace_id": "ws-1"}))
+
+	var presets ActionPresets
+	wsOK(t, resp, err, ws.ActionGitLabActionPresetsReset, &presets)
+	for _, preset := range presets.MR {
+		if preset.ID == "ship" {
+			t.Fatalf("mr presets = %+v, want the custom preset gone after reset", presets.MR)
+		}
+	}
+	stored, err := store.GetActionPresets(ctx, "ws-1")
+	if err != nil {
+		t.Fatalf("reload presets: %v", err)
+	}
+	if len(stored.MR) != 0 {
+		t.Errorf("stored mr presets = %+v, want the row deleted", stored.MR)
+	}
+}
+
+func TestWSResetActionPresetsRequiresWorkspaceID(t *testing.T) {
+	svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsResetActionPresets(svc)(context.Background(),
+		wsRequest(t, ws.ActionGitLabActionPresetsReset, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabActionPresetsReset, ws.ErrorCodeBadRequest, "workspace_id required")
+}
+
+// --- Cleanup ---
+
+// cleanupTaskDeleter records the task IDs a cleanup sweep deletes.
+type cleanupTaskDeleter struct{ ids []string }
+
+func (d *cleanupTaskDeleter) DeleteTask(_ context.Context, taskID string) error {
+	d.ids = append(d.ids, taskID)
+	return nil
+}
+
+func TestWSCleanupReviewTasksDeletesTasksForMergedMRs(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	svc, store, _ := newWSFixture(t, client)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	deleter := &cleanupTaskDeleter{}
+	svc.SetTaskDeleter(deleter)
+
+	const project, iid = "team/repo", 7
+	watch := &ReviewWatch{WorkspaceID: "ws-1", Enabled: true, CleanupPolicy: CleanupPolicyAlways}
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("create review watch: %v", err)
+	}
+	if _, err := store.ReserveReviewMRTask(ctx, watch.ID, project, iid, "url"); err != nil {
+		t.Fatalf("reserve review MR task: %v", err)
+	}
+	if err := store.AssignReviewMRTaskID(ctx, watch.ID, project, iid, "task-merged"); err != nil {
+		t.Fatalf("assign review MR task: %v", err)
+	}
+	client.SeedMR(project, &MR{IID: iid, State: gitlabStateMerged})
+
+	resp, err := wsCleanupReviewTasks(svc)(ctx,
+		wsRequest(t, ws.ActionGitLabCleanupReviewTasks, nil))
+
+	var body map[string]int
+	wsOK(t, resp, err, ws.ActionGitLabCleanupReviewTasks, &body)
+	if body[respKeyDeleted] != 1 {
+		t.Errorf("payload = %v, want {%q:1}", body, respKeyDeleted)
+	}
+	if len(deleter.ids) != 1 || deleter.ids[0] != "task-merged" {
+		t.Errorf("deleted task ids = %v, want [task-merged]", deleter.ids)
+	}
+}
+
+func TestWSCleanupReviewTasksReportsMissingDeleterAsInternalError(t *testing.T) {
+	svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsCleanupReviewTasks(svc)(context.Background(),
+		wsRequest(t, ws.ActionGitLabCleanupReviewTasks, nil))
+
+	wsErr(t, resp, err, ws.ActionGitLabCleanupReviewTasks, ws.ErrorCodeInternalError,
+		"task deleter not configured")
+}
+
+func TestWSCleanupIssueTasksDeletesTasksForClosedIssues(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	svc, store, _ := newWSFixture(t, client)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	deleter := &cleanupTaskDeleter{}
+	svc.SetTaskDeleter(deleter)
+
+	const project, iid = "team/repo", 5
+	watch := &IssueWatch{WorkspaceID: "ws-1", Enabled: true, CleanupPolicy: CleanupPolicyAlways}
+	if err := store.CreateIssueWatch(ctx, watch); err != nil {
+		t.Fatalf("create issue watch: %v", err)
+	}
+	if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, project, iid, "url"); err != nil {
+		t.Fatalf("reserve issue task: %v", err)
+	}
+	if err := store.AssignIssueWatchTaskID(ctx, watch.ID, project, iid, "task-closed"); err != nil {
+		t.Fatalf("assign issue task: %v", err)
+	}
+	client.SeedIssue(project, &Issue{IID: iid, State: gitlabStateClosed})
+
+	resp, err := wsCleanupIssueTasks(svc)(ctx,
+		wsRequest(t, ws.ActionGitLabCleanupIssueTasks, nil))
+
+	var body map[string]int
+	wsOK(t, resp, err, ws.ActionGitLabCleanupIssueTasks, &body)
+	if body[respKeyDeleted] != 1 {
+		t.Errorf("payload = %v, want {%q:1}", body, respKeyDeleted)
+	}
+	if len(deleter.ids) != 1 || deleter.ids[0] != "task-closed" {
+		t.Errorf("deleted task ids = %v, want [task-closed]", deleter.ids)
+	}
+}
+
+func TestWSCleanupIssueTasksReportsMissingDeleterAsInternalError(t *testing.T) {
+	svc, _, _ := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsCleanupIssueTasks(svc)(context.Background(),
+		wsRequest(t, ws.ActionGitLabCleanupIssueTasks, nil))
+
+	wsErr(t, resp, err, ws.ActionGitLabCleanupIssueTasks, ws.ErrorCodeInternalError,
+		"task deleter not configured")
+}

apps/backend/internal/gitlab/handlers_ws_test.go

@@ -0,0 +1,484 @@
+package gitlab
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"testing"
+
+	"github.com/kandev/kandev/internal/common/logger"
+	ws "github.com/kandev/kandev/pkg/websocket"
+)
+
+// --- Shared WS handler fixture ---
+//
+// The WS surface in handlers.go is the legacy, workspace-unscoped GitLab
+// action set: every handler closes over the package Service and translates a
+// ws.Message into one service call. These helpers drive a handler directly
+// and decode the envelope it returns, so the tests can assert the wire
+// contract (error code, reason string, payload keys) rather than merely
+// executing the closure.
+
+func newWSFixture(t *testing.T, client Client) (*Service, *Store, *logger.Logger) {
+	t.Helper()
+	log := newTestLogger(t)
+	store := newTestStore(t)
+	svc := NewService(DefaultHost, client, AuthMethodPAT, nil, log)
+	svc.SetStore(store)
+	return svc, store, log
+}
+
+// wsRequest builds a request envelope with payload marshalled as JSON.
+func wsRequest(t *testing.T, action string, payload interface{}) *ws.Message {
+	t.Helper()
+	msg, err := ws.NewRequest("req-42", action, payload)
+	if err != nil {
+		t.Fatalf("build %s request: %v", action, err)
+	}
+	return msg
+}
+
+// wsRawRequest builds a request envelope carrying a verbatim payload, used to
+// exercise the malformed-payload branches that a marshalled Go value cannot
+// produce.
+func wsRawRequest(action, rawPayload string) *ws.Message {
+	return &ws.Message{
+		ID:      "req-42",
+		Type:    ws.MessageTypeRequest,
+		Action:  action,
+		Payload: json.RawMessage(rawPayload),
+	}
+}
+
+// wsOK asserts a success envelope that echoes the request's correlation ID and
+// action, then decodes the payload into out.
+func wsOK(t *testing.T, resp *ws.Message, err error, action string, out interface{}) {
+	t.Helper()
+	if err != nil {
+		t.Fatalf("handler returned a transport error: %v", err)
+	}
+	if resp == nil {
+		t.Fatal("handler returned a nil message")
+	}
+	if resp.Type != ws.MessageTypeResponse {
+		t.Fatalf("type = %q, want %q (payload: %s)", resp.Type, ws.MessageTypeResponse, resp.Payload)
+	}
+	if resp.ID != "req-42" {
+		t.Errorf("id = %q, want the request id echoed back", resp.ID)
+	}
+	if resp.Action != action {
+		t.Errorf("action = %q, want %q", resp.Action, action)
+	}
+	if out == nil {
+		return
+	}
+	if decodeErr := json.Unmarshal(resp.Payload, out); decodeErr != nil {
+		t.Fatalf("decode payload %s: %v", resp.Payload, decodeErr)
+	}
+}
+
+// wsErr asserts an error envelope with the expected code and reason and
+// returns the decoded error payload.
+func wsErr(t *testing.T, resp *ws.Message, err error, action, code, reason string) ws.ErrorPayload {
+	t.Helper()
+	if err != nil {
+		t.Fatalf("handler returned a transport error: %v", err)
+	}
+	if resp == nil {
+		t.Fatal("handler returned a nil message")
+	}
+	if resp.Type != ws.MessageTypeError {
+		t.Fatalf("type = %q, want %q (payload: %s)", resp.Type, ws.MessageTypeError, resp.Payload)
+	}
+	if resp.ID != "req-42" {
+		t.Errorf("id = %q, want the request id echoed back", resp.ID)
+	}
+	if resp.Action != action {
+		t.Errorf("action = %q, want %q", resp.Action, action)
+	}
+	var payload ws.ErrorPayload
+	if decodeErr := json.Unmarshal(resp.Payload, &payload); decodeErr != nil {
+		t.Fatalf("decode error payload %s: %v", resp.Payload, decodeErr)
+	}
+	if payload.Code != code {
+		t.Errorf("error code = %q, want %q", payload.Code, code)
+	}
+	if reason != "" && payload.Message != reason {
+		t.Errorf("error message = %q, want %q", payload.Message, reason)
+	}
+	return payload
+}
+
+// failingSecretProvider makes Service.GetStatus fail at its findTokenSecret
+// step, which is the only internal-error path wsStatus has.
+type failingSecretProvider struct{ err error }
+
+func (f *failingSecretProvider) List(context.Context) ([]*SecretListItem, error) {
+	return nil, f.err
+}
+
+func (f *failingSecretProvider) Reveal(context.Context, string) (string, error) {
+	return "", f.err
+}
+
+// --- Status ---
+
+func TestWSStatusReturnsServiceStatus(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	client.SetUser("alice")
+	svc, _, log := newWSFixture(t, client)
+
+	resp, err := wsStatus(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabStatus, nil))
+
+	var status Status
+	wsOK(t, resp, err, ws.ActionGitLabStatus, &status)
+	if !status.Authenticated {
+		t.Error("authenticated = false, want true for the mock client")
+	}
+	if status.Username != "alice" {
+		t.Errorf("username = %q, want alice", status.Username)
+	}
+	if status.AuthMethod != AuthMethodPAT {
+		t.Errorf("auth_method = %q, want %q", status.AuthMethod, AuthMethodPAT)
+	}
+	if status.Host != DefaultHost {
+		t.Errorf("host = %q, want %q", status.Host, DefaultHost)
+	}
+}
+
+func TestWSStatusMapsSecretLookupFailureToInternalError(t *testing.T) {
+	log := newTestLogger(t)
+	svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT,
+		&failingSecretProvider{err: errors.New("vault offline")}, log)
+
+	resp, err := wsStatus(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabStatus, nil))
+
+	payload := wsErr(t, resp, err, ws.ActionGitLabStatus, ws.ErrorCodeInternalError, "")
+	if payload.Message == "" {
+		t.Error("internal error message is empty; the cause must reach the client")
+	}
+}
+
+// --- Task MR listing ---
+
+func TestWSListTaskMRsByTaskReturnsOnlyThatTasksRows(t *testing.T) {
+	svc, store, log := newWSFixture(t, NewMockClient(DefaultHost))
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	seedTask(t, store, "task-a", "ws-1")
+	seedTask(t, store, "task-b", "ws-1")
+	if err := store.UpsertTaskMR(ctx, newTestMR("task-a", "repo-1", "group/a", 11)); err != nil {
+		t.Fatalf("seed task-a MR: %v", err)
+	}
+	if err := store.UpsertTaskMR(ctx, newTestMR("task-b", "repo-1", "group/b", 22)); err != nil {
+		t.Fatalf("seed task-b MR: %v", err)
+	}
+
+	resp, err := wsListTaskMRs(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabTaskMRsList, map[string]string{"task_id": "task-a"}))
+
+	var body struct {
+		TaskMRs []*TaskMR `json:"task_mrs"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabTaskMRsList, &body)
+	if len(body.TaskMRs) != 1 {
+		t.Fatalf("task_mrs = %d rows, want exactly task-a's single row", len(body.TaskMRs))
+	}
+	if body.TaskMRs[0].ProjectPath != "group/a" || body.TaskMRs[0].MRIID != 11 {
+		t.Errorf("row = %s!%d, want group/a!11", body.TaskMRs[0].ProjectPath, body.TaskMRs[0].MRIID)
+	}
+}
+
+func TestWSListTaskMRsByWorkspaceGroupsRowsByTask(t *testing.T) {
+	svc, store, log := newWSFixture(t, NewMockClient(DefaultHost))
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	seedTask(t, store, "task-a", "ws-1")
+	seedTask(t, store, "task-b", "ws-1")
+	for _, mr := range []*TaskMR{
+		newTestMR("task-a", "repo-1", "group/a", 11),
+		newTestMR("task-b", "repo-1", "group/b", 22),
+	} {
+		if err := store.UpsertTaskMR(ctx, mr); err != nil {
+			t.Fatalf("seed %s MR: %v", mr.TaskID, err)
+		}
+	}
+
+	resp, err := wsListTaskMRs(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabTaskMRsList, map[string]string{"workspace_id": "ws-1"}))
+
+	var body TaskMRsResponse
+	wsOK(t, resp, err, ws.ActionGitLabTaskMRsList, &body)
+	if len(body.TaskMRs) != 2 {
+		t.Fatalf("grouped tasks = %d, want 2 (%v)", len(body.TaskMRs), body.TaskMRs)
+	}
+	if got := body.TaskMRs["task-a"]; len(got) != 1 || got[0].MRIID != 11 {
+		t.Errorf("task-a group = %+v, want the single !11 row", got)
+	}
+}
+
+// TestWSListTaskMRsRejectsMalformedPayload pins the comment in handlers.go: a
+// payload that fails to parse must not fall through to the "list everything in
+// the workspace" branch, which would return more rows than the caller asked
+// for.
+func TestWSListTaskMRsRejectsMalformedPayload(t *testing.T) {
+	svc, store, log := newWSFixture(t, NewMockClient(DefaultHost))
+	seedWorkspace(t, store, "ws-1")
+
+	resp, err := wsListTaskMRs(svc, log)(context.Background(),
+		wsRawRequest(ws.ActionGitLabTaskMRsList, `{"task_id":12345}`))
+
+	wsErr(t, resp, err, ws.ActionGitLabTaskMRsList, ws.ErrorCodeBadRequest, errMsgInvalidPayload)
+}
+
+func TestWSListTaskMRsRequiresWorkspaceOrTask(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsListTaskMRs(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabTaskMRsList, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabTaskMRsList, ws.ErrorCodeBadRequest,
+		"workspace_id or task_id required")
+}
+
+func TestWSGetTaskMRRequiresTaskID(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+
+	for name, msg := range map[string]*ws.Message{
+		"absent":    wsRequest(t, ws.ActionGitLabTaskMRGet, map[string]string{}),
+		"malformed": wsRawRequest(ws.ActionGitLabTaskMRGet, `{"task_id":[]}`),
+	} {
+		t.Run(name, func(t *testing.T) {
+			resp, err := wsGetTaskMR(svc, log)(context.Background(), msg)
+			wsErr(t, resp, err, ws.ActionGitLabTaskMRGet, ws.ErrorCodeBadRequest, "task_id required")
+		})
+	}
+}
+
+func TestWSGetTaskMRReturnsRowsForTask(t *testing.T) {
+	svc, store, log := newWSFixture(t, NewMockClient(DefaultHost))
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	seedTask(t, store, "task-a", "ws-1")
+	if err := store.UpsertTaskMR(ctx, newTestMR("task-a", "repo-1", "group/a", 7)); err != nil {
+		t.Fatalf("seed MR: %v", err)
+	}
+
+	resp, err := wsGetTaskMR(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabTaskMRGet, map[string]string{"task_id": "task-a"}))
+
+	var body struct {
+		TaskMRs []*TaskMR `json:"task_mrs"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabTaskMRGet, &body)
+	if len(body.TaskMRs) != 1 || body.TaskMRs[0].MRIID != 7 {
+		t.Fatalf("task_mrs = %+v, want the single !7 row", body.TaskMRs)
+	}
+}
+
+// --- MR feedback / files / commits ---
+
+func TestWSGetMRFeedbackValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+	handler := wsGetMRFeedback(svc, log)
+
+	cases := map[string]*ws.Message{
+		"missing project": wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"iid": 3}),
+		"zero iid":        wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"project": "g/p", "iid": 0}),
+		"negative iid":    wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"project": "g/p", "iid": -1}),
+		"malformed":       wsRawRequest(ws.ActionGitLabMRFeedbackGet, `{"project":true}`),
+	}
+	for name, msg := range cases {
+		t.Run(name, func(t *testing.T) {
+			resp, err := handler(context.Background(), msg)
+			wsErr(t, resp, err, ws.ActionGitLabMRFeedbackGet, ws.ErrorCodeBadRequest, "project and iid required")
+		})
+	}
+}
+
+func TestWSGetMRFeedbackReturnsClientFeedback(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	client.SetUser("alice")
+	iid := client.SeedMR("group/p", &MR{Title: "feat: x", State: "opened"})
+	client.SeedApprovals("group/p", iid, []MRApproval{{Username: "bob"}}, 1)
+	svc, _, log := newWSFixture(t, client)
+
+	resp, err := wsGetMRFeedback(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRFeedbackGet, map[string]interface{}{"project": "group/p", "iid": iid}))
+
+	var feedback MRFeedback
+	wsOK(t, resp, err, ws.ActionGitLabMRFeedbackGet, &feedback)
+	if len(feedback.Approvals) != 1 || feedback.Approvals[0].Username != "bob" {
+		t.Errorf("approvals = %+v, want bob's single approval", feedback.Approvals)
+	}
+}
+
+func TestWSGetMRFilesReturnsSeededFiles(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	iid := client.SeedMR("group/p", &MR{Title: "feat: x"})
+	client.SeedFiles("group/p", iid, []MRFile{{Filename: "a.go", Additions: 3}})
+	svc, _, log := newWSFixture(t, client)
+
+	resp, err := wsGetMRFiles(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRFilesGet, map[string]interface{}{"project": "group/p", "iid": iid}))
+
+	var body struct {
+		Files []MRFile `json:"files"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabMRFilesGet, &body)
+	if len(body.Files) != 1 || body.Files[0].Filename != "a.go" {
+		t.Fatalf("files = %+v, want the single a.go entry", body.Files)
+	}
+}
+
+func TestWSGetMRFilesValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsGetMRFiles(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRFilesGet, map[string]interface{}{"project": "", "iid": 1}))
+
+	wsErr(t, resp, err, ws.ActionGitLabMRFilesGet, ws.ErrorCodeBadRequest, "project and iid required")
+}
+
+func TestWSGetMRCommitsReturnsSeededCommits(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	iid := client.SeedMR("group/p", &MR{Title: "feat: x"})
+	client.SeedCommits("group/p", iid, []MRCommitInfo{{SHA: "abc123", Message: "feat: x"}})
+	svc, _, log := newWSFixture(t, client)
+
+	resp, err := wsGetMRCommits(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRCommitsGet, map[string]interface{}{"project": "group/p", "iid": iid}))
+
+	var body struct {
+		Commits []MRCommitInfo `json:"commits"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabMRCommitsGet, &body)
+	if len(body.Commits) != 1 || body.Commits[0].SHA != "abc123" {
+		t.Fatalf("commits = %+v, want the single abc123 entry", body.Commits)
+	}
+}
+
+func TestWSGetMRCommitsValidatesProjectAndIID(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsGetMRCommits(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRCommitsGet, map[string]interface{}{"project": "g/p"}))
+
+	wsErr(t, resp, err, ws.ActionGitLabMRCommitsGet, ws.ErrorCodeBadRequest, "project and iid required")
+}
+
+// --- Sync ---
+
+func TestWSSyncTaskMRRequiresTaskID(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsSyncTaskMR(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabTaskMRSync, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabTaskMRSync, ws.ErrorCodeBadRequest, "task_id required")
+}
+
+func TestWSSyncTaskMRReportsMissingStoreAsInternalError(t *testing.T) {
+	log := newTestLogger(t)
+	svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, log)
+
+	resp, err := wsSyncTaskMR(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabTaskMRSync, map[string]string{"task_id": "task-a"}))
+
+	wsErr(t, resp, err, ws.ActionGitLabTaskMRSync, ws.ErrorCodeInternalError,
+		"gitlab store not configured")
+}
+
+func TestWSSyncTaskMRReturnsRefreshedRows(t *testing.T) {
+	client := NewMockClient(DefaultHost)
+	iid := client.SeedMR("group/p", &MR{Title: "feat: refreshed", State: "opened"})
+	svc, store, log := newWSFixture(t, client)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	seedTask(t, store, "task-a", "ws-1")
+	row := newTestMR("task-a", "repo-1", "group/p", iid)
+	row.MRTitle = "stale title"
+	if err := store.UpsertTaskMR(ctx, row); err != nil {
+		t.Fatalf("seed MR row: %v", err)
+	}
+
+	resp, err := wsSyncTaskMR(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabTaskMRSync, map[string]string{"task_id": "task-a"}))
+
+	var body struct {
+		TaskMRs []*TaskMR `json:"task_mrs"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabTaskMRSync, &body)
+	if len(body.TaskMRs) != 1 {
+		t.Fatalf("task_mrs = %d rows, want 1", len(body.TaskMRs))
+	}
+	if body.TaskMRs[0].MRTitle != "feat: refreshed" {
+		t.Errorf("title = %q, want the upstream title to have replaced the stale one",
+			body.TaskMRs[0].MRTitle)
+	}
+}
+
+// --- Stats ---
+
+// statsRecordingClient records the filters GetStats sends upstream so the test
+// can assert the three distinct queries, not just the aggregate numbers.
+type statsRecordingClient struct {
+	*MockClient
+	mrFilters []string
+}
+
+func (c *statsRecordingClient) SearchMRsPaged(ctx context.Context, filter, customQuery string, page, perPage int) (*MRSearchPage, error) {
+	c.mrFilters = append(c.mrFilters, filter)
+	return c.MockClient.SearchMRsPaged(ctx, filter, customQuery, page, perPage)
+}
+
+func TestWSGetStatsReturnsCountsAndQueriesEachScope(t *testing.T) {
+	client := &statsRecordingClient{MockClient: NewMockClient(DefaultHost)}
+	client.SetUser("alice")
+	client.SeedMR("group/p", &MR{Title: "one", State: "opened"})
+	client.SeedMR("group/q", &MR{Title: "two", State: "opened"})
+	client.SeedIssue("group/p", &Issue{IID: 1, Title: "bug", State: "opened"})
+	svc, _, log := newWSFixture(t, client)
+
+	resp, err := wsGetStats(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabStats, nil))
+
+	var stats Stats
+	wsOK(t, resp, err, ws.ActionGitLabStats, &stats)
+	if stats.OpenMRs != 2 {
+		t.Errorf("open_mrs = %d, want 2", stats.OpenMRs)
+	}
+	if stats.MRsAwaitingMyReview != 2 {
+		t.Errorf("mrs_awaiting_my_review = %d, want 2", stats.MRsAwaitingMyReview)
+	}
+	if stats.OpenIssuesAssignedMe != 1 {
+		t.Errorf("open_issues_assigned_me = %d, want 1", stats.OpenIssuesAssignedMe)
+	}
+	want := []string{"scope=created_by_me&state=opened", "reviewer_username=alice&state=opened"}
+	if len(client.mrFilters) != len(want) {
+		t.Fatalf("MR search filters = %q, want %q", client.mrFilters, want)
+	}
+	for i, filter := range want {
+		if client.mrFilters[i] != filter {
+			t.Errorf("MR search filter[%d] = %q, want %q", i, client.mrFilters[i], filter)
+		}
+	}
+}
+
+// TestWSGetStatsDegradesToZeroWithoutAuthenticatedUser pins the documented
+// behavior of Service.GetStats: an unresolvable user yields empty counts
+// rather than an error envelope, so the dashboard still renders.
+func TestWSGetStatsDegradesToZeroWithoutAuthenticatedUser(t *testing.T) {
+	svc, _, log := newWSFixture(t, NewMockClient(DefaultHost))
+
+	resp, err := wsGetStats(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabStats, nil))
+
+	var stats Stats
+	wsOK(t, resp, err, ws.ActionGitLabStats, &stats)
+	if stats.OpenMRs != 0 || stats.OpenIssuesAssignedMe != 0 {
+		t.Errorf("stats = %+v, want zeroed counts when no user resolves", stats)
+	}
+}

apps/backend/internal/gitlab/handlers_ws_watches_test.go

@@ -0,0 +1,563 @@
+package gitlab
+
+import (
+	"context"
+	"testing"
+
+	"github.com/kandev/kandev/internal/common/logger"
+	ws "github.com/kandev/kandev/pkg/websocket"
+)
+
+// newWatchWSFixture builds a service whose watch-dependency validators accept
+// the "ws-1" fixture identifiers, so the WS create/update handlers exercise the
+// real validation path rather than a stub that waves everything through.
+func newWatchWSFixture(t *testing.T) (*Service, *Store, *logger.Logger) {
+	t.Helper()
+	svc, store := newDependencyTestService(t)
+	seedWorkspace(t, store, "ws-1")
+	seedWorkspace(t, store, "ws-2")
+	return svc, store, newTestLogger(t)
+}
+
+// --- Review watches ---
+
+func TestWSListReviewWatchesScopesToWorkspaceWhenGiven(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	mine := validReviewWatchRow()
+	other := validReviewWatchRow()
+	other.WorkspaceID = "ws-2"
+	for _, w := range []*ReviewWatch{mine, other} {
+		if err := store.CreateReviewWatch(ctx, w); err != nil {
+			t.Fatalf("seed review watch: %v", err)
+		}
+	}
+
+	resp, err := wsListReviewWatches(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabReviewWatchesList, map[string]string{"workspace_id": "ws-1"}))
+
+	var body struct {
+		Watches []*ReviewWatch `json:"watches"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabReviewWatchesList, &body)
+	if len(body.Watches) != 1 {
+		t.Fatalf("watches = %d, want only the ws-1 watch", len(body.Watches))
+	}
+	if body.Watches[0].WorkspaceID != "ws-1" {
+		t.Errorf("watch workspace = %q, want ws-1", body.Watches[0].WorkspaceID)
+	}
+}
+
+// TestWSListReviewWatchesWithoutWorkspaceListsAll pins the deliberate fallback
+// in wsListReviewWatches: an absent workspace_id lists every workspace's
+// watches. If that ever needs to become a rejection, this test is the one to
+// change.
+func TestWSListReviewWatchesWithoutWorkspaceListsAll(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	other := validReviewWatchRow()
+	other.WorkspaceID = "ws-2"
+	for _, w := range []*ReviewWatch{validReviewWatchRow(), other} {
+		if err := store.CreateReviewWatch(ctx, w); err != nil {
+			t.Fatalf("seed review watch: %v", err)
+		}
+	}
+
+	resp, err := wsListReviewWatches(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabReviewWatchesList, map[string]string{}))
+
+	var body struct {
+		Watches []*ReviewWatch `json:"watches"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabReviewWatchesList, &body)
+	if len(body.Watches) != 2 {
+		t.Fatalf("watches = %d, want both workspaces' watches", len(body.Watches))
+	}
+}
+
+func TestWSListReviewWatchesRejectsMalformedPayload(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsListReviewWatches(svc, log)(context.Background(),
+		wsRawRequest(ws.ActionGitLabReviewWatchesList, `{"workspace_id":7}`))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewWatchesList, ws.ErrorCodeBadRequest, errMsgInvalidPayload)
+}
+
+func TestWSCreateReviewWatchPersistsAndReturnsRow(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+
+	resp, err := wsCreateReviewWatch(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabReviewWatchCreate, validReviewWatchRequest()))
+
+	var created ReviewWatch
+	wsOK(t, resp, err, ws.ActionGitLabReviewWatchCreate, &created)
+	if created.ID == "" {
+		t.Fatal("created watch has no id")
+	}
+	if created.WorkspaceID != "ws-1" || created.RepositoryID != "repo-valid" {
+		t.Errorf("created watch = %+v, want the requested workspace/repository", created)
+	}
+	stored, err := store.GetReviewWatch(ctx, created.ID)
+	if err != nil || stored == nil {
+		t.Fatalf("watch was not persisted: row=%v err=%v", stored, err)
+	}
+}
+
+func TestWSCreateReviewWatchRejectsInvalidDependencies(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+	req := validReviewWatchRequest()
+	req.RepositoryID = "repo-other" // belongs to ws-2
+
+	resp, err := wsCreateReviewWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabReviewWatchCreate, req))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewWatchCreate, ws.ErrorCodeInternalError, "")
+}
+
+func TestWSCreateReviewWatchRejectsMalformedPayload(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsCreateReviewWatch(svc, log)(context.Background(),
+		wsRawRequest(ws.ActionGitLabReviewWatchCreate, `{"workspace_id":[]}`))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewWatchCreate, ws.ErrorCodeBadRequest, errMsgInvalidPayload)
+}
+
+func TestWSUpdateReviewWatchAppliesPatch(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	resp, err := wsUpdateReviewWatch(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabReviewWatchUpdate, map[string]interface{}{
+			"id":      watch.ID,
+			"enabled": false,
+		}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabReviewWatchUpdate, &body)
+	if !body["updated"] {
+		t.Errorf("payload = %v, want {\"updated\":true}", body)
+	}
+	stored, err := store.GetReviewWatch(ctx, watch.ID)
+	if err != nil {
+		t.Fatalf("reload watch: %v", err)
+	}
+	if stored.Enabled {
+		t.Error("watch is still enabled; the patch did not reach the store")
+	}
+}
+
+func TestWSUpdateReviewWatchRequiresID(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsUpdateReviewWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabReviewWatchUpdate, map[string]interface{}{"enabled": false}))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewWatchUpdate, ws.ErrorCodeBadRequest, errMsgIDRequired)
+}
+
+func TestWSUpdateReviewWatchReportsUnknownIDAsInternalError(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsUpdateReviewWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabReviewWatchUpdate, map[string]interface{}{
+			"id": "does-not-exist", "enabled": false,
+		}))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewWatchUpdate, ws.ErrorCodeInternalError, "")
+}
+
+func TestWSDeleteReviewWatchRemovesRow(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("seed review watch: %v", err)
+	}
+
+	resp, err := wsDeleteReviewWatch(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabReviewWatchDelete, map[string]string{"id": watch.ID}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabReviewWatchDelete, &body)
+	if !body[respKeyDeleted] {
+		t.Errorf("payload = %v, want {\"deleted\":true}", body)
+	}
+	remaining, err := store.ListReviewWatches(ctx, "ws-1")
+	if err != nil {
+		t.Fatalf("list review watches: %v", err)
+	}
+	if len(remaining) != 0 {
+		t.Errorf("watches remaining = %d, want 0", len(remaining))
+	}
+}
+
+func TestWSDeleteReviewWatchRequiresID(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsDeleteReviewWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabReviewWatchDelete, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewWatchDelete, ws.ErrorCodeBadRequest, errMsgIDRequired)
+}
+
+func TestWSTriggerReviewWatchRequiresID(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsTriggerReviewWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabReviewTrigger, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewTrigger, ws.ErrorCodeBadRequest, errMsgIDRequired)
+}
+
+func TestWSTriggerReviewWatchReportsUnknownWatch(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsTriggerReviewWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabReviewTrigger, map[string]string{"id": "nope"}))
+
+	wsErr(t, resp, err, ws.ActionGitLabReviewTrigger, ws.ErrorCodeInternalError, "")
+}
+
+// TestWSTriggerAllReviewChecksCountsOnlyEnabledWatches seeds one enabled and
+// one disabled watch over the same MR. The count is the number of MRs found
+// across enabled watches, so a handler that ignored the enabled flag would
+// report 2 and one that always returned 0 would report 0 — only the correct
+// filter yields 1.
+func TestWSTriggerAllReviewChecksCountsOnlyEnabledWatches(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	client := NewMockClient(DefaultHost)
+	client.SetUser("kandev-tester")
+	client.SeedMR("group/p", &MR{IID: 1, Title: "needs review", WebURL: "https://x/1",
+		Reviewers: []MRReviewer{{Username: "kandev-tester"}}})
+	svc.workspaceClients["ws-1"] = client
+
+	disabled := validReviewWatchRow()
+	disabled.Enabled = false
+	for _, watch := range []*ReviewWatch{validReviewWatchRow(), disabled} {
+		if err := store.CreateReviewWatch(ctx, watch); err != nil {
+			t.Fatalf("seed review watch: %v", err)
+		}
+	}
+
+	resp, err := wsTriggerAllReviewChecks(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabReviewTriggerAll, nil))
+
+	var body map[string]int
+	wsOK(t, resp, err, ws.ActionGitLabReviewTriggerAll, &body)
+	if body["count"] != 1 {
+		t.Errorf("count = %d, want 1 — the enabled watch contributes its MR and "+
+			"the disabled one contributes nothing", body["count"])
+	}
+}
+
+// --- MR watches ---
+
+func TestWSListMRWatchesSelectsBranchBySessionThenTaskThenAll(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	seedTask(t, store, "task-a", "ws-1")
+	seedTask(t, store, "task-b", "ws-1")
+	if _, err := svc.CreateMRWatch(ctx, "sess-a", "task-a", "repo-1", "group/a", 1, "feat/a"); err != nil {
+		t.Fatalf("seed watch a: %v", err)
+	}
+	if _, err := svc.CreateMRWatch(ctx, "sess-b", "task-b", "repo-1", "group/b", 2, "feat/b"); err != nil {
+		t.Fatalf("seed watch b: %v", err)
+	}
+
+	tests := []struct {
+		name    string
+		payload map[string]string
+		want    []string
+	}{
+		{"by session", map[string]string{"session_id": "sess-a"}, []string{"group/a"}},
+		{"by task", map[string]string{"task_id": "task-b"}, []string{"group/b"}},
+		{"all active", map[string]string{}, []string{"group/a", "group/b"}},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			resp, err := wsListMRWatches(svc, log)(ctx,
+				wsRequest(t, ws.ActionGitLabMRWatchesList, test.payload))
+
+			var body struct {
+				Watches []*MRWatch `json:"watches"`
+			}
+			wsOK(t, resp, err, ws.ActionGitLabMRWatchesList, &body)
+			if len(body.Watches) != len(test.want) {
+				t.Fatalf("watches = %d, want %d (%v)", len(body.Watches), len(test.want), body.Watches)
+			}
+			got := map[string]bool{}
+			for _, w := range body.Watches {
+				got[w.ProjectPath] = true
+			}
+			for _, project := range test.want {
+				if !got[project] {
+					t.Errorf("watch for %q missing from %v", project, got)
+				}
+			}
+		})
+	}
+}
+
+func TestWSListMRWatchesRejectsMalformedPayload(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsListMRWatches(svc, log)(context.Background(),
+		wsRawRequest(ws.ActionGitLabMRWatchesList, `{"session_id":{}}`))
+
+	wsErr(t, resp, err, ws.ActionGitLabMRWatchesList, ws.ErrorCodeBadRequest, errMsgInvalidPayload)
+}
+
+func TestWSDeleteMRWatchRemovesRow(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	seedTask(t, store, "task-a", "ws-1")
+	watch, err := svc.CreateMRWatch(ctx, "sess-a", "task-a", "repo-1", "group/a", 1, "feat/a")
+	if err != nil {
+		t.Fatalf("seed watch: %v", err)
+	}
+
+	resp, err := wsDeleteMRWatch(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabMRWatchDelete, map[string]string{"id": watch.ID}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabMRWatchDelete, &body)
+	if !body[respKeyDeleted] {
+		t.Errorf("payload = %v, want {\"deleted\":true}", body)
+	}
+	remaining, err := svc.ListActiveMRWatches(ctx)
+	if err != nil {
+		t.Fatalf("list active watches: %v", err)
+	}
+	if len(remaining) != 0 {
+		t.Errorf("watches remaining = %d, want 0", len(remaining))
+	}
+}
+
+func TestWSDeleteMRWatchRequiresID(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsDeleteMRWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabMRWatchDelete, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabMRWatchDelete, ws.ErrorCodeBadRequest, errMsgIDRequired)
+}
+
+// --- Issue watches ---
+
+func TestWSListIssueWatchesScopesToWorkspaceWhenGiven(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	other := validIssueWatchRow()
+	other.WorkspaceID = "ws-2"
+	for _, w := range []*IssueWatch{validIssueWatchRow(), other} {
+		if err := store.CreateIssueWatch(ctx, w); err != nil {
+			t.Fatalf("seed issue watch: %v", err)
+		}
+	}
+
+	resp, err := wsListIssueWatches(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabIssueWatchesList, map[string]string{"workspace_id": "ws-2"}))
+
+	var body struct {
+		Watches []*IssueWatch `json:"watches"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabIssueWatchesList, &body)
+	if len(body.Watches) != 1 || body.Watches[0].WorkspaceID != "ws-2" {
+		t.Fatalf("watches = %+v, want only the ws-2 watch", body.Watches)
+	}
+}
+
+func TestWSListIssueWatchesWithoutWorkspaceListsAll(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	other := validIssueWatchRow()
+	other.WorkspaceID = "ws-2"
+	for _, w := range []*IssueWatch{validIssueWatchRow(), other} {
+		if err := store.CreateIssueWatch(ctx, w); err != nil {
+			t.Fatalf("seed issue watch: %v", err)
+		}
+	}
+
+	resp, err := wsListIssueWatches(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabIssueWatchesList, map[string]string{}))
+
+	var body struct {
+		Watches []*IssueWatch `json:"watches"`
+	}
+	wsOK(t, resp, err, ws.ActionGitLabIssueWatchesList, &body)
+	if len(body.Watches) != 2 {
+		t.Fatalf("watches = %d, want both workspaces' watches", len(body.Watches))
+	}
+}
+
+func TestWSListIssueWatchesRejectsMalformedPayload(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsListIssueWatches(svc, log)(context.Background(),
+		wsRawRequest(ws.ActionGitLabIssueWatchesList, `{"workspace_id":false}`))
+
+	wsErr(t, resp, err, ws.ActionGitLabIssueWatchesList, ws.ErrorCodeBadRequest, errMsgInvalidPayload)
+}
+
+func TestWSCreateIssueWatchPersistsAndReturnsRow(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+
+	resp, err := wsCreateIssueWatch(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabIssueWatchCreate, validIssueWatchRequest()))
+
+	var created IssueWatch
+	wsOK(t, resp, err, ws.ActionGitLabIssueWatchCreate, &created)
+	if created.ID == "" {
+		t.Fatal("created watch has no id")
+	}
+	stored, err := store.GetIssueWatch(ctx, created.ID)
+	if err != nil || stored == nil {
+		t.Fatalf("watch was not persisted: row=%v err=%v", stored, err)
+	}
+}
+
+func TestWSCreateIssueWatchRejectsInvalidDependencies(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+	req := validIssueWatchRequest()
+	req.AgentProfileID = "agent-missing"
+
+	resp, err := wsCreateIssueWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabIssueWatchCreate, req))
+
+	wsErr(t, resp, err, ws.ActionGitLabIssueWatchCreate, ws.ErrorCodeInternalError, "")
+}
+
+func TestWSCreateIssueWatchRejectsMalformedPayload(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsCreateIssueWatch(svc, log)(context.Background(),
+		wsRawRequest(ws.ActionGitLabIssueWatchCreate, `"a string, not an object"`))
+
+	wsErr(t, resp, err, ws.ActionGitLabIssueWatchCreate, ws.ErrorCodeBadRequest, errMsgInvalidPayload)
+}
+
+func TestWSUpdateIssueWatchAppliesPatch(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(ctx, watch); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+
+	resp, err := wsUpdateIssueWatch(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabIssueWatchUpdate, map[string]interface{}{
+			"id": watch.ID, "enabled": false,
+		}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabIssueWatchUpdate, &body)
+	if !body["updated"] {
+		t.Errorf("payload = %v, want {\"updated\":true}", body)
+	}
+	stored, err := store.GetIssueWatch(ctx, watch.ID)
+	if err != nil {
+		t.Fatalf("reload watch: %v", err)
+	}
+	if stored.Enabled {
+		t.Error("watch is still enabled; the patch did not reach the store")
+	}
+}
+
+func TestWSUpdateIssueWatchRequiresID(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsUpdateIssueWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabIssueWatchUpdate, map[string]interface{}{"enabled": true}))
+
+	wsErr(t, resp, err, ws.ActionGitLabIssueWatchUpdate, ws.ErrorCodeBadRequest, errMsgIDRequired)
+}
+
+func TestWSDeleteIssueWatchRemovesRow(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(ctx, watch); err != nil {
+		t.Fatalf("seed issue watch: %v", err)
+	}
+
+	resp, err := wsDeleteIssueWatch(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabIssueWatchDelete, map[string]string{"id": watch.ID}))
+
+	var body map[string]bool
+	wsOK(t, resp, err, ws.ActionGitLabIssueWatchDelete, &body)
+	if !body[respKeyDeleted] {
+		t.Errorf("payload = %v, want {\"deleted\":true}", body)
+	}
+	remaining, err := store.ListIssueWatches(ctx, "ws-1")
+	if err != nil {
+		t.Fatalf("list issue watches: %v", err)
+	}
+	if len(remaining) != 0 {
+		t.Errorf("watches remaining = %d, want 0", len(remaining))
+	}
+}
+
+func TestWSDeleteIssueWatchRequiresID(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsDeleteIssueWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabIssueWatchDelete, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabIssueWatchDelete, ws.ErrorCodeBadRequest, errMsgIDRequired)
+}
+
+func TestWSTriggerIssueWatchRequiresID(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsTriggerIssueWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabIssueTrigger, map[string]string{}))
+
+	wsErr(t, resp, err, ws.ActionGitLabIssueTrigger, ws.ErrorCodeBadRequest, errMsgIDRequired)
+}
+
+func TestWSTriggerIssueWatchReportsUnknownWatch(t *testing.T) {
+	svc, _, log := newWatchWSFixture(t)
+
+	resp, err := wsTriggerIssueWatch(svc, log)(context.Background(),
+		wsRequest(t, ws.ActionGitLabIssueTrigger, map[string]string{"id": "nope"}))
+
+	wsErr(t, resp, err, ws.ActionGitLabIssueTrigger, ws.ErrorCodeInternalError, "")
+}
+
+// TestWSTriggerAllIssueChecksCountsOnlyEnabledWatches is the issue mirror of
+// the review test above, and discriminates the same three outcomes.
+func TestWSTriggerAllIssueChecksCountsOnlyEnabledWatches(t *testing.T) {
+	svc, store, log := newWatchWSFixture(t)
+	ctx := context.Background()
+	client := NewMockClient(DefaultHost)
+	client.SetUser("kandev-tester")
+	client.SeedIssue("group/p", &Issue{IID: 3, Title: "bug", WebURL: "https://x/3"})
+	svc.workspaceClients["ws-1"] = client
+
+	disabled := validIssueWatchRow()
+	disabled.Enabled = false
+	for _, watch := range []*IssueWatch{validIssueWatchRow(), disabled} {
+		if err := store.CreateIssueWatch(ctx, watch); err != nil {
+			t.Fatalf("seed issue watch: %v", err)
+		}
+	}
+
+	resp, err := wsTriggerAllIssueChecks(svc, log)(ctx,
+		wsRequest(t, ws.ActionGitLabIssueTriggerAll, nil))
+
+	var body map[string]int
+	wsOK(t, resp, err, ws.ActionGitLabIssueTriggerAll, &body)
+	if body["count"] != 1 {
+		t.Errorf("count = %d, want 1 — the enabled watch contributes its issue and "+
+			"the disabled one contributes nothing", body["count"])
+	}
+}

apps/backend/internal/gitlab/noop_client_contract_test.go

@@ -0,0 +1,139 @@
+package gitlab
+
+import (
+	"context"
+	"errors"
+	"reflect"
+	"testing"
+)
+
+// noopClientNonErroringMethods are the only NoopClient methods that must NOT
+// report ErrNoClient. They answer questions that have a meaningful answer even
+// with no GitLab configured, and callers branch on those answers rather than on
+// an error.
+//
+// GetProtectedBranch is the subtle one: (nil, nil) is not "no data", it is the
+// affirmative answer "this branch is not protected". PATClient returns exactly
+// that for a 404 (see pat_client_actions.go), so NoopClient reporting
+// ErrNoClient here would make the unconfigured path *more* restrictive than a
+// real GitLab that simply has no protection rule.
+var noopClientNonErroringMethods = map[string]struct{}{
+	"Host":                 {},
+	"IsAuthenticated":      {},
+	"GetAuthenticatedUser": {},
+	"GetProtectedBranch":   {},
+}
+
+// TestNoopClientDegradesUniformly is the contract for the unconfigured-GitLab
+// path: every data method must return ErrNoClient rather than a nil error with
+// a nil result, because callers that see (nil, nil) dereference it. Driving the
+// method set by reflection rather than a hand-written list means a method added
+// to the Client interface cannot quietly skip this guarantee, and it also
+// proves no method panics on zero-valued arguments.
+func TestNoopClientDegradesUniformly(t *testing.T) {
+	client := NewNoopClient("")
+	clientType := reflect.TypeOf(client)
+	ctxValue := reflect.ValueOf(context.Background())
+
+	exercised := 0
+	for i := range clientType.NumMethod() {
+		method := clientType.Method(i)
+		if !method.IsExported() {
+			continue
+		}
+		t.Run(method.Name, func(t *testing.T) {
+			results := method.Func.Call(noopCallArgs(reflect.ValueOf(client), method.Type, ctxValue))
+			err := trailingError(results)
+
+			if _, exempt := noopClientNonErroringMethods[method.Name]; exempt {
+				if err != nil {
+					t.Fatalf("%s returned %v, want no error", method.Name, err)
+				}
+				return
+			}
+			if !errors.Is(err, ErrNoClient) {
+				t.Fatalf("%s returned err = %v, want ErrNoClient", method.Name, err)
+			}
+			// A method that reports ErrNoClient must not also hand back a
+			// half-built value the caller might use.
+			for j := range len(results) - 1 {
+				if result := results[j]; result.IsValid() && !result.IsZero() {
+					t.Errorf("%s returned a non-zero value %v alongside ErrNoClient",
+						method.Name, result.Interface())
+				}
+			}
+		})
+		exercised++
+	}
+	if exercised == 0 {
+		t.Fatal("no exported methods were exercised; the reflection walk is broken")
+	}
+}
+
+// noopCallArgs builds the receiver plus zero-valued arguments for one method,
+// substituting the real context for the first context.Context parameter.
+func noopCallArgs(receiver reflect.Value, methodType reflect.Type, ctxValue reflect.Value) []reflect.Value {
+	ctxType := reflect.TypeOf((*context.Context)(nil)).Elem()
+	args := make([]reflect.Value, 0, methodType.NumIn())
+	args = append(args, receiver)
+	for i := 1; i < methodType.NumIn(); i++ {
+		paramType := methodType.In(i)
+		if paramType == ctxType {
+			args = append(args, ctxValue)
+			continue
+		}
+		args = append(args, reflect.Zero(paramType))
+	}
+	return args
+}
+
+// trailingError extracts the error from a call's results, or nil when the
+// method does not return one.
+func trailingError(results []reflect.Value) error {
+	if len(results) == 0 {
+		return nil
+	}
+	last := results[len(results)-1]
+	if last.Type() != reflect.TypeOf((*error)(nil)).Elem() {
+		return nil
+	}
+	if last.IsNil() {
+		return nil
+	}
+	return last.Interface().(error)
+}
+
+// TestNoopClientAuthAnswersAreExplicitlyNegative pins the values behind the
+// three exemptions above: "not authenticated, no username" is the answer the
+// status endpoint renders, and it must be a definite negative rather than an
+// error the caller has to interpret.
+func TestNoopClientAuthAnswersAreExplicitlyNegative(t *testing.T) {
+	client := NewNoopClient("https://gitlab.acme.test")
+	ctx := context.Background()
+
+	authenticated, err := client.IsAuthenticated(ctx)
+	if err != nil || authenticated {
+		t.Errorf("IsAuthenticated = (%v, %v), want (false, nil)", authenticated, err)
+	}
+	username, err := client.GetAuthenticatedUser(ctx)
+	if err != nil || username != "" {
+		t.Errorf("GetAuthenticatedUser = (%q, %v), want (\"\", nil)", username, err)
+	}
+	if host := client.Host(); host != "https://gitlab.acme.test" {
+		t.Errorf("Host = %q, want the configured host reported even while unauthenticated", host)
+	}
+}
+
+// TestNoopClientReportsBranchesAsUnprotected pins the exemption above: the
+// unconfigured client must give the same "not protected" answer a real GitLab
+// gives for a branch with no protection rule, not an error.
+func TestNoopClientReportsBranchesAsUnprotected(t *testing.T) {
+	branch, err := NewNoopClient("").GetProtectedBranch(context.Background(), "group/p", "main")
+
+	if err != nil {
+		t.Fatalf("GetProtectedBranch err = %v, want nil", err)
+	}
+	if branch != nil {
+		t.Errorf("protected branch = %+v, want nil (not protected)", branch)
+	}
+}

apps/backend/internal/gitlab/store_watches_scope_test.go

@@ -0,0 +1,449 @@
+package gitlab
+
+import (
+	"context"
+	"errors"
+	"testing"
+)
+
+// newMRWatchScopeStore seeds two workspaces, one task in each, and one MR
+// watch per task, returning the store plus the two watch IDs. The
+// *ForWorkspace lookups reach the watch through its owning task, so this is
+// the minimum fixture that can tell a scoped lookup from an unscoped one.
+func newMRWatchScopeStore(t *testing.T) (*Store, *MRWatch, *MRWatch) {
+	t.Helper()
+	store := newTestStore(t)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	seedWorkspace(t, store, "ws-2")
+	seedTask(t, store, "task-1", "ws-1")
+	seedTask(t, store, "task-2", "ws-2")
+	mine := &MRWatch{SessionID: "sess-1", TaskID: "task-1", RepositoryID: "repo-1", ProjectPath: "group/a", MRIID: 1}
+	foreign := &MRWatch{SessionID: "sess-2", TaskID: "task-2", RepositoryID: "repo-2", ProjectPath: "group/b", MRIID: 2}
+	for _, watch := range []*MRWatch{mine, foreign} {
+		if err := store.CreateMRWatch(ctx, watch); err != nil {
+			t.Fatalf("seed MR watch: %v", err)
+		}
+	}
+	return store, mine, foreign
+}
+
+func TestStoreListMRWatchesBySessionForWorkspaceExcludesForeignSessions(t *testing.T) {
+	store, mine, foreign := newMRWatchScopeStore(t)
+	ctx := context.Background()
+
+	own, err := store.ListMRWatchesBySessionForWorkspace(ctx, "ws-1", mine.SessionID)
+	if err != nil {
+		t.Fatalf("list own session watches: %v", err)
+	}
+	if len(own) != 1 || own[0].ID != mine.ID {
+		t.Fatalf("watches = %+v, want only the ws-1 watch", own)
+	}
+
+	// The same session id queried from the other workspace must return
+	// nothing: the scope comes from the owning task's workspace, not from
+	// the caller-supplied session id.
+	crossed, err := store.ListMRWatchesBySessionForWorkspace(ctx, "ws-2", mine.SessionID)
+	if err != nil {
+		t.Fatalf("list cross-workspace session watches: %v", err)
+	}
+	if len(crossed) != 0 {
+		t.Errorf("cross-workspace lookup returned %+v, want nothing", crossed)
+	}
+
+	foreignScoped, err := store.ListMRWatchesBySessionForWorkspace(ctx, "ws-2", foreign.SessionID)
+	if err != nil {
+		t.Fatalf("list ws-2 session watches: %v", err)
+	}
+	if len(foreignScoped) != 1 || foreignScoped[0].ID != foreign.ID {
+		t.Errorf("ws-2 watches = %+v, want its own watch", foreignScoped)
+	}
+}
+
+func TestStoreListMRWatchesByTaskForWorkspaceExcludesForeignTasks(t *testing.T) {
+	store, mine, _ := newMRWatchScopeStore(t)
+	ctx := context.Background()
+
+	own, err := store.ListMRWatchesByTaskForWorkspace(ctx, "ws-1", "task-1")
+	if err != nil {
+		t.Fatalf("list own task watches: %v", err)
+	}
+	if len(own) != 1 || own[0].ID != mine.ID {
+		t.Fatalf("watches = %+v, want only the ws-1 watch", own)
+	}
+
+	crossed, err := store.ListMRWatchesByTaskForWorkspace(ctx, "ws-1", "task-2")
+	if err != nil {
+		t.Fatalf("list cross-workspace task watches: %v", err)
+	}
+	if len(crossed) != 0 {
+		t.Errorf("cross-workspace lookup returned %+v, want nothing", crossed)
+	}
+}
+
+// TestStoreListMRWatchesForWorkspaceHidesOrphanedRows pins the documented
+// behavior: a watch whose task row is gone has no resolvable workspace, so the
+// workspace-facing lookups must not surface it even though the unscoped
+// listing still does.
+func TestStoreListMRWatchesForWorkspaceHidesOrphanedRows(t *testing.T) {
+	store, mine, _ := newMRWatchScopeStore(t)
+	ctx := context.Background()
+	if _, err := store.db.Exec(`DELETE FROM tasks WHERE id = ?`, "task-1"); err != nil {
+		t.Fatalf("delete owning task: %v", err)
+	}
+
+	scoped, err := store.ListMRWatchesByTaskForWorkspace(ctx, "ws-1", "task-1")
+	if err != nil {
+		t.Fatalf("list orphaned task watches: %v", err)
+	}
+	if len(scoped) != 0 {
+		t.Errorf("workspace lookup surfaced the orphaned watch %+v", scoped)
+	}
+
+	unscoped, err := store.ListMRWatchesByTask(ctx, "task-1")
+	if err != nil {
+		t.Fatalf("list unscoped task watches: %v", err)
+	}
+	if len(unscoped) != 1 || unscoped[0].ID != mine.ID {
+		t.Errorf("unscoped listing = %+v, want the orphaned row still present", unscoped)
+	}
+}
+
+func TestStoreDeleteMRWatchesByTaskIDRemovesOnlyThatTasksWatches(t *testing.T) {
+	store, _, foreign := newMRWatchScopeStore(t)
+	ctx := context.Background()
+
+	deleted, err := store.DeleteMRWatchesByTaskID(ctx, "task-1")
+	if err != nil {
+		t.Fatalf("delete MR watches by task: %v", err)
+	}
+	if deleted != 1 {
+		t.Errorf("rows affected = %d, want 1", deleted)
+	}
+	remaining, err := store.ListActiveMRWatches(ctx)
+	if err != nil {
+		t.Fatalf("list active watches: %v", err)
+	}
+	if len(remaining) != 1 || remaining[0].ID != foreign.ID {
+		t.Fatalf("remaining watches = %+v, want only task-2's watch", remaining)
+	}
+}
+
+func TestStoreDeleteMRWatchesByTaskIDIsZeroForUnknownTask(t *testing.T) {
+	store, _, _ := newMRWatchScopeStore(t)
+
+	deleted, err := store.DeleteMRWatchesByTaskID(context.Background(), "no-such-task")
+	if err != nil {
+		t.Fatalf("delete MR watches by task: %v", err)
+	}
+	if deleted != 0 {
+		t.Errorf("rows affected = %d, want 0", deleted)
+	}
+}
+
+// --- Service-level session lookups ---
+
+func TestServiceGetMRWatchBySessionAndRepoDiscriminatesRepositories(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	seedTask(t, store, "task-1", "ws-1")
+	svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, newTestLogger(t))
+	svc.SetStore(store)
+	for _, repo := range []struct {
+		id      string
+		project string
+		iid     int
+	}{{"repo-a", "group/a", 1}, {"repo-b", "group/b", 2}} {
+		if _, err := svc.CreateMRWatch(ctx, "sess-1", "task-1", repo.id, repo.project, repo.iid, "feat/x"); err != nil {
+			t.Fatalf("seed watch for %s: %v", repo.id, err)
+		}
+	}
+
+	watch, err := svc.GetMRWatchBySessionAndRepo(ctx, "sess-1", "repo-b")
+	if err != nil {
+		t.Fatalf("get watch by session and repo: %v", err)
+	}
+	if watch == nil || watch.ProjectPath != "group/b" {
+		t.Fatalf("watch = %+v, want repo-b's group/b watch", watch)
+	}
+
+	missing, err := svc.GetMRWatchBySessionAndRepo(ctx, "sess-1", "repo-missing")
+	if err != nil {
+		t.Fatalf("get watch for unknown repo: %v", err)
+	}
+	if missing != nil {
+		t.Errorf("watch = %+v, want nil for an unknown repository", missing)
+	}
+}
+
+func TestServiceMRWatchLookupsRequireAStore(t *testing.T) {
+	svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, newTestLogger(t))
+	ctx := context.Background()
+
+	if _, err := svc.GetMRWatchBySession(ctx, "sess-1"); !errors.Is(err, errStoreUnavailable) {
+		t.Errorf("GetMRWatchBySession error = %v, want errStoreUnavailable", err)
+	}
+	if _, err := svc.GetMRWatchBySessionAndRepo(ctx, "sess-1", "repo-a"); !errors.Is(err, errStoreUnavailable) {
+		t.Errorf("GetMRWatchBySessionAndRepo error = %v, want errStoreUnavailable", err)
+	}
+}
+
+// TestServiceGetMRWatchBySessionOnlyMatchesTheLegacyRepositorylessRow pins the
+// documented meaning of GetMRWatchBySession: it is the legacy single-repo
+// accessor, keyed on an empty repository_id. A repo-keyed watch on the same
+// session must not answer it, otherwise a multi-repo session would silently
+// resolve to an arbitrary one of its repositories.
+func TestServiceGetMRWatchBySessionOnlyMatchesTheLegacyRepositorylessRow(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	seedTask(t, store, "task-1", "ws-1")
+	svc := NewService(DefaultHost, NewMockClient(DefaultHost), AuthMethodPAT, nil, newTestLogger(t))
+	svc.SetStore(store)
+	if _, err := svc.CreateMRWatch(ctx, "sess-1", "task-1", "repo-a", "group/a", 1, "feat/x"); err != nil {
+		t.Fatalf("seed repo-keyed watch: %v", err)
+	}
+
+	watch, err := svc.GetMRWatchBySession(ctx, "sess-1")
+	if err != nil {
+		t.Fatalf("get watch by session: %v", err)
+	}
+	if watch != nil {
+		t.Fatalf("watch = %+v, want nil — a repo-keyed watch is not the legacy row", watch)
+	}
+
+	legacy, err := svc.CreateMRWatch(ctx, "sess-1", "task-1", "", "group/legacy", 2, "feat/y")
+	if err != nil {
+		t.Fatalf("seed legacy watch: %v", err)
+	}
+	watch, err = svc.GetMRWatchBySession(ctx, "sess-1")
+	if err != nil {
+		t.Fatalf("get watch by session: %v", err)
+	}
+	if watch == nil || watch.ID != legacy.ID {
+		t.Fatalf("watch = %+v, want the legacy repository-less watch %s", watch, legacy.ID)
+	}
+}
+
+// --- Reservation release ---
+
+func TestStoreReleaseReviewMRTaskAllowsARetryOnTheSameMR(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("create review watch: %v", err)
+	}
+
+	reserved, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("first reserve: %v", err)
+	}
+	if !reserved {
+		t.Fatal("first reservation was refused")
+	}
+	again, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("second reserve: %v", err)
+	}
+	if again {
+		t.Fatal("the same MR was reserved twice without a release")
+	}
+
+	if err := store.ReleaseReviewMRTask(ctx, watch.ID, "group/p", 1); err != nil {
+		t.Fatalf("release reservation: %v", err)
+	}
+
+	retried, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("reserve after release: %v", err)
+	}
+	if !retried {
+		t.Error("the MR could not be reserved again after its reservation was released")
+	}
+}
+
+// TestStoreReleaseReviewMRTaskForGenerationIgnoresStaleGenerations guards the
+// ownership check: a release carrying a generation the reservation no longer
+// belongs to must leave the row alone, so a slow poller from before a watch
+// reset cannot free a reservation the current generation owns.
+func TestStoreReleaseReviewMRTaskForGenerationIgnoresStaleGenerations(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	watch := validReviewWatchRow()
+	if err := store.CreateReviewWatch(ctx, watch); err != nil {
+		t.Fatalf("create review watch: %v", err)
+	}
+	if _, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url"); err != nil {
+		t.Fatalf("reserve: %v", err)
+	}
+
+	if err := store.ReleaseReviewMRTaskForGeneration(ctx, watch.ID, 999, "group/p", 1); err != nil {
+		t.Fatalf("stale-generation release: %v", err)
+	}
+	stillHeld, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("reserve after stale release: %v", err)
+	}
+	if stillHeld {
+		t.Fatal("a stale-generation release freed the reservation")
+	}
+
+	generation, err := store.reviewWatchGeneration(ctx, watch.ID)
+	if err != nil {
+		t.Fatalf("load watch generation: %v", err)
+	}
+	if err := store.ReleaseReviewMRTaskForGeneration(ctx, watch.ID, generation, "group/p", 1); err != nil {
+		t.Fatalf("current-generation release: %v", err)
+	}
+	freed, err := store.ReserveReviewMRTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("reserve after current release: %v", err)
+	}
+	if !freed {
+		t.Error("a current-generation release did not free the reservation")
+	}
+}
+
+func TestStoreReleaseIssueWatchTaskAllowsARetryOnTheSameIssue(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(ctx, watch); err != nil {
+		t.Fatalf("create issue watch: %v", err)
+	}
+	if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil {
+		t.Fatalf("reserve: %v", err)
+	}
+
+	if err := store.ReleaseIssueWatchTask(ctx, watch.ID, "group/p", 1); err != nil {
+		t.Fatalf("release reservation: %v", err)
+	}
+
+	retried, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("reserve after release: %v", err)
+	}
+	if !retried {
+		t.Error("the issue could not be reserved again after its reservation was released")
+	}
+}
+
+func TestStoreReleaseIssueWatchTaskForGenerationIgnoresStaleGenerations(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	seedWorkspace(t, store, "ws-1")
+	watch := validIssueWatchRow()
+	if err := store.CreateIssueWatch(ctx, watch); err != nil {
+		t.Fatalf("create issue watch: %v", err)
+	}
+	if _, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url"); err != nil {
+		t.Fatalf("reserve: %v", err)
+	}
+
+	if err := store.ReleaseIssueWatchTaskForGeneration(ctx, watch.ID, 999, "group/p", 1); err != nil {
+		t.Fatalf("stale-generation release: %v", err)
+	}
+	stillHeld, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("reserve after stale release: %v", err)
+	}
+	if stillHeld {
+		t.Fatal("a stale-generation release freed the reservation")
+	}
+
+	generation, err := store.issueWatchGeneration(ctx, watch.ID)
+	if err != nil {
+		t.Fatalf("load watch generation: %v", err)
+	}
+	if err := store.ReleaseIssueWatchTaskForGeneration(ctx, watch.ID, generation, "group/p", 1); err != nil {
+		t.Fatalf("current-generation release: %v", err)
+	}
+	freed, err := store.ReserveIssueWatchTask(ctx, watch.ID, "group/p", 1, "url")
+	if err != nil {
+		t.Fatalf("reserve after current release: %v", err)
+	}
+	if !freed {
+		t.Error("a current-generation release did not free the reservation")
+	}
+}
+
+// TestStoreIssueWatchGenerationDefaultsForUnknownWatch covers the sql.ErrNoRows
+// branch: a missing watch reports generation 1 rather than an error, so a
+// release racing a watch deletion degrades instead of failing the poll.
+func TestStoreIssueWatchGenerationDefaultsForUnknownWatch(t *testing.T) {
+	store := newTestStore(t)
+
+	generation, err := store.issueWatchGeneration(context.Background(), "no-such-watch")
+	if err != nil {
+		t.Fatalf("issue watch generation: %v", err)
+	}
+	if generation != 1 {
+		t.Errorf("generation = %d, want 1 for an unknown watch", generation)
+	}
+}
+
+// --- Repository provider identity ---
+
+// TestValidateTaskMRRepositoryIdentityFailsClosedOnEmptyProviderHost encodes
+// the invariant from apps/backend/CLAUDE.md: a provider-backed repository is
+// keyed by workspace, provider, normalized provider_host origin, owner and
+// name. A legacy row that carries a gitlab provider and a matching
+// owner/name but no provider_host has unknown identity, so it must fail
+// closed rather than being inferred from owner/name alone.
+func TestValidateTaskMRRepositoryIdentityFailsClosedOnEmptyProviderHost(t *testing.T) {
+	const host = "https://gitlab.self-managed.test"
+	_, store, _ := newTaskMRLinkService(t, host)
+	seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
+	// Provider and owner/name match the MR exactly; only the host is blank.
+	if _, err := store.db.Exec(`UPDATE repositories
+		SET provider = 'gitlab', provider_host = '', provider_owner = 'acme', provider_name = 'api'
+		WHERE id = ?`, "repo-1"); err != nil {
+		t.Fatalf("seed hostless repository identity: %v", err)
+	}
+
+	err := store.ValidateTaskMRRepositoryIdentity(
+		context.Background(), "ws-1", "task-1", "repo-1", host, "acme/api")
+
+	if !errors.Is(err, ErrTaskMRRepositoryMismatch) {
+		t.Fatalf("error = %v, want ErrTaskMRRepositoryMismatch — an owner/name match "+
+			"without a provider_host must not establish identity", err)
+	}
+}
+
+// TestValidateTaskMRRepositoryIdentityRejectsSameProjectOnAnotherHost is the
+// other half of the same invariant: identical owner/name on a different
+// GitLab installation is a different repository.
+func TestValidateTaskMRRepositoryIdentityRejectsSameProjectOnAnotherHost(t *testing.T) {
+	const host = "https://gitlab.self-managed.test"
+	_, store, _ := newTaskMRLinkService(t, host)
+	seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
+	setTaskMRRepositoryIdentity(t, store, "repo-1", "https://gitlab.other.test", "acme/api")
+
+	err := store.ValidateTaskMRRepositoryIdentity(
+		context.Background(), "ws-1", "task-1", "repo-1", host, "acme/api")
+
+	if !errors.Is(err, ErrTaskMRRepositoryMismatch) {
+		t.Fatalf("error = %v, want ErrTaskMRRepositoryMismatch for the same project "+
+			"on a different host", err)
+	}
+}
+
+// TestValidateTaskMRRepositoryIdentityAcceptsMatchingHostAndProject is the
+// positive control for the two rejections above: with the host present and
+// matching, the same owner/name is accepted.
+func TestValidateTaskMRRepositoryIdentityAcceptsMatchingHostAndProject(t *testing.T) {
+	const host = "https://gitlab.self-managed.test"
+	_, store, _ := newTaskMRLinkService(t, host)
+	seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
+	setTaskMRRepositoryIdentity(t, store, "repo-1", host, "acme/api")
+
+	if err := store.ValidateTaskMRRepositoryIdentity(
+		context.Background(), "ws-1", "task-1", "repo-1", host, "acme/api"); err != nil {
+		t.Fatalf("identity check rejected an exact host+project match: %v", err)
+	}
+}