workspace-server/internal/handlers/approvals.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"context"
 	"encoding/json"
 	"log"
 	"net/http"
@@ -82,19 +83,44 @@ func (h *ApprovalsHandler) Create(c *gin.Context) {
 	c.JSON(http.StatusCreated, gin.H{"approval_id": approvalID, "status": "pending"})
 }
 
-// ListAll handles GET /approvals/pending
-// Returns all pending approvals across all workspaces (for canvas polling).
-// Auto-expires approvals older than 10 minutes.
-func (h *ApprovalsHandler) ListAll(c *gin.Context) {
-	ctx := c.Request.Context()
+// approvalAutoExpireAfter is the age threshold for auto-expiring
+// pending approvals. Both ListAll (org-wide) and List
+// (workspace-scoped) call autoExpireStaleApprovals before their
+// SELECT so a workspace-scoped poll doesn't surface approvals
+// that should have been auto-expired (the asymmetry that
+// Researcher's RCA flagged — workspace-scoped List lacked the
+// expiry that ListAll already had). Shared threshold prevents
+// drift between the two views.
+const approvalAutoExpireAfter = "10 minutes"
 
-	// Auto-expire stale approvals (older than 10 min)
+// autoExpireStaleApprovals runs the shared auto-expiry UPDATE on
+// any pending approval older than approvalAutoExpireAfter.
+// Called by both ListAll and List so a workspace-scoped poll
+// surfaces the same view as the org-wide poll (Researcher RCA:
+// ListAll auto-expired; List did not — same workflow surfaced
+// different state depending on which endpoint the canvas
+// polled). Errors are LOGGED not returned: a transient auto-
+// expire failure must not block the read path (the read is
+// for display; a stale approval still surfaces with the
+// status=pending, which is a non-fatal inconsistency the
+// next call will catch).
+func (h *ApprovalsHandler) autoExpireStaleApprovals(ctx context.Context) {
 	if _, err := db.DB.ExecContext(ctx, `
 		UPDATE approval_requests SET status = 'denied', decided_by = 'auto-expired', decided_at = now()
-		WHERE status = 'pending' AND created_at < now() - interval '10 minutes'
+		WHERE status = 'pending' AND created_at < now() - interval '`+approvalAutoExpireAfter+`'
 	`); err != nil {
 		log.Printf("approvals: auto-expire failed: %v", err)
 	}
+}
+
+// ListAll handles GET /approvals/pending
+// Returns all pending approvals across all workspaces (for canvas polling).
+// Auto-expires approvals older than 10 minutes (shared threshold
+// with List — see approvalAutoExpireAfter).
+func (h *ApprovalsHandler) ListAll(c *gin.Context) {
+	ctx := c.Request.Context()
+
+	h.autoExpireStaleApprovals(ctx)
 
 	rows, err := db.DB.QueryContext(ctx, `
 		SELECT a.id, a.workspace_id, w.name, a.action, a.reason, a.status, a.created_at
@@ -135,10 +161,19 @@ func (h *ApprovalsHandler) ListAll(c *gin.Context) {
 }
 
 // List handles GET /workspaces/:id/approvals
+// Returns the workspace's approvals (pending + decided) for
+// the canvas. Auto-expires pending approvals older than the
+// shared approvalAutoExpireAfter threshold BEFORE the SELECT
+// so a workspace-scoped poll surfaces the same view as the
+// org-wide ListAll (Researcher RCA: the prior asymmetry let
+// a stale approval surface in workspace-scoped polling
+// after the org-wide polling had already auto-expired it).
 func (h *ApprovalsHandler) List(c *gin.Context) {
 	workspaceID := c.Param("id")
 	ctx := c.Request.Context()
 
+	h.autoExpireStaleApprovals(ctx)
+
 	rows, err := db.DB.QueryContext(ctx, `
 		SELECT id, task_id, action, reason, status, decided_by, decided_at, created_at
 		FROM approval_requests WHERE workspace_id = $1

workspace-server/internal/handlers/approvals_test.go

@@ -175,6 +175,49 @@ func TestApprovals_ListAll_WithResults(t *testing.T) {
 
 // ---------- ApprovalsHandler: List ----------
 
+// TestApprovals_List_RunsAutoExpireBeforeSelect is the
+// regression test for the workspace-scoped List auto-expiry
+// fix (Researcher RCA: ListAll auto-expired; List did not —
+// same workflow surfaced different state depending on which
+// endpoint the canvas polled). The test asserts the
+// auto-expire UPDATE runs BEFORE the workspace-scoped SELECT,
+// and that the shared approvalAutoExpireAfter threshold is
+// reflected in the SQL. If a future refactor removes the
+// auto-expiry call from List (re-introducing the asymmetry),
+// this test fails because the UPDATE expectation won't be
+// met before the SELECT runs.
+func TestApprovals_List_RunsAutoExpireBeforeSelect(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	handler := NewApprovalsHandler(newTestBroadcaster())
+
+	// The auto-expire UPDATE must come first. The SQL must
+	// include the shared 10-minute threshold so a future
+	// change to approvalAutoExpireAfter propagates here.
+	mock.ExpectExec("UPDATE approval_requests SET status = 'denied', decided_by = 'auto-expired', decided_at = now\\(\\) WHERE status = 'pending' AND created_at < now\\(\\) - interval '10 minutes'").
+		WillReturnResult(sqlmock.NewResult(0, 0))
+
+	// Then the workspace-scoped SELECT (mock empty so the
+	// test focuses on the UPDATE-then-SELECT ordering).
+	mock.ExpectQuery("SELECT id, task_id, action, reason, status, decided_by, decided_at, created_at FROM approval_requests WHERE workspace_id").
+		WithArgs("ws-1").
+		WillReturnRows(sqlmock.NewRows([]string{"id", "task_id", "action", "reason", "status", "decided_by", "decided_at", "created_at"}))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/ws-1/approvals", nil)
+
+	handler.List(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("sqlmock expectations not met (likely the auto-expire UPDATE was removed or ran AFTER the SELECT): %v", err)
+	}
+}
+
 func TestApprovals_List_ForWorkspace(t *testing.T) {
 	mock := setupTestDB(t)
 	setupTestRedis(t)