diff --git a/internal/server/agent_cancel_test.go b/internal/server/agent_cancel_test.go index dd04fa6b77c0bd1e6ae1532bcd39e68f61bff9b6..68d1f10132db3fb9f6b1e4251b744c59887f613e 100644 --- a/internal/server/agent_cancel_test.go +++ b/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 diff --git a/internal/server/proto.go b/internal/server/proto.go index f747e98791bbe95c2a510d4eb665abe205f7260c..f388d51bb87490a484bcb06b16e4698058bae134 100644 --- a/internal/server/proto.go +++ b/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):