mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-05 17:40:11 +02:00
d38da27ed4
* fix(cli): show the server's conflict message instead of the generic 409 template
Every 409 this API returns is a deterministic refusal that names its own fix
("a skill with this name already exists", "set parent_id (--parent) to <id>").
The CLI replaced all of them with a template that says the opposite — that the
state changed underneath you and you should re-fetch and retry. Agents took the
retry hint literally: GH #6264 reports 15+ identical retries over 10 minutes
followed by hours spent chasing an optimistic-concurrency theory that never
existed, and GH #5948 is a second user misdiagnosing the same way. MUL-4417 had
already written the useful message server-side; it just never reached anyone.
Route 409 through the same server-message extraction 400/422 already uses, so
roughly forty hand-written conflict messages across skills, agents, runtimes,
labels, projects and comments become visible by default. A body we cannot
recognize still falls back to the template, so this never dumps a raw response.
extractServerMessage now prefers prose over a bare identifier, because a few
endpoints put a stable code in "error" and the sentence in "message".
MUL-5619
Co-authored-by: multica-agent <github@multica.ai>
* fix(comments): stop telling a wrong --parent that it posted a top-level comment
The reply guard returns one message for two different mistakes. A resumed
session that carries a previous turn's --parent forward (GH #6264) did not ask
for a top-level comment, but is told it did — which sends it looking for a
new-thread opt-in (GH #5383) instead of correcting the parent it already passed.
Split the copy: name the rejected parent when one was supplied, and keep the
existing top-level wording for the parentless case. Both still point at the
trigger comment to use.
MUL-5619
Co-authored-by: multica-agent <github@multica.ai>
* 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>
---------
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
84 lines
3.0 KiB
Go
84 lines
3.0 KiB
Go
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())
|
|
}
|
|
}
|