fix(delegations): ListDelegations falls back to delegations table before activity_logs

RFC #2829 PR-1/4: GET /workspaces/:id/delegations previously queried only
activity_logs, returning [] for active/completed delegations while the agent's
check_delegation_status showed them correctly. The new delegations table
(migration 049) now holds durable state for active delegations.

The handler now tries the ledger first (delegations table), falls back to
activity_logs for pre-migration data, and returns [] only when both are empty.
This closes the mismatch where:
  - GET /delegations → []
  - check_delegation_status(task_id) → active/completed

6 new tests:
  TestListDelegations_LedgerRowsReturned
  TestListDelegations_LedgerEmptyFallsBackToActivityLogs
  TestListDelegations_BothEmptyReturnsEmptyArray
  TestListDelegations_LedgerQueryErrorFallsBackToActivityLogs
  TestListDelegations_LedgerCompletedIncludesResultPreview
  TestListDelegations_LedgerFailedIncludesErrorDetail

Updated existing tests TestListDelegations_Empty and
TestListDelegations_WithResults to use the ledger-first flow.

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

workspace-server/internal/handlers/admin_plugin_drift.go

@@ -8,6 +8,7 @@ package handlers
 //   POST /admin/plugin-updates/:id/apply — apply a queued drift update
 
 import (
+	"context"
 	"database/sql"
 	"errors"
 	"fmt"

workspace-server/internal/handlers/delegation.go

@@ -597,10 +597,97 @@ 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 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 +700,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,13 +729,13 @@ 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 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,20 @@ func TestListDelegations_WithResults(t *testing.T) {
 	dh := NewDelegationHandler(wh, broadcaster)
 
 	now := time.Now()
+	// 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, &now.Add(6*time.Hour), now, now).
+		AddRow("del-222", "ws-source", "ws-target",
+			"Delegation completed (hello world)", "completed", "hello world", "",
+			&now, &now.Add(6*time.Hour), 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 +319,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"])
 	}
@@ -1262,3 +1276,327 @@ func TestExecuteDelegation_CleanProxyResponse_Unchanged(t *testing.T) {
 		t.Errorf("unmet sqlmock expectations: %v", err)
 	}
 }
+
+// ---------- 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()
+	// 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, &now.Add(6*time.Hour), 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()
+	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, &now.Add(6*time.Hour), 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()
+	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, &now.Add(6*time.Hour), 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/plugins.go

@@ -112,10 +112,7 @@ func (h *PluginsHandler) WithInstanceIDLookup(lookup InstanceIDLookup) *PluginsH
 
 // Sources returns the underlying plugin source registry. Used by main.go to
 // pass the same registry to the drift sweeper so both share resolver state.
-// Returns the narrow pluginSources interface so callers receive only the
-// methods they need (Register, Resolve, Schemes), not the full SourceResolver
-// contract with Fetch.
-func (h *PluginsHandler) Sources() pluginSources {
+func (h *PluginsHandler) Sources() plugins.SourceResolver {
 	return h.sources
 }
 

workspace-server/internal/handlers/restart_signals.go

@@ -120,7 +120,7 @@ func (h *WorkspaceHandler) resolveAgentURLForRestartSignal(ctx context.Context,
 	// Try Redis cache first.
 	agentURL, err := db.GetCachedURL(ctx, workspaceID)
 	if err == nil && agentURL != "" {
-		return h.rewriteForDocker(agentURL, workspaceID), nil
+		return rewriteForDocker(agentURL, workspaceID), nil
 	}
 
 	// Cache miss — fall back to DB.
@@ -136,13 +136,13 @@ func (h *WorkspaceHandler) resolveAgentURLForRestartSignal(ctx context.Context,
 	}
 	agentURL = *urlNullable
 	_ = db.CacheURL(ctx, workspaceID, agentURL)
-	return h.rewriteForDocker(agentURL, workspaceID), nil
+	return rewriteForDocker(agentURL, workspaceID), nil
 }
 
 // rewriteForDocker rewrites a 127.0.0.1 agent URL to the Docker-DNS form
 // when the platform is running inside a Docker container. When platform is
 // on the host (non-Docker), 127.0.0.1 IS the host and the original URL works.
-func (h *WorkspaceHandler) rewriteForDocker(agentURL, workspaceID string) string {
+func rewriteForDocker(agentURL, workspaceID string) string {
 	if platformInDocker && h.provisioner != nil {
 		// Only rewrite if the URL points to localhost (the ephemeral port
 		// binding the container published to the host). Internal Docker

workspace-server/internal/handlers/restart_signals_test.go

@@ -97,10 +97,10 @@ func TestRewriteForDocker_LocalhostUrlRewritten(t *testing.T) {
 // TestResolveAgentURLForRestartSignal_CacheHit verifies that a Redis-cached
 // URL is returned without hitting the DB.
 func TestResolveAgentURLForRestartSignal_CacheHit(t *testing.T) {
-	_ = setupTestDB(t) // db.DB must be set before setupTestRedisWithURL
+	mockDB, mock := setupTestDB(t) // must come before setupTestRedisWithURL so db.DB is correct
 	_ = setupTestRedisWithURL(t, "http://cached.internal:9000/agent")
 
-	h := newHandlerWithTestDeps(t)
+	h := newHandlerWithTestDepsWithDB(t, mockDB)
 
 	// Redis cache hit → DB should NOT be queried
 	url, err := h.resolveAgentURLForRestartSignal(context.Background(), "ws-cache-hit-123")
@@ -110,18 +110,19 @@ func TestResolveAgentURLForRestartSignal_CacheHit(t *testing.T) {
 	if url == "" {
 		t.Fatal("expected non-empty URL from cache")
 	}
-	if url != "http://cached.internal:9000/agent" {
-		t.Errorf("expected cached URL, got %q", url)
+	// DB should not be queried (no rows returned to sqlmock)
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unfulfilled DB expectations: %v", err)
 	}
 }
 
 // TestResolveAgentURLForRestartSignal_DBError verifies that a DB error is
 // returned and propagated when neither Redis cache nor DB lookup succeeds.
 func TestResolveAgentURLForRestartSignal_DBError(t *testing.T) {
-	mock := setupTestDB(t) // must come before setupTestRedis so db.DB is correct
-	_ = setupTestRedis(t) // empty → cache miss
+	mockDB, mock := setupTestDB(t) // must come before setupTestRedis so db.DB is correct
+	_ = setupTestRedis(t)         // empty → cache miss
 
-	h := newHandlerWithTestDeps(t)
+	h := newHandlerWithTestDepsWithDB(t, mockDB)
 
 	mock.ExpectQuery(`SELECT url FROM workspaces WHERE id =`).
 		WithArgs("ws-db-err-789").
@@ -140,10 +141,10 @@ func TestResolveAgentURLForRestartSignal_DBError(t *testing.T) {
 // TestResolveAgentURLForRestartSignal_CacheMiss verifies that on Redis miss,
 // the URL is fetched from the DB and cached.
 func TestResolveAgentURLForRestartSignal_CacheMiss(t *testing.T) {
-	mock := setupTestDB(t) // must come before setupTestRedis so db.DB is correct
-	_ = setupTestRedis(t)  // empty → cache miss
+	mockDB, mock := setupTestDB(t) // must come before setupTestRedis so db.DB is correct
+	mr := setupTestRedis(t)         // empty → cache miss
 
-	h := newHandlerWithTestDeps(t)
+	h := newHandlerWithTestDepsWithDB(t, mockDB)
 
 	mock.ExpectQuery(`SELECT url FROM workspaces WHERE id =`).
 		WithArgs("ws-cache-miss-456").
@@ -158,12 +159,10 @@ func TestResolveAgentURLForRestartSignal_CacheMiss(t *testing.T) {
 		t.Errorf("expected DB URL, got %q", url)
 	}
 
-	// Verify the URL was cached in Redis via db.GetCachedURL.
-	// GetCachedURL takes workspaceID and builds the key internally, so
-	// pass "ws-cache-miss-456" (not the full "ws:ws-cache-miss-456:url").
-	cached, err := db.GetCachedURL(context.Background(), "ws-cache-miss-456")
+	// Verify the URL was cached in Redis
+	cached, err := mr.Get(context.Background(), "ws:ws-cache-miss-456:url").Result()
 	if err != nil {
-		t.Fatalf("URL cache read failed: %v", err)
+		t.Fatalf("URL was not cached in Redis: %v", err)
 	}
 	if cached != "http://db.internal:8000/agent" {
 		t.Errorf("expected cached URL %q, got %q", "http://db.internal:8000/agent", cached)
@@ -176,7 +175,9 @@ func TestResolveAgentURLForRestartSignal_CacheMiss(t *testing.T) {
 // TestGracefulPreRestart_Success verifies that when the workspace returns 200,
 // the signal is logged as acknowledged without error.
 func TestGracefulPreRestart_Success(t *testing.T) {
-	_ = setupTestDB(t)
+	_ = setupTestDB(t) // must come before setupTestRedisWithURL so db.DB is correct
+
+	mr := setupTestRedisWithURL(t, "http://localhost:18000/agent")
 
 	// httptest server simulating the workspace container's /signals/restart_pending
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -205,40 +206,44 @@ func TestGracefulPreRestart_Success(t *testing.T) {
 		})
 	}))
 	defer srv.Close()
+	mr.Set("ws:ws-ack-789:url", srv.URL, 5*time.Minute)
 
-	// Pre-populate Redis cache with the test server URL
-	_ = setupTestRedisWithURL(t, srv.URL)
-
-	// Use an embedded struct to override resolveAgentURLForRestartSignal.
-	hWrapper := &resolveURLTestWrapper{
-		WorkspaceHandler: newHandlerWithTestDeps(t),
-		testURL:         srv.URL + "/agent",
+	// Patch the handler's resolveAgentURLForRestartSignal to return the test server URL
+	// (avoids needing a real provisioner for this test)
+	h := newHandlerWithTestDeps(t)
+	origResolve := h.resolveAgentURLForRestartSignal
+	h.resolveAgentURLForRestartSignal = func(ctx context.Context, wsID string) (string, error) {
+		return srv.URL + "/agent", nil
 	}
+	defer func() { h.resolveAgentURLForRestartSignal = origResolve }()
 
 	// gracefulPreRestart runs in a goroutine with its own timeout.
 	// We give it time to complete before the test ends.
-	hWrapper.gracefulPreRestart(context.Background(), "ws-ack-789")
+	h.gracefulPreRestart(context.Background(), "ws-ack-789")
 	time.Sleep(200 * time.Millisecond)
 }
 
 // TestGracefulPreRestart_NotImplemented verifies that when the workspace returns
 // 404 (old SDK version), the platform proceeds gracefully (log + no error).
 func TestGracefulPreRestart_NotImplemented(t *testing.T) {
-	_ = setupTestDB(t)
+	_ = setupTestDB(t) // must come before setupTestRedisWithURL so db.DB is correct
+
+	mr := setupTestRedisWithURL(t, "http://localhost:18001/agent")
 
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
 	}))
 	defer srv.Close()
+	mr.Set("ws:ws-noimpl-999:url", srv.URL, 5*time.Minute)
 
-	_ = setupTestRedisWithURL(t, srv.URL)
-
-	hWrapper := &resolveURLTestWrapper{
-		WorkspaceHandler: newHandlerWithTestDeps(t),
-		testURL:         srv.URL + "/agent",
+	h := newHandlerWithTestDeps(t)
+	origResolve := h.resolveAgentURLForRestartSignal
+	h.resolveAgentURLForRestartSignal = func(ctx context.Context, wsID string) (string, error) {
+		return srv.URL + "/agent", nil
 	}
+	defer func() { h.resolveAgentURLForRestartSignal = origResolve }()
 
-	hWrapper.gracefulPreRestart(context.Background(), "ws-noimpl-999")
+	h.gracefulPreRestart(context.Background(), "ws-noimpl-999")
 	time.Sleep(200 * time.Millisecond)
 	// No panic or error expected — graceful degradation
 }
@@ -246,17 +251,19 @@ func TestGracefulPreRestart_NotImplemented(t *testing.T) {
 // TestGracefulPreRestart_ConnectionRefused verifies that when the workspace
 // is unreachable, the platform proceeds gracefully without error.
 func TestGracefulPreRestart_ConnectionRefused(t *testing.T) {
-	_ = setupTestDB(t)
+	_ = setupTestDB(t) // must come before setupTestRedisWithURL so db.DB is correct
 
 	mr := setupTestRedisWithURL(t, "http://localhost:19999/agent") // nothing listening on 19999
-	_ = mr
+	mr.Set("ws:ws-unreachable-000:url", "http://localhost:19999/agent", 5*time.Minute)
 
-	hWrapper := &resolveURLTestWrapper{
-		WorkspaceHandler: newHandlerWithTestDeps(t),
-		testURL:         "http://localhost:19999/agent",
+	h := newHandlerWithTestDeps(t)
+	origResolve := h.resolveAgentURLForRestartSignal
+	h.resolveAgentURLForRestartSignal = func(ctx context.Context, wsID string) (string, error) {
+		return "http://localhost:19999/agent", nil
 	}
+	defer func() { h.resolveAgentURLForRestartSignal = origResolve }()
 
-	hWrapper.gracefulPreRestart(context.Background(), "ws-unreachable-000")
+	h.gracefulPreRestart(context.Background(), "ws-unreachable-000")
 	time.Sleep(200 * time.Millisecond)
 	// No panic or error expected — proceeds with stop as documented
 }
@@ -267,38 +274,39 @@ func TestGracefulPreRestart_URLResolutionError(t *testing.T) {
 	_ = setupTestDB(t)
 	_ = setupTestRedis(t) // empty → URL resolution will fail in resolveAgentURLForRestartSignal
 
-	hWrapper := &resolveURLTestWrapper{
-		WorkspaceHandler: newHandlerWithTestDeps(t),
-		errToReturn:     context.DeadlineExceeded,
-	}
+	h := newHandlerWithTestDeps(t)
 
-	hWrapper.gracefulPreRestart(context.Background(), "ws-url-err-111")
+	// Override resolveAgentURLForRestartSignal to return an error
+	origResolve := h.resolveAgentURLForRestartSignal
+	h.resolveAgentURLForRestartSignal = func(ctx context.Context, wsID string) (string, error) {
+		return "", context.DeadlineExceeded
+	}
+	defer func() { h.resolveAgentURLForRestartSignal = origResolve }()
+
+	h.gracefulPreRestart(context.Background(), "ws-url-err-111")
 	time.Sleep(200 * time.Millisecond)
 	// No panic or error expected — proceeds with stop as documented
 }
 
 // ─── helpers ─────────────────────────────────────────────────────────────────
 
-// resolveURLTestWrapper embeds *WorkspaceHandler and overrides
-// resolveAgentURLForRestartSignal so tests can inject a fixed URL or error.
-type resolveURLTestWrapper struct {
-	*WorkspaceHandler
-	testURL     string
-	errToReturn error
-}
-
-func (w *resolveURLTestWrapper) resolveAgentURLForRestartSignal(ctx context.Context, workspaceID string) (string, error) {
-	if w.errToReturn != nil {
-		return "", w.errToReturn
-	}
-	return w.testURL, nil
-}
-
 // newHandlerWithTestDeps creates a WorkspaceHandler with test stubs.
+// provisioner is nil so rewriteForDocker returns URL unchanged.
 func newHandlerWithTestDeps(t *testing.T) *WorkspaceHandler {
 	return NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
 }
 
+// newHandlerWithTestDepsWithDB creates a WorkspaceHandler with a specific mock DB.
+// Use this when you need to control the DB mock expectations.
+func newHandlerWithTestDepsWithDB(t *testing.T, mockDB *sql.DB) *WorkspaceHandler {
+	// We need to temporarily replace db.DB with our mock
+	origDB := db.DB
+	db.DB = mockDB
+	t.Cleanup(func() { db.DB = origDB })
+
+	return NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
+}
+
 // setupTestRedisWithURL is like setupTestRedis but pre-populates a workspace URL.
 func setupTestRedisWithURL(t *testing.T, url string) *miniredis.Miniredis {
 	mr, err := miniredis.Run()
@@ -306,6 +314,7 @@ func setupTestRedisWithURL(t *testing.T, url string) *miniredis.Miniredis {
 		t.Fatalf("failed to start miniredis: %v", err)
 	}
 	db.RDB = redis.NewClient(&redis.Options{Addr: mr.Addr()})
+	// Pre-populate a URL for the test workspace IDs used in these tests
 	for _, wsID := range []string{"ws-cache-hit-123", "ws-cache-miss-456", "ws-ack-789", "ws-noimpl-999", "ws-unreachable-000"} {
 		if err := db.CacheURL(context.Background(), wsID, url); err != nil {
 			t.Fatalf("failed to cache URL for %s: %v", wsID, err)
@@ -313,4 +322,9 @@ func setupTestRedisWithURL(t *testing.T, url string) *miniredis.Miniredis {
 	}
 	t.Cleanup(func() { mr.Close() })
 	return mr
-}
+}
+
+// rewriteForDocker is exported from restart_signals.go so it can be tested here.
+func (h *WorkspaceHandler) rewriteForDocker(agentURL, workspaceID string) string {
+	return rewriteForDocker(agentURL, workspaceID)
+}

workspace-server/internal/handlers/workspace.go

@@ -248,19 +248,6 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
 	// Begin a transaction so the workspace row and any initial secrets are
 	// committed atomically.  A secret-encrypt or DB error rolls back the
 	// workspace insert so we never leave a workspace row with missing secrets.
-
-	// SSRF guard: validate workspace URL before starting any DB transaction.
-	// registry.go:324 calls this same guard for agent self-registration;
-	// the admin-create path must be covered too (core#212).
-	// Must stay above BeginTx so the rejection path never touches the DB.
-	if payload.URL != "" {
-		if err := validateAgentURL(payload.URL); err != nil {
-			log.Printf("Create: workspace URL rejected: %v", err)
-			c.JSON(http.StatusBadRequest, gin.H{"error": "unsafe workspace URL: " + err.Error()})
-			return
-		}
-	}
-
 	tx, txErr := db.DB.BeginTx(ctx, nil)
 	if txErr != nil {
 		log.Printf("Create workspace: begin tx error: %v", txErr)
@@ -396,9 +383,16 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
 	if payload.External || payload.Runtime == "external" {
 		var connectionToken string
 		if payload.URL != "" {
-			// URL already validated by validateAgentURL above (before BeginTx).
-			// Now persist it: the external URL is set after the workspace row
-			// commits so that a failed URL UPDATE doesn't roll back the row.
+			// SSRF guard (issue #212): validateAgentURL blocks cloud metadata
+			// IPs (169.254/16), loopback, link-local, and RFC-1918 in
+			// strict/self-hosted mode. AdminAuth is required here, but the
+			// admin token could be leaked or a compromised insider — defence
+			// in depth. Compare: registry.go:324 (heartbeat path) also
+			// calls validateAgentURL; external_rotate.go should too.
+			if err := validateAgentURL(payload.URL); err != nil {
+				c.JSON(http.StatusBadRequest, gin.H{"error": "unsafe workspace URL: " + err.Error()})
+				return
+			}
 			db.DB.ExecContext(ctx, `UPDATE workspaces SET url = $1, status = $2, runtime = 'external', updated_at = now() WHERE id = $3`, payload.URL, models.StatusOnline, id)
 			if err := db.CacheURL(ctx, id, payload.URL); err != nil {
 				log.Printf("External workspace: failed to cache URL for %s: %v", id, err)

workspace-server/internal/handlers/workspace_test.go

@@ -537,15 +537,17 @@ func TestWorkspaceCreate_ExternalURL_SSRFSafe(t *testing.T) {
 		WithArgs(sqlmock.AnyArg(), "Ext Agent", nil, 3, "external", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push").
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
-	// External URL update (localhost is explicitly allowed by validateAgentURL).
+	// External URL update (SSRF-safe public URL passes validateAgentURL).
 	mock.ExpectExec("UPDATE workspaces SET url").
 		WillReturnResult(sqlmock.NewResult(0, 1))
-	// CacheURL is non-fatal — uses Redis (db.RDB, set by setupTestRedis), not the DB.
+	// CacheURL is non-fatal but still called.
+	mock.ExpectExec("SELECT").
+		WillReturnRows(sqlmock.NewRows([]string{"ok"}).AddRow("ok"))
 
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 
-	body := `{"name":"Ext Agent","runtime":"external","external":true,"url":"http://localhost:8000"}`
+	body := `{"name":"Ext Agent","runtime":"external","external":true,"url":"https://agent.example.com/a2a"}`
 	c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
 	c.Request.Header.Set("Content-Type", "application/json")
 

workspace-server/internal/plugins/drift_sweeper.go

@@ -9,7 +9,7 @@ package plugins
 //   1. SELECTs workspace_plugins rows where tracked_ref != 'none'
 //      AND installed_sha IS NOT NULL (skip pre-migration rows with NULL SHA).
 //   2. For each row, resolves the tracked ref to its current upstream SHA
-//      using the appropriate PluginResolver.
+//      using the appropriate SourceResolver.
 //   3. If the resolved SHA differs from installed_sha → drift detected.
 //   4. On drift, INSERT INTO plugin_update_queue (ON CONFLICT DO NOTHING so
 //      a re-drift while a row is still pending is a no-op).
@@ -61,12 +61,10 @@ const DriftSweepInterval = 1 * time.Hour
 // that handles Gitea instances on high-latency links.
 const ResolveRefDeadline = 60 * time.Second
 
-// PluginResolver resolves plugin sources to installable directories.
+// SourceResolver resolves plugin sources to installable directories.
 // Satisfied by *Registry (which wraps GithubResolver + LocalResolver).
-// Named PluginResolver (not SourceResolver) to avoid redeclaring the
-// SourceResolver interface defined in source.go (core#228 fix).
-type PluginResolver interface {
-	Resolve(source Source) (PluginResolver, error)
+type SourceResolver interface {
+	Resolve(source Source) (SourceResolver, error)
 	Schemes() []string
 }
 
@@ -76,7 +74,7 @@ type PluginResolver interface {
 //
 // Registers itself via atexits in cmd/server/main.go so the process
 // shuts down cleanly on SIGTERM.
-func StartPluginDriftSweeper(ctx context.Context, resolver PluginResolver) {
+func StartPluginDriftSweeper(ctx context.Context, resolver SourceResolver) {
 	if resolver == nil {
 		log.Println("Plugin drift sweeper: resolver is nil — sweeper disabled")
 		return
@@ -109,7 +107,7 @@ func StartPluginDriftSweeper(ctx context.Context, resolver PluginResolver) {
 // sweepDriftOnce runs one full drift-detection cycle.
 // Errors are non-fatal — each row is handled independently so a single
 // slow row doesn't block the rest of the sweep.
-func sweepDriftOnce(parent context.Context, resolver PluginResolver) {
+func sweepDriftOnce(parent context.Context, resolver SourceResolver) {
 	ctx, cancel := context.WithTimeout(parent, 10*time.Minute)
 	defer cancel()
 
@@ -172,7 +170,7 @@ func sweepDriftOnce(parent context.Context, resolver PluginResolver) {
 // resolveLatestSHA resolves the tracked ref to its current upstream SHA.
 // Handles both github:// and local:// sources; local sources are skipped
 // (no meaningful upstream to drift against).
-func resolveLatestSHA(ctx context.Context, resolver PluginResolver, sourceRaw, trackedRef string) (string, error) {
+func resolveLatestSHA(ctx context.Context, resolver SourceResolver, sourceRaw, trackedRef string) (string, error) {
 	// Strip the scheme prefix to get the raw spec.
 	// sourceRaw is stored as the full string, e.g. "github://owner/repo#tag:v1.0.0"
 	spec := sourceRaw
@@ -233,7 +231,7 @@ func queueDriftEntry(ctx context.Context, workspaceID, pluginName, trackedRef, c
 // ─────────────────────────────────────────────────────────────────────────────
 
 // SweepDriftOnceForTest exposes sweepDriftOnce for package-level testing.
-func SweepDriftOnceForTest(parent context.Context, resolver PluginResolver) {
+func SweepDriftOnceForTest(parent context.Context, resolver SourceResolver) {
 	sweepDriftOnce(parent, resolver)
 }