From c7cd7ed4a86e91e967445a91b8bd20a258cf62be Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Fri, 30 Oct 2026 20:56:41 +0000
Subject: [PATCH 1/4] test: show that temporary display names delete Coder
 resources and #422 items 4 and 5

Refs #422

Change-Id: I9e7ac05664d59ab6e7b8edadadff1d7517d3884e
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/controller/cleanup_sweep_test.go     | 167 +++++++++++++-----
 .../controller/coderaiprovider_delete_test.go |  25 +--
 .../controller/coderaiprovider_record_test.go |  59 ++++---
 .../controller/codermcpserver_delete_test.go  |  13 +-
 .../controller/codermcpserver_record_test.go  |  54 +++---
 5 files changed, 211 insertions(+), 107 deletions(-)

diff --git a/internal/controller/cleanup_sweep_test.go b/internal/controller/cleanup_sweep_test.go
index 9b5a8b8a5..0c2120fee 100644
--- a/internal/controller/cleanup_sweep_test.go
+++ b/internal/controller/cleanup_sweep_test.go
@@ -14,6 +14,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/events"
 	clocktesting "k8s.io/utils/clock/testing"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
@@ -131,8 +132,8 @@ func TestCleanupSweepAIProvider(t *testing.T) {
 }
 
 // #353 invariants 5 and 6: a sweep never touches the provider of a living
-// object, finds a provider without a recorded ID only by its nonce, and
-// leaves a provider of the same name that the object did not create.
+// object, and deletes only through a control plane of the record's
+// deployment.
 func TestCleanupSweepAIProviderCases(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
@@ -156,29 +157,6 @@ func TestCleanupSweepAIProviderCases(t *testing.T) {
 		_, ok := s.fake.aiProvider("openai")
 		require.False(t, ok, "only the UID decides that the object is gone")
 	})
-	for name, nonce := range map[string]bool{"nonce matches": true, "nonce differs": false} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-			s := newSweepEnv(ctx, t)
-			createTestControlPlane(ctx, t, s.namespace, "coder-b", s.fake.server.URL)
-			configMap := &corev1.ConfigMap{}
-			require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: s.namespace, Name: "ck8s-cleanup-" + string(s.provider.UID)}, configMap))
-			configMap.Data["coderID"] = "" // The create committed, but its ID was never recorded.
-			require.NoError(t, k8sClient.Update(ctx, configMap))
-			if nonce {
-				s.fake.mu.Lock()
-				for _, p := range s.fake.aiProviders {
-					p.displayName = "coder-k8s:" + string(s.provider.UID)
-				}
-				s.fake.mu.Unlock()
-			}
-			s.clock.Step(6 * time.Minute)
-			require.Zero(t, s.sweep(ctx, t).RequeueAfter)
-			_, ok := s.fake.aiProvider("openai")
-			require.Equal(t, !nonce, ok, "only the provider with the record's nonce is deleted")
-			require.Nil(t, cleanupRecord(ctx, t, s.namespace, s.provider.UID), "Coder holds no provider of the record")
-		})
-	}
 	t.Run("delete fails", func(t *testing.T) {
 		t.Parallel()
 		s := newSweepEnv(ctx, t)
@@ -230,24 +208,6 @@ func TestCleanupSweepAIProviderCases(t *testing.T) {
 		_, ok := s.fake.aiProvider("openai")
 		require.False(t, ok)
 	})
-	t.Run("same name and nonce, another type", func(t *testing.T) {
-		t.Parallel()
-		s := newSweepEnv(ctx, t)
-		createTestControlPlane(ctx, t, s.namespace, "coder-b", s.fake.server.URL)
-		configMap := &corev1.ConfigMap{}
-		require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: s.namespace, Name: "ck8s-cleanup-" + string(s.provider.UID)}, configMap))
-		configMap.Data["coderID"] = ""
-		require.NoError(t, k8sClient.Update(ctx, configMap))
-		s.fake.mu.Lock()
-		for _, p := range s.fake.aiProviders {
-			p.displayName, p.providerType = "coder-k8s:"+string(s.provider.UID), codersdk.AIProviderTypeAnthropic
-		}
-		s.fake.mu.Unlock()
-		s.clock.Step(6 * time.Minute)
-		s.sweep(ctx, t)
-		_, ok := s.fake.aiProvider("openai")
-		require.True(t, ok, "only the nonce and the recorded type prove ownership")
-	})
 	t.Run("stale cache of the record", func(t *testing.T) {
 		t.Parallel()
 		s := newSweepEnv(ctx, t)
@@ -320,3 +280,124 @@ func TestCleanupSweepAIProviderCases(t *testing.T) {
 		require.True(t, ok, "a ConfigMap that is not a valid record is never acted on")
 	})
 }
+
+// #422 item 1: without a recorded provider ID, a sweep deletes nothing. A
+// person can edit the display name of a provider, so the temporary display
+// name proves nothing. Coder cannot change a provider's name or type, so the
+// record goes only when Coder holds no provider of that name and type.
+// Otherwise it stays for manual cleanup.
+func TestCleanupSweepAIProviderWithoutProviderID(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	for name, tc := range map[string]struct {
+		committed bool                                                                           // The create commits, and its answer is lost.
+		person    func(context.Context, *testing.T, sweepEnv, *codersdk.CreateAIProviderRequest) // A person's change in Coder after the object went.
+		kept      bool
+	}{
+		"create never committed": {},
+		"lost create answer":     {committed: true, kept: true},
+		"lost create answer, display name edited": {committed: true, kept: true, person: func(ctx context.Context, t *testing.T, s sweepEnv, _ *codersdk.CreateAIProviderRequest) {
+			_, err := s.fake.client(t, 2*time.Second).UpdateAIProvider(ctx, "openai", codersdk.UpdateAIProviderRequest{DisplayName: ptr.To("Renamed by a person")})
+			require.NoError(t, err)
+		}},
+		"a person's provider with the temporary display name": {kept: true, person: createPersonsAIProvider},
+		"a person's provider of another type": {person: func(ctx context.Context, t *testing.T, s sweepEnv, req *codersdk.CreateAIProviderRequest) {
+			req.Type, req.DisplayName = codersdk.AIProviderTypeAnthropic, "Team Anthropic"
+			createPersonsAIProvider(ctx, t, s, req)
+		}},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			s := leaveAIProviderWithoutID(ctx, t, tc.committed)
+			if tc.person != nil {
+				tc.person(ctx, t, s, &codersdk.CreateAIProviderRequest{
+					Type: codersdk.AIProviderTypeOpenAI, Name: "openai", DisplayName: "coder-k8s:" + string(s.provider.UID),
+					BaseURL: s.provider.Spec.BaseURL, APIKeys: []string{testAIProviderKey},
+				})
+			}
+			_, before := s.fake.aiProvider("openai")
+			deletes := s.fake.requestCount(routeDeleteAIProvider)
+			createTestControlPlane(ctx, t, s.namespace, "coder-b", s.fake.server.URL)
+			s.clock.Step(6 * time.Minute)
+
+			result := s.sweep(ctx, t)
+			require.Equal(t, deletes, s.fake.requestCount(routeDeleteAIProvider), "a record without a provider ID deletes nothing")
+			_, after := s.fake.aiProvider("openai")
+			require.Equal(t, before, after)
+			if !tc.kept {
+				require.Zero(t, result.RequeueAfter)
+				require.Nil(t, cleanupRecord(ctx, t, s.namespace, s.provider.UID), "Coder holds no provider that can be the object's")
+				require.Empty(t, s.recorder.Events)
+				return
+			}
+			require.Equal(t, 10*time.Minute, result.RequeueAfter)
+			require.NotNil(t, cleanupRecord(ctx, t, s.namespace, s.provider.UID), "a provider can still be the object's, so the record stays")
+			require.Len(t, s.recorder.Events, 1)
+			require.Contains(t, <-s.recorder.Events, "ManualCleanupNeeded")
+		})
+	}
+}
+
+// #422 item 1: an object with an unconfirmed create is deleted while a
+// person's provider of its name carries its temporary display name. The
+// delete and the sweep both leave that provider, the record waits for manual
+// cleanup, and it goes once the person deletes the provider. Not parallel: it
+// reads process-wide metrics.
+func TestCoderAIProviderTemporaryDisplayNameDeletesNothing(t *testing.T) {
+	ctx := context.Background()
+	env, _, provider := newAIProviderCreateEnv(ctx, t)
+	env.fake.failNext(routeCreateAIProvider, fakeFault{Reset: true})
+	got, _ := env.reconcile(ctx, t, provider)
+	requireAIProviderStep(t, got, false, "ConfirmingCreate")
+	_, committed := env.fake.aiProvider("openai")
+	require.False(t, committed, "the create never committed")
+	s := newSweeper(t, env, got)
+	createPersonsAIProvider(ctx, t, s, &codersdk.CreateAIProviderRequest{
+		Type: codersdk.AIProviderTypeOpenAI, Name: "openai", DisplayName: "coder-k8s:" + string(got.UID),
+		BaseURL: got.Spec.BaseURL, APIKeys: []string{testAIProviderKey},
+	})
+
+	gone, _ := env.deleteAIProvider(ctx, t, got)
+	require.Nil(t, gone, "the finalizer goes without a delete")
+	require.Zero(t, env.fake.requestCount(routeDeleteAIProvider), "a display name never proves ownership")
+	_, ok := env.fake.aiProvider("openai")
+	require.True(t, ok, "the person's provider stays")
+	require.NotNil(t, cleanupRecord(ctx, t, env.namespace, got.UID), "the record stays for a sweep")
+
+	s.clock.Step(6 * time.Minute)
+	require.Equal(t, 10*time.Minute, s.sweep(ctx, t).RequeueAfter)
+	require.Zero(t, env.fake.requestCount(routeDeleteAIProvider))
+	require.Contains(t, <-s.recorder.Events, "ManualCleanupNeeded")
+	require.Equal(t, 1.0, providerMetric(t, "coder_k8s_cleanup_records_unmatched"))
+
+	stored, _ := env.fake.aiProvider("openai")
+	require.NoError(t, env.fake.client(t, 2*time.Second).DeleteAIProvider(ctx, stored.id.String()))
+	require.Zero(t, s.sweep(ctx, t).RequeueAfter)
+	require.Nil(t, cleanupRecord(ctx, t, env.namespace, got.UID), "the provider is gone, so the record goes")
+	require.Zero(t, providerMetric(t, "coder_k8s_cleanup_records_unmatched"))
+}
+
+// leaveAIProviderWithoutID leaves the record of an object whose create answer
+// was lost, so the record has no provider ID: the control plane goes, then
+// the object. With committed, the create committed in Coder. Otherwise Coder
+// holds no provider.
+func leaveAIProviderWithoutID(ctx context.Context, t *testing.T, committed bool) sweepEnv {
+	t.Helper()
+	env, _, provider := newAIProviderCreateEnv(ctx, t)
+	env.fake.failNext(routeCreateAIProvider, fakeFault{AfterCommit: committed, Reset: true})
+	got, _ := env.reconcile(ctx, t, provider)
+	requireAIProviderStep(t, got, false, "ConfirmingCreate")
+	require.NoError(t, k8sClient.Delete(ctx, &coderv1alpha1.CoderControlPlane{ObjectMeta: metav1.ObjectMeta{Namespace: env.namespace, Name: "coder"}}))
+	gone, _ := env.deleteAIProvider(ctx, t, got)
+	require.Nil(t, gone)
+	require.Empty(t, cleanupRecord(ctx, t, env.namespace, got.UID)["coderID"])
+	_, ok := env.fake.aiProvider("openai")
+	require.Equal(t, committed, ok)
+	return newSweeper(t, env, got)
+}
+
+func createPersonsAIProvider(ctx context.Context, t *testing.T, s sweepEnv, req *codersdk.CreateAIProviderRequest) {
+	t.Helper()
+	_, err := s.fake.client(t, 2*time.Second).CreateAIProvider(ctx, *req)
+	require.NoError(t, err)
+}
diff --git a/internal/controller/coderaiprovider_delete_test.go b/internal/controller/coderaiprovider_delete_test.go
index 3f70dd89b..ee3aac5f6 100644
--- a/internal/controller/coderaiprovider_delete_test.go
+++ b/internal/controller/coderaiprovider_delete_test.go
@@ -116,18 +116,18 @@ func TestCoderAIProviderDelete(t *testing.T) {
 	})
 }
 
-// Plan S7: with only a create marker, a read decides. The controller deletes
-// a provider that carries this object's nonce, and never one that does not.
+// Plan S7, #422 item 1: with only a create marker, the controller deletes
+// nothing. A person can edit the display name of a provider, so not even the
+// temporary display name proves that this object created it.
 func TestCoderAIProviderDeletePendingCreate(t *testing.T) {
 	t.Parallel()
 	for _, tc := range []struct {
 		name        string
 		displayName string // of a provider that holds the name in Coder; empty for none
-		deletes     int
 	}{
-		{name: "create committed", displayName: "nonce", deletes: 1},
-		{name: "nothing created", deletes: 0},
-		{name: "someone else's provider", displayName: "Team OpenAI", deletes: 0},
+		{name: "a person's provider with the temporary display name", displayName: "nonce"},
+		{name: "nothing created"},
+		{name: "someone else's provider", displayName: "Team OpenAI"},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
@@ -150,9 +150,9 @@ func TestCoderAIProviderDeletePendingCreate(t *testing.T) {
 			require.NoError(t, k8sClient.Status().Update(ctx, provider))
 			got, _ := env.deleteAIProvider(ctx, t, provider)
 			require.Nil(t, got, "the finalizer went")
-			require.Equal(t, tc.deletes, env.fake.requestCount(routeDeleteAIProvider))
+			require.Zero(t, env.fake.requestCount(routeDeleteAIProvider))
 			_, found := env.fake.aiProvider(provider.Name)
-			require.Equal(t, tc.displayName != "" && tc.deletes == 0, found, "only this object's provider is deleted")
+			require.Equal(t, tc.displayName != "", found, "the provider stays")
 		})
 	}
 }
@@ -177,8 +177,9 @@ func TestCoderAIProviderDeletePendingCreateNeedsSiteOwner(t *testing.T) {
 }
 
 // A 404 right after a create request does not prove absence: the request
-// can still commit. The finalizer stays until the settle window ends, and a
-// create that commits in the meantime is deleted.
+// can still commit. The finalizer stays until the settle window ends. A
+// create that commits in the meantime stays in Coder: without its ID, nothing
+// proves that it is this object's (#422 item 1).
 func TestCoderAIProviderDeleteRecentCreate(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
@@ -204,9 +205,9 @@ func TestCoderAIProviderDeleteRecentCreate(t *testing.T) {
 	require.NoError(t, k8sClient.Status().Update(ctx, got))
 	got, _ = env.reconcileDeleting(ctx, t, got)
 	require.Nil(t, got, "the finalizer went")
-	require.Equal(t, 1, env.fake.requestCount(routeDeleteAIProvider))
+	require.Zero(t, env.fake.requestCount(routeDeleteAIProvider))
 	_, found := env.fake.aiProvider(provider.Name)
-	require.False(t, found, "the late create was deleted")
+	require.True(t, found, "the late create stays")
 }
 
 // A name lookup that answers this object's provider without an ID is
diff --git a/internal/controller/coderaiprovider_record_test.go b/internal/controller/coderaiprovider_record_test.go
index f4840478d..efa530a04 100644
--- a/internal/controller/coderaiprovider_record_test.go
+++ b/internal/controller/coderaiprovider_record_test.go
@@ -281,36 +281,43 @@ func TestCoderAIProviderSetupNeedsRecordDependencies(t *testing.T) {
 	}
 }
 
-// #374 review round 3: when the control plane reaches another Coder
-// deployment while the object lives, the provider looks gone, but the
-// record still locates it in the old deployment. The controller keeps the
-// record and creates nothing in the new deployment.
+// #374 review round 3, #422 item 4: when the control plane reaches another
+// Coder deployment while the object lives, the provider looks gone, but the
+// record still locates it in the old deployment. With or without a pending
+// spec edit, the controller stalls with CoderDeploymentChanged, not
+// DeletedExternally, keeps the record and creates nothing in the new
+// deployment.
 func TestCoderAIProviderDeploymentSwitchKeepsRecord(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
-	env, _, provider := newReadyAIProvider(ctx, t)
-	before := cleanupRecord(ctx, t, env.namespace, provider.UID)
-	env.fake.mu.Lock()
-	env.fake.deploymentID = uuid.NewString() // A new database behind the same control plane.
-	for id := range env.fake.aiProviders {
-		delete(env.fake.aiProviders, id)
-	}
-	env.fake.mu.Unlock()
-	provider.Spec.DisplayName = "OpenAI (edited)"
-	require.NoError(t, k8sClient.Update(ctx, provider))
-	got, _ := env.reconcile(ctx, t, provider)
-	got, _ = env.reconcile(ctx, t, got)
-	require.Contains(t, requireAIProviderStep(t, got, true, "CoderDeploymentChanged"), before["deploymentID"])
-	require.Equal(t, 1, env.fake.requestCount(routeCreateAIProvider), "no create in the new deployment")
-	require.Equal(t, before, cleanupRecord(ctx, t, env.namespace, provider.UID), "the record keeps the old locator")
+	for name, edit := range map[string]bool{"applied": false, "pending spec edit": true} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			env, _, provider := newReadyAIProvider(ctx, t)
+			before := cleanupRecord(ctx, t, env.namespace, provider.UID)
+			env.fake.mu.Lock()
+			env.fake.deploymentID = uuid.NewString() // A new database behind the same control plane.
+			for id := range env.fake.aiProviders {
+				delete(env.fake.aiProviders, id)
+			}
+			env.fake.mu.Unlock()
+			if edit {
+				provider.Spec.DisplayName = "OpenAI (edited)"
+				require.NoError(t, k8sClient.Update(ctx, provider))
+			}
+			got, _ := env.reconcile(ctx, t, provider)
+			require.Contains(t, requireAIProviderStep(t, got, true, "CoderDeploymentChanged"), before["deploymentID"])
+			require.Equal(t, 1, env.fake.requestCount(routeCreateAIProvider), "no create in the new deployment")
+			require.Equal(t, before, cleanupRecord(ctx, t, env.namespace, provider.UID), "the record keeps the old locator")
 
-	// #374 review round 4: deleting the object in this state keeps the
-	// record too, so a sweep in the old deployment can still find it.
-	require.Empty(t, got.Status.ProviderID)
-	gone, _ := env.deleteAIProvider(ctx, t, got)
-	require.Nil(t, gone, "the finalizer goes without a delete")
-	require.Zero(t, env.fake.requestCount(routeDeleteAIProvider), "no delete in the new deployment")
-	require.Equal(t, before, cleanupRecord(ctx, t, env.namespace, provider.UID), "the record stays for a sweep")
+			// #374 review round 4: deleting the object in this state keeps the
+			// record too, so a sweep in the old deployment can still find it.
+			gone, _ := env.deleteAIProvider(ctx, t, got)
+			require.Nil(t, gone, "the finalizer goes without a delete")
+			require.Zero(t, env.fake.requestCount(routeDeleteAIProvider), "no delete in the new deployment")
+			require.Equal(t, before, cleanupRecord(ctx, t, env.namespace, provider.UID), "the record stays for a sweep")
+		})
+	}
 }
 
 // #353 (#362 follow-up 7): a create whose answer was lost leaves a record
diff --git a/internal/controller/codermcpserver_delete_test.go b/internal/controller/codermcpserver_delete_test.go
index e7021ca29..8ffe8482a 100644
--- a/internal/controller/codermcpserver_delete_test.go
+++ b/internal/controller/codermcpserver_delete_test.go
@@ -141,11 +141,12 @@ func TestCoderMCPServerDeleteWithoutOperatorAccess(t *testing.T) {
 	require.Len(t, env.fake.mcpServers(), 1)
 }
 
-// Plan 4.10 item 2: with only a create marker, the slug read decides. A
-// config with this object's nonce is deleted. Someone else's config with the
-// slug is never touched. With no config, the controller waits until a late
-// create cannot commit any more, then releases. #353: the record goes only
-// when no config can still be the object's, as in a sweep.
+// Plan 4.10 item 2: with only a create marker, the slug read decides, and
+// the controller deletes nothing (#422 item 1): a person can edit the slug
+// and the display name of a config, so not even the temporary display name
+// proves that this object created it. With no config, the controller waits
+// until a late create cannot commit any more, then releases. #353: the
+// record goes only when no config can still be the object's, as in a sweep.
 func TestCoderMCPServerDeletePendingCreate(t *testing.T) {
 	t.Parallel()
 	for _, tc := range []struct {
@@ -156,7 +157,7 @@ func TestCoderMCPServerDeletePendingCreate(t *testing.T) {
 		deletes int
 		record  bool // The record stays.
 	}{
-		{name: "nonce found", create: createWithNonce, release: true, deletes: 1},
+		{name: "a person's config with the temporary display name", create: createWithNonce, release: true, record: true},
 		{name: "someone else's config", create: createOther, release: true, record: true},
 		{name: "nonce under another slug", create: createWithNonceUnderOtherSlug, age: 3 * time.Minute, release: true, record: true},
 		{name: "no config, recent create", age: time.Second, record: true},
diff --git a/internal/controller/codermcpserver_record_test.go b/internal/controller/codermcpserver_record_test.go
index 1f97ae5ea..04e7ab40b 100644
--- a/internal/controller/codermcpserver_record_test.go
+++ b/internal/controller/codermcpserver_record_test.go
@@ -169,27 +169,40 @@ func TestCoderMCPServerPendingCreateAfterDeploymentSwitch(t *testing.T) {
 	require.Equal(t, before, env.mcpServerRecord(ctx, t, got.UID), "the record stays for a sweep")
 }
 
-// #353: a status without the config ID, but with a create marker, keeps the
-// recorded ID. After the sync the display name is no longer the nonce, so
-// only the recorded ID still finds the config for a delete.
+// #353, #422 item 5: a status without the config ID keeps the recorded ID.
+// After the sync the display name is no longer the nonce, so only the
+// recorded ID still proves that the config with the slug is this object's.
+// The controller takes it back instead of reporting SlugConflict, and a
+// delete uses it.
 func TestCoderMCPServerKeepsRecordedIDWithoutStatusID(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
-	env := newMCPServerEnv(ctx, t, true)
-	env.keySecret(ctx, t, testMCPKey)
-	got, id := env.applied(ctx, t, nil)
-	got.Status.ConfigID = ""
-	got.Status.CreateAttemptTime = &metav1.Time{Time: time.Now().Add(-time.Hour)}
-	require.NoError(t, k8sClient.Status().Update(ctx, got))
+	for name, lose := range map[string]func(*coderv1alpha1.CoderMCPServerStatus){
+		"create marker kept": func(s *coderv1alpha1.CoderMCPServerStatus) {
+			s.ConfigID, s.CreateAttemptTime = "", &metav1.Time{Time: time.Now().Add(-time.Hour)}
+		},
+		"status lost everything": func(s *coderv1alpha1.CoderMCPServerStatus) { *s = coderv1alpha1.CoderMCPServerStatus{} },
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			env := newMCPServerEnv(ctx, t, true)
+			env.keySecret(ctx, t, testMCPKey)
+			got, id := env.applied(ctx, t, nil)
+			lose(&got.Status)
+			require.NoError(t, k8sClient.Status().Update(ctx, got))
 
-	got, _ = env.reconcile(ctx, t, got)
-	requireMCPServerStep(t, got, true, "SlugConflict")
-	require.Equal(t, id.String(), env.mcpServerRecord(ctx, t, got.UID)["coderID"], "the record keeps the config ID")
+			got, _ = env.reconcile(ctx, t, got)
+			requireMCPServerApplied(t, got)
+			require.Equal(t, id.String(), got.Status.ConfigID, "the recorded ID proves the config")
+			require.Equal(t, 1, env.posts(), "no new create")
+			require.Equal(t, id.String(), env.mcpServerRecord(ctx, t, got.UID)["coderID"], "the record keeps the config ID")
 
-	gone, _ := env.deleteMCPServer(ctx, t, got)
-	require.Nil(t, gone)
-	require.Empty(t, env.fake.mcpServers(), "the delete used the recorded ID")
-	require.Nil(t, env.mcpServerRecord(ctx, t, got.UID))
+			gone, _ := env.deleteMCPServer(ctx, t, got)
+			require.Nil(t, gone)
+			require.Empty(t, env.fake.mcpServers(), "the delete used the recorded ID")
+			require.Nil(t, env.mcpServerRecord(ctx, t, got.UID))
+		})
+	}
 }
 
 // #353: a config from before records existed gets one, with the deployment
@@ -265,8 +278,9 @@ func TestCoderMCPServerRecordAfterLostCreateAnswer(t *testing.T) {
 }
 
 // #353: a config that is gone before a full apply is created again. The
-// record write before that create drops the old ID, so a delete finds the new
-// config by its nonce instead of trusting the 404 of the old ID.
+// record write before that create drops the old ID, so a delete does not
+// trust the 404 of the old ID. Without the new ID it deletes nothing (#422
+// item 1), and the record stays for a sweep.
 func TestCoderMCPServerRecreateDropsGoneID(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
@@ -285,8 +299,8 @@ func TestCoderMCPServerRecreateDropsGoneID(t *testing.T) {
 
 	gone, _ := env.deleteMCPServer(ctx, t, got)
 	require.Nil(t, gone)
-	require.Empty(t, env.fake.mcpServers(), "the delete found the new config by its nonce")
-	require.Nil(t, env.mcpServerRecord(ctx, t, got.UID))
+	require.Len(t, env.fake.mcpServers(), 1, "a display name never proves ownership")
+	require.NotNil(t, env.mcpServerRecord(ctx, t, got.UID), "the record stays for a sweep")
 }
 
 // #353: when the control plane reaches another Coder deployment while the

From 854f942f373c19be5137b5ef2f92bcbb1a40d20f Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Fri, 30 Oct 2026 21:04:39 +0000
Subject: [PATCH 2/4] fix: delete Coder resources only by recorded ID and
 report CoderDeploymentChanged for providers

Without a recorded ID, the CoderAIProvider sweep and both live delete
paths no longer delete a provider or MCP server config whose display name
is the temporary nonce: a person can edit display names. The record stays
and the sweep reports it with ManualCleanupNeeded.

A CoderAIProvider whose control plane reaches another Coder deployment
now reports CoderDeploymentChanged instead of DeletedExternally, and a
CoderMCPServer whose status lost its config ID recognizes its config by
the recorded ID instead of stalling with SlugConflict.

Refs #422

Change-Id: I5346eac9b37454579e102d5a8b52c550fb808bde
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 docs/how-to/manage-ai-providers.md            | 18 ++++++-
 docs/how-to/manage-mcp-servers.md             |  2 +-
 internal/controller/cleanup_sweep.go          | 29 +++++------
 internal/controller/coderaiprovider_delete.go | 49 ++++++++++---------
 internal/controller/coderaiprovider_record.go | 32 ++++++++++--
 internal/controller/coderaiprovider_sync.go   | 11 +++--
 internal/controller/codermcpserver_create.go  | 27 +++++++---
 internal/controller/codermcpserver_delete.go  | 16 +++---
 8 files changed, 120 insertions(+), 64 deletions(-)

diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md
index bd0f6f60d..1c6f27683 100644
--- a/docs/how-to/manage-ai-providers.md
+++ b/docs/how-to/manage-ai-providers.md
@@ -79,6 +79,7 @@ The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled ob
 | `NameConflict` | Coder has a provider with this name that the operator did not create. |
 | `NameReservedByEnv` | The control plane can seed a provider with this name. See [Seeding limits](#seeding-limits). |
 | `CoderRejected` | Coder rejected the create or update request. Fix the spec. |
+| `CoderDeploymentChanged` | The control plane now reaches another Coder deployment than the one that holds the provider. See [Cleanup records](#cleanup-records). |
 | `DisabledExternally`, `KeyChangedExternally`, `DeletedExternally` | Someone disabled the provider, changed its key or deleted it in Coder. `DisabledExternally` also follows the credential kill switch, after the Secret was repaired. See [Kill switches](#kill-switches). |
 
 ## Kill switches
@@ -122,7 +123,7 @@ These limits remain:
 
 ## Delete a provider
 
-Deleting the object deletes the provider in Coder. With only an unconfirmed create, the operator reads Coder first and deletes only a provider that this object created. A create request can still commit for a short time, so within 2 minutes of that request the operator reads again before it trusts that the provider is absent.
+Deleting the object deletes the provider in Coder. The operator deletes only the provider ID that it recorded. With only an unconfirmed create, it deletes nothing: a person can edit the display name of any provider, so not even the display-name nonce of the create proves that this object created a provider. When Coder has a provider of this name and type, the finalizer goes without a delete, and the [cleanup record](#cleanup-records) stays for manual cleanup. A create request can still commit for a short time, so within 2 minutes of that request the operator reads again before it trusts that the provider is absent.
 
 - When Coder is unreachable, or the control plane has no operator access, the finalizer `coder.com/ai-provider-cleanup` stays and the operator retries every 60 seconds. See [Stuck finalizer](#stuck-finalizer).
 - When the control plane is gone, the operator cannot delete the provider now. The finalizer goes at once, and the operator logs the provider ID. The [cleanup record](#cleanup-records) stays, so a later sweep can delete the provider.
@@ -141,11 +142,24 @@ The operator removes the record when Coder confirms that the provider is deleted
 
 1. It waits until the object with that UID is gone and 5 minutes have passed since the last create, so that a create still in flight commits first.
 2. It looks for a ready `CoderControlPlane` in the object's namespace whose Coder answers the record's deployment ID now. It never matches by URL or name: a new Coder database behind the same URL has another deployment ID and is never swept. A control plane in another namespace never sweeps the record, because Coder reports its deployment ID itself and a control plane that someone else runs could claim any ID.
-3. Through that control plane it deletes the recorded provider ID. Without a recorded ID, it deletes only the provider of that name whose display name is still the record's nonce.
+3. Through that control plane it deletes the recorded provider ID. Without a recorded ID, it deletes nothing (see below).
 4. It removes the record when Coder answers the delete with success or with its own 404. Any other answer keeps the record, and the operator retries every minute.
 
 So after a namespace deletion with an external database, the leftover provider goes once you create the namespace again with a control plane that uses the same database. The metric `coder_k8s_cleanup_records_swept_total` (label `kind`) counts the swept records.
 
+A record without a provider ID is left by a create that the operator could not confirm. The sweep deletes nothing for such a record: a person can edit the display name of any provider, so the nonce does not prove that the operator created it. Coder cannot change the name or the type of a provider, so when Coder has no provider of the object's name, or only one of another type, the sweep removes the record. Otherwise the record stays, the gauge `coder_k8s_cleanup_records_unmatched` counts it, and the operator writes a `ManualCleanupNeeded` Warning Event on the ConfigMap that names the provider. Clean up by hand:
+
+1. Read the record and its Event:
+
+    ```bash
+    kubectl -n coder-system get configmap ck8s-cleanup-<object UID> -o yaml
+    kubectl -n coder-system get events --field-selector reason=ManualCleanupNeeded
+    ```
+
+2. In the dashboard, open **AI settings > Providers** and find the provider that the Event names.
+3. When that provider is a leftover of the deleted object, delete it. The next sweep, within 10 minutes, then removes the record.
+4. When a person owns that provider, keep it and delete the ConfigMap.
+
 When no ready control plane reaches the record's deployment, the record stays. The gauge `coder_k8s_cleanup_records_unmatched` counts these records, and the operator writes a `NoMatchingDeployment` Warning Event on the ConfigMap every 10 minutes. When that Coder deployment never comes back, delete the provider in the Coder dashboard (the Event names the object, and the record holds the provider ID), then delete the ConfigMap. Find the records with `kubectl -n coder-system get configmaps -l coder.com/cleanup-record`. The operator gets write access to ConfigMaps only in that namespace (the Role `manager-role` and the RoleBinding `coder-k8s-cleanup-records`). Do not give other users write access to ConfigMaps there: a record names a Coder provider ID that a cleanup deletes. If you install the operator in another namespace, change the namespace of both.
 
 ### Stuck finalizer
diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md
index 5f98561f1..247e92f13 100644
--- a/docs/how-to/manage-mcp-servers.md
+++ b/docs/how-to/manage-mcp-servers.md
@@ -207,7 +207,7 @@ Disabling stops every chat from using the server, also chats that selected it ea
 
 Deleting the object deletes the config in Coder. Coder removes the server from every chat and deletes the users' OAuth2 tokens for it. The finalizer `coder.com/mcp-server-cleanup` keeps the object until then.
 
-- The operator deletes only a config that this object created. With only an unconfirmed create, it reads Coder first. It deletes a config that carries this object's temporary display name, and it never touches another config with the slug. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent. When another config holds the slug, or a config with the temporary display name has another slug, the finalizer goes without a delete, and the [cleanup record](#cleanup-records) stays. A record without a config ID also stays when the object reports `SlugConflict` after a create: a person can have renamed the config of that create.
+- The operator deletes only the config ID that it recorded. With only an unconfirmed create, it deletes nothing: a person can edit the slug and the display name of any config, so not even the temporary display name proves that this object created a config. When a config holds the slug, or a config with the temporary display name has another slug, the finalizer goes without a delete, and the [cleanup record](#cleanup-records) stays for manual cleanup. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent. A record without a config ID also stays when the object reports `SlugConflict` after a create: a person can have renamed the config of that create.
 - When Coder is unreachable, or the control plane has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer.
 - When the control plane is gone, the operator cannot delete the config now. The finalizer goes at once. The operator logs the slug and the config ID with the reason `ControlPlaneGone`. The [cleanup record](#cleanup-records) stays, so a later sweep can delete the config.
 - When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. An object that you do not delete holds the control plane for only 30 seconds, so delete the objects together with the control plane. A foreground deletion (`kubectl delete --cascade=foreground`) does not wait.
diff --git a/internal/controller/cleanup_sweep.go b/internal/controller/cleanup_sweep.go
index 5604811f8..c06bfec3c 100644
--- a/internal/controller/cleanup_sweep.go
+++ b/internal/controller/cleanup_sweep.go
@@ -63,9 +63,8 @@ func init() {
 // CleanupRecordReconciler sweeps the cleanup records of objects that went
 // without a confirmed delete in Coder (#353). It acts only through a ready
 // control plane whose current Coder deployment ID equals the record's, and
-// it deletes only the recorded provider or MCP server config ID, the provider
-// with the recorded nonce, or the AI Gateway keys whose names start with the
-// recorded prefix.
+// it deletes only the recorded provider or MCP server config ID, or the AI
+// Gateway keys whose names start with the recorded prefix.
 type CleanupRecordReconciler struct {
 	client.Client
 	// APIReader reads the object of a record live, so a stale cache never
@@ -306,12 +305,13 @@ func (r *CleanupRecordReconciler) matchingControlPlanes(ctx context.Context, rec
 	return matches, unknown, nil
 }
 
-// sweepAIProvider deletes the provider of record in Coder. It returns gone
-// when Coder confirms that it holds no provider of the record: a delete
-// answered with success or Coder's own 404, a read with Coder's own 404 for
-// a site owner, or a provider of that name without the record's nonce. It
-// deletes only the recorded ID or the provider whose display name is the
-// recorded nonce (invariant 5).
+// sweepAIProvider deletes the recorded provider ID in Coder (invariant 5),
+// and returns gone when Coder answers that delete with success or its own
+// 404. Without a recorded ID it deletes nothing: a person can edit the display
+// name of a provider, so the nonce does not prove that this object created
+// it (#422). Coder cannot change the name or the type of a provider, so it
+// then returns gone when a read by a site owner finds no provider of that
+// name and type, and kept otherwise.
 func sweepAIProvider(ctx context.Context, sdk *codersdk.Client, record *cleanupRecord) (outcome sweepOutcome, why string, err error) {
 	if record.Nonce != aiProviderNoncePrefix+string(record.UID) {
 		return sweepRetry, "", fmt.Errorf("assertion failed: cleanup record of %s has nonce %q, not the nonce of its UID", record.UID, record.Nonce)
@@ -336,16 +336,13 @@ func sweepAIProvider(ctx context.Context, sdk *codersdk.Client, record *cleanupR
 		if err := coderAnswerFor("AI provider", record.Name, found.Name); err != nil {
 			return sweepRetry, err.Error(), nil //nolint:nilerr // A wrong answer retries the sweep.
 		}
-		if found.DisplayName != record.Nonce || string(found.Type) != record.Type {
-			// Another provider holds the name, so no create of this object
-			// committed after the settle window. The nonce and the type
-			// prove ownership, as on the controller's own paths.
-			return sweepGone, "the provider of that name is not this object's", nil
-		}
 		if found.ID == uuid.Nil {
 			return sweepRetry, "", errors.New("assertion failed: Coder answered an AI provider without an ID")
 		}
-		id = found.ID.String()
+		if string(found.Type) != record.Type {
+			return sweepGone, "the provider of that name has another type", nil
+		}
+		return sweepKept, fmt.Sprintf("AI provider %s (ID %s)", found.Name, found.ID), nil
 	}
 	if err := sdk.DeleteAIProvider(ctx, id); err != nil && !isCoderResourceNotFound(err) {
 		return sweepRetry, "delete provider " + id + ": " + coderErrorSummary(err), nil
diff --git a/internal/controller/coderaiprovider_delete.go b/internal/controller/coderaiprovider_delete.go
index e7b636af1..02ba3e2fb 100644
--- a/internal/controller/coderaiprovider_delete.go
+++ b/internal/controller/coderaiprovider_delete.go
@@ -140,7 +140,12 @@ func (r *CoderAIProviderReconciler) deleteStep(ctx context.Context, provider *co
 			return r.releaseWithoutDelete(ctx, provider, fmt.Sprintf("CoderControlPlane now reaches Coder deployment %s, but the provider is in deployment %s", id, record.DeploymentID)), nil, nil
 		}
 	}
-	step, err = r.deleteInCoder(ctx, sdk, provider)
+	unproven, step, err := r.deleteInCoder(ctx, sdk, provider)
+	if err == nil && step == nil && unproven {
+		// A provider that can be this object's stays in Coder, so the record
+		// stays for a sweep, which reports it for manual cleanup (#422).
+		return true, nil, nil
+	}
 	if err == nil && step == nil {
 		// Coder confirmed that it holds no provider of this object. Only
 		// then does the record go (#353 invariant 7). A failed record
@@ -161,29 +166,32 @@ func (r *CoderAIProviderReconciler) deleteStep(ctx context.Context, provider *co
 	return step == nil, step, nil
 }
 
-// deleteInCoder deletes the provider of this object in Coder and returns nil
-// when Coder holds none. With only a create marker, a read decides first:
-// only the nonce and the type prove that the provider is this object's.
-func (r *CoderAIProviderReconciler) deleteInCoder(ctx context.Context, sdk *codersdk.Client, provider *coderv1alpha1.CoderAIProvider) (*aiProviderStep, error) {
+// deleteInCoder deletes the provider of this object in Coder and returns a
+// nil step when it is done. Without a provider ID in the status or the record
+// it deletes nothing: a person can edit the display name of a provider, so
+// the nonce does not prove that this object created it (#422). unproven then
+// reports a provider of this name and type, which it leaves although it can
+// be this object's.
+func (r *CoderAIProviderReconciler) deleteInCoder(ctx context.Context, sdk *codersdk.Client, provider *coderv1alpha1.CoderAIProvider) (unproven bool, step *aiProviderStep, err error) {
 	id := provider.Status.ProviderID
 	if id == "" {
 		// A failed status write after the create can leave the provider ID
 		// only in the record. It proves ownership after the nonce is gone.
 		record, _, err := readCleanupRecord(ctx, r.APIReader, r.OperatorNamespace, provider.UID)
 		if err != nil {
-			return nil, err
+			return false, nil, err
 		}
 		if record != nil && record.CoderID != "" {
 			id = record.CoderID
 			if parsed, err := uuid.Parse(id); err != nil || parsed == uuid.Nil || parsed.String() != id {
-				return nil, fmt.Errorf("assertion failed: the cleanup record of AI provider %s/%s has coderID %q, which is not a canonical UUID", provider.Namespace, provider.Name, id)
+				return false, nil, fmt.Errorf("assertion failed: the cleanup record of AI provider %s/%s has coderID %q, which is not a canonical UUID", provider.Namespace, provider.Name, id)
 			}
 		}
 	}
 	if id != "" {
 		if provider.Status.ProviderID != "" {
 			if err := checkAIProviderID(provider); err != nil {
-				return nil, err
+				return false, nil, err
 			}
 		}
 	} else {
@@ -194,36 +202,33 @@ func (r *CoderAIProviderReconciler) deleteInCoder(ctx context.Context, sdk *code
 			// Coder has no watch, so the wait uses the delete poll.
 			if step := requireSiteOwner(ctx, sdk); step != nil {
 				step.requeue = aiProviderDeletePoll
-				return step, nil
+				return false, step, nil
 			}
 			if wait := aiProviderCreateSettle - time.Since(provider.Status.CreateAttemptTime.Time); wait > 0 {
 				// A create sent moments ago can still commit after this read.
-				return aiProviderWait(wait, "ConfirmingCreate",
+				return false, aiProviderWait(wait, "ConfirmingCreate",
 					"Provider %s is not in Coder, but its create request was sent at %s and can still commit. The controller reads again before it releases the finalizer.",
 					provider.Name, provider.Status.CreateAttemptTime.UTC().Format(time.RFC3339)), nil
 			}
-			return nil, nil
+			return false, nil, nil
 		case err != nil:
-			return aiProviderWait(aiProviderDeletePoll, "CoderUnavailable", "get provider %s: %s", provider.Name, coderErrorSummary(err)), nil
+			return false, aiProviderWait(aiProviderDeletePoll, "CoderUnavailable", "get provider %s: %s", provider.Name, coderErrorSummary(err)), nil
 		}
 		if err := coderAnswerFor("AI provider", provider.Name, found.Name); err != nil {
 			log.FromContext(ctx).Error(err, "Coder answered something other than the controller asked for")
-			return aiProviderWait(aiProviderDeletePoll, "CoderAnswerMismatch", "%s", err.Error()), nil
-		}
-		if found.DisplayName != aiProviderNonce(provider) || string(found.Type) != provider.Spec.Type {
-			// Someone else's provider holds the name, so no create of this
-			// object committed. The controller never deletes it.
-			return nil, nil
+			return false, aiProviderWait(aiProviderDeletePoll, "CoderAnswerMismatch", "%s", err.Error()), nil
 		}
 		if found.ID == uuid.Nil {
-			return nil, fmt.Errorf("assertion failed: Coder answered AI provider %s without an ID", provider.Name)
+			return false, nil, fmt.Errorf("assertion failed: Coder answered AI provider %s without an ID", provider.Name)
 		}
-		id = found.ID.String()
+		// Coder cannot change the type of a provider, so another type proves
+		// that no create of this object committed.
+		return string(found.Type) == provider.Spec.Type, nil, nil
 	}
 	if err := sdk.DeleteAIProvider(ctx, id); err != nil && !isCoderResourceNotFound(err) {
-		return aiProviderWait(aiProviderDeletePoll, "CoderUnavailable", "delete provider %s (ID %s): %s", provider.Name, id, coderErrorSummary(err)), nil
+		return false, aiProviderWait(aiProviderDeletePoll, "CoderUnavailable", "delete provider %s (ID %s): %s", provider.Name, id, coderErrorSummary(err)), nil
 	}
-	return nil, nil
+	return false, nil, nil
 }
 
 // releaseWithoutDelete logs what can stay in Coder, because nothing can
diff --git a/internal/controller/coderaiprovider_record.go b/internal/controller/coderaiprovider_record.go
index ddcfd47b4..fbccf38ad 100644
--- a/internal/controller/coderaiprovider_record.go
+++ b/internal/controller/coderaiprovider_record.go
@@ -34,9 +34,7 @@ func (r *CoderAIProviderReconciler) writeAIProviderRecord(
 		// The control plane now reaches another Coder database. The record
 		// is the only locator of what this object made in the old one, so
 		// the controller neither overwrites it nor creates in the new one.
-		return aiProviderStalled(aiProviderResync, "CoderDeploymentChanged",
-			"Provider %s (ID %q) is in Coder deployment %s, but CoderControlPlane %s now reaches deployment %s. Delete the provider there, then delete ConfigMap %s/%s. Until then the controller creates nothing.",
-			provider.Name, previous.CoderID, previous.DeploymentID, provider.Spec.ControlPlaneRef.Name, deploymentID, r.OperatorNamespace, cleanupRecordObjectName(provider.UID)), nil
+		return r.aiProviderDeploymentChanged(provider, previous, deploymentID), nil
 	}
 	record := cleanupRecord{
 		Kind: cleanupRecordKindAIProvider, UID: provider.UID, Namespace: provider.Namespace, Name: provider.Name,
@@ -61,6 +59,34 @@ func (r *CoderAIProviderReconciler) writeAIProviderRecord(
 	return nil, writeCleanupRecord(ctx, r.Client, r.OperatorNamespace, existing, record)
 }
 
+// aiProviderDeploymentChanged is the stall of an object whose record names
+// another Coder deployment than the one that the control plane reaches now.
+func (r *CoderAIProviderReconciler) aiProviderDeploymentChanged(provider *coderv1alpha1.CoderAIProvider, record *cleanupRecord, deploymentID string) *aiProviderStep {
+	return aiProviderStalled(aiProviderResync, "CoderDeploymentChanged",
+		"Provider %s (ID %q) is in Coder deployment %s, but CoderControlPlane %s now reaches deployment %s. Delete the provider there, then delete ConfigMap %s/%s. Until then the controller creates nothing.",
+		provider.Name, record.CoderID, record.DeploymentID, provider.Spec.ControlPlaneRef.Name, deploymentID, r.OperatorNamespace, cleanupRecordObjectName(provider.UID))
+}
+
+// recordedDeploymentChanged returns the CoderDeploymentChanged stall when the
+// record names another Coder deployment than the one that the control plane
+// reaches now, or a wait when the deployment ID read fails. Without a record
+// it returns nil. A provider that looks deleted can then still exist in the
+// old deployment.
+func (r *CoderAIProviderReconciler) recordedDeploymentChanged(ctx context.Context, sdk *codersdk.Client, provider *coderv1alpha1.CoderAIProvider) (*aiProviderStep, error) {
+	record, _, err := readCleanupRecord(ctx, r.APIReader, r.OperatorNamespace, provider.UID)
+	if err != nil || record == nil {
+		return nil, err
+	}
+	deploymentID, err := coderDeploymentID(ctx, sdk)
+	if err != nil {
+		return aiProviderWait(aiProviderRetry, "CoderUnavailable", "read the Coder deployment ID: %s", coderErrorSummary(err)), nil
+	}
+	if deploymentID != record.DeploymentID {
+		return r.aiProviderDeploymentChanged(provider, record, deploymentID), nil
+	}
+	return nil, nil
+}
+
 // backfillAIProviderRecord writes the missing record of a provider that
 // exists in Coder from before records existed. It runs before every other
 // check, so a stalled object (a bad base URL or key Secret) gets its record
diff --git a/internal/controller/coderaiprovider_sync.go b/internal/controller/coderaiprovider_sync.go
index 07d0c797d..55bc04f92 100644
--- a/internal/controller/coderaiprovider_sync.go
+++ b/internal/controller/coderaiprovider_sync.go
@@ -43,7 +43,7 @@ func (r *CoderAIProviderReconciler) syncAIProvider(
 	case isCoderResourceNotFound(err):
 		// Only Coder's own 404 proves that the provider is gone. Any other
 		// error, a proxy 404 included, keeps ownership (#327).
-		return r.providerGone(provider, applied)
+		return r.providerGone(ctx, sdk, provider, applied)
 	case err != nil:
 		return aiProviderWait(aiProviderRetry, "CoderUnavailable", "get provider %s: %s", provider.Name, coderErrorSummary(err)), false, nil
 	}
@@ -130,7 +130,7 @@ func (r *CoderAIProviderReconciler) syncAIProvider(
 		} else if !isCoderResourceNotFound(readErr) {
 			return aiProviderWait(aiProviderRetry, "CoderUnavailable", "get provider %s after a 404 update: %s", provider.Name, coderErrorSummary(readErr)), false, nil
 		}
-		return r.providerGone(provider, applied)
+		return r.providerGone(ctx, sdk, provider, applied)
 	case isCoderAuthFailure(status) || status == http.StatusTooManyRequests:
 		// No Kubernetes event follows a repaired token or role, so retry.
 		return aiProviderWait(aiProviderRetry, "CoderUnavailable", "update provider %s: %s", provider.Name, coderErrorSummary(err)), false, nil
@@ -171,8 +171,13 @@ func (r *CoderAIProviderReconciler) syncAIProvider(
 
 // providerGone handles a provider that is missing in Coder. After a full
 // apply it is a kill switch. Before one, the object creates it again.
-func (r *CoderAIProviderReconciler) providerGone(provider *coderv1alpha1.CoderAIProvider, applied bool) (*aiProviderStep, bool, error) {
+func (r *CoderAIProviderReconciler) providerGone(ctx context.Context, sdk *codersdk.Client, provider *coderv1alpha1.CoderAIProvider, applied bool) (*aiProviderStep, bool, error) {
 	provider.Status.Enabled = nil
+	// The control plane can reach another Coder deployment now, which
+	// answers 404 for a provider of the old one (#422).
+	if step, err := r.recordedDeploymentChanged(ctx, sdk, provider); step != nil || err != nil {
+		return step, false, err
+	}
 	if applied {
 		return aiProviderStalled(aiProviderResync, "DeletedExternally",
 			"Provider %s (ID %s) was deleted in Coder. The controller does not create it again until the spec changes.", provider.Name, provider.Status.ProviderID), false, nil
diff --git a/internal/controller/codermcpserver_create.go b/internal/controller/codermcpserver_create.go
index 970fbdb8f..cc2fc1848 100644
--- a/internal/controller/codermcpserver_create.go
+++ b/internal/controller/codermcpserver_create.go
@@ -55,7 +55,16 @@ func (r *CoderMCPServerReconciler) createOrRecord(
 		return keepRejectedMCPServerCreate(step, rejected), nil
 	}
 	if found != nil {
-		if step := recordMCPServer(server, *found); step != nil {
+		// A status that lost the config ID still has it in the record (#422).
+		record, _, err := readCleanupRecord(ctx, r.APIReader, r.OperatorNamespace, server.UID)
+		if err != nil {
+			return nil, err
+		}
+		recordedID := ""
+		if record != nil {
+			recordedID = record.CoderID
+		}
+		if step := recordMCPServer(server, *found, recordedID); step != nil {
 			return step, nil
 		}
 		// Store the ID before the sync replaces the nonce (#353).
@@ -101,11 +110,12 @@ func mcpServerRequireSiteOwner(ctx context.Context, sdk *codersdk.Client) *mcpSe
 }
 
 // recordMCPServer decides whether found, the config with the spec's slug, is
-// the config that this object created. Only the nonce proves it. Anything
-// else is a config that the controller never touches (no adoption). It
-// returns nil when it recorded the ID.
-func recordMCPServer(server *coderv1alpha1.CoderMCPServer, found codersdk.MCPServerConfig) *mcpServerStep {
-	if found.DisplayName != mcpServerNonce(server) {
+// the config that this object created. Only the nonce or recordedID, the
+// config ID of the cleanup record, proves it. Anything else is a config that
+// the controller never touches (no adoption). It returns nil when it recorded
+// the ID.
+func recordMCPServer(server *coderv1alpha1.CoderMCPServer, found codersdk.MCPServerConfig, recordedID string) *mcpServerStep {
+	if found.DisplayName != mcpServerNonce(server) && found.ID.String() != recordedID {
 		// No create of this object can commit while another config holds the
 		// slug, so nothing of this object can exist in Coder.
 		server.Status.CreateAttemptTime, server.Status.Credentials = nil, nil
@@ -186,8 +196,9 @@ func confirmRejectedMCPServerCreate(
 	case step != nil:
 		return keepRejectedMCPServerCreate(step, !retryable)
 	case found != nil:
-		// Ours (the create committed) or someone else's (SlugConflict).
-		return recordMCPServer(server, *found)
+		// Ours (the create committed) or someone else's (SlugConflict). The
+		// create dropped the recorded ID, so only the nonce can prove it.
+		return recordMCPServer(server, *found, "")
 	}
 	if step := mcpServerRequireSiteOwner(ctx, sdk); step != nil {
 		return keepRejectedMCPServerCreate(step, !retryable)
diff --git a/internal/controller/codermcpserver_delete.go b/internal/controller/codermcpserver_delete.go
index e5ebc58c2..b6b9db325 100644
--- a/internal/controller/codermcpserver_delete.go
+++ b/internal/controller/codermcpserver_delete.go
@@ -165,11 +165,12 @@ func (r *CoderMCPServerReconciler) deleteStep(ctx context.Context, server *coder
 // deleteMCPServerInCoder deletes the config of this object in Coder and
 // returns a nil step when it is done. Without status.configID it deletes
 // recordedID, the config ID of the cleanup record: a failed status write
-// after the create can leave the ID only there. With only a create marker, a
-// read decides first: only the nonce proves that the config is this
-// object's. Only Coder's own 404 proves absence (#337). unproven reports a
-// config that it leaves although it can be this object's: another config
-// with the slug, or a config with the nonce under another slug.
+// after the create can leave the ID only there. Without either ID it deletes
+// nothing: a person can edit the slug and the display name of a config, so
+// the nonce does not prove that this object created it (#422). Only Coder's
+// own 404 proves absence (#337). unproven reports a config that it leaves
+// although it can be this object's: a config with the slug, or one with the
+// nonce under another slug.
 func deleteMCPServerInCoder(ctx context.Context, sdk *codersdk.Client, server *coderv1alpha1.CoderMCPServer, orgID uuid.UUID, recordedID string) (unproven bool, step *mcpServerStep, err error) {
 	var id uuid.UUID
 	if configID := cmp.Or(server.Status.ConfigID, recordedID); configID != "" {
@@ -206,12 +207,9 @@ func deleteMCPServerInCoder(ctx context.Context, sdk *codersdk.Client, server *c
 					server.Spec.Slug, server.Status.CreateAttemptTime.UTC().Format(time.RFC3339)), nil
 			}
 			return mcpServerWithDisplayName(configs, mcpServerNonce(server)) != nil, nil, nil
-		case found.DisplayName != mcpServerNonce(server):
-			// Someone else's config holds the slug, or a person renamed this
-			// object's config. The controller never deletes it.
+		default:
 			return true, nil, nil
 		}
-		id = found.ID
 	}
 	if err := sdk.DeleteMCPServerConfig(ctx, orgID, id); err != nil && !isCoderResourceNotFound(err) {
 		return false, mcpServerWait(mcpServerDeletePoll, "CoderUnavailable", "delete MCP server config %s (ID %s): %s", server.Spec.Slug, id, coderErrorSummary(err)), nil

From f690612b4249458eff49bf6f768e6bc2cf1088d7 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Fri, 30 Oct 2026 21:11:41 +0000
Subject: [PATCH 3/4] test: keep the provider record after NameConflict and
 check the deployment before recorded-ID recovery

Refs #422

Change-Id: Iaf70ca73c8056032dcb3a7c18a63dd838781a251
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 .../controller/coderaiprovider_delete_test.go | 25 +++++++++++++++++++
 .../controller/codermcpserver_record_test.go  | 25 +++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/internal/controller/coderaiprovider_delete_test.go b/internal/controller/coderaiprovider_delete_test.go
index ee3aac5f6..711bb1823 100644
--- a/internal/controller/coderaiprovider_delete_test.go
+++ b/internal/controller/coderaiprovider_delete_test.go
@@ -12,6 +12,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
@@ -157,6 +158,30 @@ func TestCoderAIProviderDeletePendingCreate(t *testing.T) {
 	}
 }
 
+// #422: when a person edits the display name of the provider of a lost
+// create answer, the controller reports NameConflict and drops its create
+// marker. A delete then keeps the record, so a sweep can report the provider
+// for manual cleanup.
+func TestCoderAIProviderDeleteKeepsRecordAfterNameConflict(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	env, _, provider := newAIProviderCreateEnv(ctx, t)
+	env.fake.failNext(routeCreateAIProvider, fakeFault{AfterCommit: true, Reset: true})
+	got, _ := env.reconcile(ctx, t, provider)
+	requireAIProviderStep(t, got, false, "ConfirmingCreate")
+	_, err := env.fake.client(t, 2*time.Second).UpdateAIProvider(ctx, "openai", codersdk.UpdateAIProviderRequest{DisplayName: ptr.To("Renamed by a person")})
+	require.NoError(t, err)
+	got, _ = env.reconcile(ctx, t, got)
+	requireAIProviderStep(t, got, true, "NameConflict")
+	require.Nil(t, got.Status.CreateAttemptTime)
+
+	gone, _ := env.deleteAIProvider(ctx, t, got)
+	require.Nil(t, gone)
+	_, ok := env.fake.aiProvider("openai")
+	require.True(t, ok, "the controller never deletes a provider that it cannot prove its own")
+	require.NotNil(t, cleanupRecord(ctx, t, env.namespace, got.UID), "the record stays for a sweep")
+}
+
 // A 404 proves that no create of this object committed only when the
 // operator user is a site owner. Otherwise the finalizer stays.
 func TestCoderAIProviderDeletePendingCreateNeedsSiteOwner(t *testing.T) {
diff --git a/internal/controller/codermcpserver_record_test.go b/internal/controller/codermcpserver_record_test.go
index 04e7ab40b..173e993fc 100644
--- a/internal/controller/codermcpserver_record_test.go
+++ b/internal/controller/codermcpserver_record_test.go
@@ -205,6 +205,31 @@ func TestCoderMCPServerKeepsRecordedIDWithoutStatusID(t *testing.T) {
 	}
 }
 
+// #422 item 5: the recorded ID proves a config only in the record's Coder
+// deployment. After a status loss and a switch to another deployment that
+// holds a config with the same ID, the controller stalls with
+// CoderDeploymentChanged and writes nothing.
+func TestCoderMCPServerRecordedIDNeedsRecordedDeployment(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	env := newMCPServerEnv(ctx, t, true)
+	env.keySecret(ctx, t, testMCPKey)
+	got, _ := env.applied(ctx, t, nil)
+	before := env.mcpServerRecord(ctx, t, got.UID)
+	got.Status = coderv1alpha1.CoderMCPServerStatus{}
+	require.NoError(t, k8sClient.Status().Update(ctx, got))
+	env.fake.mu.Lock()
+	env.fake.deploymentID = uuid.NewString() // A copy of the database under a new deployment ID.
+	env.fake.mu.Unlock()
+	mark := len(env.writeRoutes())
+
+	got, _ = env.reconcile(ctx, t, got)
+	require.Contains(t, requireMCPServerStep(t, got, true, "CoderDeploymentChanged"), before["deploymentID"])
+	require.Empty(t, got.Status.ConfigID)
+	require.Empty(t, env.writesSince(mark), "no write in another deployment")
+	require.Equal(t, before, env.mcpServerRecord(ctx, t, got.UID), "the record keeps the old locator")
+}
+
 // #353: a config from before records existed gets one, with the deployment
 // ID that Coder answers now.
 func TestCoderMCPServerRecordBackfill(t *testing.T) {

From f0125c2154b24f7dcb79e74d38b01c1da49a6d58 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Fri, 30 Oct 2026 21:15:12 +0000
Subject: [PATCH 4/4] fix: keep ID-less provider records after NameConflict and
 check the deployment before recorded-ID recovery

Refs #422

Change-Id: I320c0b1544475654f0a085c8ffd32e98a5003899
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 docs/how-to/manage-ai-providers.md            | 2 +-
 internal/controller/coderaiprovider_delete.go | 9 ++++++---
 internal/controller/codermcpserver_create.go  | 8 ++++++--
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md
index 1c6f27683..88b10fe97 100644
--- a/docs/how-to/manage-ai-providers.md
+++ b/docs/how-to/manage-ai-providers.md
@@ -123,7 +123,7 @@ These limits remain:
 
 ## Delete a provider
 
-Deleting the object deletes the provider in Coder. The operator deletes only the provider ID that it recorded. With only an unconfirmed create, it deletes nothing: a person can edit the display name of any provider, so not even the display-name nonce of the create proves that this object created a provider. When Coder has a provider of this name and type, the finalizer goes without a delete, and the [cleanup record](#cleanup-records) stays for manual cleanup. A create request can still commit for a short time, so within 2 minutes of that request the operator reads again before it trusts that the provider is absent.
+Deleting the object deletes the provider in Coder. The operator deletes only the provider ID that it recorded. With only an unconfirmed create, it deletes nothing: a person can edit the display name of any provider, so not even the display-name nonce of the create proves that this object created a provider. When Coder has a provider of this name and type, the finalizer goes without a delete, and the [cleanup record](#cleanup-records) stays for manual cleanup. A create request can still commit for a short time, so within 2 minutes of that request the operator reads again before it trusts that the provider is absent. A record without a provider ID also stays when the object reports `NameConflict` after a create: a person can have edited the display name of the provider of that create.
 
 - When Coder is unreachable, or the control plane has no operator access, the finalizer `coder.com/ai-provider-cleanup` stays and the operator retries every 60 seconds. See [Stuck finalizer](#stuck-finalizer).
 - When the control plane is gone, the operator cannot delete the provider now. The finalizer goes at once, and the operator logs the provider ID. The [cleanup record](#cleanup-records) stays, so a later sweep can delete the provider.
diff --git a/internal/controller/coderaiprovider_delete.go b/internal/controller/coderaiprovider_delete.go
index 02ba3e2fb..50faa503a 100644
--- a/internal/controller/coderaiprovider_delete.go
+++ b/internal/controller/coderaiprovider_delete.go
@@ -61,14 +61,17 @@ func (r *CoderAIProviderReconciler) deleteStep(ctx context.Context, provider *co
 	if provider.Status.ProviderID == "" && provider.Status.CreateAttemptTime == nil {
 		// Earlier Coder answers cleared the create marker. A record with a
 		// provider ID still locates a provider, for example in a deployment
-		// that the control plane no longer reaches (CoderDeploymentChanged),
-		// so only a record without one proves that nothing exists.
+		// that the control plane no longer reaches (CoderDeploymentChanged).
+		// A record without one stays too: a NameConflict clears the marker
+		// also when a person edited the display name of the provider of a
+		// lost create answer. The sweep removes it once Coder holds no
+		// provider of that name and type (#422).
 		record, _, err := readCleanupRecord(ctx, r.APIReader, r.OperatorNamespace, provider.UID)
 		if err != nil && !errors.Is(err, errNotACleanupRecord) {
 			return false, nil, err
 		}
 		if record == nil || record.CoderID == "" {
-			return true, nil, deleteCleanupRecord(ctx, r.Client, r.APIReader, r.OperatorNamespace, provider.UID)
+			return true, nil, nil
 		}
 	}
 	ref := types.NamespacedName{Namespace: provider.Namespace, Name: provider.Spec.ControlPlaneRef.Name}
diff --git a/internal/controller/codermcpserver_create.go b/internal/controller/codermcpserver_create.go
index cc2fc1848..ac9a7e498 100644
--- a/internal/controller/codermcpserver_create.go
+++ b/internal/controller/codermcpserver_create.go
@@ -55,13 +55,17 @@ func (r *CoderMCPServerReconciler) createOrRecord(
 		return keepRejectedMCPServerCreate(step, rejected), nil
 	}
 	if found != nil {
-		// A status that lost the config ID still has it in the record (#422).
+		// A status that lost the config ID still has it in the record, which
+		// proves the config only in the record's deployment (#422).
 		record, _, err := readCleanupRecord(ctx, r.APIReader, r.OperatorNamespace, server.UID)
 		if err != nil {
 			return nil, err
 		}
 		recordedID := ""
-		if record != nil {
+		if record != nil && record.CoderID == found.ID.String() {
+			if step, err := r.recordedDeploymentChanged(ctx, sdk, server); step != nil || err != nil {
+				return step, err
+			}
 			recordedID = record.CoderID
 		}
 		if step := recordMCPServer(server, *found, recordedID); step != nil {