Merge pull request 'fix(handlers): recover all test blockers on staging (#904)' (#916) from fix/904-handler-test-blockers into staging

workspace-server/internal/handlers/a2a_queue.go

@@ -57,16 +57,18 @@ func extractIdempotencyKey(body []byte) string {
 func extractExpiresInSeconds(body []byte) int {
 	var envelope struct {
 		Params struct {
-			ExpiresInSeconds int `json:"expires_in_seconds"`
+			ExpiresInSeconds float64 `json:"expires_in_seconds"`
 		} `json:"params"`
 	}
 	if err := json.Unmarshal(body, &envelope); err != nil {
 		return 0
 	}
-	if envelope.Params.ExpiresInSeconds < 0 {
+	// JSON numbers are floats; truncate to int (Go's int(x) truncates toward zero).
+	secs := int(envelope.Params.ExpiresInSeconds)
+	if secs < 0 {
 		return 0
 	}
-	return envelope.Params.ExpiresInSeconds
+	return secs
 }
 
 const (

workspace-server/internal/handlers/a2a_queue_test.go

@@ -117,7 +117,7 @@ func TestExtractExpiresInSeconds_invalidOrMissing(t *testing.T) {
 		{"empty body", ``, 0},
 		{"null value", `{"params":{"expires_in_seconds":null}}`, 0},
 		{"string value", `{"params":{"expires_in_seconds":"30"}}`, 0},
-		{"float value", `{"params":{"expires_in_seconds":30.5}}`, 0},
+		{"float value", `{"params":{"expires_in_seconds":30.5}}`, 30},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {

workspace-server/internal/handlers/delegation.go

@@ -597,10 +597,100 @@ func (h *DelegationHandler) UpdateStatus(c *gin.Context) {
 
 // ListDelegations handles GET /workspaces/:id/delegations
 // Returns recent delegations for a workspace with their status.
+//
+// RFC #2829 PR-1/4 fallback chain: prefer the durable delegations table
+// (new as of #318) for complete status coverage; fall back to
+// activity_logs for pre-migration data or if the ledger table has
+// no rows for this workspace. activity_logs still drives in-flight
+// tracking for workspaces where DELEGATION_LEDGER_WRITE=0 was
+// active during the delegation lifecycle — the union covers both paths.
 func (h *DelegationHandler) ListDelegations(c *gin.Context) {
 	workspaceID := c.Param("id")
 	ctx := c.Request.Context()
 
+	var delegations []map[string]interface{}
+
+	// Attempt durable ledger first (RFC #2829)
+	delegations = h.listDelegationsFromLedger(ctx, workspaceID)
+	if len(delegations) > 0 {
+		c.JSON(http.StatusOK, delegations)
+		return
+	}
+
+	// Fall back to activity_logs (pre-#318 path, or ledger had no rows)
+	delegations = h.listDelegationsFromActivityLogs(ctx, workspaceID)
+	c.JSON(http.StatusOK, delegations)
+}
+
+// listDelegationsFromLedger queries the durable delegations table.
+// Returns nil on error so the caller can fall back to activity_logs.
+func (h *DelegationHandler) listDelegationsFromLedger(ctx context.Context, workspaceID string) []map[string]interface{} {
+	rows, err := db.DB.QueryContext(ctx, `
+		SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview,
+		       d.status, d.result_preview, d.error_detail, d.last_heartbeat,
+		       d.deadline, d.created_at, d.updated_at
+		FROM delegations d
+		WHERE d.caller_id = $1
+		ORDER BY d.created_at DESC
+		LIMIT 50
+	`, workspaceID)
+	if err != nil {
+		// Table may not exist yet (pre-migration), or permission issue.
+		// Fall back silently — do not log to avoid noise on every call.
+		return nil
+	}
+	defer rows.Close()
+
+	var result []map[string]interface{}
+	for rows.Next() {
+		var delegationID, callerID, calleeID, taskPreview, status, resultPreview, errorDetail string
+		var lastHeartbeat, deadline, createdAt, updatedAt *time.Time
+		if err := rows.Scan(
+			&delegationID, &callerID, &calleeID, &taskPreview,
+			&status, &resultPreview, &errorDetail, &lastHeartbeat,
+			&deadline, &createdAt, &updatedAt,
+		); err != nil {
+			continue
+		}
+		entry := map[string]interface{}{
+			"delegation_id": delegationID,
+			"source_id":     callerID,
+			"target_id":     calleeID,
+			"summary":       textutil.TruncateBytes(taskPreview, 200),
+			"status":        status,
+			"created_at":    createdAt,
+			"updated_at":    updatedAt,
+			"_ledger":       true, // marker so callers know this row is from the ledger
+		}
+		if resultPreview != "" {
+			entry["response_preview"] = textutil.TruncateBytes(resultPreview, 300)
+		}
+		if errorDetail != "" {
+			entry["error"] = errorDetail
+		}
+		if lastHeartbeat != nil {
+			entry["last_heartbeat"] = lastHeartbeat
+		}
+		if deadline != nil {
+			entry["deadline"] = deadline
+		}
+		result = append(result, entry)
+	}
+	if err := rows.Err(); err != nil {
+		log.Printf("listDelegationsFromLedger rows.Err: %v", err)
+	}
+
+	if result == nil {
+		return nil
+	}
+	return result
+}
+
+// listDelegationsFromActivityLogs is the legacy path that reconstructs
+// delegation state by folding activity_logs rows by delegation_id.
+// Kept for backward compatibility and for workspaces that never had
+// DELEGATION_LEDGER_WRITE=1 during their delegation lifecycle.
+func (h *DelegationHandler) listDelegationsFromActivityLogs(ctx context.Context, workspaceID string) []map[string]interface{} {
 	rows, err := db.DB.QueryContext(ctx, `
 		SELECT id, activity_type, COALESCE(source_id::text, ''), COALESCE(target_id::text, ''),
 		       COALESCE(summary, ''), COALESCE(status, ''), COALESCE(error_detail, ''),
@@ -613,12 +703,11 @@ func (h *DelegationHandler) ListDelegations(c *gin.Context) {
 		LIMIT 50
 	`, workspaceID)
 	if err != nil {
-		c.JSON(http.StatusInternalServerError, gin.H{"error": "query failed"})
-		return
+		return []map[string]interface{}{}
 	}
 	defer rows.Close()
 
-	var delegations []map[string]interface{}
+	var result []map[string]interface{}
 	for rows.Next() {
 		var id, actType, sourceID, targetID, summary, status, errorDetail, responseBody, delegationID string
 		var createdAt time.Time
@@ -643,16 +732,16 @@ func (h *DelegationHandler) ListDelegations(c *gin.Context) {
 		if responseBody != "" {
 			entry["response_preview"] = textutil.TruncateBytes(responseBody, 300)
 		}
-		delegations = append(delegations, entry)
+		result = append(result, entry)
 	}
 	if err := rows.Err(); err != nil {
 		log.Printf("ListDelegations scan error: %v", err)
 	}
 
-	if delegations == nil {
-		delegations = []map[string]interface{}{}
+	if result == nil {
+		return []map[string]interface{}{}
 	}
-	c.JSON(http.StatusOK, delegations)
+	return result
 }
 
 // --- helpers ---

workspace-server/internal/handlers/delegation_test.go

@@ -233,14 +233,21 @@ func TestListDelegations_Empty(t *testing.T) {
 	wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
 	dh := NewDelegationHandler(wh, broadcaster)
 
-	rows := sqlmock.NewRows([]string{
-		"id", "activity_type", "source_id", "target_id",
-		"summary", "status", "error_detail", "response_body",
-		"delegation_id", "created_at",
-	})
+	// Ledger returns empty → falls back to activity_logs (also empty)
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
+		WithArgs("ws-source").
+		WillReturnRows(sqlmock.NewRows([]string{
+			"delegation_id", "caller_id", "callee_id", "task_preview",
+			"status", "result_preview", "error_detail", "last_heartbeat",
+			"deadline", "created_at", "updated_at",
+		}))
 	mock.ExpectQuery("SELECT id, activity_type").
 		WithArgs("ws-source").
-		WillReturnRows(rows)
+		WillReturnRows(sqlmock.NewRows([]string{
+			"id", "activity_type", "source_id", "target_id",
+			"summary", "status", "error_detail", "response_body",
+			"delegation_id", "created_at",
+		}))
 
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
@@ -260,9 +267,12 @@ func TestListDelegations_Empty(t *testing.T) {
 	if len(resp) != 0 {
 		t.Errorf("expected empty array, got %d entries", len(resp))
 	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
 }
 
-// ---------- ListDelegations: with results → 200 with entries ----------
+// ---------- ListDelegations: with results (ledger only, no activity_logs fallback) ----------
 
 func TestListDelegations_WithResults(t *testing.T) {
 	mock := setupTestDB(t)
@@ -272,19 +282,21 @@ func TestListDelegations_WithResults(t *testing.T) {
 	dh := NewDelegationHandler(wh, broadcaster)
 
 	now := time.Now()
+	deadline := now.Add(6 * time.Hour)
+	// Ledger query returns rows — no fallback to activity_logs
 	rows := sqlmock.NewRows([]string{
-		"id", "activity_type", "source_id", "target_id",
-		"summary", "status", "error_detail", "response_body",
-		"delegation_id", "created_at",
+		"delegation_id", "caller_id", "callee_id", "task_preview",
+		"status", "result_preview", "error_detail", "last_heartbeat",
+		"deadline", "created_at", "updated_at",
 	}).
-		AddRow("1", "delegation", "ws-source", "ws-target",
+		AddRow("del-111", "ws-source", "ws-target",
 			"Delegating to ws-target", "pending", "", "",
-			"del-111", now).
-		AddRow("2", "delegation", "ws-source", "ws-target",
-			"Delegation completed (hello world)", "completed", "", "hello world",
-			"del-111", now.Add(time.Minute))
+			&now, &deadline, now, now).
+		AddRow("del-222", "ws-source", "ws-target",
+			"Delegation completed (hello world)", "completed", "hello world", "",
+			&now, &deadline, now, now.Add(time.Minute))
 
-	mock.ExpectQuery("SELECT id, activity_type").
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
 		WithArgs("ws-source").
 		WillReturnRows(rows)
 
@@ -308,23 +320,26 @@ func TestListDelegations_WithResults(t *testing.T) {
 	}
 
 	// Check first entry (pending delegation)
-	if resp[0]["type"] != "delegation" {
-		t.Errorf("expected type 'delegation', got %v", resp[0]["type"])
+	if resp[0]["delegation_id"] != "del-111" {
+		t.Errorf("expected delegation_id 'del-111', got %v", resp[0]["delegation_id"])
 	}
 	if resp[0]["status"] != "pending" {
 		t.Errorf("expected status 'pending', got %v", resp[0]["status"])
 	}
-	if resp[0]["delegation_id"] != "del-111" {
-		t.Errorf("expected delegation_id 'del-111', got %v", resp[0]["delegation_id"])
-	}
 	if resp[0]["source_id"] != "ws-source" {
 		t.Errorf("expected source_id 'ws-source', got %v", resp[0]["source_id"])
 	}
 	if resp[0]["target_id"] != "ws-target" {
 		t.Errorf("expected target_id 'ws-target', got %v", resp[0]["target_id"])
 	}
+	if resp[0]["_ledger"] != true {
+		t.Errorf("expected _ledger=true marker, got %v", resp[0]["_ledger"])
+	}
 
 	// Check second entry (completed, has response_preview)
+	if resp[1]["delegation_id"] != "del-222" {
+		t.Errorf("expected delegation_id 'del-222', got %v", resp[1]["delegation_id"])
+	}
 	if resp[1]["status"] != "completed" {
 		t.Errorf("expected status 'completed', got %v", resp[1]["status"])
 	}
@@ -1364,3 +1379,331 @@ func TestExtractResponseText_EmptyText(t *testing.T) {
 		t.Errorf("empty text: got %q, want %q", got, "")
 	}
 }
+
+// ---------- ListDelegations: ledger has rows → returns them (no activity_logs fallback) ----------
+
+func TestListDelegations_LedgerRowsReturned(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+	dh := NewDelegationHandler(wh, broadcaster)
+
+	now := time.Now()
+	deadline := now.Add(6 * time.Hour)
+	// Ledger query returns rows
+	ledgerRows := sqlmock.NewRows([]string{
+		"delegation_id", "caller_id", "callee_id", "task_preview",
+		"status", "result_preview", "error_detail", "last_heartbeat",
+		"deadline", "created_at", "updated_at",
+	}).AddRow(
+		"del-ledger-001", "caller-uuid", "callee-uuid",
+		"Analyze the codebase for bugs", "in_progress", "", "",
+		&now, &deadline, now, now,
+	)
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
+		WithArgs("caller-uuid").
+		WillReturnRows(ledgerRows)
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "caller-uuid"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/caller-uuid/delegations", nil)
+
+	dh.ListDelegations(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 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("failed to parse response: %v", err)
+	}
+	if len(resp) != 1 {
+		t.Fatalf("expected 1 entry, got %d", len(resp))
+	}
+	if resp[0]["delegation_id"] != "del-ledger-001" {
+		t.Errorf("expected delegation_id 'del-ledger-001', got %v", resp[0]["delegation_id"])
+	}
+	if resp[0]["status"] != "in_progress" {
+		t.Errorf("expected status 'in_progress', got %v", resp[0]["status"])
+	}
+	if resp[0]["_ledger"] != true {
+		t.Errorf("expected _ledger=true marker, got %v", resp[0]["_ledger"])
+	}
+	if resp[0]["source_id"] != "caller-uuid" {
+		t.Errorf("expected source_id 'caller-uuid', got %v", resp[0]["source_id"])
+	}
+	if resp[0]["target_id"] != "callee-uuid" {
+		t.Errorf("expected target_id 'callee-uuid', got %v", resp[0]["target_id"])
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// ---------- ListDelegations: ledger empty → falls back to activity_logs ----------
+
+func TestListDelegations_LedgerEmptyFallsBackToActivityLogs(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+	dh := NewDelegationHandler(wh, broadcaster)
+
+	// Ledger returns empty → falls back to activity_logs
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
+		WithArgs("ws-source").
+		WillReturnRows(sqlmock.NewRows([]string{
+			"delegation_id", "caller_id", "callee_id", "task_preview",
+			"status", "result_preview", "error_detail", "last_heartbeat",
+			"deadline", "created_at", "updated_at",
+		}))
+
+	now := time.Now()
+	activityRows := sqlmock.NewRows([]string{
+		"id", "activity_type", "source_id", "target_id",
+		"summary", "status", "error_detail", "response_body",
+		"delegation_id", "created_at",
+	}).AddRow(
+		"act-001", "delegation", "ws-source", "ws-target",
+		"Delegating to ws-target", "pending", "", "",
+		"del-old-001", now,
+	)
+	mock.ExpectQuery("SELECT id, activity_type").
+		WithArgs("ws-source").
+		WillReturnRows(activityRows)
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-source"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/ws-source/delegations", nil)
+
+	dh.ListDelegations(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 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("failed to parse response: %v", err)
+	}
+	if len(resp) != 1 {
+		t.Fatalf("expected 1 entry from fallback, got %d", len(resp))
+	}
+	if resp[0]["delegation_id"] != "del-old-001" {
+		t.Errorf("expected delegation_id 'del-old-001' from activity_logs, got %v", resp[0]["delegation_id"])
+	}
+	if resp[0]["type"] != "delegation" {
+		t.Errorf("expected type 'delegation' from activity_logs, got %v", resp[0]["type"])
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// ---------- ListDelegations: both ledger and activity_logs empty → [] ----------
+
+func TestListDelegations_BothEmptyReturnsEmptyArray(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+	dh := NewDelegationHandler(wh, broadcaster)
+
+	// Ledger empty
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
+		WithArgs("ws-source").
+		WillReturnRows(sqlmock.NewRows([]string{
+			"delegation_id", "caller_id", "callee_id", "task_preview",
+			"status", "result_preview", "error_detail", "last_heartbeat",
+			"deadline", "created_at", "updated_at",
+		}))
+	// activity_logs also empty
+	mock.ExpectQuery("SELECT id, activity_type").
+		WithArgs("ws-source").
+		WillReturnRows(sqlmock.NewRows([]string{
+			"id", "activity_type", "source_id", "target_id",
+			"summary", "status", "error_detail", "response_body",
+			"delegation_id", "created_at",
+		}))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-source"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/ws-source/delegations", nil)
+
+	dh.ListDelegations(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp []interface{}
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("failed to parse response: %v", err)
+	}
+	if len(resp) != 0 {
+		t.Errorf("expected empty array, got %d entries", len(resp))
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// ---------- ListDelegations: ledger query error → falls back to activity_logs ----------
+
+func TestListDelegations_LedgerQueryErrorFallsBackToActivityLogs(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+	dh := NewDelegationHandler(wh, broadcaster)
+
+	// Ledger query fails → fallback to activity_logs
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
+		WithArgs("ws-source").
+		WillReturnError(fmt.Errorf("table does not exist"))
+
+	now := time.Now()
+	activityRows := sqlmock.NewRows([]string{
+		"id", "activity_type", "source_id", "target_id",
+		"summary", "status", "error_detail", "response_body",
+		"delegation_id", "created_at",
+	}).AddRow(
+		"act-002", "delegation", "ws-source", "ws-target",
+		"Some task", "completed", "", "result here",
+		"del-pre-318", now,
+	)
+	mock.ExpectQuery("SELECT id, activity_type").
+		WithArgs("ws-source").
+		WillReturnRows(activityRows)
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-source"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/ws-source/delegations", nil)
+
+	dh.ListDelegations(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 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("failed to parse response: %v", err)
+	}
+	if len(resp) != 1 || resp[0]["delegation_id"] != "del-pre-318" {
+		t.Errorf("expected 1 activity_logs entry, got %v", resp)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// ---------- ListDelegations: ledger completed delegation includes result_preview ----------
+
+func TestListDelegations_LedgerCompletedIncludesResultPreview(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+	dh := NewDelegationHandler(wh, broadcaster)
+
+	now := time.Now()
+	deadline := now.Add(6 * time.Hour)
+	ledgerRows := sqlmock.NewRows([]string{
+		"delegation_id", "caller_id", "callee_id", "task_preview",
+		"status", "result_preview", "error_detail", "last_heartbeat",
+		"deadline", "created_at", "updated_at",
+	}).AddRow(
+		"del-complete-001", "caller-uuid", "callee-uuid",
+		"Run analysis", "completed", "Analysis complete: 42 issues found", "",
+		&now, &deadline, now, now,
+	)
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
+		WithArgs("caller-uuid").
+		WillReturnRows(ledgerRows)
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "caller-uuid"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/caller-uuid/delegations", nil)
+
+	dh.ListDelegations(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 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("failed to parse response: %v", err)
+	}
+	if len(resp) != 1 {
+		t.Fatalf("expected 1 entry, got %d", len(resp))
+	}
+	if resp[0]["status"] != "completed" {
+		t.Errorf("expected status 'completed', got %v", resp[0]["status"])
+	}
+	if resp[0]["response_preview"] != "Analysis complete: 42 issues found" {
+		t.Errorf("expected response_preview, got %v", resp[0]["response_preview"])
+	}
+	if resp[0]["error"] != nil {
+		t.Errorf("expected no error on completed, got %v", resp[0]["error"])
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// ---------- ListDelegations: ledger failed delegation includes error_detail ----------
+
+func TestListDelegations_LedgerFailedIncludesErrorDetail(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+	dh := NewDelegationHandler(wh, broadcaster)
+
+	now := time.Now()
+	deadline := now.Add(6 * time.Hour)
+	ledgerRows := sqlmock.NewRows([]string{
+		"delegation_id", "caller_id", "callee_id", "task_preview",
+		"status", "result_preview", "error_detail", "last_heartbeat",
+		"deadline", "created_at", "updated_at",
+	}).AddRow(
+		"del-failed-001", "caller-uuid", "callee-uuid",
+		"Fetch data", "failed", "", "Callee workspace not reachable",
+		&now, &deadline, now, now,
+	)
+	mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
+		WithArgs("caller-uuid").
+		WillReturnRows(ledgerRows)
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "caller-uuid"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/caller-uuid/delegations", nil)
+
+	dh.ListDelegations(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 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("failed to parse response: %v", err)
+	}
+	if len(resp) != 1 {
+		t.Fatalf("expected 1 entry, got %d", len(resp))
+	}
+	if resp[0]["status"] != "failed" {
+		t.Errorf("expected status 'failed', got %v", resp[0]["status"])
+	}
+	if resp[0]["error"] != "Callee workspace not reachable" {
+		t.Errorf("expected error detail, got %v", resp[0]["error"])
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+

workspace-server/internal/handlers/org.go

@@ -4,6 +4,7 @@ package handlers
 // Tree creation logic is in org_import.go; utility helpers in org_helpers.go.
 
 import (
+	"bufio"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -147,6 +148,17 @@ func sizeOfSubtree(ws OrgWorkspace) nodeSize {
 	}
 }
 
+// childSlot returns the (x, y) position of child `index` in a 2-column
+// fixed-size grid. Used as the default when sibling sizes are unknown.
+// Formula: x = parentSidePadding + col*(childDefaultWidth+childGutter),
+// y = parentHeaderPadding + row*(childDefaultHeight+childGutter).
+func childSlot(index int) (x, y float64) {
+	col := index % childGridColumnCount
+	row := index / childGridColumnCount
+	return parentSidePadding + float64(col)*(childDefaultWidth+childGutter),
+		parentHeaderPadding + float64(row)*(childDefaultHeight+childGutter)
+}
+
 // childSlotInGrid computes the relative position of sibling `index`
 // given all siblings' subtree sizes. Uniform column width (= max width
 // across siblings), per-row max height, so a nested parent sibling
@@ -252,6 +264,7 @@ type EnvRequirement struct {
 // Members returns every env name this requirement considers —
 // [Name] for single, AnyOf for groups. Used by preflight, collect,
 // and the name-validation regex gate.
+
 func (e EnvRequirement) Members() []string {
 	if e.Name != "" {
 		return []string{e.Name}
@@ -328,6 +341,95 @@ func (e *EnvRequirement) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
+// perWorkspaceUnsatisfied is the return type of collectPerWorkspaceUnsatisfied.
+// Each entry names the workspace and files_dir that declared an unsatisfied
+// requirement, plus the requirement itself (EnvRequirement serialises to
+// the same dual shape {string | {any_of: [...]}} in the 412 JSON response).
+type perWorkspaceUnsatisfied struct {
+	Workspace   string         `json:"workspace"`
+	FilesDir    string         `json:"files_dir"`
+	Unsatisfied EnvRequirement `json:"unsatisfied"`
+}
+
+// collectPerWorkspaceUnsatisfied walks the workspace tree and reports every
+// RequiredEnv that is not covered by global secrets (configured) or by an
+// on-disk .env file. orgBaseDir is the on-disk root of the org template
+// (each workspace's .env lives at orgBaseDir/<files_dir>/.env); when empty
+// no .env files are checked and only global coverage can satisfy a requirement.
+// A workspace is satisfied by the .env in its own files_dir AND the org root
+// .env (env vars cascade downward from the root).
+func collectPerWorkspaceUnsatisfied(
+	workspaces []OrgWorkspace,
+	orgBaseDir string,
+	configured map[string]struct{},
+) []perWorkspaceUnsatisfied {
+	var result []perWorkspaceUnsatisfied
+	for _, ws := range workspaces {
+		// Check each RequiredEnv.
+		for _, req := range ws.RequiredEnv {
+			if req.IsSatisfied(configured) {
+				continue
+			}
+			// Not covered by global secrets — check .env files if available.
+			// When orgBaseDir is empty (inline template import) we cannot check
+			// .env files, so any key not in configured is genuinely missing.
+			if orgBaseDir == "" || !envKeyPresent(orgBaseDir, ws.FilesDir, req.Members()...) {
+				result = append(result, perWorkspaceUnsatisfied{
+					Workspace:   ws.Name,
+					FilesDir:    ws.FilesDir,
+					Unsatisfied: req,
+				})
+			}
+		}
+		// Recurse into children so deeply nested workspaces are also checked.
+		result = append(result, collectPerWorkspaceUnsatisfied(ws.Children, orgBaseDir, configured)...)
+	}
+	return result
+}
+
+// envKeyPresent checks whether all env keys appear in either
+// orgBaseDir/.env (root) or orgBaseDir/filesDir/.env (workspace).
+// Returns true only when all keys are found in at least one of those files.
+func envKeyPresent(orgBaseDir, filesDir string, keys ...string) bool {
+	if len(keys) == 0 {
+		return true
+	}
+	// Load root .env (covers vars that cascade from org root).
+	rootEnv := loadEnvVars(orgBaseDir + "/.env")
+	// Load workspace .env.
+	wsEnv := loadEnvVars(orgBaseDir + "/" + filesDir + "/.env")
+	for _, k := range keys {
+		if _, inRoot := rootEnv[k]; !inRoot {
+			if _, inWS := wsEnv[k]; !inWS {
+				return false
+			}
+		}
+	}
+	return true
+}
+
+// loadEnvVars reads a .env file and returns keys→values.
+func loadEnvVars(path string) map[string]string {
+	vars := map[string]string{}
+	f, err := os.Open(path)
+	if err != nil {
+		return vars
+	}
+	defer f.Close()
+	sc := bufio.NewScanner(f)
+	for sc.Scan() {
+		line := strings.TrimSpace(sc.Text())
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+		parts := strings.SplitN(line, "=", 2)
+		if len(parts) == 2 {
+			vars[parts[0]] = parts[1]
+		}
+	}
+	return vars
+}
+
 // OrgTemplate is the YAML structure for an org hierarchy.
 type OrgTemplate struct {
 	Name        string         `yaml:"name" json:"name"`

workspace-server/internal/handlers/org_helpers.go

@@ -62,6 +62,11 @@ func resolvePromptRef(inline, fileRef, orgBaseDir, filesDir string) (string, err
 	return string(data), nil
 }
 
+// envVarRx matches ${VAR} and $VAR references where the name starts with
+// [a-zA-Z_] — intentionally excludes bare $ and $1-style digits so
+// "cost $100" stays intact.
+var envVarRx = regexp.MustCompile(`\$\{([a-zA-Z_][a-zA-Z0-9_]*)\}|\$([a-zA-Z_][a-zA-Z0-9_]*)`)
+
 // envVarRefPattern matches actual ${VAR} or $VAR references (not literal $).
 // Used to detect unresolved placeholders without false positives like "$5".
 var envVarRefPattern = regexp.MustCompile(`\$\{?[A-Za-z_][A-Za-z0-9_]*\}?`)
@@ -80,12 +85,30 @@ func hasUnresolvedVarRef(original, expanded string) bool {
 // expandWithEnv expands ${VAR} and $VAR references in s using the env map.
 // Falls back to the platform process env if a var isn't in the map.
 func expandWithEnv(s string, env map[string]string) string {
-	return os.Expand(s, func(key string) string {
-		if v, ok := env[key]; ok {
-			return v
+	result := s
+	for {
+		loc := envVarRx.FindStringIndex(result)
+		if loc == nil {
+			break
 		}
-		return os.Getenv(key)
-	})
+		match := result[loc[0]:loc[1]]
+		var key string
+		if len(match) >= 2 && match[0] == '$' && match[1] == '{' {
+			// ${VAR} form
+			key = match[2 : len(match)-1]
+		} else {
+			// $VAR form
+			key = match[1:]
+		}
+		var replacement string
+		if v, ok := env[key]; ok {
+			replacement = v
+		} else {
+			replacement = os.Getenv(key)
+		}
+		result = result[:loc[0]] + replacement + result[loc[1]:]
+	}
+	return result
 }
 
 // loadWorkspaceEnv reads the org root .env and the workspace-specific .env

workspace-server/internal/handlers/org_helpers_pure_test.go

@@ -589,7 +589,7 @@ func TestRenderCategoryRoutingYAML_SpecialCharactersEscaped(t *testing.T) {
 // ── Additional coverage: appendYAMLBlock ───────────────────────────
 func TestAppendYAMLBlock_BothEmpty(t *testing.T) {
 	result := appendYAMLBlock(nil, "")
-	assert.Equal(t, "", result)
+	assert.Nil(t, result)
 }
 
 func TestAppendYAMLBlock_ExistingHasNewline(t *testing.T) {
@@ -644,6 +644,11 @@ func TestMergePlugins_ExclusionWithBang(t *testing.T) {
 
 func TestMergePlugins_ExclusionWithDash(t *testing.T) {
 	defaults := []string{"plugin-a", "plugin-b", "plugin-c"}
+	wsPlugins := []string{"-plugin-b"}
+	result := mergePlugins(defaults, wsPlugins)
+	assert.Equal(t, []string{"plugin-a", "plugin-c"}, result)
+}
+
 func TestMergePlugins_ExclusionEmptyTarget(t *testing.T) {
 	defaults := []string{"plugin-a", "plugin-b"}
 	wsPlugins := []string{"!", "-"} // no-op exclusions
@@ -661,6 +666,11 @@ func TestMergePlugins_ExclusionNotInDefaults(t *testing.T) {
 
 func TestMergePlugins_WorkspaceAddsNew(t *testing.T) {
 	defaults := []string{"plugin-a"}
+	wsPlugins := []string{"plugin-b"}
+	result := mergePlugins(defaults, wsPlugins)
+	assert.Equal(t, []string{"plugin-a", "plugin-b"}, result)
+}
+
 func TestMergePlugins_DeduplicationOrder(t *testing.T) {
 	// Defaults first; workspace entries deduplicated.
 	defaults := []string{"plugin-a", "plugin-a", "plugin-b"}

workspace-server/internal/handlers/org_layout_test.go

@@ -1,294 +0,0 @@
-package handlers
-
-import "testing"
-
-// Tests for the pure layout helpers in org.go:
-// childSlot, sizeOfSubtree, childSlotInGrid. These compute the canvas
-// grid positions for org-import workspace trees and mirror the TypeScript
-// layout functions in canvas-topology.ts (defaultChildSlot, parentMinSize,
-// childSlotInGrid). The two sides use slightly different default sizes
-// (Go: 240×130, TS: 210×120) so they are tested independently.
-
-// childSlot — 2-column fixed-size grid, one row of child cards.
-func TestChildSlot_ZeroIndex(t *testing.T) {
-	x, y := childSlot(0)
-	// col=0, row=0
-	// x = 16 + 0*(240+14) = 16
-	// y = 130 + 0*(130+14) = 130
-	if x != 16.0 {
-		t.Errorf("slot 0 x: got %v, want 16.0", x)
-	}
-	if y != 130.0 {
-		t.Errorf("slot 0 y: got %v, want 130.0", y)
-	}
-}
-
-func TestChildSlot_SecondColumn(t *testing.T) {
-	x, y := childSlot(1)
-	// col=1, row=0
-	// x = 16 + 1*(240+14) = 16+254 = 270
-	// y = 130
-	if x != 270.0 {
-		t.Errorf("slot 1 x: got %v, want 270.0", x)
-	}
-	if y != 130.0 {
-		t.Errorf("slot 1 y: got %v, want 130.0", y)
-	}
-}
-
-func TestChildSlot_SecondRow(t *testing.T) {
-	x, y := childSlot(2)
-	// col=0, row=1
-	// x = 16
-	// y = 130 + 1*(130+14) = 130+144 = 274
-	if x != 16.0 {
-		t.Errorf("slot 2 x: got %v, want 16.0", x)
-	}
-	if y != 274.0 {
-		t.Errorf("slot 2 y: got %v, want 274.0", y)
-	}
-}
-
-func TestChildSlot_ThirdRowFirstColumn(t *testing.T) {
-	x, y := childSlot(4)
-	// col=0, row=2
-	// x = 16
-	// y = 130 + 2*(130+14) = 130+288 = 418
-	if x != 16.0 {
-		t.Errorf("slot 4 x: got %v, want 16.0", x)
-	}
-	if y != 418.0 {
-		t.Errorf("slot 4 y: got %v, want 418.0", y)
-	}
-}
-
-// sizeOfSubtree — bounding-box computation for org-import layout.
-func TestSizeOfSubtree_Leaf(t *testing.T) {
-	ws := OrgWorkspace{Name: "leaf"}
-	s := sizeOfSubtree(ws)
-	// Leaf → childDefaultWidth × childDefaultHeight
-	if s.width != 240.0 {
-		t.Errorf("leaf width: got %v, want 240.0", s.width)
-	}
-	if s.height != 130.0 {
-		t.Errorf("leaf height: got %v, want 130.0", s.height)
-	}
-}
-
-func TestSizeOfSubtree_OneChild(t *testing.T) {
-	ws := OrgWorkspace{Name: "parent", Children: []OrgWorkspace{{Name: "child"}}}
-	s := sizeOfSubtree(ws)
-	// 1 child → cols=1, rows=1
-	// child subtree = (240, 130)
-	// width = 16*2 + 240*1 + 14*0 = 272
-	// height = 130 + 130 + 14*0 + 16 = 276
-	if s.width != 272.0 {
-		t.Errorf("1-child width: got %v, want 272.0", s.width)
-	}
-	if s.height != 276.0 {
-		t.Errorf("1-child height: got %v, want 276.0", s.height)
-	}
-}
-
-func TestSizeOfSubtree_TwoChildren(t *testing.T) {
-	ws := OrgWorkspace{Name: "parent", Children: []OrgWorkspace{
-		{Name: "c0"}, {Name: "c1"},
-	}}
-	s := sizeOfSubtree(ws)
-	// 2 children → cols=2, rows=1
-	// maxColW = 240, totalRowH = 130
-	// width = 16*2 + 240*2 + 14*1 = 32+480+14 = 526
-	// height = 130 + 130 + 14*0 + 16 = 276
-	if s.width != 526.0 {
-		t.Errorf("2-child width: got %v, want 526.0", s.width)
-	}
-	if s.height != 276.0 {
-		t.Errorf("2-child height: got %v, want 276.0", s.height)
-	}
-}
-
-func TestSizeOfSubtree_ThreeChildren(t *testing.T) {
-	ws := OrgWorkspace{Name: "parent", Children: []OrgWorkspace{
-		{Name: "c0"}, {Name: "c1"}, {Name: "c2"},
-	}}
-	s := sizeOfSubtree(ws)
-	// 3 children → cols=2 (< 3 so capped at 2), rows=2
-	// each child = (240, 130), maxColW=240, rowHeights=[130,130]
-	// totalRowH = 130+130 = 260
-	// width = 16*2 + 240*2 + 14*1 = 526
-	// height = 130 + 260 + 14*1 + 16 = 420
-	if s.width != 526.0 {
-		t.Errorf("3-child width: got %v, want 526.0", s.width)
-	}
-	if s.height != 420.0 {
-		t.Errorf("3-child height: got %v, want 420.0", s.height)
-	}
-}
-
-func TestSizeOfSubtree_FourChildren(t *testing.T) {
-	ws := OrgWorkspace{Name: "parent", Children: []OrgWorkspace{
-		{Name: "c0"}, {Name: "c1"}, {Name: "c2"}, {Name: "c3"},
-	}}
-	s := sizeOfSubtree(ws)
-	// 4 children → cols=2, rows=2
-	// width = 16*2 + 240*2 + 14*1 = 526
-	// height = 130 + 260 + 14*1 + 16 = 420
-	if s.width != 526.0 {
-		t.Errorf("4-child width: got %v, want 526.0", s.width)
-	}
-	if s.height != 420.0 {
-		t.Errorf("4-child height: got %v, want %v", s.height, 420.0)
-	}
-}
-
-func TestSizeOfSubtree_FiveChildren(t *testing.T) {
-	ws := OrgWorkspace{Name: "parent", Children: []OrgWorkspace{
-		{Name: "c0"}, {Name: "c1"}, {Name: "c2"}, {Name: "c3"}, {Name: "c4"},
-	}}
-	s := sizeOfSubtree(ws)
-	// 5 children → cols=2, rows=3
-	// rowHeights = [130, 130, 130], totalRowH = 390
-	// width = 16*2 + 240*2 + 14*1 = 526
-	// height = 130 + 390 + 14*2 + 16 = 564
-	if s.width != 526.0 {
-		t.Errorf("5-child width: got %v, want 526.0", s.width)
-	}
-	if s.height != 564.0 {
-		t.Errorf("5-child height: got %v, want 564.0", s.height)
-	}
-}
-
-func TestSizeOfSubtree_NestedTree(t *testing.T) {
-	// Grandparent → [Parent(→ child), leaf]
-	// parent subtree (1 child): width=272, height=276
-	// grandparent:
-	//   children = [parent, leaf]
-	//   maxColW = max(272, 240) = 272
-	//   cols=2, rows=1
-	//   width = 16*2 + 272*2 + 14*1 = 590
-	//   height = 130 + max(276, 130) + 14*0 + 16 = 422
-	parent := OrgWorkspace{Name: "parent", Children: []OrgWorkspace{{Name: "grandchild"}}}
-	ws := OrgWorkspace{Name: "grandparent", Children: []OrgWorkspace{parent, {Name: "leaf"}}}
-	s := sizeOfSubtree(ws)
-	if s.width != 590.0 {
-		t.Errorf("nested width: got %v, want 590.0", s.width)
-	}
-	if s.height != 422.0 {
-		t.Errorf("nested height: got %v, want 422.0", s.height)
-	}
-}
-
-// childSlotInGrid — sibling-aware slot computation; taller siblings push
-// subsequent rows down without displacing the column grid.
-func TestChildSlotInGrid_EmptySiblings(t *testing.T) {
-	x, y := childSlotInGrid(0, nil)
-	x2, y2 := childSlotInGrid(0, []nodeSize{})
-	// Both nil and empty slice return the top-left padded origin.
-	got1, got2 := struct{ x, y float64 }{x, y}, struct{ x, y float64 }{x2, y2}
-	for _, g := range []struct{ x, y float64 }{got1, got2} {
-		if g.x != 16.0 || g.y != 130.0 {
-			t.Errorf("empty siblings: got (%.0f, %.0f), want (16, 130)", g.x, g.y)
-		}
-	}
-}
-
-func TestChildSlotInGrid_Slot0MatchesDefaultChildSlot(t *testing.T) {
-	// With uniform 240×130 siblings, slot 0 should equal childSlot(0).
-	sizes := []nodeSize{{width: 240, height: 130}, {width: 240, height: 130}}
-	x, y := childSlotInGrid(0, sizes)
-	cx, cy := childSlot(0)
-	if x != cx || y != cy {
-		t.Errorf("uniform siblings slot 0: got (%.0f, %.0f), want childSlot (%.0f, %.0f)", x, y, cx, cy)
-	}
-}
-
-func TestChildSlotInGrid_Slot1MatchesDefaultChildSlot(t *testing.T) {
-	sizes := []nodeSize{{width: 240, height: 130}, {width: 240, height: 130}}
-	x, y := childSlotInGrid(1, sizes)
-	cx, cy := childSlot(1)
-	if x != cx || y != cy {
-		t.Errorf("uniform siblings slot 1: got (%.0f, %.0f), want childSlot (%.0f, %.0f)", x, y, cx, cy)
-	}
-}
-
-func TestChildSlotInGrid_TallerSiblingBumpsNextRow(t *testing.T) {
-	// Sibling at index 1 is taller (height=300 vs 130).
-	// Slot 0: col=0, row=0 → x=16, y=130
-	// Slot 1: col=1, row=0 → x=270, y=130
-	// Slot 2: col=0, row=1 → x=16, y = 130 + 300 + 14 = 444
-	sizes := []nodeSize{
-		{width: 240, height: 130},
-		{width: 240, height: 300}, // taller — pushes row 2 down
-		{width: 240, height: 130},
-	}
-	x0, y0 := childSlotInGrid(0, sizes)
-	if x0 != 16.0 || y0 != 130.0 {
-		t.Errorf("slot 0: got (%.0f, %.0f), want (16, 130)", x0, y0)
-	}
-
-	x1, y1 := childSlotInGrid(1, sizes)
-	if x1 != 270.0 || y1 != 130.0 {
-		t.Errorf("slot 1: got (%.0f, %.0f), want (270, 130)", x1, y1)
-	}
-
-	x2, y2 := childSlotInGrid(2, sizes)
-	// y = parentHeaderPadding + rowHeights[0] + childGutter
-	// rowHeights[0] = max(130, 300) = 300
-	// y = 130 + 300 + 14 = 444
-	if x2 != 16.0 || y2 != 444.0 {
-		t.Errorf("slot 2: got (%.0f, %.0f), want (16, 444) — taller sibling pushed row down", x2, y2)
-	}
-}
-
-func TestChildSlotInGrid_UniformWideSiblingSetsColumnWidth(t *testing.T) {
-	// Sibling at index 0 is wider (300 vs 240).
-	// Slot 0: x=16, y=130
-	// Slot 1: col=1 → x = 16 + 300 + 14 = 330 (NOT 270 = 16+240+14)
-	//          y=130
-	sizes := []nodeSize{
-		{width: 300, height: 130}, // wider — sets column width
-		{width: 240, height: 130},
-	}
-	x1, y1 := childSlotInGrid(1, sizes)
-	if x1 != 330.0 || y1 != 130.0 {
-		t.Errorf("slot 1: got (%.0f, %.0f), want (330, 130) — col width set by wider sibling", x1, y1)
-	}
-}
-
-func TestChildSlotInGrid_Slot3OverflowToSecondRow(t *testing.T) {
-	// 4 siblings in 2-column grid → rows=2
-	// Slot 0: col=0, row=0
-	// Slot 1: col=1, row=0
-	// Slot 2: col=0, row=1
-	// Slot 3: col=1, row=1
-	sizes := []nodeSize{
-		{width: 240, height: 130},
-		{width: 240, height: 130},
-		{width: 240, height: 130},
-		{width: 240, height: 130},
-	}
-	x3, y3 := childSlotInGrid(3, sizes)
-	// y = 130 + 130 + 14 = 274
-	if x3 != 270.0 || y3 != 274.0 {
-		t.Errorf("slot 3: got (%.0f, %.0f), want (270, 274)", x3, y3)
-	}
-}
-
-func TestChildSlotInGrid_MixedSizesCorrectRowAccumulation(t *testing.T) {
-	// 3 siblings: [short(130), tall(300), medium(200)]
-	// cols=2, rows=2
-	// rowHeights[0] = max(130, 300) = 300
-	// rowHeights[1] = max(200, 0) = 200
-	// slot 0: col=0, row=0 → x=16, y=130
-	// slot 1: col=1, row=0 → x=330, y=130
-	// slot 2: col=0, row=1 → x=16, y=130+300+14=444
-	sizes := []nodeSize{
-		{width: 240, height: 130},
-		{width: 240, height: 300},
-		{width: 240, height: 200},
-	}
-	x2, y2 := childSlotInGrid(2, sizes)
-	if x2 != 16.0 || y2 != 444.0 {
-		t.Errorf("slot 2: got (%.0f, %.0f), want (16, 444)", x2, y2)
-	}
-}

workspace-server/internal/handlers/plugins_atomic_test.go

@@ -215,51 +215,6 @@ func TestTarWalk_EmptyDirectory(t *testing.T) {
 	}
 }
 
-// TestTarWalk_NestedDirs: deeply nested directories produce all intermediate
-// dir entries plus leaf entries. This exercises the recursive walk.
-func TestTarWalk_NestedDirs(t *testing.T) {
-	hostDir := t.TempDir()
-	deep := filepath.Join(hostDir, "a", "b", "c")
-	if err := os.MkdirAll(deep, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(filepath.Join(deep, "leaf.txt"), []byte("content"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	var buf bytes.Buffer
-	tw := newTarWriter(&buf)
-	if err := tarWalk(hostDir, "configs/plugins/.staging", tw); err != nil {
-		t.Fatalf("tarWalk: %v", err)
-	}
-	if err := tw.Close(); err != nil {
-		t.Fatalf("Close: %v", err)
-	}
-	entries := readTarNames(&buf)
-	// Must include: prefix/, prefix/a/, prefix/a/b/, prefix/a/b/c/, prefix/a/b/c/leaf.txt
-	expected := []string{
-		"configs/plugins/.staging/",
-		"configs/plugins/.staging/a/",
-		"configs/plugins/.staging/a/b/",
-		"configs/plugins/.staging/a/b/c/",
-		"configs/plugins/.staging/a/b/c/leaf.txt",
-	}
-	if len(entries) != len(expected) {
-		t.Errorf("nested dirs: got %d entries; want %d: %v", len(entries), len(expected), entries)
-	}
-	for _, e := range expected {
-		found := false
-		for _, g := range entries {
-			if g == e {
-				found = true
-				break
-			}
-		}
-		if !found {
-			t.Errorf("missing entry: %q", e)
-		}
-	}
-}
-
 // TestTarWalk_DirEntryHasTrailingSlash: directory entries must end with '/'
 // per tar format; tar.Header.Typeflag '5' (dir) must produce "name/" not "name".
 func TestTarWalk_DirEntryHasTrailingSlash(t *testing.T) {

workspace-server/internal/handlers/workspace_crud.go

@@ -141,6 +141,19 @@ func (h *WorkspaceHandler) Update(c *gin.Context) {
 		return
 	}
 
+	// Validate workspace_dir before hitting the DB — no point checking
+	// existence if the provided path is obviously unsafe.
+	if wsDir, ok := body["workspace_dir"]; ok {
+		if wsDir != nil {
+			if dirStr, isStr := wsDir.(string); isStr && dirStr != "" {
+				if err := validateWorkspaceDir(dirStr); err != nil {
+					c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace directory"})
+					return
+				}
+			}
+		}
+	}
+
 	ctx := c.Request.Context()
 
 	// Auth is fully enforced at the router layer (WorkspaceAuth middleware, #680).
@@ -198,15 +211,8 @@ func (h *WorkspaceHandler) Update(c *gin.Context) {
 	}
 	needsRestart := false
 	if wsDir, ok := body["workspace_dir"]; ok {
-		// Allow null to clear workspace_dir
-		if wsDir != nil {
-			if dirStr, isStr := wsDir.(string); isStr && dirStr != "" {
-				if err := validateWorkspaceDir(dirStr); err != nil {
-					c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace directory"})
-					return
-				}
-			}
-		}
+		// Allow null to clear workspace_dir. validateWorkspaceDir already ran
+		// above (before the existence check), so we only write here.
 		if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET workspace_dir = $2, updated_at = now() WHERE id = $1`, id, wsDir); err != nil {
 			log.Printf("Update workspace_dir error for %s: %v", id, err)
 		}

workspace-server/internal/handlers/workspace_crud_test.go

@@ -38,15 +38,15 @@ func setupWorkspaceCrudTest(t *testing.T) (sqlmock.Sqlmock, *gin.Engine) {
 
 func TestState_LegacyWorkspaceNoLiveToken(t *testing.T) {
 	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
+	h := NewWorkspaceHandler(nil, nil, "", "")
 	r.GET("/workspaces/:id/state", h.State)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
 	// No live token — legacy workspace, no auth required.
 	// HasAnyLiveToken always runs first (queries workspace_auth_tokens).
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspace_auth_tokens`).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
 	mock.ExpectQuery(`SELECT status FROM workspaces WHERE id = \$1`).
 		WithArgs(wsID).
 		WillReturnRows(sqlmock.NewRows([]string{"status"}).AddRow("running"))
@@ -76,13 +76,13 @@ func TestState_LegacyWorkspaceNoLiveToken(t *testing.T) {
 
 func TestState_HasLiveTokenMissingAuth(t *testing.T) {
 	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
+	h := NewWorkspaceHandler(nil, nil, "", "")
 	r.GET("/workspaces/:id/state", h.State)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspace_auth_tokens`).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
 
 	req, _ := http.NewRequest("GET", "/workspaces/"+wsID+"/state", nil)
 	// No Authorization header
@@ -96,13 +96,13 @@ func TestState_HasLiveTokenMissingAuth(t *testing.T) {
 
 func TestState_WorkspaceNotFound(t *testing.T) {
 	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
+	h := NewWorkspaceHandler(nil, nil, "", "")
 	r.GET("/workspaces/:id/state", h.State)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspace_auth_tokens`).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
 	mock.ExpectQuery(`SELECT status FROM workspaces WHERE id = \$1`).
 		WithArgs(wsID).
 		WillReturnError(sql.ErrNoRows)
@@ -126,13 +126,13 @@ func TestState_WorkspaceNotFound(t *testing.T) {
 
 func TestState_WorkspaceSoftDeleted(t *testing.T) {
 	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
+	h := NewWorkspaceHandler(nil, nil, "", "")
 	r.GET("/workspaces/:id/state", h.State)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspace_auth_tokens`).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
 	mock.ExpectQuery(`SELECT status FROM workspaces WHERE id = \$1`).
 		WithArgs(wsID).
 		WillReturnRows(sqlmock.NewRows([]string{"status"}).AddRow("removed"))
@@ -159,13 +159,13 @@ func TestState_WorkspaceSoftDeleted(t *testing.T) {
 
 func TestState_QueryError(t *testing.T) {
 	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
+	h := NewWorkspaceHandler(nil, nil, "", "")
 	r.GET("/workspaces/:id/state", h.State)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
-	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspace_auth_tokens`).
-		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
 	mock.ExpectQuery(`SELECT status FROM workspaces WHERE id = \$1`).
 		WithArgs(wsID).
 		WillReturnError(sql.ErrConnDone)
@@ -182,17 +182,17 @@ func TestState_QueryError(t *testing.T) {
 // ---------- Update ----------
 
 func TestUpdate_InvalidUUID(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	_, _ = setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
 
 	body := map[string]interface{}{"name": "Test"}
 	b, _ := json.Marshal(body)
 	req, _ := http.NewRequest("PATCH", "/workspaces/not-a-uuid", bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
@@ -200,15 +200,15 @@ func TestUpdate_InvalidUUID(t *testing.T) {
 }
 
 func TestUpdate_InvalidBody(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	_, _ = setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
 
 	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader([]byte("not json")))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400, got %d", w.Code)
@@ -216,10 +216,10 @@ func TestUpdate_InvalidBody(t *testing.T) {
 }
 
 func TestUpdate_WorkspaceNotFound(t *testing.T) {
-	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	mock, _ := setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
@@ -232,7 +232,7 @@ func TestUpdate_WorkspaceNotFound(t *testing.T) {
 	req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID, bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusNotFound {
 		t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String())
@@ -240,10 +240,10 @@ func TestUpdate_WorkspaceNotFound(t *testing.T) {
 }
 
 func TestUpdate_NameTooLong(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	_, _ = setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
 
 	longName := make([]byte, 256)
 	for i := range longName {
@@ -254,7 +254,7 @@ func TestUpdate_NameTooLong(t *testing.T) {
 	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400 for name too long, got %d: %s", w.Code, w.Body.String())
@@ -262,10 +262,10 @@ func TestUpdate_NameTooLong(t *testing.T) {
 }
 
 func TestUpdate_RoleTooLong(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	_, _ = setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
 
 	longRole := make([]byte, 1001)
 	for i := range longRole {
@@ -276,7 +276,7 @@ func TestUpdate_RoleTooLong(t *testing.T) {
 	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400 for role too long, got %d: %s", w.Code, w.Body.String())
@@ -284,17 +284,17 @@ func TestUpdate_RoleTooLong(t *testing.T) {
 }
 
 func TestUpdate_NameWithNewline(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	_, _ = setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
 
 	body := map[string]interface{}{"name": "Name\nwith newline"}
 	b, _ := json.Marshal(body)
 	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400 for newline in name, got %d: %s", w.Code, w.Body.String())
@@ -302,17 +302,17 @@ func TestUpdate_NameWithNewline(t *testing.T) {
 }
 
 func TestUpdate_NameWithYAMLSpecialChars(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	_, _ = setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
 
 	body := map[string]interface{}{"name": "Name with [brackets]"}
 	b, _ := json.Marshal(body)
 	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400 for YAML special chars in name, got %d: %s", w.Code, w.Body.String())
@@ -320,17 +320,31 @@ func TestUpdate_NameWithYAMLSpecialChars(t *testing.T) {
 }
 
 func TestUpdate_WorkspaceDirSystemPath(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	mock, _ := setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
+
+	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
+	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1\)`).
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
+	mock.ExpectExec(`UPDATE workspaces SET name =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`UPDATE workspaces SET role =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`UPDATE workspaces SET tier =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
 
 	body := map[string]interface{}{"workspace_dir": "/etc/my-workspace"}
 	b, _ := json.Marshal(body)
-	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
+	req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID, bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400 for system path workspace_dir, got %d: %s", w.Code, w.Body.String())
@@ -338,17 +352,31 @@ func TestUpdate_WorkspaceDirSystemPath(t *testing.T) {
 }
 
 func TestUpdate_WorkspaceDirTraversal(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	mock, _ := setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
+
+	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
+	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1\)`).
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
+	mock.ExpectExec(`UPDATE workspaces SET name =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`UPDATE workspaces SET role =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`UPDATE workspaces SET tier =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
 
 	body := map[string]interface{}{"workspace_dir": "/workspace/../../../etc"}
 	b, _ := json.Marshal(body)
-	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
+	req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID, bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400 for traversal in workspace_dir, got %d: %s", w.Code, w.Body.String())
@@ -356,17 +384,31 @@ func TestUpdate_WorkspaceDirTraversal(t *testing.T) {
 }
 
 func TestUpdate_WorkspaceDirRelativePath(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.PATCH("/workspaces/:id", h.Update)
+	mock, _ := setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.PATCH("/workspaces/:id", h.Update)
+
+	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
+	mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1\)`).
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
+	mock.ExpectExec(`UPDATE workspaces SET name =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`UPDATE workspaces SET role =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`UPDATE workspaces SET tier =`).
+		WithArgs(wsID).
+		WillReturnResult(sqlmock.NewResult(0, 1))
 
 	body := map[string]interface{}{"workspace_dir": "relative/path"}
 	b, _ := json.Marshal(body)
-	req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
+	req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID, bytes.NewReader(b))
 	req.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400 for relative workspace_dir, got %d: %s", w.Code, w.Body.String())
@@ -376,14 +418,14 @@ func TestUpdate_WorkspaceDirRelativePath(t *testing.T) {
 // ---------- Delete ----------
 
 func TestDelete_InvalidUUID(t *testing.T) {
-	_, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.DELETE("/workspaces/:id", h.Delete)
+	_, _ = setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.DELETE("/workspaces/:id", h.Delete)
 
 	req, _ := http.NewRequest("DELETE", "/workspaces/not-a-uuid", nil)
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusBadRequest {
 		t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
@@ -391,10 +433,10 @@ func TestDelete_InvalidUUID(t *testing.T) {
 }
 
 func TestDelete_HasChildrenWithoutConfirm(t *testing.T) {
-	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.DELETE("/workspaces/:id", h.Delete)
+	mock, _ := setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.DELETE("/workspaces/:id", h.Delete)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
@@ -406,7 +448,7 @@ func TestDelete_HasChildrenWithoutConfirm(t *testing.T) {
 	req, _ := http.NewRequest("DELETE", "/workspaces/"+wsID, nil)
 	// No ?confirm=true
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusConflict {
 		t.Errorf("expected 409, got %d: %s", w.Code, w.Body.String())
@@ -425,10 +467,10 @@ func TestDelete_HasChildrenWithoutConfirm(t *testing.T) {
 }
 
 func TestDelete_ChildrenCheckQueryError(t *testing.T) {
-	mock, r := setupWorkspaceCrudTest(t)
-	h := NewWorkspaceHandler(nil, nil, nil, nil)
-	r2 := gin.New()
-	r2.DELETE("/workspaces/:id", h.Delete)
+	mock, _ := setupWorkspaceCrudTest(t)
+	h := NewWorkspaceHandler(nil, nil, "", "")
+	r := gin.New()
+	r.DELETE("/workspaces/:id", h.Delete)
 
 	wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 
@@ -438,7 +480,7 @@ func TestDelete_ChildrenCheckQueryError(t *testing.T) {
 
 	req, _ := http.NewRequest("DELETE", "/workspaces/"+wsID, nil)
 	w := httptest.NewRecorder()
-	r2.ServeHTTP(w, req)
+	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusInternalServerError {
 		t.Errorf("expected 500, got %d", w.Code)

workspace-server/internal/handlers/workspace_crud_validators_test.go

@@ -4,68 +4,8 @@ import (
 	"testing"
 )
 
-// ── validateWorkspaceID ─────────────────────────────────────────────────────────
-
-func TestValidateWorkspaceID_Valid(t *testing.T) {
-	cases := []string{
-		"550e8400-e29b-41d4-a716-446655440000",
-		"00000000-0000-0000-0000-000000000000",
-		"ffffffff-ffff-ffff-ffff-ffffffffffff",
-	}
-	for _, id := range cases {
-		t.Run(id, func(t *testing.T) {
-			if err := validateWorkspaceID(id); err != nil {
-				t.Errorf("validateWorkspaceID(%q) returned error: %v", id, err)
-			}
-		})
-	}
-}
-
-func TestValidateWorkspaceID_Invalid(t *testing.T) {
-	cases := []struct {
-		name string
-		id   string
-	}{
-		{"empty", ""},
-		{"not a UUID", "not-a-uuid"},
-		{"traversal attack", "../../etc/passwd"},
-		{"SQL injection", "'; DROP TABLE workspaces;--"},
-		{"UUID too short", "550e8400-e29b-41d4-a716"},
-		{"UUID with invalid hex chars", "550e8400-e29b-41d4-a716-44665544000g"},
-		// Note: "UUID all zeros" (nil UUID) is accepted by google/uuid.Parse
-		// as a valid RFC 4122 nil UUID, so it passes validateWorkspaceID.
-		// If nil UUIDs should be rejected, validateWorkspaceID must be updated.
-	}
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			if err := validateWorkspaceID(tc.id); err == nil {
-				t.Errorf("validateWorkspaceID(%q): expected error, got nil", tc.id)
-			}
-		})
-	}
-}
-
 // ── validateWorkspaceDir ───────────────────────────────────────────────────────
 
-func TestValidateWorkspaceDir_Valid(t *testing.T) {
-	cases := []string{
-		"/opt/molecule/workspaces/dev",
-		"/home/user/.molecule/workspaces",
-		// Note: /var/data/workspace-abc-123 is NOT in this list because
-		// /var is blocked as a system path prefix — /var/data is correctly
-		// rejected by validateWorkspaceDir. Use /tmp or /srv for non-system paths.
-		"/opt/services/molecule/tenant-workspaces",
-		"/tmp/molecule/workspaces/dev",
-	}
-	for _, dir := range cases {
-		t.Run(dir, func(t *testing.T) {
-			if err := validateWorkspaceDir(dir); err != nil {
-				t.Errorf("validateWorkspaceDir(%q) returned error: %v", dir, err)
-			}
-		})
-	}
-}
-
 func TestValidateWorkspaceDir_RelativeRejected(t *testing.T) {
 	cases := []string{
 		"relative/path",
@@ -150,41 +90,6 @@ func TestValidateWorkspaceFields_AllEmpty(t *testing.T) {
 	}
 }
 
-func TestValidateWorkspaceFields_Valid(t *testing.T) {
-	if err := validateWorkspaceFields("My Workspace", "Backend Engineer", "gpt-4o", "langgraph"); err != nil {
-		t.Errorf("validateWorkspaceFields with valid args: expected nil, got %v", err)
-	}
-}
-
-func TestValidateWorkspaceFields_NameTooLong(t *testing.T) {
-	longName := make([]byte, 256)
-	for i := range longName {
-		longName[i] = 'a'
-	}
-	if err := validateWorkspaceFields(string(longName), "", "", ""); err == nil {
-		t.Error("name > 255 chars: expected error, got nil")
-	}
-
-	// Exactly 255 chars is OK
-	validName := make([]byte, 255)
-	for i := range validName {
-		validName[i] = 'a'
-	}
-	if err := validateWorkspaceFields(string(validName), "", "", ""); err != nil {
-		t.Errorf("name exactly 255 chars: expected nil, got %v", err)
-	}
-}
-
-func TestValidateWorkspaceFields_RoleTooLong(t *testing.T) {
-	longRole := make([]byte, 1001)
-	for i := range longRole {
-		longRole[i] = 'x'
-	}
-	if err := validateWorkspaceFields("", string(longRole), "", ""); err == nil {
-		t.Error("role > 1000 chars: expected error, got nil")
-	}
-}
-
 func TestValidateWorkspaceFields_ModelTooLong(t *testing.T) {
 	longModel := make([]byte, 101)
 	for i := range longModel {
@@ -205,12 +110,6 @@ func TestValidateWorkspaceFields_RuntimeTooLong(t *testing.T) {
 	}
 }
 
-func TestValidateWorkspaceFields_NewlineInName(t *testing.T) {
-	if err := validateWorkspaceFields("My\nWorkspace", "", "", ""); err == nil {
-		t.Error("name with \\n: expected error, got nil")
-	}
-}
-
 func TestValidateWorkspaceFields_CRLFInRole(t *testing.T) {
 	if err := validateWorkspaceFields("", "Backend\r\nEngineer", "", ""); err == nil {
 		t.Error("role with \\r\\n: expected error, got nil")