🤖 fix: trust only Coder's own 404 on the CoderAIProvider create lookups (#350)

## Summary

The `CoderAIProvider` create path now trusts only Coder's own 404 on its
name lookups, as the delete path (#325) and the sync and disable paths
(#335) already do.

Fixes #338
Refs #154

## Why

A 404 that Coder did not send (an HTML page from a proxy, or Coder's
`Route not found.` for a wrong base URL) counted as "no such provider"
in two places in `internal/controller/coderaiprovider_create.go`:

1. The name lookup before a create, which also confirms a pending create
(`createOrConfirm`). With a create marker, such a 404 could clear the
marker of a create that committed. A delete at that point released the
finalizer without a delete in Coder, and Coder kept a provider that
holds the key. Coder created that provider with `enabled: false`, so it
did not use the key.
2. The read after a rejected create (`confirmRejectedCreate`). Such a
404 cleared the marker in the same way.

## What

Both lookups use `isCoderResourceNotFound`: a 404 whose message is
Coder's fixed `Resource not found or you do not have access to this
resource`. Any other answer falls through to the existing waits. The
lookup waits with `CoderUnavailable` (30 s), keeps the marker and sends
no create. After a rejected create, the object waits with
`ConfirmingRejectedCreate` and keeps the marker.

Docs nit from #338: the credential kill switch paragraph in
`docs/how-to/manage-ai-providers.md` now says that a failed Coder call
for the disable, including a 404 that Coder did not send, retries every
30 seconds with a message that starts with `Coder can still send the
key`.

## Tests

- `TestCoderAIProviderCreateLookupNotCoder404`: an HTML 404 and `Route
not found.` on the name lookup, for a new create and for a pending
create whose create committed. Each one waits with `CoderUnavailable`,
sends no create, and a pending create keeps its marker. The next
reconcile is `Ready`, with one provider in Coder.
- `TestCoderAIProviderRejectedCreateNotCoder404`: after a 400 create
answer, a non-Coder 404 on the confirmation read keeps the marker
(`ConfirmingRejectedCreate`). The next read stalls with `CoderRejected`,
and the create is not sent again.
- The existing name-conflict test now injects Coder's own 404 message.

Mutation checks (each one breaks a named test, then I reverted it):
`isCoderNotFound` back on the name lookup, and on the read after a
rejected create.

Local gates on the pushed tree, exit 0 each: `make verify-vendor`, `make
test`, `make test-integration`, `make build`, `make lint`, `go tool
govulncheck ./...`, `go test -race ./internal/controller/`, `make
codegen manifests build-installer docs-reference` with no diff, and
`make docs-check`. Before the rebase, two runs of `make test` and one
race run failed with envtest timeouts (`timeout waiting for process
kube-apiserver to stop`) while the host load average was about 130.
Those runs failed outside this change, and all gates pass on this tree.

---

_Generated with `mux` • Model: `anthropic:claude-opus-5-5` • Thinking:
`high` • Cost: `$129.14`_

<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high
costs=129.14 -->

Signed-off-by: Thomas Kosiewski <tk@coder.com>

Author: ThomasK33

docs/how-to/manage-ai-providers.md

@@ -83,7 +83,7 @@ The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled ob
 
 ### The credential kill switch
 
-When the key Secret fails the credential contract for a provider that the operator created, the operator disables the provider in Coder at once. This covers every credential error: the Secret is deleted, loses its label or the destinations annotation, no longer lists the origin that Coder uses, is refused, has no such key, or holds a malformed value. The update holds only `enabled: false`. Coder keeps the old key, but no longer uses it.
+When the key Secret fails the credential contract for a provider that the operator created, the operator disables the provider in Coder at once. This covers every credential error: the Secret is deleted, loses its label or the destinations annotation, no longer lists the origin that Coder uses, is refused, has no such key, or holds a malformed value. The update holds only `enabled: false`. Coder keeps the old key, but no longer uses it. When a Coder call for the disable fails, including a 404 that Coder did not send (for example from a proxy), the operator retries every 30 seconds, and the condition message starts with `Coder can still send the key`.
 
 The object stalls with the credential reason. Repairing the Secret does not enable the provider again. To enable it, fix the Secret, then edit the spec (any change of `spec`, for example `displayName`). A short read error of the Secret, such as an API server timeout, is not a credential error: the operator retries and changes nothing.
 

internal/controller/coderaiprovider_create.go

@@ -49,7 +49,9 @@ func (r *CoderAIProviderReconciler) createOrConfirm(
 		ready.Reason == aiProviderConfirmingRejectedCreate && ready.ObservedGeneration == provider.Generation
 	found, err := sdk.AIProvider(ctx, provider.Name)
 	switch {
-	case isCoderNotFound(err):
+	case isCoderResourceNotFound(err):
+		// Only Coder's own 404 proves that the name is free. Any other
+		// answer keeps the marker and waits (#338).
 		if provider.Status.CreateAttemptTime != nil {
 			if step := requireSiteOwner(ctx, sdk); step != nil {
 				return keepRejectedCreate(step, rejected), nil
@@ -192,7 +194,7 @@ func (r *CoderAIProviderReconciler) confirmRejectedCreate(
 	case err == nil:
 		// Ours (the create committed) or someone else's (NameConflict).
 		return recordAIProvider(ctx, provider, found)
-	case !isCoderNotFound(err):
+	case !isCoderResourceNotFound(err):
 		reason := "ConfirmingCreate"
 		if !aiProviderCreateRetryable(status) {
 			// Remember the rejection, so a later read that confirms absence

internal/controller/coderaiprovider_create_test.go

@@ -2,6 +2,7 @@ package controller_test
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"testing"
 	"time"
@@ -10,6 +11,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -284,7 +286,7 @@ func TestCoderAIProviderCreateConflict(t *testing.T) {
 	require.NoError(t, err)
 	setupPosts := env.fake.requestCount(routeCreateAIProvider)
 	// The lookup does not see it yet.
-	env.fake.failNext(routeAIProvider, fakeFault{Status: http.StatusNotFound})
+	env.fake.failNext(routeAIProvider, fakeFault{Status: http.StatusNotFound, Message: coderResourceNotFound})
 
 	got, _ := env.reconcile(ctx, t, provider)
 	require.Contains(t, requireAIProviderStep(t, got, true, "NameConflict"), foreign.ID.String())
@@ -371,3 +373,65 @@ func TestCoderAIProviderRejectedCreateOwnerCheckFails(t *testing.T) {
 	require.Nil(t, got.Status.CreateAttemptTime)
 	require.Equal(t, 1, env.fake.requestCount(routeCreateAIProvider), "the rejected create was not sent again")
 }
+
+// #338: a 404 that Coder did not send on the name lookup proves nothing. A
+// pending create keeps its marker and sends no create, a new create is not
+// sent, and the next reconcile with Coder's own answer decides.
+func TestCoderAIProviderCreateLookupNotCoder404(t *testing.T) {
+	t.Parallel()
+	for name, fault := range notCoder404s {
+		for _, pending := range []bool{false, true} {
+			t.Run(fmt.Sprintf("%s/pending=%v", name, pending), func(t *testing.T) {
+				t.Parallel()
+				ctx := context.Background()
+				env, secret, provider := newAIProviderCreateEnv(ctx, t)
+				if pending {
+					// A create that committed, but whose answer was lost.
+					_, err := env.fake.client(t, 2*time.Second).CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{
+						Type: codersdk.AIProviderTypeOpenAI, Name: provider.Name, DisplayName: "coder-k8s:" + string(provider.UID),
+						BaseURL: provider.Spec.BaseURL, APIKeys: []string{testAIProviderKey},
+					})
+					require.NoError(t, err)
+					provider.Status.CreateAttemptTime = &metav1.Time{Time: time.Now()}
+					provider.Status.Credential = &coderv1alpha1.CoderAIProviderCredentialStatus{SecretName: secret.Name, SecretKey: "api-key"}
+					require.NoError(t, k8sClient.Status().Update(ctx, provider))
+				}
+				posts := env.fake.requestCount(routeCreateAIProvider)
+				env.fake.failNext(routeAIProvider, fault)
+				got, result := env.reconcile(ctx, t, provider)
+				requireAIProviderStep(t, got, false, "CoderUnavailable")
+				require.Equal(t, 30*time.Second, result.RequeueAfter)
+				require.Equal(t, posts, env.fake.requestCount(routeCreateAIProvider), "no create is sent")
+				require.Equal(t, pending, got.Status.CreateAttemptTime != nil, "a pending create keeps its marker")
+
+				got, _ = env.reconcile(ctx, t, got)
+				requireAIProviderReady(t, got)
+				require.Equal(t, 1, env.fake.aiProviderCount(), "one provider in Coder")
+			})
+		}
+	}
+}
+
+// #338: after a rejected create, the read that confirms absence also needs
+// Coder's own 404. Otherwise the marker stays and the create is not sent
+// again.
+func TestCoderAIProviderRejectedCreateNotCoder404(t *testing.T) {
+	t.Parallel()
+	for name, fault := range notCoder404s {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			ctx := context.Background()
+			env, _, provider := newAIProviderCreateEnv(ctx, t)
+			env.fake.failNext(routeAIProvider, fakeFault{Rewrite: func(answer any) any { return answer }}) // The name lookup passes.
+			env.fake.failNext(routeAIProvider, fault)                                                      // The confirmation read.
+			env.fake.failNext(routeCreateAIProvider, fakeFault{Status: http.StatusBadRequest})
+			got, _ := env.reconcile(ctx, t, provider)
+			requireAIProviderStep(t, got, false, "ConfirmingRejectedCreate")
+			require.NotNil(t, got.Status.CreateAttemptTime, "the marker stays")
+			got, _ = env.reconcile(ctx, t, got)
+			requireAIProviderStep(t, got, true, "CoderRejected")
+			require.Nil(t, got.Status.CreateAttemptTime)
+			require.Equal(t, 1, env.fake.requestCount(routeCreateAIProvider), "the rejected create was not sent again")
+		})
+	}
+}