Skip to content

MCPRemoteProxy config-handler errors leave Ready=True while Phase=Failed #6770

Description

@feiiiiii5

Bug description

validateAndHandleConfigs (cmd/thv-operator/controllers/mcpremoteproxy_controller.go:232) runs six config handlers; on error each writes only proxy.Status.Phase = Failed before Status().Update and returns. No Status.Message, no Status.ObservedGeneration, and the Ready condition is never touched — while every other failure path in the same controller writes all three (:86-95, :132-145, :326-336, :373-388). A proxy that was healthy and then loses a referenced MCPToolConfig therefore reports both verdicts at once:

phase: Failed
message: Remote proxy is running            # stale, from :1643
observedGeneration: <last healthy generation>
conditions:
  - type: Ready
    status: "True"                          # stale, from :1644-1649
    reason: DeploymentReady
    message: Deployment is ready and running

kubectl wait --for=condition=Ready and anything keying on Ready still see a healthy proxy while the operator has declared it Failed.

Steps to reproduce

Apply an MCPRemoteProxy with a valid MCPToolConfig reference, wait until it reports Ready, then delete or break that config and re-read the status after the next reconcile.

Expected behavior

Phase: Failed together with a Message naming the failed config, the current ObservedGeneration, and Ready=False — what :86-95 and :326-336 already do.

Actual behavior

Phase: Failed with the previous healthy Message and Ready=True left in place, and ObservedGeneration still on the last healthy generation.

Environment (if relevant)

  • ToolHive version: c23718d5 (main)

Additional context

Found while sweeping this controller for the same pattern as #6726, whose fix is in #6765; reyortiz3 asked for this one as a separate issue. I plan to submit a PR mirroring :86-95 on the six paths, with a regression case per handler.

The six handlers and their error blocks
handler error block
handleToolConfig :254
handleTelemetryConfig :264
handleExternalAuthConfig :274
handleAuthServerRef :288
handleOIDCConfig :298
handleAuthzConfig :308

TestUpdateMCPRemoteProxyStatus (:1026) only exercises the pod-phase paths, so none of the six config paths is covered today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

needs-triageIssue needs initial triage by a maintainer

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions