chore(server,tests): cover async cancellation cleanup behavior

Christian Rocha and Charm Crush created 2 weeks ago

Lock in that prompt cancellation is handled by background completion
events rather than a synchronous HTTP error. This prevents canceled
prompts from being reported as server failures.

Co-Authored-By: Charm Crush <[e-mail]>

Change summary

internal/server/agent_cancel_test.go | 30 ++++++++++++++++++++++++++++++
internal/server/proto.go             | 17 ++++++++---------
2 files changed, 38 insertions(+), 9 deletions(-)

Detailed changes

internal/server/agent_cancel_test.go

@@ -150,6 +150,36 @@ func TestPostAgent_ReturnsOKOnContextCanceled(t *testing.T) {
 		t.Fatal("dispatched run was never entered")
 	}
 	close(coord.release)
+
+	// Wait for the dispatched run to fully return. Backend.runAgent
+	// swallows context.Canceled, so it must not publish a
+	// notify.TypeAgentError. Publishing would dereference the synthetic
+	// workspace's nil notification broker and crash this goroutine,
+	// which is the explicit guard that a cancel produces no top-level
+	// error event.
+	require.Eventually(t, func() bool {
+		return coord.ranCount.Load() == 1
+	}, 2*time.Second, 10*time.Millisecond)
+}
+
+// TestHandleError_ContextCanceledFallsThroughTo500 documents the step 8
+// cleanup: the old context.Canceled special case in handleError was
+// removed because runtime cancellation of an agent run can no longer
+// reach handleError. The agent-prompt handler returns 202 before the run
+// starts (fire-and-forget SendMessage) and Backend.runAgent swallows
+// context.Canceled. Any context.Canceled that still reaches handleError
+// is therefore an unexpected synchronous error and falls through to the
+// default 500 like any other.
+func TestHandleError_ContextCanceledFallsThroughTo500(t *testing.T) {
+	t.Parallel()
+
+	c := &controllerV1{server: &Server{}}
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+
+	c.handleError(rec, req, context.Canceled)
+
+	require.Equal(t, http.StatusInternalServerError, rec.Code)
 }
 
 // TestPostAgent_DetachesRequestContext verifies that the dispatched run

internal/server/proto.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -1035,15 +1034,15 @@ func (c *controllerV1) handleGetWorkspacePermissionsSkip(w http.ResponseWriter,
 
 // handleError maps backend errors to HTTP status codes and writes the
 // JSON error response.
+//
+// Runtime cancellation of an agent run no longer reaches here for the
+// agent-prompt path: SendMessage is fire-and-forget (the handler returns
+// 202 before the run starts) and Backend.runAgent swallows
+// context.Canceled, surfacing the FinishReasonCanceled marker to SSE
+// subscribers instead. The remaining callers pass synchronous backend
+// errors, so context.Canceled gets no special case and would fall through
+// to the default 500 like any other unexpected error.
 func (c *controllerV1) handleError(w http.ResponseWriter, r *http.Request, err error) {
-	// A canceled agent run is not an error from the prompting
-	// client's perspective. The cancellation reaches every SSE
-	// subscriber via the FinishReasonCanceled marker on the assistant
-	// message; the still-open POST should not surface a 500.
-	if errors.Is(err, context.Canceled) {
-		w.WriteHeader(http.StatusOK)
-		return
-	}
 	status := http.StatusInternalServerError
 	switch {
 	case errors.Is(err, backend.ErrWorkspaceNotFound):