fix(runtime): return 500, not a 409 echo, when the update store fails

InitiateUpdate answered every UpdateStore.Create failure with a 409 carrying
err.Error(). The in-memory store only ever returns errUpdateInProgress, so this
looked safe — but the Redis store also wraps infrastructure failures as
"reserve active update: <dial error>" and "persist update request: <error>".

Surfacing 409 bodies in the CLI turns that into a user-visible leak of internal
addresses, and labels an outage as a conflict the caller could fix by retrying.
Classify instead: errUpdateInProgress keeps its 409 and its actionable message,
everything else is logged and answered with a 500 and fixed copy.

Also pins the prose-over-machine-code preference for validation bodies, which
the shared extractor applies to 400/422 as well as 409. Only the issue-table
endpoints are shaped that way and none is reachable from the CLI today, but the
change is intentional and should fail loudly if reverted.

MUL-5619

Co-authored-by: multica-agent <github@multica.ai>

server/internal/cli/errors_test.go

@@ -211,6 +211,29 @@ func TestExtractServerMessagePrefersProseOverMachineCode(t *testing.T) {
 	}
 }
 
+// TestFormatErrorValidationPrefersProseOverMachineCode pins an intentional
+// behavior change that rides along with the conflict fix: the prose preference
+// lives in the shared extractor, so a 400/422 whose "error" holds a machine code
+// now shows its "message" instead of the code. Only the issue-table endpoints
+// are shaped this way and none of them is reachable from the CLI today, but the
+// change is deliberate and should fail loudly if someone reverts it by accident.
+func TestFormatErrorValidationPrefersProseOverMachineCode(t *testing.T) {
+	withLang(t, "en_US.UTF-8")
+	got := FormatError(&HTTPError{
+		StatusCode: 422,
+		Body:       `{"error":"unsupported_group","code":"group_kind_unsupported","message":"This group type is not supported."}`,
+	}, false)
+	if !strings.Contains(got, "This group type is not supported.") {
+		t.Errorf("expected the prose message, got %q", got)
+	}
+
+	// A validation body carrying only a code is unchanged.
+	only := FormatError(&HTTPError{StatusCode: 422, Body: `{"error":"title_is_required"}`}, false)
+	if !strings.Contains(only, "title_is_required") {
+		t.Errorf("code-only validation body should still surface the code, got %q", only)
+	}
+}
+
 func TestFormatErrorDebugIncludesRawChain(t *testing.T) {
 	withLang(t, "en_US.UTF-8")
 	httpErr := &HTTPError{Method: "GET", Path: "/api/issues/abc", StatusCode: 404, Body: `{"error":"not found"}`}

server/internal/handler/runtime_update.go

@@ -3,6 +3,7 @@ package handler
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"log/slog"
 	"net/http"
 	"sync"
@@ -249,7 +250,19 @@ func (h *Handler) InitiateUpdate(w http.ResponseWriter, r *http.Request) {
 		uuidToString(member.UserID),
 	)
 	if err != nil {
-		writeError(w, http.StatusConflict, err.Error())
+		// Only the in-progress rejection is a conflict the caller can act on.
+		// Every other Create failure is infrastructure — the Redis store wraps
+		// connection failures as "reserve active update: ..." / "persist update
+		// request: ..." — and echoing it back would both leak internals and
+		// label an outage as a user-fixable conflict. That was survivable while
+		// the CLI hid 409 bodies; it no longer is, now that they are shown by
+		// default.
+		if errors.Is(err, errUpdateInProgress) {
+			writeError(w, http.StatusConflict, err.Error())
+			return
+		}
+		slog.Error("UpdateStore Create failed", "error", err, "runtime_id", uuidToString(rt.ID))
+		writeError(w, http.StatusInternalServerError, "failed to start the update")
 		return
 	}
 

server/internal/handler/runtime_update_error_classification_test.go

@@ -0,0 +1,83 @@
+package handler
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// failingUpdateStore returns a chosen error from Create. The embedded interface
+// supplies the rest of the method set; InitiateUpdate only reaches Create, so
+// anything else calling through would panic loudly rather than pass silently.
+type failingUpdateStore struct {
+	UpdateStore
+	createErr error
+}
+
+func (s *failingUpdateStore) Create(context.Context, string, string, string) (*UpdateRequest, error) {
+	return nil, s.createErr
+}
+
+// TestInitiateUpdate_InfrastructureErrorIsNotAConflict pins the classification
+// split this PR adds alongside GH #6264. InitiateUpdate used to answer every
+// UpdateStore.Create failure with a 409 carrying err.Error(). That was survivable
+// while the CLI hid conflict bodies; now that they print by default, a Redis
+// outage would show its dial address to the user and read as a conflict they
+// could fix by retrying.
+func TestInitiateUpdate_InfrastructureErrorIsNotAConflict(t *testing.T) {
+	if testHandler == nil || testPool == nil {
+		t.Skip("database not available")
+	}
+
+	// Shape mirrors RedisUpdateStore.Create wrapping a dial failure.
+	infraErr := errors.New("reserve active update: dial tcp 10.1.2.3:6379: connect: connection refused")
+
+	original := testHandler.UpdateStore
+	testHandler.UpdateStore = &failingUpdateStore{createErr: infraErr}
+	t.Cleanup(func() { testHandler.UpdateStore = original })
+
+	w := httptest.NewRecorder()
+	r := newRequest("POST", "/api/runtimes/"+testRuntimeID+"/update", map[string]any{"target_version": "v1.2.3"})
+	r = withURLParams(r, "runtimeId", testRuntimeID)
+
+	testHandler.InitiateUpdate(w, r)
+
+	if w.Code != http.StatusInternalServerError {
+		t.Fatalf("infrastructure failure: expected 500, got %d: %s", w.Code, w.Body.String())
+	}
+	body := w.Body.String()
+	for _, leak := range []string{"10.1.2.3:6379", "connection refused", "reserve active update"} {
+		if strings.Contains(body, leak) {
+			t.Fatalf("response leaked internal detail %q: %s", leak, body)
+		}
+	}
+}
+
+// TestInitiateUpdate_InProgressStillConflicts is the positive half: the one
+// Create failure a caller can actually act on keeps its 409 and its actionable
+// wording, which is exactly what the CLI now surfaces by default.
+func TestInitiateUpdate_InProgressStillConflicts(t *testing.T) {
+	if testHandler == nil || testPool == nil {
+		t.Skip("database not available")
+	}
+
+	original := testHandler.UpdateStore
+	testHandler.UpdateStore = &failingUpdateStore{createErr: errUpdateInProgress}
+	t.Cleanup(func() { testHandler.UpdateStore = original })
+
+	w := httptest.NewRecorder()
+	r := newRequest("POST", "/api/runtimes/"+testRuntimeID+"/update", map[string]any{"target_version": "v1.2.3"})
+	r = withURLParams(r, "runtimeId", testRuntimeID)
+
+	testHandler.InitiateUpdate(w, r)
+
+	if w.Code != http.StatusConflict {
+		t.Fatalf("update-in-progress: expected 409, got %d: %s", w.Code, w.Body.String())
+	}
+	if !strings.Contains(w.Body.String(), "already in progress") {
+		t.Fatalf("409 should keep its actionable message, got %s", w.Body.String())
+	}
+}