🤖 fix: delete Coder providers and MCP configs only by recorded ID (#422 items 1, 4, 5)

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. 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.
@@ -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

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.

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