feat(agents): register, visualize, chat with, and evaluate external NAT agents#745
Closed
marcusds wants to merge 11 commits into
Closed
feat(agents): register, visualize, chat with, and evaluate external NAT agents#745marcusds wants to merge 11 commits into
marcusds wants to merge 11 commits into
Conversation
Studio had no way to bring in an existing NAT agent or see its workflow. Add register + visualize paths on the Agents surface and a first-class notion of agents that run outside NeMo Platform. - Register modal: "Connect running agent" (by URL) or "Paste config" (NAT YAML) - Onboarding empty state with dual CTA + NAT docs link - Workflow tab on the agent panel: managed -> config graph + YAML; external -> A2A card skills + endpoint - Agent entity gains source/endpoint/card; POST /agents takes config XOR url and fetches the A2A agent card for external agents - External agents cannot be deployed/evaluated (API 400 + UI gating); chat tab shows an external-endpoint message instead of deploy prompts Backend: a2a.py card fetch (timeout/size-capped), unified create validator, deploy guard; tests for a2a, create, and deploy paths. Deferred: A2A chat proxy. Flagged for review: SSRF on server-side card fetch, Python SDK (Stainless) regen. Signed-off-by: mschwab <mschwab@nvidia.com>
External agents run outside NeMo Platform, so there is no NMP deployment to read logs from (NAT serve/A2A exposes no logs endpoint). Show an explanatory message on the Logs tab instead of an empty deployment log viewer, matching the Deploy/Chat gating. Signed-off-by: mschwab <mschwab@nvidia.com>
Studio's chat playground can now talk to a registered external agent. The
gateway exposes POST /agents/{name}/chat/completions for external agents: it
accepts an OpenAI chat/completions request, forwards the latest user turn to
the agent over A2A message/send (JSON-RPC, no A2A client dependency), and
returns the reply in OpenAI shape (SSE when the client streams).
- a2a.py: send_a2a_message + extract_message_text (Task/Message parts)
- gateway.py: external-only chat route with OpenAI<->A2A translation; 400 managed
- ChatPlaygroundContent: external agents render ModelChat against the bridge
- tests: extract/send (respx + real socket) and route (stream, non-stream,
latest-user forwarding, 400/404/502)
MVP: single-turn (latest user message; no server-side context_id) and no auth
to the external agent. SSRF on the outbound call flagged for review.
Signed-off-by: mschwab <mschwab@nvidia.com>
nat eval drives an agent over NAT's /generate protocol; external agents expose
only A2A, so System-A eval couldn't reach them. Bridge /generate/full -> A2A
message/send inside the agent-name gateway proxy, so external agents evaluate
through the existing nat eval pipeline with unchanged eval configs.
- gateway.py: _serve_agent_proxy branches external -> _serve_external_generate,
translating {"input_message": q} -> A2A message/send and emitting the
data: {"value": ...} line nat eval reads
- AgentDetailsContent: un-gate Evaluate + Recent Evaluations for external;
Deploy stays gated
- tests: generate bridge (full path, missing input_message, non-generate path)
Eval config is unchanged (agent-agnostic; agent comes from --endpoint).
Signed-off-by: mschwab <mschwab@nvidia.com>
External chat used non-streaming message/send + one buffered SSE chunk, so the UI froze until the agent finished. The streaming path now uses A2A message/stream and forwards each text delta as an OpenAI chat.completion.chunk: token-streaming agents show progress, buffering agents (e.g. NAT react_agent) still send one final chunk. Non-streaming requests keep using message/send. - a2a.py: stream_a2a_message (SSE JSON-RPC) + extract_stream_delta - gateway.py: streaming path emits OpenAI chunks from A2A deltas; a mid-stream error is surfaced as content since the HTTP status is already 200 - tests: stream delta extraction, delta forwarding, jsonrpc + mid-stream errors Signed-off-by: mschwab <mschwab@nvidia.com>
Simplify the Register Existing Agent modal to endpoint-URL only for now: remove the "Paste config" YAML tab, the mode toggle, and the now-unused parseAgentConfig helper + test. Update the empty-state copy to match. Backend create still accepts config XOR url; this is a UI scope reduction only. Signed-off-by: mschwab <mschwab@nvidia.com>
Codex + roundtable review of the external-agent feature. Fixes:
Security / robustness (backend, a2a.py + gateway.py):
- Invoke external agents at the vetted registered endpoint, never the card's
self-reported url (remote-controlled → SSRF/exfil target)
- Collapse card-fetch errors to one generic message (distinct errors were an
internal port-scan oracle); log the specific reason server-side
- Cap card + message + stream response bytes with an incremental read (the old
cap ran after the body was fully buffered); log A2A failures
- Chat route catches Exception -> 500 like its siblings
- Reject structurally invalid endpoint URLs in the create schema (http://[ 500)
CLI/SDK parity:
- Bridge /agents/{name}/-/v1/chat/completions (nemo agents invoke / SDK) to A2A,
not just /generate — external agents were unreachable via the standard path
Frontend:
- Redact api_key/token/secret in the Workflow tab config/card view
- Resolve the panel agent via GET /agents/{name} not a page-1 list scan
(external agents past page 1 were mis-rendered as managed)
- Validate the endpoint URL in the register form
- Repair Studio tests broken by the dual-CTA empty state + copy change; add a
GET-by-name mock handler
Tests: a2a stream/size, chat-via-proxy bridge, malformed-URL 422, redactSecrets.
Signed-off-by: mschwab <mschwab@nvidia.com>
stream_a2a_message capped bytes via aiter_lines(), which buffers a line internally until a newline — a newline-less body would grow unbounded before the per-line check ran. Parse SSE lines from a byte-capped buffer over aiter_bytes() instead, so total bytes are bounded regardless of newlines. Add a test that a newline-less body trips the cap. Signed-off-by: mschwab <mschwab@nvidia.com>
External agent cards are captured once at registration, and an external
endpoint can go down invisibly (looks live until you try to chat). Add:
- POST /agents/{name}/refresh — re-fetch the A2A card and update the stored copy
- GET /agents/{name}/reachability — lightweight liveness probe (well-known path,
short timeout, never raises)
- Studio: a reachability badge (polled) + "Refresh card" button on the external
agent's detail panel (panel-only — a per-row table probe would hammer the
backend with N outbound calls)
Backend: probe_agent_reachable util + AgentReachability model + route/probe
tests. Reuses CREATE/READ perms (no new perm → no OPA policy change).
Signed-off-by: mschwab <mschwab@nvidia.com>
#3: add is_external_agent(agent) (backend, mirrors is_container_deployment_mode) and isExternalAgent() (frontend helper); replace the 4 backend + 6 frontend inline `source == "external"` / `source === 'external'` checks so the next AgentSource value only touches one predicate. #6: extract the external-agent logs notice into a reusable ExternalAgentNotice component + a shared EXTERNAL_AGENT_HEADING constant, so the "runs outside NeMo Platform" phrasing lives in one place. Signed-off-by: mschwab <mschwab@nvidia.com>
Chat was single-turn — only the latest user message reached the agent, so it had no memory across turns. NAT's A2A executor is message-only (no server-side contextId memory), so threading contextId wouldn't help; instead fold the OpenAI conversation history (which the client sends each turn) into the A2A message. A single turn is sent verbatim; multi-turn becomes a transcript of prior user/assistant turns followed by the latest user message. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
marked this pull request as ready for review
July 17, 2026 17:29
Contributor
📝 WalkthroughWalkthroughExternal-agent support adds A2A discovery and messaging, dual-mode agent creation, refresh and reachability APIs, gateway bridging for OpenAI/NAT requests, deployment restrictions, and Studio registration, status, workflow, and chat interfaces. ChangesExternal agent platform flow
Studio external-agent experience
Sequence Diagram(s)sequenceDiagram
participant Studio
participant AgentsAPI
participant ExternalAgent
Studio->>AgentsAPI: Register agent with endpoint URL
AgentsAPI->>ExternalAgent: Fetch A2A card
ExternalAgent-->>AgentsAPI: Card metadata
AgentsAPI-->>Studio: External Agent
Studio->>AgentsAPI: Poll reachability or refresh card
AgentsAPI->>ExternalAgent: Probe or fetch card
ExternalAgent-->>AgentsAPI: Reachability/card response
AgentsAPI-->>Studio: Status or updated card
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsx (1)
65-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd registration-flow coverage.
Cover valid submission, URL validation, API failure rendering, and navigation after registration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsx` around lines 65 - 78, Add registration-flow tests around onSubmit in RegisterAgentModal covering successful valid submission, invalid URL validation preventing submission, createAgent API failure rendering through errorText, and navigation after successful registration. Reuse the existing form and routing/test utilities, and verify the expected createAgent arguments and navigation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/nemo-agents/openapi/openapi.yaml`:
- Around line 2785-2793: Update the CreateAgentRequest schema to encode the
documented XOR between config and url using oneOf. Add branches that require
config and url respectively, while preserving the existing name requirement and
allowing each valid managed or external agent shape; both fields must fail
validation.
In `@plugins/nemo-agents/src/nemo_agents_plugin/a2a.py`:
- Around line 213-214: The HTTPError handling in A2AMessageError must not expose
the external endpoint; update plugins/nemo-agents/src/nemo_agents_plugin/a2a.py
lines 213-214 to omit endpoint while retaining the exception type context. In
plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py lines 633-637,
return a generic chat 502 and log only sanitized metadata; apply the same
generic generate 502 response and sanitized logging at lines 651-655.
- Around line 276-284: Update the SSE processing loop in the response stream
around resp.aiter_bytes() to buffer raw bytes and split complete lines on b"\n"
before UTF-8 decoding, preserving partial code points across chunks. Decode each
complete line only after splitting, while retaining the existing size-limit
accounting and JSON payload handling.
- Around line 142-149: Update the probe loop in the liveness-check function
around AsyncClient.get so requests use httpx.AsyncClient.stream instead of
buffering the full response body, while preserving HTTPError handling and
returning immediately when resp.status_code equals 200.
In `@plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py`:
- Around line 143-148: Update the exception handling in the proxy lookup and
direct chat route in gateway.py (lines 143-148 and 563-568): log the underlying
entity-client failure server-side, then raise HTTPException with status 500 and
a generic detail instead of exposing str(exc). Preserve the existing 404
handling for NemoEntityNotFoundError.
- Around line 537-542: Update the A2A streaming flow in gateway.py around the
A2A iterator handling to prime the iterator before returning the streaming
response, mapping failures that occur before the first delta to HTTP 502 while
preserving committed-stream error behavior. In
plugins/nemo-agents/tests/unit/test_gateway.py lines 674-694, add coverage
asserting an immediate failure returns 502 and separately verify errors
occurring after streaming has begun retain the existing behavior.
- Around line 668-672: Update the dispatcher around _external_a2a_endpoint so
only POST requests can invoke _external_openai_chat or
_serve_external_generate_full; return an HTTP 405 response for all other methods
before reading the request body or calling either external-agent handler.
- Around line 477-490: Update the conversation-turn handling around
_message_text so the final turn is accepted only when its role is user,
preserving each prior turn’s original User or Assistant label rather than
forcing the latest turn to User. Do not silently discard system messages; handle
them according to the existing conversation contract, while retaining the
current empty-input behavior.
In `@plugins/nemo-agents/src/nemo_agents_plugin/schema.py`:
- Around line 59-66: Implement one shared outbound URL validation policy and
reuse it from _require_config_xor_url and all request paths in
plugins/nemo-agents/src/nemo_agents_plugin/a2a.py at lines 92-96, 142-145,
208-209, and 268-269. Reject userinfo, localhost, private, loopback, link-local,
and other reserved destinations (or enforce the established public/allowlisted
policy), and validate every redirect target before following it. Ensure card
fetches, probes, message requests, and stream requests all apply the same check.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/AgentDetailsContent.tsx`:
- Around line 61-77: Update the managed-agent check in AgentDetailsContent so an
undefined agent is treated as loading rather than managed: require agent to be
defined before deriving or using isExternal for managed-only controls such as
Deploy. Preserve the existing external-agent behavior once agent data is loaded.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/ExternalAgentStatus.tsx`:
- Around line 33-37: Update ExternalAgentStatus reachability handling to consume
the query's isError state alongside reachability and isLoading. Treat failed
queries as an unknown/check-failed state rather than “Unreachable,” including
the corresponding status rendering and any duplicated logic around the
reachability display.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/redactSecrets.ts`:
- Around line 17-21: Update the secret-key branch in redactSecrets so any value
whose key matches SECRET_KEY is replaced with MASK regardless of its type or
nesting shape; only recurse with redactSecrets for non-sensitive keys.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.ts`:
- Around line 27-32: Update the model collection in summarizeAgentWorkflow to
include only the configured LLM whose key matches workflow.llm_name, rather than
all entries in config.llms. Preserve the existing filtering of missing
model_name values and ensure workflows without a matching selection produce no
model entry.
- Around line 27-44: Update summarizeAgentWorkflow in
web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.ts:
resolve workflow.llm_name against config.llms and return only the selected LLM’s
model in models, rather than collecting every configured LLM; preserve the
existing empty/missing-value behavior. Update the deduplication test in
web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.test.ts
lines 43-51 to configure multiple LLMs and assert that only the model referenced
by workflow.llm_name is returned.
In
`@web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsx`:
- Around line 65-74: Update the name schema in the RegisterAgent form validation
to trim whitespace before enforcing the minimum length, using
z.string().trim().min(1, 'Name is required'). Keep the existing onSubmit
trimming in place so validated names cannot become empty after submission.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsx`:
- Around line 65-78: Add registration-flow tests around onSubmit in
RegisterAgentModal covering successful valid submission, invalid URL validation
preventing submission, createAgent API failure rendering through errorText, and
navigation after successful registration. Reuse the existing form and
routing/test utilities, and verify the expected createAgent arguments and
navigation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 71b168c2-5426-419e-b0e2-ee1017eb0a09
📒 Files selected for processing (34)
plugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/src/nemo_agents_plugin/a2a.pyplugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.pyplugins/nemo-agents/src/nemo_agents_plugin/api/v2/deployments.pyplugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.pyplugins/nemo-agents/src/nemo_agents_plugin/entities.pyplugins/nemo-agents/src/nemo_agents_plugin/schema.pyplugins/nemo-agents/tests/unit/test_a2a.pyplugins/nemo-agents/tests/unit/test_agents_api.pyplugins/nemo-agents/tests/unit/test_deployments_api.pyplugins/nemo-agents/tests/unit/test_gateway.pyweb/packages/studio/src/components/dataViews/AgentsDataView/index.test.tsxweb/packages/studio/src/components/dataViews/AgentsDataView/index.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/AgentDetailsContent.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/AgentWorkflowContent.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/ChatPlaygroundContent.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/ExternalAgentNotice.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/ExternalAgentStatus.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/index.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/redactSecrets.test.tsweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/redactSecrets.tsweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentCard.test.tsweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentCard.tsweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.test.tsweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.tsweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/types.tsweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/useAgentPanel.tsweb/packages/studio/src/constants/links.tsweb/packages/studio/src/mocks/handlers.tsweb/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsxweb/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/type.tsweb/packages/studio/src/routes/agents/AgentsListRoute/index.test.tsxweb/packages/studio/src/routes/agents/AgentsListRoute/index.tsxweb/packages/studio/src/util/agents.ts
Comment on lines
2785
to
+2793
| type: object | ||
| required: | ||
| - name | ||
| - config | ||
| title: CreateAgentRequest | ||
| description: Request body for ``POST /v2/workspaces/{workspace}/agents``. | ||
| description: "Request body for ``POST /v2/workspaces/{workspace}/agents``.\n\ | ||
| \nCreates either a **managed** agent (supply ``config`` \u2014 a NAT workflow\ | ||
| \ the\nplatform will run) or an **external** agent (supply ``url`` \u2014\ | ||
| \ a pointer to\na NAT agent already running elsewhere, whose A2A card the\ | ||
| \ platform fetches\nat creation). Provide exactly one of ``config`` or ``url``." |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Encode the config/url XOR in OpenAPI.
The constraint is documentation-only, so generated clients accept both invalid shapes. Add oneOf branches requiring config or url; providing both will then fail both-branch exclusivity.
Proposed schema change
required:
- name
+ oneOf:
+ - required:
+ - config
+ - required:
+ - url📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type: object | |
| required: | |
| - name | |
| - config | |
| title: CreateAgentRequest | |
| description: Request body for ``POST /v2/workspaces/{workspace}/agents``. | |
| description: "Request body for ``POST /v2/workspaces/{workspace}/agents``.\n\ | |
| \nCreates either a **managed** agent (supply ``config`` \u2014 a NAT workflow\ | |
| \ the\nplatform will run) or an **external** agent (supply ``url`` \u2014\ | |
| \ a pointer to\na NAT agent already running elsewhere, whose A2A card the\ | |
| \ platform fetches\nat creation). Provide exactly one of ``config`` or ``url``." | |
| type: object | |
| required: | |
| - name | |
| oneOf: | |
| - required: | |
| - config | |
| - required: | |
| - url | |
| title: CreateAgentRequest | |
| description: "Request body for ``POST /v2/workspaces/{workspace}/agents``.\n\ | |
| \nCreates either a **managed** agent (supply ``config`` \u2014 a NAT workflow\ | |
| \ the\nplatform will run) or an **external** agent (supply ``url`` \u2014\ | |
| \ a pointer to\na NAT agent already running elsewhere, whose A2A card the\ | |
| \ platform fetches\nat creation). Provide exactly one of ``config`` or ``url``." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/nemo-agents/openapi/openapi.yaml` around lines 2785 - 2793, Update
the CreateAgentRequest schema to encode the documented XOR between config and
url using oneOf. Add branches that require config and url respectively, while
preserving the existing name requirement and allowing each valid managed or
external agent shape; both fields must fail validation.
Comment on lines
+142
to
+149
| async with httpx.AsyncClient(timeout=_PROBE_TIMEOUT_S) as client: | ||
| for path in AGENT_CARD_PATHS: | ||
| try: | ||
| resp = await client.get(_card_url(base, path)) | ||
| except httpx.HTTPError: | ||
| continue | ||
| if resp.status_code == 200: | ||
| return True |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby context.
git ls-files plugins/nemo-agents/src/nemo_agents_plugin/a2a.py
wc -l plugins/nemo-agents/src/nemo_agents_plugin/a2a.py
sed -n '110,180p' plugins/nemo-agents/src/nemo_agents_plugin/a2a.py
# Find any response-size cap logic or related probe helpers.
rg -n "stream\(|response-size|size cap|card|probe|AsyncClient|get\(" plugins/nemo-agents/src/nemo_agents_plugin -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 45225
plugins/nemo-agents/src/nemo_agents_plugin/a2a.py:142-149: Stream the probe request. client.get() buffers the full body for a liveness check; use client.stream() and return on resp.status_code == 200.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/nemo-agents/src/nemo_agents_plugin/a2a.py` around lines 142 - 149,
Update the probe loop in the liveness-check function around AsyncClient.get so
requests use httpx.AsyncClient.stream instead of buffering the full response
body, while preserving HTTPError handling and returning immediately when
resp.status_code equals 200.
Comment on lines
+213
to
+214
| except httpx.HTTPError as exc: | ||
| raise A2AMessageError(f"could not reach agent at {endpoint} ({exc.__class__.__name__})") from exc |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent external endpoint secrets from reaching clients or logs.
plugins/nemo-agents/src/nemo_agents_plugin/a2a.py#L213-L214: remove the raw endpoint fromA2AMessageError.plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L633-L637: return a generic chat 502 and log only sanitized metadata.plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L651-L655: return a generic generate 502 and log only sanitized metadata.
📍 Affects 2 files
plugins/nemo-agents/src/nemo_agents_plugin/a2a.py#L213-L214(this comment)plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L633-L637plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L651-L655
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/nemo-agents/src/nemo_agents_plugin/a2a.py` around lines 213 - 214,
The HTTPError handling in A2AMessageError must not expose the external endpoint;
update plugins/nemo-agents/src/nemo_agents_plugin/a2a.py lines 213-214 to omit
endpoint while retaining the exception type context. In
plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py lines 633-637,
return a generic chat 502 and log only sanitized metadata; apply the same
generic generate 502 response and sanitized logging at lines 651-655.
Comment on lines
+276
to
+284
| total = 0 | ||
| buffer = "" | ||
| async for chunk in resp.aiter_bytes(): | ||
| total += len(chunk) | ||
| if total > _MAX_MESSAGE_BYTES: | ||
| raise A2AMessageError("agent stream exceeded the size limit") | ||
| buffer += chunk.decode("utf-8", errors="replace") | ||
| while "\n" in buffer: | ||
| line, buffer = buffer.split("\n", 1) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '240,330p' plugins/nemo-agents/src/nemo_agents_plugin/a2a.py
printf '\n--- SEARCH ---\n'
rg -n "aiter_bytes|decode\\(|SSE|event|stream" plugins/nemo-agents/src/nemo_agents_plugin/a2a.pyRepository: NVIDIA-NeMo/nemo-platform
Length of output: 5019
Decode SSE lines after buffering bytes.
aiter_bytes() can split UTF-8 code points across chunks; decoding each chunk with errors="replace" can corrupt data: payloads before json.loads(). Buffer bytes and split on b"\n" first, or use an incremental decoder.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/nemo-agents/src/nemo_agents_plugin/a2a.py` around lines 276 - 284,
Update the SSE processing loop in the response stream around resp.aiter_bytes()
to buffer raw bytes and split complete lines on b"\n" before UTF-8 decoding,
preserving partial code points across chunks. Decode each complete line only
after splitting, while retaining the existing size-limit accounting and JSON
payload handling.
Comment on lines
+143
to
+148
| try: | ||
| agent = await entity_client.get(Agent, name=name, workspace=workspace) | ||
| except NemoEntityNotFoundError as exc: | ||
| raise HTTPException(status_code=404, detail=f"Agent '{name}' not found in workspace '{workspace}'.") from exc | ||
| except Exception as exc: | ||
| raise HTTPException(status_code=500, detail=str(exc)) from exc |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Stop exposing entity-client exception details.
plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L143-L148: log the proxy lookup failure and return a generic 500.plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L563-L568: apply the same handling to the direct chat route.
📍 Affects 1 file
plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L143-L148(this comment)plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py#L563-L568
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py` around lines
143 - 148, Update the exception handling in the proxy lookup and direct chat
route in gateway.py (lines 143-148 and 563-568): log the underlying
entity-client failure server-side, then raise HTTPException with status 500 and
a generic detail instead of exposing str(exc). Preserve the existing 404
handling for NemoEntityNotFoundError.
Comment on lines
+33
to
+37
| const { data: reachability, isLoading } = useAgentsExternalAgentReachability( | ||
| workspace, | ||
| agentName, | ||
| { query: { enabled: !!agentName, refetchInterval: REACHABILITY_POLL_MS } } | ||
| ); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not report reachability API failures as “Unreachable.”
After a query error, isLoading is false and reachability is undefined, so the endpoint is incorrectly blamed. Handle isError as an unknown/check-failed state.
Proposed fix
- const { data: reachability, isLoading } = useAgentsExternalAgentReachability(
+ const { data: reachability, isLoading, isError } = useAgentsExternalAgentReachability(
...
- const status = isLoading ? 'checking' : reachability?.reachable ? 'reachable' : 'unreachable';
+ const status = isLoading
+ ? 'checking'
+ : isError
+ ? 'unknown'
+ : reachability?.reachable
+ ? 'reachable'
+ : 'unreachable';Also applies to: 55-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/ExternalAgentStatus.tsx`
around lines 33 - 37, Update ExternalAgentStatus reachability handling to
consume the query's isError state alongside reachability and isLoading. Treat
failed queries as an unknown/check-failed state rather than “Unreachable,”
including the corresponding status rendering and any duplicated logic around the
reachability display.
Comment on lines
+17
to
+21
| Object.entries(value as Record<string, unknown>).map(([key, val]) => { | ||
| if (SECRET_KEY.test(key) && (typeof val === 'string' || typeof val === 'number')) { | ||
| return [key, MASK]; | ||
| } | ||
| return [key, redactSecrets(val)]; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Mask the entire value under secret-looking keys.
Nested secret containers leak values—for example, { authorization: { value: 'Bearer …' } } renders unchanged. Once the key is sensitive, mask its value regardless of shape.
Proposed fix
- if (SECRET_KEY.test(key) && (typeof val === 'string' || typeof val === 'number')) {
+ if (SECRET_KEY.test(key)) {
return [key, MASK];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Object.entries(value as Record<string, unknown>).map(([key, val]) => { | |
| if (SECRET_KEY.test(key) && (typeof val === 'string' || typeof val === 'number')) { | |
| return [key, MASK]; | |
| } | |
| return [key, redactSecrets(val)]; | |
| Object.entries(value as Record<string, unknown>).map(([key, val]) => { | |
| if (SECRET_KEY.test(key)) { | |
| return [key, MASK]; | |
| } | |
| return [key, redactSecrets(val)]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/redactSecrets.ts`
around lines 17 - 21, Update the secret-key branch in redactSecrets so any value
whose key matches SECRET_KEY is replaced with MASK regardless of its type or
nesting shape; only recurse with redactSecrets for non-sensitive keys.
Comment on lines
+27
to
+32
| const workflow = config?.workflow; | ||
| const wiredNames = new Set(workflow?.tool_names ?? []); | ||
|
|
||
| const models = Object.values(config?.llms ?? {}) | ||
| .map((llm) => llm?.model_name) | ||
| .filter((m): m is string => !!m); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show only the workflow-selected LLM model.
models currently includes every configured LLM, even though workflow.llm_name selects one. This misrepresents workflows containing auxiliary LLM configurations.
Proposed fix
- const models = Object.values(config?.llms ?? {})
- .map((llm) => llm?.model_name)
- .filter((m): m is string => !!m);
+ const selectedModel = workflow?.llm_name
+ ? config?.llms?.[workflow.llm_name]?.model_name
+ : undefined;
...
- models: [...new Set(models)],
+ models: selectedModel ? [selectedModel] : [],Also applies to: 40-44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.ts`
around lines 27 - 32, Update the model collection in summarizeAgentWorkflow to
include only the configured LLM whose key matches workflow.llm_name, rather than
all entries in config.llms. Preserve the existing filtering of missing
model_name values and ensure workflows without a matching selection produce no
model entry.
Comment on lines
+27
to
+44
| const workflow = config?.workflow; | ||
| const wiredNames = new Set(workflow?.tool_names ?? []); | ||
|
|
||
| const models = Object.values(config?.llms ?? {}) | ||
| .map((llm) => llm?.model_name) | ||
| .filter((m): m is string => !!m); | ||
|
|
||
| const tools: WorkflowToolNode[] = Object.entries(config?.functions ?? {}).map(([name, fn]) => ({ | ||
| name, | ||
| type: fn?._type ?? 'unknown', | ||
| wired: wiredNames.has(name), | ||
| })); | ||
|
|
||
| return { | ||
| workflowType: workflow?._type, | ||
| llmName: workflow?.llm_name, | ||
| models: [...new Set(models)], | ||
| tools, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Derive models from workflow.llm_name, not every configured LLM.
web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.ts#L27-L44: resolve the selected LLM and return only its model.web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.test.ts#L43-L51: replace the deduplication case with multiple LLMs and assert that only the referenced LLM appears.
📍 Affects 2 files
web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.ts#L27-L44(this comment)web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.test.ts#L43-L51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.ts`
around lines 27 - 44, Update summarizeAgentWorkflow in
web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.ts:
resolve workflow.llm_name against config.llms and return only the selected LLM’s
model in models, rather than collecting every configured LLM; preserve the
existing empty/missing-value behavior. Update the deduplication test in
web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/summarizeAgentWorkflow.test.ts
lines 43-51 to configure multiple LLMs and assert that only the model referenced
by workflow.llm_name is returned.
Comment on lines
+65
to
+74
| const onSubmit: SubmitHandler<RegisterAgentFormData> = async (formData) => { | ||
| setSubmitError(undefined); | ||
| try { | ||
| await createAgent({ | ||
| workspace, | ||
| data: { | ||
| name: formData.name.trim(), | ||
| description: formData.description?.trim() || '', | ||
| url: formData.url.trim(), | ||
| }, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the modal file and related schema definitions
git ls-files 'web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/*' \
'web/packages/studio/src/routes/agents/AgentsListRoute/**' \
| sed 's#^`#FILE` #'
echo '--- index.tsx ---'
cat -n web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsx | sed -n '1,220p'
echo '--- search for RegisterAgentFormData / zod schema ---'
rg -n "RegisterAgentFormData|z\.string\(\)\.min\(1|trim\(\)\.min\(1|Name is required|description" web/packages/studio/src/routes/agents/AgentsListRoute -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 8778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read any directly related file(s) if the schema lives nearby
fd -a '.*(schema|validation|form).*' web/packages/studio/src/routes/agents/AgentsListRoute -t fRepository: NVIDIA-NeMo/nemo-platform
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsx | sed -n '1,220p'
echo '---'
rg -n "RegisterAgentFormData|z\.string\(\)\.min\(1|trim\(\)\.min\(1|Name is required|description" web/packages/studio/src/routes/agents/AgentsListRoute -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 7778
Trim name before min(1). z.string().min(1) still accepts spaces, so submit trims them to '' and sends an invalid request. Use z.string().trim().min(1, 'Name is required') in web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/type.ts:17.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/packages/studio/src/routes/agents/AgentsListRoute/RegisterAgentModal/index.tsx`
around lines 65 - 74, Update the name schema in the RegisterAgent form
validation to trim whitespace before enforcing the minimum length, using
z.string().trim().min(1, 'Name is required'). Keep the existing onSubmit
trimming in place so validated names cannot become empty after submission.
Contributor
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
1 participant
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
Studio had no way to bring in an existing NAT agent or work with one that runs outside NeMo Platform. This adds a first-class external agent (registered by URL, discovered via its A2A card) and makes it work across the Agents surface — register, visualize, chat, evaluate — with managed-only actions correctly gated.
What's included
Register & visualize
configXORurl.)Entity + unified create
Agentgainssource(managed|external),endpoint,card.POST /agentstakesconfigXORurl; aurlfetches the A2A card (timeout + size-capped) and stores an external agent. No NAT deployment involved.Chat (A2A bridge, streaming, multi-turn)
/agents/{name}/chat/completions(Studio) and the agent-name proxy/-/v1/chat/completions(CLInemo agents invoke/ SDK) bridge OpenAI chat ↔ A2A. Streaming usesmessage/stream; non-streamingmessage/send.contextIdmemory), so the OpenAI history is folded into the message (single turn verbatim; multi-turn as a transcript).Evaluate (A2A bridge)
nat evaldrives agents over NAT/generate; the proxy bridges/generate/full→ A2A, so external agents evaluate through the existingnat evalpipeline with unchanged eval configs.Health & lifecycle
POST /agents/{name}/refreshre-fetches the card;GET /agents/{name}/reachabilityis a liveness probe. Studio shows a reachability badge + Refresh card button on the external agent panel.Gating
How it works
Dependency-free hand-rolled A2A JSON-RPC (
a2a.py): card fetch,message/send,message/stream, liveness probe. The gateway translates OpenAIchat/completionsand NAT/generate↔ A2A. External agents flow through the same agent-name proxy as managed ones, so CLI/eval tooling works unchanged. A singleis_external_agent/isExternalAgentpredicate drives the managed/external branch.Testing
nat a2a serveagent: register by URL, chat, and a fullnat evalrun through the bridge (accuracy 0.79).Review
Went through Codex + a 3-persona roundtable. Fixed: card.url→vetted endpoint (SSRF/exfil), generic card-fetch error (port-scan oracle), effective size caps, A2A failure logging, CLI/SDK invoke parity, credential redaction in the Workflow tab, agent-panel pagination (external past page 1 mis-rendered), malformed-URL 422, and the broken Studio tests.
Known / follow-ups
host_url, evaluator BYO URLs all connect unguarded), so no per-feature guard was added; a private-IP policy would be a platform-wide change.docs/studio/agents.mdxupdate pending (follow-up).include_in_schema=False, refresh/reachability are in-schema).web-typecheckjob fails on pre-existing errors in intake/eval-session files (Trace.input/output,@axe-core/playwright) authored on main (commit 9215278), untouched by this branch. Clears when main's typecheck is fixed.🤖 Generated with Claude Code
Summary by CodeRabbit