fix(api): map degenerate GetResult to a clean 500 before writing status

Addresses the P1 follow-up on #59: returning an error from
ProjectOrDegraded.MarshalJSON does not "surface" the contract violation —
envelope.WriteJSON discards the encode error after the 200 status and a partial
JSON frame have already been flushed, leaving the client with a truncated,
unparseable 200 (worse than the previous null).

Validate the invariant upstream instead: newGetProjectResponse now returns an
error when the GetResult sets neither Project nor Degraded, and the get handler
maps that to a 500 envelope before any status/body is written. MarshalJSON keeps
the error branch only as an unreachable last-resort backstop, with the comment
corrected to say so. Adds TestProjectsAPI_GetEmptyResultIs500 to lock the
clean-500 behavior.

Refs #59.

Author: i-trytoohard

backend/internal/httpd/controllers/dto.go

@@ -56,26 +56,37 @@ func (p ProjectOrDegraded) MarshalJSON() ([]byte, error) {
 	case p.Project != nil:
 		return json.Marshal(p.Project)
 	default:
-		// Neither variant set — the spec declares `project` as a required
-		// oneOf[Project, Degraded], so emitting `null` would silently breach
-		// the contract. Fail loudly instead: a GetResult with both pointers
-		// nil is a Manager bug, and the encode error surfaces it rather than
-		// shipping an invalid body.
-		return nil, errors.New("controllers: ProjectOrDegraded has neither Project nor Degraded set")
+		// Unreachable in practice: the handler validates the GetResult via
+		// newGetProjectResponse and writes a 500 before committing the 200
+		// status, so this never encodes. Kept as a last-resort backstop —
+		// erroring is still better than emitting a contract-breaking `null`,
+		// though by here the status is already sent, so the real guard is
+		// upstream.
+		return nil, errEmptyProjectOrDegraded
 	}
 }
 
+// errEmptyProjectOrDegraded marks a GetResult that set neither variant — a
+// Manager-contract violation. newGetProjectResponse returns it so the handler
+// can map it to a 500 before any response bytes are written.
+var errEmptyProjectOrDegraded = errors.New("controllers: GetResult has neither Project nor Degraded set")
+
 // JSONSchemaOneOf is read by swaggest's reflector (apispec.Build) to emit the
 // oneOf for this field; it is not used at runtime.
 func (ProjectOrDegraded) JSONSchemaOneOf() []interface{} {
 	return []interface{}{project.Project{}, project.Degraded{}}
 }
 
 // newGetProjectResponse maps the internal GetResult onto the wire envelope —
-// the explicit project→httpd boundary the result type exists for.
-func newGetProjectResponse(res project.GetResult) GetProjectResponse {
+// the explicit project→httpd boundary the result type exists for. It errors
+// when the result sets neither variant, so the handler can return a clean 500
+// BEFORE writing the 200 status rather than flushing a truncated body.
+func newGetProjectResponse(res project.GetResult) (GetProjectResponse, error) {
+	if res.Project == nil && res.Degraded == nil {
+		return GetProjectResponse{}, errEmptyProjectOrDegraded
+	}
 	return GetProjectResponse{
 		Status:  res.Status,
 		Project: ProjectOrDegraded{Project: res.Project, Degraded: res.Degraded},
-	}
+	}, nil
 }

backend/internal/httpd/controllers/projects.go

@@ -101,7 +101,14 @@ func (c *ProjectsController) get(w http.ResponseWriter, r *http.Request) {
 		writeProjectError(w, r, err, http.StatusInternalServerError)
 		return
 	}
-	envelope.WriteJSON(w, http.StatusOK, newGetProjectResponse(got))
+	resp, err := newGetProjectResponse(got)
+	if err != nil {
+		// GetResult set neither variant — a Manager-contract violation. Map it
+		// to a 500 here, before any status/body is written.
+		writeProjectError(w, r, err, http.StatusInternalServerError)
+		return
+	}
+	envelope.WriteJSON(w, http.StatusOK, resp)
 }
 
 func (c *ProjectsController) updateConfig(w http.ResponseWriter, r *http.Request) {

backend/internal/httpd/controllers/projects_test.go

@@ -1,6 +1,7 @@
 package controllers_test
 
 import (
+	"context"
 	"encoding/json"
 	"io"
 	"log/slog"
@@ -13,10 +14,37 @@ import (
 	"testing"
 
 	"github.com/aoagents/agent-orchestrator/backend/internal/config"
+	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
 	"github.com/aoagents/agent-orchestrator/backend/internal/httpd"
 	"github.com/aoagents/agent-orchestrator/backend/internal/project"
 )
 
+// emptyGetManager returns a GetResult that sets neither Project nor Degraded —
+// a Manager-contract violation — so the test can prove the handler answers a
+// clean 500 before writing the 200 status, rather than flushing a truncated
+// body when the discriminated union fails to marshal. Other methods are
+// unused; the embedded nil interface would panic if one were called.
+type emptyGetManager struct{ project.Manager }
+
+func (emptyGetManager) Get(context.Context, domain.ProjectID) (project.GetResult, error) {
+	return project.GetResult{}, nil
+}
+
+// TestProjectsAPI_GetEmptyResultIs500 locks the fix for the discriminated-union
+// invariant: a degenerate GetResult must surface as a parseable 500 envelope,
+// not a 200 with truncated JSON.
+func TestProjectsAPI_GetEmptyResultIs500(t *testing.T) {
+	log := slog.New(slog.NewTextHandler(io.Discard, nil))
+	srv := httptest.NewServer(httpd.NewRouterWithAPI(config.Config{}, log, nil, httpd.APIDeps{
+		Projects: emptyGetManager{},
+	}))
+	t.Cleanup(srv.Close)
+
+	body, status, headers := doRequest(t, srv, "GET", "/api/v1/projects/whatever", "")
+	assertJSON(t, headers)
+	assertErrorCode(t, body, status, http.StatusInternalServerError, "INTERNAL_ERROR")
+}
+
 func newTestServer(t *testing.T) *httptest.Server {
 	t.Helper()
 	log := slog.New(slog.NewTextHandler(io.Discard, nil))