fix(mcp): include SandboxAgents in list_agents and invoke_agent#2249
Open
anxkhn wants to merge 1 commit into
Open
fix(mcp): include SandboxAgents in list_agents and invoke_agent#2249anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
listReadyAgents only enumerated v1alpha2.Agent and gated readiness on the literal reason "DeploymentReady", so ready SandboxAgents were never listed. A SandboxAgent reports its Ready condition with reason "WorkloadReady" (reconciler.reconcileSandboxAgentStatus), while a declarative Agent uses "DeploymentReady", so a fully ready SandboxAgent (Accepted=True, Ready=True) was invisible to the list_agents tool and the kagent://agents resource, and invoke_agent reported it as "not found or not ready" even though it appears in the REST /agents API and the UI. List both Agent and SandboxAgent and evaluate readiness over the shared AgentObject interface, accepting both AgentReadyReasonDeploymentReady and AgentReadyReasonWorkloadReady, matching the A2A registrar (isAgentReady) and the REST agents handler. Update the tool and resource descriptions to say "accepted + ready" now that both workload types are covered. Closes: kagent-dev#2123 Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the MCP agent-discovery path so that list_agents, the kagent://agents resource, and the readiness filtering they rely on include both declarative Agent CRs and substrate-backed SandboxAgent CRs, using the shared v1alpha2.AgentObject interface and the same “Accepted + Ready” semantics already used by the REST /agents endpoint and the A2A registrar.
Changes:
- Expand
MCPHandler.listReadyAgentsto list bothAgentListandSandboxAgentList. - Replace hard-coded condition-string readiness checks with a shared
readyAgentSummary(v1alpha2.AgentObject)helper that recognizes bothDeploymentReadyandWorkloadReadyready reasons. - Add a regression unit test ensuring ready
SandboxAgents are included and their descriptions are surfaced.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| go/core/internal/mcp/mcp_handler.go | Lists both Agent and SandboxAgent CRs and applies interface-based readiness checks for MCP agent discovery. |
| go/core/internal/mcp/mcp_handler_test.go | Adds a regression test covering ready vs. not-ready SandboxAgent inclusion and description propagation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+185
to
+187
| summary := AgentSummary{ | ||
| Ref: agent.GetNamespace() + "/" + agent.GetName(), | ||
| } |
Comment on lines
+90
to
+95
| func readyConditions(readyReason string) []metav1.Condition { | ||
| return []metav1.Condition{ | ||
| {Type: "Accepted", Status: metav1.ConditionTrue, Reason: "AgentReconciled"}, | ||
| {Type: "Ready", Status: metav1.ConditionTrue, Reason: readyReason}, | ||
| } | ||
| } |
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.
What
list_agents, thekagent://agentsMCP resource, andinvoke_agentnow returnSandboxAgents (substrate-backed agents) in addition to declarativeAgents. Afully ready
SandboxAgentwas previously invisible to all three, even though itshows up in the REST
/agentsAPI and the UI.Why
MCPHandler.listReadyAgents(go/core/internal/mcp/mcp_handler.go) had twocompounding faults:
v1alpha2.AgentList, soSandboxAgentCRs were neverenumerated at all.
condition.Reason == "DeploymentReady".A
SandboxAgentreports itsReadycondition with reasonWorkloadReady, whilea declarative
AgentusesDeploymentReady:reconcileSandboxAgentStatussetsReason = AgentReadyReasonWorkloadReadywhen the workload is ready (
reconciler.go).reconcileAgentStatussetsReason = AgentReadyReasonDeploymentReady(
reconciler.go).So a
SandboxAgentwithAccepted=TrueandReady=True(reasonWorkloadReady)was dropped by both faults.
listReadyAgentsfeedshandleListAgents(thelist_agentstool) andreadAgentsResource(thekagent://agentsresource), andinvoke_agentrelies on the same readiness notion, so such an agent was reportedas "not found or not ready" over MCP while being fully usable via the REST API.
The correct, two-reason, interface-based readiness check already exists elsewhere
in the codebase and is what this change reuses:
isAgentReadyin the A2A registrar (a2a/a2a_registrar.go) accepts bothAgentReadyReasonDeploymentReadyandAgentReadyReasonWorkloadReadyover theAgentObjectinterface.httpserver/handlers/agents.go) does the same.Fix
v1alpha2.AgentListandv1alpha2.SandboxAgentList, and evaluatereadiness through a small
readyAgentSummary(v1alpha2.AgentObject)helper thatoperates on the shared
AgentObjectinterface.reconciler.AgentReadyReasonDeploymentReadyandreconciler.AgentReadyReasonWorkloadReady, and uses the typed condition-typeconstants (
v1alpha2.AgentConditionTypeReady/...Accepted) withmetav1.ConditionTrue, matching the A2A registrar and the REST handler insteadof re-implementing the check with hardcoded strings.
list_agentstool andkagent://agentsresource descriptions from"accepted + deploymentReady" to "accepted + ready" now that both workload types
are covered.
No CRD/API types change, so no code generation is required.
Tests
Added
TestListReadyAgents_IncludesSandboxAgentsingo/core/internal/mcp/mcp_handler_test.go(table of fake objects via thecontroller-runtime fake client already used in this file). It asserts that:
Agent(DeploymentReady) is listed,SandboxAgent(WorkloadReady) is listed,SandboxAgentis excluded, andSandboxAgent's description is surfaced.The test fails against the current code (the
SandboxAgentis absent and itsdescription is empty) and passes with the fix. Revert the
mcp_handler.gohunksto reproduce.
How to verify
Scope
Limited to the MCP agent-listing path. It reuses the existing readiness semantics
(the same two
Readyreasons the REST API and A2A registrar already honor) ratherthan introducing new behavior, so declarative
Agentresults are unchanged andonly genuinely ready
SandboxAgents are added.Checklist
git commit -s).go build ./core/...,go test -race ./core/internal/mcp/..., andmake -C go lintall pass.go/core/internal/mcp/mcp_handler.goand its test.