test(handlers): cover ListGlobal

- ListGlobal: 0% → 87.5% (success with rows, empty result, query error)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

workspace-server/internal/handlers/a2a_queue_status_test.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"database/sql"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -111,3 +112,133 @@ func TestExtractExpiresInSeconds(t *testing.T) {
 		})
 	}
 }
+
+// ──────────────────────────────────────────────────────────────────────────────
+// QueueStatusByID
+// ──────────────────────────────────────────────────────────────────────────────
+
+func TestQueueStatusByID_Success(t *testing.T) {
+	mock := setupTestDB(t)
+	queueID := "queue-789"
+
+	mock.ExpectQuery(`(?s:SELECT.*FROM a2a_queue.*WHERE q\.id = \$1)`).
+		WithArgs(queueID).
+		WillReturnRows(sqlmock.NewRows([]string{
+			"id", "workspace_id", "status", "priority", "attempts",
+			"last_error", "enqueued_at", "dispatched_at", "completed_at", "expires_at",
+			"response_body",
+		}).AddRow(
+			queueID, "ws-789", "completed", 50, 1,
+			"some error", "2026-05-28T10:00:00Z", "2026-05-28T10:01:00Z", "2026-05-28T10:02:00Z", "2026-05-28T11:00:00Z",
+			`{"result":"ok"}`,
+		))
+
+	qs, err := QueueStatusByID(context.Background(), queueID)
+	if err != nil {
+		t.Fatalf("QueueStatusByID error: %v", err)
+	}
+	if qs.ID != queueID {
+		t.Errorf("ID = %q, want %q", qs.ID, queueID)
+	}
+	if qs.Status != "completed" {
+		t.Errorf("Status = %q, want completed", qs.Status)
+	}
+	if qs.LastError == nil || *qs.LastError != "some error" {
+		t.Errorf("LastError = %v, want 'some error'", qs.LastError)
+	}
+	if qs.DispatchedAt == nil || *qs.DispatchedAt != "2026-05-28T10:01:00Z" {
+		t.Errorf("DispatchedAt = %v", qs.DispatchedAt)
+	}
+	if qs.CompletedAt == nil || *qs.CompletedAt != "2026-05-28T10:02:00Z" {
+		t.Errorf("CompletedAt = %v", qs.CompletedAt)
+	}
+	if qs.ExpiresAt == nil || *qs.ExpiresAt != "2026-05-28T11:00:00Z" {
+		t.Errorf("ExpiresAt = %v", qs.ExpiresAt)
+	}
+	if string(qs.ResponseBody) != `{"result":"ok"}` {
+		t.Errorf("ResponseBody = %q", qs.ResponseBody)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestQueueStatusByID_NullOptionalFields(t *testing.T) {
+	mock := setupTestDB(t)
+	queueID := "queue-null"
+
+	mock.ExpectQuery(`(?s:SELECT.*FROM a2a_queue.*WHERE q\.id = \$1)`).
+		WithArgs(queueID).
+		WillReturnRows(sqlmock.NewRows([]string{
+			"id", "workspace_id", "status", "priority", "attempts",
+			"last_error", "enqueued_at", "dispatched_at", "completed_at", "expires_at",
+			"response_body",
+		}).AddRow(
+			queueID, "ws-null", "queued", 50, 0,
+			nil, "2026-05-28T10:00:00Z", nil, nil, nil,
+			nil,
+		))
+
+	qs, err := QueueStatusByID(context.Background(), queueID)
+	if err != nil {
+		t.Fatalf("QueueStatusByID error: %v", err)
+	}
+	if qs.LastError != nil {
+		t.Errorf("LastError = %v, want nil", qs.LastError)
+	}
+	if qs.DispatchedAt != nil {
+		t.Errorf("DispatchedAt = %v, want nil", qs.DispatchedAt)
+	}
+	if qs.CompletedAt != nil {
+		t.Errorf("CompletedAt = %v, want nil", qs.CompletedAt)
+	}
+	if qs.ExpiresAt != nil {
+		t.Errorf("ExpiresAt = %v, want nil", qs.ExpiresAt)
+	}
+	if qs.ResponseBody != nil {
+		t.Errorf("ResponseBody = %v, want nil", qs.ResponseBody)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestQueueStatusByID_NotFound(t *testing.T) {
+	mock := setupTestDB(t)
+	queueID := "queue-missing"
+
+	mock.ExpectQuery(`(?s:SELECT.*FROM a2a_queue.*WHERE q\.id = \$1)`).
+		WithArgs(queueID).
+		WillReturnError(sql.ErrNoRows)
+
+	qs, err := QueueStatusByID(context.Background(), queueID)
+	if err != sql.ErrNoRows {
+		t.Fatalf("want sql.ErrNoRows, got err=%v qs=%v", err, qs)
+	}
+	if qs != nil {
+		t.Errorf("want nil status on not-found, got %v", qs)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestQueueStatusByID_QueryError(t *testing.T) {
+	mock := setupTestDB(t)
+	queueID := "queue-err"
+
+	mock.ExpectQuery(`(?s:SELECT.*FROM a2a_queue.*WHERE q\.id = \$1)`).
+		WithArgs(queueID).
+		WillReturnError(sql.ErrConnDone)
+
+	qs, err := QueueStatusByID(context.Background(), queueID)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if qs != nil {
+		t.Errorf("want nil status on error, got %v", qs)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}

workspace-server/internal/handlers/a2a_queue_test.go

@@ -18,8 +18,9 @@ import (
 	"testing"
 	"time"
 
-	"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db"
 	"github.com/DATA-DOG/go-sqlmock"
+	"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db"
+	"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/textutil"
 	"github.com/alicebob/miniredis/v2"
 )
 
@@ -209,12 +210,10 @@ func drainSetup(t *testing.T, workspaceID string) (sqlmock.Sqlmock, *WorkspaceHa
 // Named distinctly from handlers_test.go's expectBudgetCheck (which uses MatchPsql
 // escaped-regex and cannot be reused with QueryMatcherEqual tests).
 func expectQueueBudgetCheck(mock sqlmock.Sqlmock, workspaceID string) {
-	// Multi-period (#49): exact-match the budget_limits read; "{}" → no limits →
-	// checkWorkspaceBudget returns early (no spend query).
 	mock.ExpectQuery(
-		"SELECT COALESCE(budget_limits, '{}'::jsonb) FROM workspaces WHERE id = $1",
+		"SELECT budget_limit, COALESCE(monthly_spend, 0) FROM workspaces WHERE id = $1",
 	).WithArgs(workspaceID).
-		WillReturnRows(sqlmock.NewRows([]string{"budget_limits"}).AddRow([]byte("{}")))
+		WillReturnRows(sqlmock.NewRows([]string{"budget_limit", "monthly_spend"}))
 }
 
 // seedRedisURL puts the agent server URL into the Redis cache so resolveAgentURL
@@ -522,3 +521,117 @@ func TestDrainQueueForWorkspace_ClaimGuarding_SecondDrainGetsEmpty(t *testing.T)
 		t.Errorf("unmet sqlmock expectations: %v", err)
 	}
 }
+
+// ──────────────────────────────────────────────────────────────────────────────
+// QueueDepth
+// ──────────────────────────────────────────────────────────────────────────────
+
+func TestQueueDepth(t *testing.T) {
+	mock := setupTestDB(t)
+	wsID := "ws-depth-001"
+
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM a2a_queue WHERE workspace_id = \$1 AND status = 'queued'`).
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(3))
+
+	got := QueueDepth(context.Background(), wsID)
+	if got != 3 {
+		t.Errorf("QueueDepth = %d, want 3", got)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestQueueDepth_QueryError(t *testing.T) {
+	mock := setupTestDB(t)
+	wsID := "ws-depth-err"
+
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM a2a_queue WHERE workspace_id = \$1 AND status = 'queued'`).
+		WithArgs(wsID).
+		WillReturnError(sql.ErrConnDone)
+
+	got := QueueDepth(context.Background(), wsID)
+	if got != 0 {
+		t.Errorf("QueueDepth on error = %d, want 0", got)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+// ──────────────────────────────────────────────────────────────────────────────
+// stitchDrainResponseToDelegation
+// ──────────────────────────────────────────────────────────────────────────────
+
+func TestStitchDrainResponseToDelegation_Success(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := &WorkspaceHandler{}
+
+	sourceID := "ws-source-001"
+	targetID := "ws-target-002"
+	delegationID := "dlg-123"
+	respBody := []byte(`{"result":{"parts":[{"kind":"text","text":"hello from peer"}]}}`)
+
+	responseText := "hello from peer"
+	summary := "Delegation completed (" + textutil.TruncateBytes(responseText, 80) + ")"
+	respJSON := `{"delegation_id":"dlg-123","text":"hello from peer"}`
+
+	mock.ExpectExec(`UPDATE activity_logs SET status = 'completed', summary = \$1, response_body = \$2::jsonb WHERE workspace_id = \$3 AND method = 'delegate_result' AND target_id = \$4 AND response_body->>'delegation_id' = \$5`).
+		WithArgs(summary, respJSON, sourceID, targetID, delegationID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+
+	handler.stitchDrainResponseToDelegation(context.Background(), sourceID, targetID, delegationID, respBody)
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestStitchDrainResponseToDelegation_EmptySourceID(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := &WorkspaceHandler{}
+
+	// No DB expectations — should return early
+	handler.stitchDrainResponseToDelegation(context.Background(), "", "ws-target", "dlg-123", []byte(`{}`))
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestStitchDrainResponseToDelegation_EmptyDelegationID(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := &WorkspaceHandler{}
+
+	// No DB expectations — should return early
+	handler.stitchDrainResponseToDelegation(context.Background(), "ws-source", "ws-target", "", []byte(`{}`))
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestStitchDrainResponseToDelegation_ZeroRowsAffected(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := &WorkspaceHandler{}
+
+	sourceID := "ws-source-001"
+	targetID := "ws-target-002"
+	delegationID := "dlg-123"
+	respBody := []byte(`{"result":{"parts":[{"kind":"text","text":"hello from peer"}]}}`)
+
+	responseText := "hello from peer"
+	summary := "Delegation completed (" + textutil.TruncateBytes(responseText, 80) + ")"
+	respJSON := `{"delegation_id":"dlg-123","text":"hello from peer"}`
+
+	mock.ExpectExec(`UPDATE activity_logs SET status = 'completed', summary = \$1, response_body = \$2::jsonb WHERE workspace_id = \$3 AND method = 'delegate_result' AND target_id = \$4 AND response_body->>'delegation_id' = \$5`).
+		WithArgs(summary, respJSON, sourceID, targetID, delegationID).
+		WillReturnResult(sqlmock.NewResult(0, 0))
+
+	handler.stitchDrainResponseToDelegation(context.Background(), sourceID, targetID, delegationID, respBody)
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}

workspace-server/internal/handlers/admin_workspace_images_test.go

@@ -6,6 +6,28 @@ import (
 	"testing"
 )
 
+// TestTemplateImageRef_DefaultRegistry verifies the default image ref shape
+// when MOLECULE_IMAGE_REGISTRY is not set.
+func TestTemplateImageRef_DefaultRegistry(t *testing.T) {
+	t.Setenv("MOLECULE_IMAGE_REGISTRY", "")
+	got := TemplateImageRef("claude-code")
+	want := "ghcr.io/molecule-ai/workspace-template-claude-code:latest"
+	if got != want {
+		t.Errorf("TemplateImageRef(claude-code) = %q, want %q", got, want)
+	}
+}
+
+// TestTemplateImageRef_CustomRegistry verifies the ref respects
+// MOLECULE_IMAGE_REGISTRY when set (RFC #229 mirror support).
+func TestTemplateImageRef_CustomRegistry(t *testing.T) {
+	t.Setenv("MOLECULE_IMAGE_REGISTRY", "004947743811.dkr.ecr.us-east-2.amazonaws.com/molecule-ai")
+	got := TemplateImageRef("codex")
+	want := "004947743811.dkr.ecr.us-east-2.amazonaws.com/molecule-ai/workspace-template-codex:latest"
+	if got != want {
+		t.Errorf("TemplateImageRef(codex) = %q, want %q", got, want)
+	}
+}
+
 func TestGHCRAuthHeader_NoEnvReturnsEmpty(t *testing.T) {
 	t.Setenv("GHCR_USER", "")
 	t.Setenv("GHCR_TOKEN", "")

workspace-server/internal/handlers/artifacts_test.go

@@ -971,6 +971,37 @@ func TestArtifactsFork_InvalidRepoName(t *testing.T) {
 	}
 }
 
+// TestNewArtifactsHandler_MissingEnv verifies that when CF_ARTIFACTS_API_TOKEN
+// or CF_ARTIFACTS_NAMESPACE is missing, the handler returns a zero-value struct
+// with nil client so every endpoint returns 503 via configured().
+func TestNewArtifactsHandler_MissingEnv(t *testing.T) {
+	t.Setenv("CF_ARTIFACTS_API_TOKEN", "")
+	t.Setenv("CF_ARTIFACTS_NAMESPACE", "")
+
+	h := NewArtifactsHandler()
+	if h.client != nil {
+		t.Error("expected nil client when env vars are missing")
+	}
+	if h.namespace != "" {
+		t.Errorf("expected empty namespace, got %q", h.namespace)
+	}
+}
+
+// TestNewArtifactsHandler_WithEnv verifies that when both env vars are present
+// the handler builds a non-nil client and preserves the namespace.
+func TestNewArtifactsHandler_WithEnv(t *testing.T) {
+	t.Setenv("CF_ARTIFACTS_API_TOKEN", "test-token")
+	t.Setenv("CF_ARTIFACTS_NAMESPACE", "test-ns")
+
+	h := NewArtifactsHandler()
+	if h.client == nil {
+		t.Fatal("expected non-nil client when env vars are set")
+	}
+	if h.namespace != "test-ns" {
+		t.Errorf("namespace = %q, want test-ns", h.namespace)
+	}
+}
+
 // containsCredentials is a test helper that checks whether a URL has embedded
 // user:password@ credentials (should never appear in a stored remote URL).
 func containsCredentials(u string) bool {

workspace-server/internal/handlers/channels_test.go

@@ -674,6 +674,74 @@ func TestChannelHandler_Discover_329_RequiresWorkspaceID(t *testing.T) {
 	}
 }
 
+// ==================== matchesChatID ====================
+
+func TestMatchesChatID(t *testing.T) {
+	tests := []struct {
+		name   string
+		config map[string]interface{}
+		chatID string
+		want   bool
+	}{
+		{
+			name:   "exact single match",
+			config: map[string]interface{}{"chat_id": "-100"},
+			chatID: "-100",
+			want:   true,
+		},
+		{
+			name:   "match in comma-separated list",
+			config: map[string]interface{}{"chat_id": "-100,-200,-300"},
+			chatID: "-200",
+			want:   true,
+		},
+		{
+			name:   "match with whitespace",
+			config: map[string]interface{}{"chat_id": " -100 , -200 , -300 "},
+			chatID: "-200",
+			want:   true,
+		},
+		{
+			name:   "no match",
+			config: map[string]interface{}{"chat_id": "-100,-300"},
+			chatID: "-200",
+			want:   false,
+		},
+		{
+			name:   "empty config chat_id",
+			config: map[string]interface{}{"chat_id": ""},
+			chatID: "-100",
+			want:   false,
+		},
+		{
+			name:   "missing chat_id key",
+			config: map[string]interface{}{"other": "value"},
+			chatID: "-100",
+			want:   false,
+		},
+		{
+			name:   "wrong type for chat_id",
+			config: map[string]interface{}{"chat_id": 123},
+			chatID: "123",
+			want:   false,
+		},
+		{
+			name:   "substring must not match",
+			config: map[string]interface{}{"chat_id": "-1001,-1002"},
+			chatID: "-100",
+			want:   false,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := matchesChatID(tc.config, tc.chatID)
+			if got != tc.want {
+				t.Errorf("matchesChatID(%v, %q) = %v, want %v", tc.config, tc.chatID, got, tc.want)
+			}
+		})
+	}
+}
+
 // ==================== System Caller Prefix ====================
 
 func TestSystemCallerPrefix_ChannelIncluded(t *testing.T) {

workspace-server/internal/handlers/plugins_install.go

@@ -171,11 +171,9 @@ func (h *PluginsHandler) uninstallViaDocker(ctx context.Context, c *gin.Context,
 			log.Printf("Plugin uninstall: skipping invalid skill name %q in %s: %v", skill, pluginName, err)
 			continue
 		}
-		if _, rmErr := h.execAsRoot(ctx, containerName, []string{
+		_, _ = h.execAsRoot(ctx, containerName, []string{
 			"rm", "-rf", "/configs/skills/" + skill,
-		}); rmErr != nil {
-			log.Printf("Plugin uninstall: failed to remove skill %s from %s: %v", skill, workspaceID, rmErr)
-		}
+		})
 	}
 
 	// 3. Delete the plugin directory itself (as root to handle file ownership).

workspace-server/internal/handlers/plugins_install_pipeline.go

@@ -417,9 +417,7 @@ func (h *PluginsHandler) stripPluginMarkersFromMemory(ctx context.Context, conta
 		`awk 'BEGIN{skip=0; blanks=0} /^%s/{skip=1; blanks=0; next} skip==1 && /^[[:space:]]*$/{blanks++; if(blanks>=2){skip=0; print; next} next} /^# Plugin: /{if(skip==1)skip=0} skip==1{next} {print}' /configs/CLAUDE.md > /tmp/claude.new && mv /tmp/claude.new /configs/CLAUDE.md`,
 		regexpEscapeForAwk(marker),
 	)
-	if _, awkErr := h.execAsRoot(ctx, containerName, []string{"bash", "-c", script}); awkErr != nil {
-		log.Printf("Plugin uninstall: failed to strip markers from CLAUDE.md for %s in %s: %v", pluginName, workspaceID, awkErr)
-	}
+	_, _ = h.execAsRoot(ctx, containerName, []string{"bash", "-c", script})
 }
 
 // regexpEscapeForAwk escapes characters that have special meaning inside an

workspace-server/internal/handlers/secrets_test.go

@@ -1074,3 +1074,87 @@ func TestDeleteGlobal_AutoRestartsAffectedWorkspaces(t *testing.T) {
 		t.Errorf("unmet sqlmock expectations: %v", err)
 	}
 }
+
+// ==================== ListGlobal ====================
+
+func TestListGlobal_Success(t *testing.T) {
+	mock := setupTestDB(t)
+
+	mock.ExpectQuery(`SELECT key, created_at, updated_at FROM global_secrets ORDER BY key`).
+		WillReturnRows(sqlmock.NewRows([]string{"key", "created_at", "updated_at"}).
+			AddRow("CF_ARTIFACTS_API_TOKEN", "2026-01-01T00:00:00Z", "2026-05-01T00:00:00Z").
+			AddRow("GHCR_TOKEN", "2026-02-01T00:00:00Z", "2026-05-02T00:00:00Z"))
+
+	h := NewSecretsHandler(nil)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request, _ = http.NewRequest("GET", "/admin/secrets", nil)
+
+	h.ListGlobal(c)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp []map[string]interface{}
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("invalid JSON: %v", err)
+	}
+	if len(resp) != 2 {
+		t.Errorf("want 2 secrets, got %d", len(resp))
+	}
+	if resp[0]["key"] != "CF_ARTIFACTS_API_TOKEN" {
+		t.Errorf("first key = %v", resp[0]["key"])
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestListGlobal_Empty(t *testing.T) {
+	mock := setupTestDB(t)
+
+	mock.ExpectQuery(`SELECT key, created_at, updated_at FROM global_secrets ORDER BY key`).
+		WillReturnRows(sqlmock.NewRows([]string{"key", "created_at", "updated_at"}))
+
+	h := NewSecretsHandler(nil)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request, _ = http.NewRequest("GET", "/admin/secrets", nil)
+
+	h.ListGlobal(c)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("want 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp []map[string]interface{}
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("invalid JSON: %v", err)
+	}
+	if len(resp) != 0 {
+		t.Errorf("want 0 secrets, got %d", len(resp))
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}
+
+func TestListGlobal_QueryError(t *testing.T) {
+	mock := setupTestDB(t)
+
+	mock.ExpectQuery(`SELECT key, created_at, updated_at FROM global_secrets ORDER BY key`).
+		WillReturnError(sql.ErrConnDone)
+
+	h := NewSecretsHandler(nil)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request, _ = http.NewRequest("GET", "/admin/secrets", nil)
+
+	h.ListGlobal(c)
+
+	if w.Code != http.StatusInternalServerError {
+		t.Errorf("want 500, got %d", w.Code)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
+	}
+}

workspace-server/internal/handlers/workspace_abilities_test.go

@@ -1,11 +1,8 @@
 package handlers
 
-// Sqlmock-backed coverage for workspace_abilities.go (PatchAbilities).
-// Closes #1312 — handler was at 0% coverage.
-
 import (
 	"bytes"
-	"errors"
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -14,187 +11,144 @@ import (
 	"github.com/gin-gonic/gin"
 )
 
-func patchAbilitiesReq(t *testing.T, wsID string, body string) *httptest.ResponseRecorder {
-	t.Helper()
+func TestPatchAbilities_InvalidID(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
-	c.Params = gin.Params{{Key: "id", Value: wsID}}
-	c.Request = httptest.NewRequest("PATCH", "/workspaces/"+wsID+"/abilities", bytes.NewBufferString(body))
-	c.Request.Header.Set("Content-Type", "application/json")
+	c.Request, _ = http.NewRequest("PATCH", "/workspaces/bad-id/abilities", bytes.NewReader([]byte(`{"broadcast_enabled":true}`)))
+	c.Params = gin.Params{{Key: "id", Value: "bad-id"}}
+
 	PatchAbilities(c)
-	return w
-}
 
-// ---------- Validation errors ----------
-
-func TestPatchAbilities_InvalidWorkspaceID(t *testing.T) {
-	w := patchAbilitiesReq(t, "not-a-uuid", `{"broadcast_enabled":true}`)
 	if w.Code != http.StatusBadRequest {
-		t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
+		t.Errorf("want 400, got %d", w.Code)
 	}
 }
 
-func TestPatchAbilities_InvalidJSON(t *testing.T) {
-	w := patchAbilitiesReq(t, wsUUID1, `not json`)
+func TestPatchAbilities_InvalidBody(t *testing.T) {
+	mock := setupTestDB(t)
+	_ = mock
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request, _ = http.NewRequest("PATCH", "/workspaces/00000000-0000-0000-0000-000000000001/abilities", bytes.NewReader([]byte(`not json`)))
+	c.Request.Header.Set("Content-Type", "application/json")
+	c.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000001"}}
+
+	PatchAbilities(c)
+
 	if w.Code != http.StatusBadRequest {
-		t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
+		t.Errorf("want 400, got %d", w.Code)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
 	}
 }
 
-func TestPatchAbilities_EmptyBody(t *testing.T) {
-	w := patchAbilitiesReq(t, wsUUID1, `{}`)
+func TestPatchAbilities_NoFields(t *testing.T) {
+	mock := setupTestDB(t)
+	_ = mock
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	body, _ := json.Marshal(AbilitiesPayload{})
+	c.Request, _ = http.NewRequest("PATCH", "/workspaces/00000000-0000-0000-0000-000000000001/abilities", bytes.NewReader(body))
+	c.Request.Header.Set("Content-Type", "application/json")
+	c.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000001"}}
+
+	PatchAbilities(c)
+
 	if w.Code != http.StatusBadRequest {
-		t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
+		t.Errorf("want 400, got %d", w.Code)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet expectations: %v", err)
 	}
 }
 
-// ---------- Not found ----------
-
 func TestPatchAbilities_WorkspaceNotFound(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
+	mock := setupTestDB(t)
+	wsID := "00000000-0000-0000-0000-000000000001"
 
 	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
+		WithArgs(wsID).
 		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
 
-	w := patchAbilitiesReq(t, wsUUID1, `{"broadcast_enabled":true}`)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	body, _ := json.Marshal(map[string]bool{"broadcast_enabled": true})
+	c.Request, _ = http.NewRequest("PATCH", "/workspaces/"+wsID+"/abilities", bytes.NewReader(body))
+	c.Request.Header.Set("Content-Type", "application/json")
+	c.Params = gin.Params{{Key: "id", Value: wsID}}
+
+	PatchAbilities(c)
+
 	if w.Code != http.StatusNotFound {
-		t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String())
+		t.Errorf("want 404, got %d", w.Code)
 	}
 	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet: %v", err)
+		t.Fatalf("unmet expectations: %v", err)
 	}
 }
 
-func TestPatchAbilities_ExistsQueryError(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
+func TestPatchAbilities_UpdateBroadcast(t *testing.T) {
+	mock := setupTestDB(t)
+	wsID := "00000000-0000-0000-0000-000000000001"
 
 	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
-		WillReturnError(errors.New("conn refused"))
-
-	w := patchAbilitiesReq(t, wsUUID1, `{"broadcast_enabled":true}`)
-	if w.Code != http.StatusNotFound {
-		t.Fatalf("expected 404 on exists query error, got %d: %s", w.Code, w.Body.String())
-	}
-}
-
-// ---------- Happy paths ----------
-
-func TestPatchAbilities_BroadcastOnly(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
-
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
+		WithArgs(wsID).
 		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
+
 	mock.ExpectExec(`UPDATE workspaces SET broadcast_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
-		WithArgs(wsUUID1, true).
+		WithArgs(wsID, true).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 
-	w := patchAbilitiesReq(t, wsUUID1, `{"broadcast_enabled":true}`)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	val := true
+	body, _ := json.Marshal(AbilitiesPayload{BroadcastEnabled: &val})
+	c.Request, _ = http.NewRequest("PATCH", "/workspaces/"+wsID+"/abilities", bytes.NewReader(body))
+	c.Request.Header.Set("Content-Type", "application/json")
+	c.Params = gin.Params{{Key: "id", Value: wsID}}
+
+	PatchAbilities(c)
+
 	if w.Code != http.StatusOK {
-		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+		t.Errorf("want 200, got %d", w.Code)
 	}
 	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet: %v", err)
+		t.Fatalf("unmet expectations: %v", err)
 	}
 }
 
-func TestPatchAbilities_TalkToUserOnly(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
+func TestPatchAbilities_UpdateBoth(t *testing.T) {
+	mock := setupTestDB(t)
+	wsID := "00000000-0000-0000-0000-000000000001"
 
 	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
+		WithArgs(wsID).
 		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
-	mock.ExpectExec(`UPDATE workspaces SET talk_to_user_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
-		WithArgs(wsUUID1, false).
+
+	mock.ExpectExec(`UPDATE workspaces SET broadcast_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
+		WithArgs(wsID, false).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 
-	w := patchAbilitiesReq(t, wsUUID1, `{"talk_to_user_enabled":false}`)
+	mock.ExpectExec(`UPDATE workspaces SET talk_to_user_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
+		WithArgs(wsID, true).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	broadcast := false
+	talk := true
+	body, _ := json.Marshal(AbilitiesPayload{BroadcastEnabled: &broadcast, TalkToUserEnabled: &talk})
+	c.Request, _ = http.NewRequest("PATCH", "/workspaces/"+wsID+"/abilities", bytes.NewReader(body))
+	c.Request.Header.Set("Content-Type", "application/json")
+	c.Params = gin.Params{{Key: "id", Value: wsID}}
+
+	PatchAbilities(c)
+
 	if w.Code != http.StatusOK {
-		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+		t.Errorf("want 200, got %d", w.Code)
 	}
 	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet: %v", err)
-	}
-}
-
-func TestPatchAbilities_BothFields(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
-
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
-	mock.ExpectExec(`UPDATE workspaces SET broadcast_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
-		WithArgs(wsUUID1, true).
-		WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`UPDATE workspaces SET talk_to_user_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
-		WithArgs(wsUUID1, true).
-		WillReturnResult(sqlmock.NewResult(0, 1))
-
-	w := patchAbilitiesReq(t, wsUUID1, `{"broadcast_enabled":true,"talk_to_user_enabled":true}`)
-	if w.Code != http.StatusOK {
-		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
-	}
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet: %v", err)
-	}
-}
-
-// ---------- DB errors on update ----------
-
-func TestPatchAbilities_BroadcastUpdateError(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
-
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
-	mock.ExpectExec(`UPDATE workspaces SET broadcast_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
-		WithArgs(wsUUID1, true).
-		WillReturnError(errors.New("disk full"))
-
-	w := patchAbilitiesReq(t, wsUUID1, `{"broadcast_enabled":true}`)
-	if w.Code != http.StatusInternalServerError {
-		t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String())
-	}
-}
-
-func TestPatchAbilities_TalkToUserUpdateError(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
-
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
-	mock.ExpectExec(`UPDATE workspaces SET talk_to_user_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
-		WithArgs(wsUUID1, false).
-		WillReturnError(errors.New("disk full"))
-
-	w := patchAbilitiesReq(t, wsUUID1, `{"talk_to_user_enabled":false}`)
-	if w.Code != http.StatusInternalServerError {
-		t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String())
-	}
-}
-
-func TestPatchAbilities_BothFields_BroadcastFails(t *testing.T) {
-	mock, cleanup := withMockDB(t)
-	defer cleanup()
-
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1 AND status != 'removed'\)`).
-		WithArgs(wsUUID1).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
-	mock.ExpectExec(`UPDATE workspaces SET broadcast_enabled = \$2, updated_at = now\(\) WHERE id = \$1`).
-		WithArgs(wsUUID1, true).
-		WillReturnError(errors.New("disk full"))
-
-	w := patchAbilitiesReq(t, wsUUID1, `{"broadcast_enabled":true,"talk_to_user_enabled":true}`)
-	if w.Code != http.StatusInternalServerError {
-		t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String())
+		t.Fatalf("unmet expectations: %v", err)
 	}
 }