dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix: stop update_agent_profile from renaming agents when only the model changes (#2849)

Browse Source 
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
nova28
2026-08-21 00:56:02 +08:00
committed by GitHub
parent 34b17ab774
commit e9b6f45381
3 changed files with 207 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/agent/settings/controller/profile_crud.go
-32
View File
@@ -179,11 +179,6 @@ func (c *Controller) UpdateProfile(ctx context.Context, req UpdateProfileRequest
}
if req.Model != nil {
profile.Model = *req.Model
if req.Name == nil {
if newName := c.resolveProfileNameForModel(ctx, profile.AgentID, *req.Model); newName != "" {
profile.Name = newName
}
}
}
if req.FallbackModel != nil {
profile.FallbackModel = strings.TrimSpace(*req.FallbackModel)
@@ -927,30 +922,3 @@ func validateEnvVarValue(ev dto.ProfileEnvVarDTO, i int) error {
}
return nil
}
// resolveProfileNameForModel looks up the agent by ID, fetches its model list (using cache),
// and returns the display name for the given model ID. Returns empty string on failure.
func (c *Controller) resolveProfileNameForModel(ctx context.Context, agentID, modelID string) string {
agent, err := c.repo.GetAgent(ctx, agentID)
if err != nil {
return ""
}
if _, ok := c.agentRegistry.Get(agent.Name); !ok {
return ""
}
// Look up the model's display name from the host utility capability
// cache. If the cache isn't populated yet (probes not finished, agent
// not probed) we fall through to the raw model ID — better than
// blocking the save.
if c.hostUtility != nil {
if caps, ok := c.hostUtility.Get(agent.Name); ok {
for _, m := range caps.Models {
if m.ID == modelID {
return m.Name
}
}
}
}
return modelID
}
apps/backend/internal/agent/settings/controller/profile_crud_test.go
+105
View File
@@ -503,3 +503,108 @@ func TestUpdateProfile_MixedEnabledAndFallbackPersistsBoth(t *testing.T) {
stored.FallbackModel, stored.AutoFallback)
}
}
// TestUpdateProfile_ModelOnlyLeavesNameUnchanged is the regression guard for
// the MCP update_agent_profile silent-rename bug: a model-only patch (the
// shape the MCP tool sends, since it omits name unless explicitly asked for
// one) must not derive and overwrite the profile's name from the model ID.
func TestUpdateProfile_ModelOnlyLeavesNameUnchanged(t *testing.T) {
ctrl := newTestController(map[string]agents.Agent{"test-agent": &testAgent{
id: "test-agent",
name: "test-agent",
displayName: "Test Agent",
enabled: true,
}})
st := newFakeStore()
agent := &models.Agent{ID: "agent-1", Name: "test-agent"}
st.agents[agent.ID] = agent
st.byName[agent.Name] = agent
ctrl.repo = st
profile, err := ctrl.CreateProfile(context.Background(), CreateProfileRequest{
AgentID: "agent-1",
Name: "Product Manager",
Model: "opus",
})
if err != nil {
t.Fatalf("CreateProfile: %v", err)
}
model := "sonnet"
updated, err := ctrl.UpdateProfile(context.Background(), UpdateProfileRequest{
ID: profile.ID,
Model: &model,
})
if err != nil {
t.Fatalf("UpdateProfile: %v", err)
}
if updated.Name != "Product Manager" {
t.Fatalf("response name = %q, want unchanged %q", updated.Name, "Product Manager")
}
if updated.Model != model {
t.Fatalf("response model = %q, want %q", updated.Model, model)
}
stored, err := st.GetAgentProfile(context.Background(), profile.ID)
if err != nil {
t.Fatalf("GetAgentProfile: %v", err)
}
if stored.Name != "Product Manager" {
t.Fatalf("stored name = %q, want unchanged %q", stored.Name, "Product Manager")
}
if stored.AgentDisplayName != "Test Agent" {
t.Fatalf("stored agent_display_name = %q, want unchanged %q", stored.AgentDisplayName, "Test Agent")
}
if stored.Model != model {
t.Fatalf("stored model = %q, want %q", stored.Model, model)
}
}
// TestUpdateProfile_NameOnlyLeavesModelUnchanged pins the other half of the
// acceptance criteria: a name-only patch must not touch the stored model.
func TestUpdateProfile_NameOnlyLeavesModelUnchanged(t *testing.T) {
ctrl := newTestController(map[string]agents.Agent{"test-agent": &testAgent{
id: "test-agent",
name: "test-agent",
displayName: "Test Agent",
enabled: true,
}})
st := newFakeStore()
agent := &models.Agent{ID: "agent-1", Name: "test-agent"}
st.agents[agent.ID] = agent
st.byName[agent.Name] = agent
ctrl.repo = st
profile, err := ctrl.CreateProfile(context.Background(), CreateProfileRequest{
AgentID: "agent-1",
Name: "Product Manager",
Model: "opus",
})
if err != nil {
t.Fatalf("CreateProfile: %v", err)
}
newName := "Renamed"
updated, err := ctrl.UpdateProfile(context.Background(), UpdateProfileRequest{
ID: profile.ID,
Name: &newName,
})
if err != nil {
t.Fatalf("UpdateProfile: %v", err)
}
if updated.Name != newName {
t.Fatalf("response name = %q, want %q", updated.Name, newName)
}
if updated.Model != "opus" {
t.Fatalf("response model = %q, want unchanged %q", updated.Model, "opus")
}
stored, err := st.GetAgentProfile(context.Background(), profile.ID)
if err != nil {
t.Fatalf("GetAgentProfile: %v", err)
}
if stored.Name != newName {
t.Fatalf("stored name = %q, want %q", stored.Name, newName)
}
if stored.Model != "opus" {
t.Fatalf("stored model = %q, want unchanged %q", stored.Model, "opus")
}
}
apps/backend/internal/agent/settings/controller/reconciler_test.go
+102 -14
View File
@@ -88,6 +88,59 @@ func copyAgent(a *models.Agent) *models.Agent {
return &out
}
// copyProfile deep-copies a profile so the fake store behaves like a real one:
// callers get a snapshot, not a handle on stored state. Keep the legacy field
// that is intentionally omitted from the public JSON representation.
func copyProfile(p *models.AgentProfile) *models.AgentProfile {
if p == nil {
return nil
}
data, err := json.Marshal(p)
if err != nil {
panic("fakeStore: marshal profile: " + err.Error())
}
var out models.AgentProfile
if err := json.Unmarshal(data, &out); err != nil {
panic("fakeStore: unmarshal profile: " + err.Error())
}
out.DangerouslySkipPermissions = p.DangerouslySkipPermissions
return &out
}
func TestFakeStoreAgentProfileReadRequiresExplicitUpdate(t *testing.T) {
st := newFakeStore()
st.profiles["agent-1"] = []*models.AgentProfile{{
ID: "profile-1",
AgentID: "agent-1",
Name: "Original",
}}
profile, err := st.GetAgentProfile(context.Background(), "profile-1")
if err != nil {
t.Fatalf("GetAgentProfile: %v", err)
}
profile.Name = "Changed"
stored, err := st.GetAgentProfile(context.Background(), "profile-1")
if err != nil {
t.Fatalf("GetAgentProfile after local mutation: %v", err)
}
if stored.Name != "Original" {
t.Fatalf("local mutation changed stored profile name to %q", stored.Name)
}
if err := st.UpdateAgentProfile(context.Background(), profile); err != nil {
t.Fatalf("UpdateAgentProfile: %v", err)
}
stored, err = st.GetAgentProfile(context.Background(), "profile-1")
if err != nil {
t.Fatalf("GetAgentProfile after update: %v", err)
}
if stored.Name != "Changed" {
t.Fatalf("stored profile name = %q, want Changed after explicit update", stored.Name)
}
}
func (f *fakeStore) CreateAgent(_ context.Context, a *models.Agent) error {
f.nextAgentID++
a.ID = "agent-" + strconv.Itoa(f.nextAgentID)
@@ -161,8 +214,9 @@ func (f *fakeStore) UpsertAgentProfileMcpConfig(_ context.Context, config *model
func (f *fakeStore) CreateAgentProfile(_ context.Context, p *models.AgentProfile) error {
f.nextProfID++
p.ID = "profile-" + strconv.Itoa(f.nextProfID)
f.profiles[p.AgentID] = append(f.profiles[p.AgentID], p)
f.created = append(f.created, p)
stored := copyProfile(p)
f.profiles[p.AgentID] = append(f.profiles[p.AgentID], stored)
f.created = append(f.created, stored)
return nil
}
@@ -178,6 +232,9 @@ func (f *fakeStore) DuplicateAgentProfile(_ context.Context, input store.Duplica
// must re-read the source and copy the NEW state.
input.Source.Name = "Changed Name"
input.Source.UpdatedAt = input.Source.UpdatedAt.Add(time.Second)
if _, err := f.persistAgentProfile(input.Source); err != nil {
return err
}
return store.ErrProfileChanged
}
// Version check mirrors the sqlite store: the stored source rows must
@@ -201,8 +258,9 @@ func (f *fakeStore) DuplicateAgentProfile(_ context.Context, input store.Duplica
now := time.Now().UTC()
p.CreatedAt = now
p.UpdatedAt = now
f.profiles[p.AgentID] = append(f.profiles[p.AgentID], p)
f.created = append(f.created, p)
stored := copyProfile(p)
f.profiles[p.AgentID] = append(f.profiles[p.AgentID], stored)
f.created = append(f.created, stored)
if input.McpConfig != nil {
input.McpConfig.ProfileID = p.ID
f.mcpConfigs[p.ID] = input.McpConfig
@@ -210,8 +268,25 @@ func (f *fakeStore) DuplicateAgentProfile(_ context.Context, input store.Duplica
return nil
}
func (f *fakeStore) persistAgentProfile(p *models.AgentProfile) (*models.AgentProfile, error) {
stored := copyProfile(p)
for agentID, profiles := range f.profiles {
for i, existing := range profiles {
if existing.ID == p.ID {
f.profiles[agentID][i] = stored
return stored, nil
}
}
}
return nil, fmt.Errorf("agent profile not found: %s", p.ID)
}
func (f *fakeStore) UpdateAgentProfile(_ context.Context, p *models.AgentProfile) error {
f.updated = append(f.updated, p)
stored, err := f.persistAgentProfile(p)
if err != nil {
return err
}
f.updated = append(f.updated, copyProfile(stored))
return nil
}
@@ -223,8 +298,12 @@ func (f *fakeStore) UpdateAgentProfileEnabled(ctx context.Context, id string, en
profile.Enabled = enabled
profile.UserModified = true
profile.UpdatedAt = time.Now().UTC()
f.updated = append(f.updated, profile)
return profile.UpdatedAt, nil
stored, err := f.persistAgentProfile(profile)
if err != nil {
return time.Time{}, err
}
f.updated = append(f.updated, copyProfile(stored))
return stored.UpdatedAt, nil
}
func (f *fakeStore) DeleteAgentProfile(_ context.Context, id string) error {
@@ -254,7 +333,7 @@ func (f *fakeStore) GetAgentProfile(_ context.Context, id string) (*models.Agent
for _, list := range f.profiles {
for _, p := range list {
if p.ID == id {
return p, nil
return copyProfile(p), nil
}
}
}
@@ -270,7 +349,7 @@ func (f *fakeStore) GetAgentProfileIncludingDeleted(ctx context.Context, id stri
for _, list := range f.deleted {
for _, p := range list {
if p.ID == id {
return p, nil
return copyProfile(p), nil
}
}
}
@@ -283,7 +362,12 @@ func (f *fakeStore) ListAgentProfiles(_ context.Context, agentID string) ([]*mod
return nil, err
}
}
return f.profiles[agentID], nil
profiles := f.profiles[agentID]
out := make([]*models.AgentProfile, 0, len(profiles))
for _, p := range profiles {
out = append(out, copyProfile(p))
}
return out, nil
}
func (f *fakeStore) Close() error { return nil }
@@ -1063,13 +1147,17 @@ func TestProfileReconciler_MigratesCursorVariantModel(t *testing.T) {
t.Fatalf("Run: %v", err)
}
if profile.Model != "grok-4.5" {
t.Fatalf("profile model = %q, want bare Cursor model", profile.Model)
stored, err := st.GetAgentProfile(context.Background(), profile.ID)
if err != nil {
t.Fatalf("get migrated profile: %v", err)
}
if got := profile.ConfigOptions["effort"]; got != "low" {
if stored.Model != "grok-4.5" {
t.Fatalf("profile model = %q, want bare Cursor model", stored.Model)
}
if got := stored.ConfigOptions["effort"]; got != "low" {
t.Errorf("profile effort = %q, want existing profile value low", got)
}
if got := profile.ConfigOptions["fast"]; got != "true" {
if got := stored.ConfigOptions["fast"]; got != "true" {
t.Errorf("profile fast = %q, want migrated value true", got)
}
if len(st.updated) == 0 {