🤖 fix: prevent MCP schema panic in control plane status tool#58
Merged
Conversation
Replace get_control_plane_status conditions with a summary schema that avoids metav1.Time embedding issues, add a non-panic NewServer regression test, and switch make lint to go tool golangci-lint so local lint uses the vendored v2 toolchain. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$1.13`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=1.13 -->
Member
Author
|
@codex review Please review this change. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix MCP startup panic by replacing
metav1.Conditioninget_control_plane_statustool output with a JSON-schema-safe summary type, add a regression test to ensureNewServerdoes not panic during tool registration, and align local lint tooling with CI by using vendored golangci-lint v2 viago tool.Background
coder-k8scould panic on startup when MCP registeredget_control_plane_statusbecause the output schema exposedmetav1.Condition(which includesmetav1.Timeembeddingtime.Time). The schema inference path rejects that embedded custom schema shape, crashing MCP registration and preventing app startup.Implementation
controlPlaneConditionSummaryand changedgetControlPlaneStatusOutput.Conditionsto[]controlPlaneConditionSummary.summarizeMetav1Conditions(...)helper to convert condition fields and stringifyLastTransitionTimeas RFC3339Nano.get_control_plane_statushandler to return summarized conditions.internal/app/mcpapp/server_test.gowithTestNewServerDoesNotPanicusing fake clients.Makefilelint target to use vendored golangci-lint v2:go tool golangci-lint run ./...go tool golangci-lint fmt --diffValidation
make verify-vendormake testmake buildmake lintRisks
Low to medium. The API shape for a single MCP tool response changed from raw Kubernetes condition objects to summarized condition objects, but this is intentional to prevent startup panics and is scoped to MCP output formatting.
📋 Implementation Plan
Fix MCP tool schema panic (
get_control_plane_status)Context / Why
Running
coder-k8sin--app=all(the default indeploy/deployment.yaml) currently crash-loops with a panic during MCP tool registration:This prevents the operator stack from starting at all (controller + aggregated API server + MCP HTTP run in the same pod).
Evidence
internal/app/mcpapp/tools.go:getControlPlaneStatusOutputcurrently returns[]metav1.Conditionin the tool output.metav1.Conditioncontainsmetav1.Timefields, andmetav1.Timeembedstime.Time.The vendored schema generator used by MCP (
vendor/github.com/google/jsonschema-go/jsonschema) has a hard-coded custom schema fortime.TimewithType: "string"and rejects embedded structs whose custom schema isn’tType: "object".The failure occurs in
vendor/github.com/google/jsonschema-go/jsonschema/infer.goaround the embedded-field check:Scan of
internal/app/mcpapp/**/*.goconfirmsgetControlPlaneStatusOutputis the only MCP tool output type that exposesmetav1.Conditiondirectly; all other tools already use “summary” structs with stringified timestamps.Implementation plan
1) Replace
metav1.Conditionin MCP output with a summary typeFile:
internal/app/mcpapp/tools.gogetControlPlaneStatusOutputto use this summary type:2) Convert conditions when handling the tool call
File:
internal/app/mcpapp/tools.goAdd a small helper to keep tool handlers clean and ensure consistent formatting:
Then update the
get_control_plane_statustool handler to return:3) Add a regression test to prevent future panics
File:
internal/app/mcpapp/server_test.go(new)Goal: ensure that tool registration (schema inference) does not panic.
Test shape:
newScheme().k8s.io/client-go/kubernetes/fake.NewSimpleClientset().NewServer(k8sClient, clientset)inside adefer recover()block and fail the test if it panics.This catches:
metav1.Condition/metav1.Time/ other embedded-time types into MCP tool IO structs4) Validation / smoke checks
Run locally:
make testmake lint(or at minimumGOFLAGS=-mod=vendor golangci-lint runif that’s the repo norm)Optional runtime smoke:
GOFLAGS=-mod=vendor go run . --app=mcp-http(should start without panic)Readywhen running--app=all.Acceptance criteria
coder-k8sno longer panics on startup when running--app=all.get_control_plane_statustool returns conditions (in summarized form).go test ./...(viamake test) passes.Generated with
mux• Model:openai:gpt-5.3-codex• Thinking:xhighGenerated with
mux• Model:openai:gpt-5.3-codex• Thinking:xhigh• Cost:$1.13