diff --git a/api/v1alpha1/codermcpserver_types.go b/api/v1alpha1/codermcpserver_types.go index 1a05fe4e..f4f78a8d 100644 --- a/api/v1alpha1/codermcpserver_types.go +++ b/api/v1alpha1/codermcpserver_types.go @@ -308,6 +308,11 @@ type CoderMCPServerStatus struct { // ConfigID is the Coder MCP server config ID. // +optional ConfigID string `json:"configID,omitempty"` + // Enabled is the enabled flag of the Coder MCP server config at the + // controller's last read or update of it. It is empty while Coder holds no + // config of this object, or before the first read. + // +optional + Enabled *bool `json:"enabled,omitempty"` // CreateAttemptTime is set before every create request. While it is set // and ConfigID is empty, a create request can be in flight or already // committed in Coder. @@ -332,7 +337,7 @@ type CoderMCPServerStatus struct { // +kubebuilder:printcolumn:name="Organization",type=string,JSONPath=`.spec.organization` // +kubebuilder:printcolumn:name="Slug",type=string,JSONPath=`.spec.slug` // +kubebuilder:printcolumn:name="Auth",type=string,JSONPath=`.spec.auth.type` -// +kubebuilder:printcolumn:name="Enabled",type=boolean,JSONPath=`.spec.enabled` +// +kubebuilder:printcolumn:name="Enabled",type=boolean,JSONPath=`.status.enabled` // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e70d03b1..3041c3fb 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -896,6 +896,11 @@ func (in *CoderMCPServerSpec) DeepCopy() *CoderMCPServerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CoderMCPServerStatus) DeepCopyInto(out *CoderMCPServerStatus) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } if in.CreateAttemptTime != nil { in, out := &in.CreateAttemptTime, &out.CreateAttemptTime *out = (*in).DeepCopy() diff --git a/config/crd/bases/coder.com_codermcpservers.yaml b/config/crd/bases/coder.com_codermcpservers.yaml index f2af937c..21dd3e97 100644 --- a/config/crd/bases/coder.com_codermcpservers.yaml +++ b/config/crd/bases/coder.com_codermcpservers.yaml @@ -24,7 +24,7 @@ spec: - jsonPath: .spec.auth.type name: Auth type: string - - jsonPath: .spec.enabled + - jsonPath: .status.enabled name: Enabled type: boolean - jsonPath: .status.conditions[?(@.type=="Ready")].status @@ -537,6 +537,12 @@ spec: x-kubernetes-list-map-keys: - field x-kubernetes-list-type: map + enabled: + description: |- + Enabled is the enabled flag of the Coder MCP server config at the + controller's last read or update of it. It is empty while Coder holds no + config of this object, or before the first read. + type: boolean observedGeneration: description: ObservedGeneration is the generation the controller last processed. diff --git a/config/gitops/argocd-health-codermcpserver.lua b/config/gitops/argocd-health-codermcpserver.lua index 7e0ac9e8..58b40861 100644 --- a/config/gitops/argocd-health-codermcpserver.lua +++ b/config/gitops/argocd-health-codermcpserver.lua @@ -13,12 +13,19 @@ local st = obj.status if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then return hs end +-- A condition from an older generation does not describe the current spec. +local conditions = {} for _, c in ipairs(st.conditions or {}) do + if c.observedGeneration == obj.metadata.generation then + table.insert(conditions, c) + end +end +for _, c in ipairs(conditions) do if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then hs.message = c.reason .. ": " .. c.message end end -for _, c in ipairs(st.conditions or {}) do +for _, c in ipairs(conditions) do if c.type == "Ready" and c.status == "True" then hs.status = "Healthy" elseif c.type == "Stalled" and c.status == "True" then diff --git a/config/gitops/flux-healthcheck-codermcpserver.yaml b/config/gitops/flux-healthcheck-codermcpserver.yaml index ac769664..a55d4402 100644 --- a/config/gitops/flux-healthcheck-codermcpserver.yaml +++ b/config/gitops/flux-healthcheck-codermcpserver.yaml @@ -4,6 +4,6 @@ # docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. - apiVersion: coder.com/v1alpha1 kind: CoderMCPServer - inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || !status.conditions.exists(c, c.type == 'Ready' && has(c.observedGeneration) && c.observedGeneration == metadata.generation) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" diff --git a/config/gitops/health_test.go b/config/gitops/health_test.go index b91405d4..ed6d47d9 100644 --- a/config/gitops/health_test.go +++ b/config/gitops/health_test.go @@ -156,6 +156,13 @@ func mcpServerCases() []healthCase { stale := readyReconcilingStalled("ready", "Applied") stale["observedGeneration"] = int64(1) noConditions := map[string]any{"observedGeneration": int64(2)} + // An update interrupted before Coder committed it, as stored by + // controllers before the #415 fix: only the top-level generation moved. + interrupted := readyReconcilingStalled("ready", "Applied") + interrupted["appliedGeneration"] = int64(1) + for _, c := range interrupted["conditions"].([]any) { + c.(map[string]any)["observedGeneration"] = int64(1) + } deleting := mcpServerObject(readyReconcilingStalled("ready", "Applied")) deleting["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-14T10:00:00Z" deletingWait := mcpServerObject(readyReconcilingStalled("reconciling", "CoderUnavailable")) @@ -165,6 +172,7 @@ func mcpServerCases() []healthCase { {"stale observedGeneration", mcpServerObject(stale), "Progressing", "Waiting for the controller", "InProgress", ""}, {"no conditions", mcpServerObject(noConditions), "Progressing", "Waiting for the controller", "InProgress", ""}, {"applied", mcpServerObject(readyReconcilingStalled("ready", "Applied")), "Healthy", "Applied: details", "Current", ""}, + {"stale conditions", mcpServerObject(interrupted), "Progressing", "Waiting for the controller", "InProgress", ""}, {"control plane not ready", mcpServerObject(readyReconcilingStalled("reconciling", "ControlPlaneNotReady")), "Progressing", "ControlPlaneNotReady: details", "InProgress", ""}, {"Coder unavailable", mcpServerObject(readyReconcilingStalled("reconciling", "CoderUnavailable")), "Progressing", "CoderUnavailable: details", "InProgress", ""}, {"group not found", mcpServerObject(readyReconcilingStalled("stalled", "GroupNotFound")), "Degraded", "GroupNotFound: details", "Failed", ""}, diff --git a/dist/install.yaml b/dist/install.yaml index 7348b38a..67f6fa28 100644 --- a/dist/install.yaml +++ b/dist/install.yaml @@ -5229,7 +5229,7 @@ spec: - jsonPath: .spec.auth.type name: Auth type: string - - jsonPath: .spec.enabled + - jsonPath: .status.enabled name: Enabled type: boolean - jsonPath: .status.conditions[?(@.type=="Ready")].status @@ -5742,6 +5742,12 @@ spec: x-kubernetes-list-map-keys: - field x-kubernetes-list-type: map + enabled: + description: |- + Enabled is the enabled flag of the Coder MCP server config at the + controller's last read or update of it. It is empty while Coder holds no + config of this object, or before the first read. + type: boolean observedGeneration: description: ObservedGeneration is the generation the controller last processed. diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 2888ac79..1e8d5e85 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -106,6 +106,8 @@ kubectl -n coder get codermcpservers kubectl -n coder get codermcpserver example -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}: {.message}{"\n"}{end}' ``` +The `ENABLED` column shows `status.enabled`: the enabled flag of the config in Coder at the operator's last read or update. It is empty while Coder holds no config of this object, for example when the credential Secret is refused before the create, and before the first read. + The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled object waits for a change of its spec or of an input. | Reason | Meaning | @@ -228,11 +230,11 @@ These limits come from Coder, and the operator cannot change them: ## GitOps health checks -Argo CD and Flux need health rules to wait for a `CoderMCPServer`. The rules live in `config/gitops/`. This page copies them, and a unit test keeps the copies equal to the files. Both rules wait until `status.observedGeneration` equals `metadata.generation`. Then they read the conditions: +Argo CD and Flux need health rules to wait for a `CoderMCPServer`. The rules live in `config/gitops/`. This page copies them, and a unit test keeps the copies equal to the files. The controller sets `status.observedGeneration` to `metadata.generation` only when it writes the conditions for that generation. Both rules wait until `status.observedGeneration` equals `metadata.generation`. They also ignore a condition whose own `observedGeneration` is older, so a status that an older controller wrote in the middle of an update does not count as healthy. Then they read the conditions: | Conditions | Argo CD | Flux | | --- | --- | --- | -| no status, or a stale `observedGeneration` | `Progressing` | `InProgress` (`Unknown` before the first status) | +| no status, or a stale `observedGeneration` on the status or the `Ready` condition | `Progressing` | `InProgress` (`Unknown` before the first status) | | `Reconciling=True` | `Progressing` | `InProgress` | | `Ready=True` | `Healthy` | `Current` | | `Stalled=True` | `Degraded` | `Failed` | @@ -257,12 +259,19 @@ local st = obj.status if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then return hs end +-- A condition from an older generation does not describe the current spec. +local conditions = {} for _, c in ipairs(st.conditions or {}) do + if c.observedGeneration == obj.metadata.generation then + table.insert(conditions, c) + end +end +for _, c in ipairs(conditions) do if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then hs.message = c.reason .. ": " .. c.message end end -for _, c in ipairs(st.conditions or {}) do +for _, c in ipairs(conditions) do if c.type == "Ready" and c.status == "True" then hs.status = "Healthy" elseif c.type == "Stalled" and c.status == "True" then @@ -281,7 +290,7 @@ Add this entry (`config/gitops/flux-healthcheck-codermcpserver.yaml`) to `spec.h # docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. - apiVersion: coder.com/v1alpha1 kind: CoderMCPServer - inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || !status.conditions.exists(c, c.type == 'Ready' && has(c.observedGeneration) && c.observedGeneration == metadata.generation) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" ``` diff --git a/docs/reference/api/codermcpserver.md b/docs/reference/api/codermcpserver.md index 831f4856..a0b7c651 100644 --- a/docs/reference/api/codermcpserver.md +++ b/docs/reference/api/codermcpserver.md @@ -37,6 +37,7 @@ | `appliedGeneration` | integer | AppliedGeneration is the last generation whose spec Coder fully holds, or that a kill switch holds: when the controller disables the config because a credential Secret or the control plane withdrew consent, it sets this field before it sends the disable, so only a later spec edit enables the config again. Ready tells whether Coder holds the spec. | | `organizationID` | string | OrganizationID is the Coder organization ID that spec.organization resolved to first. | | `configID` | string | ConfigID is the Coder MCP server config ID. | +| `enabled` | boolean | Enabled is the enabled flag of the Coder MCP server config at the controller's last read or update of it. It is empty while Coder holds no config of this object, or before the first read. | | `createAttemptTime` | [Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#time-v1-meta) | CreateAttemptTime is set before every create request. While it is set and ConfigID is empty, a create request can be in flight or already committed in Coder. | | `credentials` | [CoderMCPServerCredentialStatus](#codermcpservercredentialstatus) array | Credentials record the Secret values last sent to Coder. | | `conditions` | [Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#condition-v1-meta) array | Conditions are Ready, Reconciling, and Stalled. | diff --git a/internal/controller/coderaiprovider_column_test.go b/internal/controller/coderaiprovider_column_test.go index 2ecf9e09..efa52ac9 100644 --- a/internal/controller/coderaiprovider_column_test.go +++ b/internal/controller/coderaiprovider_column_test.go @@ -11,20 +11,20 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" - coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" "github.com/coder/coder-k8s/internal/credentialsecret" ) -// enabledColumn returns the Enabled cell that kubectl get prints for -// provider, read as a server-side table from the API server. -func enabledColumn(ctx context.Context, t *testing.T, provider *coderv1alpha1.CoderAIProvider) any { +// enabledColumn returns the Enabled cell that kubectl get prints for obj of +// the coder.com resource, read as a server-side table from the API server. +func enabledColumn(ctx context.Context, t *testing.T, resource string, obj client.Object) any { t.Helper() server, _, err := rest.DefaultServerUrlFor(cfg) require.NoError(t, err) httpClient, err := rest.HTTPClientFor(cfg) require.NoError(t, err) - server.Path = "/apis/coder.com/v1alpha1/namespaces/" + provider.Namespace + "/coderaiproviders/" + provider.Name + server.Path = "/apis/coder.com/v1alpha1/namespaces/" + obj.GetNamespace() + "/" + resource + "/" + obj.GetName() req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.String(), nil) require.NoError(t, err) req.Header.Set("Accept", "application/json;as=Table;v=v1;g=meta.k8s.io") @@ -55,23 +55,23 @@ func TestCoderAIProviderEnabledColumn(t *testing.T) { got, _ := env.reconcile(ctx, t, env.createProvider(ctx, t, nil)) requireAIProviderStep(t, got, true, credentialsecret.ReasonSecretNotFound) require.True(t, *got.Spec.Enabled) - require.Nil(t, enabledColumn(ctx, t, got)) + require.Nil(t, enabledColumn(ctx, t, "coderaiproviders", got)) }) t.Run("kill switch, then deleted in Coder", func(t *testing.T) { t.Parallel() env, secret, provider := newReadyAIProvider(ctx, t) - require.Equal(t, true, enabledColumn(ctx, t, provider)) + require.Equal(t, true, enabledColumn(ctx, t, "coderaiproviders", provider)) consented := secret.DeepCopy() editSecret(ctx, t, secret, func(s *corev1.Secret) { delete(s.Labels, credentialsecret.LabelKey) }) got, _ := env.reconcile(ctx, t, provider) requireAIProviderStep(t, got, true, credentialsecret.ReasonNotOptedIn) - require.Equal(t, false, enabledColumn(ctx, t, got)) + require.Equal(t, false, enabledColumn(ctx, t, "coderaiproviders", got)) editSecret(ctx, t, secret, func(s *corev1.Secret) { s.Labels = consented.Labels }) require.NoError(t, env.fake.client(t, 2*time.Second).DeleteAIProvider(ctx, got.Status.ProviderID)) got, _ = env.reconcile(ctx, t, got) requireAIProviderStep(t, got, true, "DeletedExternally") - require.Nil(t, enabledColumn(ctx, t, got)) + require.Nil(t, enabledColumn(ctx, t, "coderaiproviders", got)) }) } diff --git a/internal/controller/codermcpserver_apply.go b/internal/controller/codermcpserver_apply.go index 9f29f02d..ac70378d 100644 --- a/internal/controller/codermcpserver_apply.go +++ b/internal/controller/codermcpserver_apply.go @@ -51,10 +51,11 @@ func (r *CoderMCPServerReconciler) syncMCPServer( // Only Coder's own 404 proves that the config is gone. A proxy page or // "Route not found." waits below and changes nothing (#337). case isCoderResourceNotFound(err) && applied: + server.Status.Enabled = nil return mcpServerStalled(mcpServerResync, "DeletedExternally", "MCP server config %s was deleted in Coder. The controller does not create it again until the spec changes.", id), false, nil case isCoderResourceNotFound(err): - server.Status.ConfigID, server.Status.Credentials = "", nil + server.Status.ConfigID, server.Status.Credentials, server.Status.Enabled = "", nil, nil r.sentCredentials.Delete(client.ObjectKeyFromObject(server)) return nil, true, nil case err != nil: @@ -64,6 +65,7 @@ func (r *CoderMCPServerReconciler) syncMCPServer( return mcpServerStalled(mcpServerResync, "CoderAnswerMismatch", "assertion failed: Coder answered MCP server config %s with slug %s in organization %s when asked for %s", found.ID, found.Slug, found.OrganizationID, id), false, nil } + server.Status.Enabled = &found.Enabled // user_oidc sends each chat user's login token to url (plan F14), so it is // set back even when a kill switch holds every other write. @@ -127,6 +129,7 @@ func (r *CoderMCPServerReconciler) syncMCPServer( if field := mcpServerMismatch(server.Spec.Auth, check, updated); field != "" || updated.ID != id { return mcpServerStalled(mcpServerResync, "CoderAnswerMismatch", "assertion failed: Coder answered the update of MCP server config %s with another %s", id, cmp.Or(field, "ID")), false, nil } + server.Status.Enabled = &updated.Enabled r.rememberSentCredentials(server, sent, values) } else if field := mcpServerMismatch(server.Spec.Auth, full, found); field != "" { return nil, false, fmt.Errorf("assertion failed: MCP server %s/%s differs in %s, but the sync request is empty", server.Namespace, server.Name, field) @@ -160,6 +163,7 @@ func (r *CoderMCPServerReconciler) disableForMissingGroup( return mcpServerWait(mcpServerRetry, "CoderAnswerMismatch", "Coder answered the disable of MCP server config %s with another or an enabled config. The controller disables it again.", id) } + server.Status.Enabled = &updated.Enabled return missing } diff --git a/internal/controller/codermcpserver_column_test.go b/internal/controller/codermcpserver_column_test.go new file mode 100644 index 00000000..5917f2d3 --- /dev/null +++ b/internal/controller/codermcpserver_column_test.go @@ -0,0 +1,89 @@ +package controller_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" + "github.com/coder/coder-k8s/internal/credentialsecret" +) + +// #415: the Enabled column shows what Coder holds, not spec.enabled. A +// refused object that Coder does not have shows no value. +func TestCoderMCPServerEnabledColumn(t *testing.T) { + t.Parallel() + ctx := context.Background() + t.Run("refused before a create", func(t *testing.T) { + t.Parallel() + env := newMCPServerEnv(ctx, t, true) + got, _ := env.reconcile(ctx, t, env.create(ctx, t, nil)) + requireMCPServerStep(t, got, true, credentialsecret.ReasonSecretNotFound) + require.True(t, *got.Spec.Enabled) + require.Nil(t, enabledColumn(ctx, t, "codermcpservers", got)) + }) + t.Run("opt-in withdrawn, then deleted in Coder", func(t *testing.T) { + t.Parallel() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, id := env.applied(ctx, t, nil) + require.Equal(t, true, enabledColumn(ctx, t, "codermcpservers", got)) + + optIn := env.controlPlane.Spec.MCPServers.DeepCopy() + env.setOptIn(ctx, t, nil) + got, _ = env.reconcile(ctx, t, got) + requireMCPServerStep(t, got, true, "MCPServersNotAllowed") + require.Equal(t, false, enabledColumn(ctx, t, "codermcpservers", got)) + + env.setOptIn(ctx, t, optIn) + got, _ = env.reconcile(ctx, t, got) + requireMCPServerStep(t, got, true, "DisabledExternally") + require.Equal(t, false, enabledColumn(ctx, t, "codermcpservers", got)) + + require.NoError(t, env.dashboard(t).DeleteMCPServerConfig(ctx, env.orgID, id)) + got, _ = env.reconcile(ctx, t, got) + requireMCPServerStep(t, got, true, "DeletedExternally") + require.Nil(t, enabledColumn(ctx, t, "codermcpservers", got)) + }) +} + +// #415: a status write between two steps must not advance +// status.observedGeneration past the conditions, or GitOps health checks read +// the conditions of an older generation as current. The write that records a +// committed create runs before the PATCH of the sync. +func TestCoderMCPServerInterruptedUpdateKeepsObservedGeneration(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + server := env.create(ctx, t, nil) + env.fake.failNext(routeCreateMCPServer, fakeFault{Reset: true, AfterCommit: true}) + got, _ := env.reconcile(ctx, t, server) + requireMCPServerStep(t, got, false, "ConfirmingCreate") + require.Empty(t, got.Status.ConfigID) + confirmedGeneration := got.Generation + + require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(got), got)) + got.Spec.DisplayName = "GitHub tools" + require.NoError(t, k8sClient.Update(ctx, got)) + env.fake.failNext(routeUpdateMCPServer, fakeFault{Hang: true}) + short, cancel := context.WithTimeout(ctx, hangTimeout) + defer cancel() + _, err := env.r.Reconcile(short, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(got)}) + require.Error(t, err) + + stored := &coderv1alpha1.CoderMCPServer{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(got), stored)) + require.NotEmpty(t, stored.Status.ConfigID, "the config ID was written before the PATCH") + require.Equal(t, confirmedGeneration, stored.Status.ObservedGeneration, "the config ID write does not observe the edit") + require.Equal(t, confirmedGeneration, meta.FindStatusCondition(stored.Status.Conditions, "Ready").ObservedGeneration) + require.Equal(t, "coder-k8s:"+string(stored.UID), env.stored(t, stored).view.DisplayName) + + got, _ = env.reconcile(ctx, t, stored) + requireMCPServerApplied(t, got) + require.Equal(t, "GitHub tools", env.stored(t, got).view.DisplayName) +} diff --git a/internal/controller/codermcpserver_controller.go b/internal/controller/codermcpserver_controller.go index afee054b..ba65e933 100644 --- a/internal/controller/codermcpserver_controller.go +++ b/internal/controller/codermcpserver_controller.go @@ -545,8 +545,11 @@ func (r *CoderMCPServerReconciler) patchFinalizer(ctx context.Context, server *c } // applyMCPServerStep sets the kstatus conditions for step. Messages are cut to -// the CoderTemplateTest limit. +// the CoderTemplateTest limit. status.observedGeneration moves only here, so a +// status write between two steps never claims a generation that the +// conditions do not describe yet. func applyMCPServerStep(server *coderv1alpha1.CoderMCPServer, step *mcpServerStep) { + server.Status.ObservedGeneration = server.Generation message := truncateTemplateTestMessage(step.message) ready, reconciling, stalled := metav1.ConditionFalse, metav1.ConditionTrue, metav1.ConditionFalse switch { @@ -571,7 +574,6 @@ func applyMCPServerStep(server *coderv1alpha1.CoderMCPServer, step *mcpServerSte // writeStatus writes the status only when it changed. func (r *CoderMCPServerReconciler) writeStatus(ctx context.Context, server *coderv1alpha1.CoderMCPServer, before *coderv1alpha1.CoderMCPServerStatus) error { - server.Status.ObservedGeneration = server.Generation if equality.Semantic.DeepEqual(before, &server.Status) { return nil } diff --git a/internal/controller/codermcpserver_create.go b/internal/controller/codermcpserver_create.go index 04f1d49f..fa30f258 100644 --- a/internal/controller/codermcpserver_create.go +++ b/internal/controller/codermcpserver_create.go @@ -106,7 +106,7 @@ func recordMCPServer(server *coderv1alpha1.CoderMCPServer, found codersdk.MCPSer "Coder organization %s already has an MCP server config with slug %s (ID %s). The controller does not change a config that it did not create.", server.Spec.Organization, server.Spec.Slug, found.ID) } - server.Status.ConfigID, server.Status.CreateAttemptTime = found.ID.String(), nil + server.Status.ConfigID, server.Status.CreateAttemptTime, server.Status.Enabled = found.ID.String(), nil, &found.Enabled return nil } @@ -140,7 +140,7 @@ func (r *CoderMCPServerReconciler) createMCPServer( return mcpServerWait(mcpServerConfirmPoll, "ConfirmingCreate", "The create answer does not match the request. The controller reads Coder instead of sending it again."), nil } - server.Status.ConfigID, server.Status.CreateAttemptTime = created.ID.String(), nil + server.Status.ConfigID, server.Status.CreateAttemptTime, server.Status.Enabled = created.ID.String(), nil, &created.Enabled var sent []string for _, c := range mcpServerCredentialRefs(server.Spec.Auth) { sent = append(sent, c.field) diff --git a/internal/controller/codermcpserver_disable.go b/internal/controller/codermcpserver_disable.go index 4e9e5573..fbe493fa 100644 --- a/internal/controller/codermcpserver_disable.go +++ b/internal/controller/codermcpserver_disable.go @@ -87,7 +87,7 @@ func readMCPServerToDisable( // The kill switch holds a gone config too: a pending generation must // not create an enabled config again once consent or the opt-in comes // back, before a spec edit. - server.Status.AppliedGeneration = server.Generation + server.Status.AppliedGeneration, server.Status.Enabled = server.Generation, nil return found, stalled("MCP server config %s does not exist in Coder.", id) case err != nil: return found, unchecked("get MCP server config: %s", coderErrorSummary(err)) @@ -96,6 +96,7 @@ func readMCPServerToDisable( return found, mcpServerStalled(mcpServerResync, "CoderAnswerMismatch", "assertion failed: Coder answered MCP server config %s with slug %s in organization %s when asked for %s", found.ID, found.Slug, found.OrganizationID, id) } + server.Status.Enabled = &found.Enabled if !found.Enabled { // The disable answer can be lost after Coder committed it. Whoever // disabled the config, the kill switch now holds this generation too, @@ -156,6 +157,7 @@ func (r *CoderMCPServerReconciler) sendMCPServerDisable( return mcpServerWait(mcpServerRetry, "CoderAnswerMismatch", "Coder answered the disable of MCP server config %s with another or an enabled config. The controller disables it again.", id), nil } + server.Status.Enabled = &updated.Enabled return nil, nil }