mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-25 03:55:27 +02:00
MUL-3341: clear incompatible model on runtime switch
Closes MUL-3341
This commit is contained in:
@@ -1059,6 +1059,7 @@ func (h *Handler) UpdateAgent(w http.ResponseWriter, r *http.Request) {
|
||||
// request doesn't move the agent, we still need to load the *current*
|
||||
// runtime to validate a thinking_level change. Resolve once and reuse.
|
||||
targetRuntimeID := existing.RuntimeID
|
||||
targetProvider := ""
|
||||
if req.RuntimeID != nil {
|
||||
runtimeUUID, ok := parseUUIDOrBadRequest(w, *req.RuntimeID, "runtime_id")
|
||||
if !ok {
|
||||
@@ -1086,6 +1087,7 @@ func (h *Handler) UpdateAgent(w http.ResponseWriter, r *http.Request) {
|
||||
params.RuntimeID = runtime.ID
|
||||
params.RuntimeMode = pgtype.Text{String: runtime.RuntimeMode, Valid: true}
|
||||
targetRuntimeID = runtime.ID
|
||||
targetProvider = runtime.Provider
|
||||
}
|
||||
if req.Visibility != nil {
|
||||
params.Visibility = pgtype.Text{String: *req.Visibility, Valid: true}
|
||||
@@ -1098,6 +1100,13 @@ func (h *Handler) UpdateAgent(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
if req.Model != nil {
|
||||
params.Model = pgtype.Text{String: *req.Model, Valid: true}
|
||||
} else if req.RuntimeID != nil && existing.Model.Valid && agent.ModelKnownIncompatibleWithProvider(targetProvider, existing.Model.String) {
|
||||
// Model is runtime-native. When moving an agent across known provider
|
||||
// families and the caller did not choose a replacement model, clear the
|
||||
// old value so the new runtime falls back to its own default instead of
|
||||
// receiving an obvious foreign model ID (e.g. Claude Code -> Codex).
|
||||
// Unknown/custom model strings are preserved by the helper.
|
||||
params.Model = pgtype.Text{String: "", Valid: true}
|
||||
}
|
||||
|
||||
// thinking_level handling (MUL-2339). Tri-state semantics:
|
||||
@@ -1121,10 +1130,14 @@ func (h *Handler) UpdateAgent(w http.ResponseWriter, r *http.Request) {
|
||||
// Need the target runtime's provider to validate. Re-fetch only when
|
||||
// we haven't already loaded it above (i.e. the request didn't change
|
||||
// runtime_id), to keep the no-change path one DB roundtrip.
|
||||
provider, ok := h.resolveAgentProvider(r, existing.WorkspaceID, targetRuntimeID)
|
||||
if !ok {
|
||||
writeError(w, http.StatusInternalServerError, "failed to resolve runtime for thinking_level validation")
|
||||
return
|
||||
provider := targetProvider
|
||||
if provider == "" {
|
||||
var ok bool
|
||||
provider, ok = h.resolveAgentProvider(r, existing.WorkspaceID, targetRuntimeID)
|
||||
if !ok {
|
||||
writeError(w, http.StatusInternalServerError, "failed to resolve runtime for thinking_level validation")
|
||||
return
|
||||
}
|
||||
}
|
||||
if !agent.IsKnownThinkingValue(provider, value) {
|
||||
writeError(w, http.StatusBadRequest, fmt.Sprintf("thinking_level %q is not a recognised value for runtime %q", value, provider))
|
||||
@@ -1140,10 +1153,14 @@ func (h *Handler) UpdateAgent(w http.ResponseWriter, r *http.Request) {
|
||||
// literal-invalid, never silently coerce. The caller can either
|
||||
// pass `thinking_level: ""` to clear or pick a value valid for the
|
||||
// new runtime.
|
||||
provider, ok := h.resolveAgentProvider(r, existing.WorkspaceID, targetRuntimeID)
|
||||
if !ok {
|
||||
writeError(w, http.StatusInternalServerError, "failed to resolve runtime for thinking_level validation")
|
||||
return
|
||||
provider := targetProvider
|
||||
if provider == "" {
|
||||
var ok bool
|
||||
provider, ok = h.resolveAgentProvider(r, existing.WorkspaceID, targetRuntimeID)
|
||||
if !ok {
|
||||
writeError(w, http.StatusInternalServerError, "failed to resolve runtime for thinking_level validation")
|
||||
return
|
||||
}
|
||||
}
|
||||
if !agent.IsKnownThinkingValue(provider, existing.ThinkingLevel.String) {
|
||||
writeError(w, http.StatusBadRequest, fmt.Sprintf(
|
||||
|
||||
@@ -295,6 +295,117 @@ func TestUpdateAgent_RuntimeSwitch_PreservesValidValueRejectsInvalid(t *testing.
|
||||
})
|
||||
}
|
||||
|
||||
// TestUpdateAgent_RuntimeSwitch_ClearsKnownIncompatibleModel covers the
|
||||
// runtime/model persistence bug from MUL-3341: a runtime_id-only PATCH used
|
||||
// to preserve a provider-native model string, so switching a Claude Code
|
||||
// agent to Codex could leave agent.model = "claude-..." and fail at task
|
||||
// execution. Unknown custom models are intentionally preserved because the
|
||||
// API supports manual entries and cannot prove they are invalid.
|
||||
func TestUpdateAgent_RuntimeSwitch_ClearsKnownIncompatibleModel(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
claudeRuntimeID := createClaudeProviderRuntime(t)
|
||||
codexRuntimeID := createCodexProviderRuntime(t)
|
||||
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(ctx, `DELETE FROM agent WHERE workspace_id = $1 AND name LIKE 'runtime-model-switch-%'`, testWorkspaceID)
|
||||
})
|
||||
|
||||
t.Run("runtime-only switch clears known foreign model", func(t *testing.T) {
|
||||
agentID := createAgentOnRuntimeWithModel(t, "runtime-model-switch-clear", claudeRuntimeID, "claude-sonnet-4-6")
|
||||
body := map[string]any{
|
||||
"runtime_id": codexRuntimeID,
|
||||
}
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID)
|
||||
testHandler.UpdateAgent(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200 switching runtime with incompatible model, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]any
|
||||
_ = json.NewDecoder(w.Body).Decode(&resp)
|
||||
if resp["model"] != "" {
|
||||
t.Errorf("expected model cleared across Claude->Codex runtime switch, got %v", resp["model"])
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("runtime-only switch clears provider-prefixed model not accepted by target", func(t *testing.T) {
|
||||
agentID := createAgentOnRuntimeWithModel(t, "runtime-model-switch-prefixed", claudeRuntimeID, "openai/gpt-4o")
|
||||
body := map[string]any{
|
||||
"runtime_id": codexRuntimeID,
|
||||
}
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID)
|
||||
testHandler.UpdateAgent(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200 switching runtime with provider-prefixed model, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]any
|
||||
_ = json.NewDecoder(w.Body).Decode(&resp)
|
||||
if resp["model"] != "" {
|
||||
t.Errorf("expected provider-prefixed model cleared across runtime switch, got %v", resp["model"])
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("runtime-only switch keeps exact target accepted model", func(t *testing.T) {
|
||||
agentID := createAgentOnRuntimeWithModel(t, "runtime-model-switch-accepted", claudeRuntimeID, "gpt-5.5")
|
||||
body := map[string]any{
|
||||
"runtime_id": codexRuntimeID,
|
||||
}
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID)
|
||||
testHandler.UpdateAgent(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200 preserving exact target accepted model, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]any
|
||||
_ = json.NewDecoder(w.Body).Decode(&resp)
|
||||
if resp["model"] != "gpt-5.5" {
|
||||
t.Errorf("expected exact target model preserved, got %v", resp["model"])
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("explicit replacement model wins during switch", func(t *testing.T) {
|
||||
agentID := createAgentOnRuntimeWithModel(t, "runtime-model-switch-replace", claudeRuntimeID, "claude-sonnet-4-6")
|
||||
body := map[string]any{
|
||||
"runtime_id": codexRuntimeID,
|
||||
"model": "gpt-5.5",
|
||||
}
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID)
|
||||
testHandler.UpdateAgent(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200 with explicit replacement model, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]any
|
||||
_ = json.NewDecoder(w.Body).Decode(&resp)
|
||||
if resp["model"] != "gpt-5.5" {
|
||||
t.Errorf("expected explicit model to be persisted, got %v", resp["model"])
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("unknown custom model is preserved", func(t *testing.T) {
|
||||
agentID := createAgentOnRuntimeWithModel(t, "runtime-model-switch-custom", claudeRuntimeID, "private-lab-model")
|
||||
body := map[string]any{
|
||||
"runtime_id": codexRuntimeID,
|
||||
}
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID)
|
||||
testHandler.UpdateAgent(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200 preserving unknown custom model, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]any
|
||||
_ = json.NewDecoder(w.Body).Decode(&resp)
|
||||
if resp["model"] != "private-lab-model" {
|
||||
t.Errorf("expected unknown custom model preserved, got %v", resp["model"])
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// createCodexProviderRuntime mirrors createClaudeProviderRuntime but for
|
||||
// the codex provider, so runtime-switch tests can exercise a real
|
||||
// cross-provider transition.
|
||||
@@ -370,3 +481,24 @@ func createAgentOnRuntime(t *testing.T, name, runtimeID, level string) string {
|
||||
})
|
||||
return agentID
|
||||
}
|
||||
|
||||
func createAgentOnRuntimeWithModel(t *testing.T, name, runtimeID, model string) string {
|
||||
t.Helper()
|
||||
var agentID string
|
||||
err := testPool.QueryRow(context.Background(), `
|
||||
INSERT INTO agent (
|
||||
workspace_id, name, description, runtime_mode, runtime_config,
|
||||
runtime_id, visibility, max_concurrent_tasks, owner_id,
|
||||
instructions, custom_env, custom_args, model
|
||||
)
|
||||
VALUES ($1, $2, '', 'cloud', '{}'::jsonb, $3, 'private', 1, $4, '', '{}'::jsonb, '[]'::jsonb, $5)
|
||||
RETURNING id
|
||||
`, testWorkspaceID, name, runtimeID, testUserID, model).Scan(&agentID)
|
||||
if err != nil {
|
||||
t.Fatalf("create agent on runtime %s with model %s: %v", runtimeID, model, err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM agent WHERE id = $1`, agentID)
|
||||
})
|
||||
return agentID
|
||||
}
|
||||
|
||||
@@ -172,6 +172,67 @@ func ModelSelectionSupported(providerType string) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// ModelKnownIncompatibleWithProvider reports whether a saved model is a known
|
||||
// mismatch for a target runtime provider. For first-party providers with
|
||||
// maintained static catalogs, compatibility is exact: the model must be one of
|
||||
// the IDs that runtime advertises. Unknown/custom model strings still return
|
||||
// false because the UI and CLI allow manual entries and the server should not
|
||||
// erase values it cannot confidently classify.
|
||||
func ModelKnownIncompatibleWithProvider(providerType, model string) bool {
|
||||
model = strings.TrimSpace(model)
|
||||
if model == "" {
|
||||
return false
|
||||
}
|
||||
|
||||
accepted, ok := acceptedModelIDsForProvider(providerType)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
if accepted[model] {
|
||||
return false
|
||||
}
|
||||
return isRuntimeSpecificModelID(model)
|
||||
}
|
||||
|
||||
func acceptedModelIDsForProvider(providerType string) (map[string]bool, bool) {
|
||||
switch {
|
||||
case providerType == "claude":
|
||||
return modelIDSet(claudeStaticModels()), true
|
||||
case providerType == "codex":
|
||||
return modelIDSet(codexStaticModels()), true
|
||||
case providerType == "gemini":
|
||||
return modelIDSet(geminiStaticModels()), true
|
||||
default:
|
||||
return nil, false
|
||||
}
|
||||
}
|
||||
|
||||
func modelIDSet(models []Model) map[string]bool {
|
||||
out := make(map[string]bool, len(models))
|
||||
for _, m := range models {
|
||||
out[m.ID] = true
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func isRuntimeSpecificModelID(model string) bool {
|
||||
if strings.Contains(model, "/") {
|
||||
return true
|
||||
}
|
||||
return modelHasKnownPrefix(model) ||
|
||||
modelIDSet(claudeStaticModels())[model] ||
|
||||
modelIDSet(codexStaticModels())[model] ||
|
||||
modelIDSet(geminiStaticModels())[model]
|
||||
}
|
||||
|
||||
func modelHasKnownPrefix(model string) bool {
|
||||
return strings.HasPrefix(model, "claude-") ||
|
||||
strings.HasPrefix(model, "gpt-") ||
|
||||
strings.HasPrefix(model, "gemini-") ||
|
||||
strings.HasPrefix(model, "auto-gemini-") ||
|
||||
isOpenAIReasoningSeriesID(model)
|
||||
}
|
||||
|
||||
// cachedDiscovery invokes fn and caches the result for modelCacheTTL.
|
||||
// The cache is keyed on providerType only; callers that need to
|
||||
// distinguish discovery by host/user should include that in the key
|
||||
|
||||
@@ -149,6 +149,78 @@ func TestCodexStaticModelsExposesGPT55(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestModelKnownIncompatibleWithProvider(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
provider string
|
||||
model string
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "claude model is incompatible with codex",
|
||||
provider: "codex",
|
||||
model: "claude-sonnet-4-6",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "codex model is compatible with codex",
|
||||
provider: "codex",
|
||||
model: "gpt-5.5",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "codex model is incompatible with claude",
|
||||
provider: "claude",
|
||||
model: "o3",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "exact claude model is compatible with claude",
|
||||
provider: "claude",
|
||||
model: "claude-opus-4-7",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "provider-prefixed openai model is incompatible with codex",
|
||||
provider: "codex",
|
||||
model: "openai/gpt-4o",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "provider-prefixed anthropic model is incompatible with claude",
|
||||
provider: "claude",
|
||||
model: "anthropic/claude-opus-4.7",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "known openai-looking model outside codex catalog is incompatible",
|
||||
provider: "codex",
|
||||
model: "gpt-99",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "unknown custom model is not classified",
|
||||
provider: "codex",
|
||||
model: "private-lab-model",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "unknown target provider does not clear",
|
||||
provider: "opencode",
|
||||
model: "claude-sonnet-4-6",
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := ModelKnownIncompatibleWithProvider(tc.provider, tc.model); got != tc.want {
|
||||
t.Fatalf("ModelKnownIncompatibleWithProvider(%q, %q) = %v, want %v", tc.provider, tc.model, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestInferCopilotProvider(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"gpt-5.5": "openai",
|
||||
|
||||
Reference in New Issue
Block a user