feat(agents): external NAT agents — backend (register, chat, eval, health)#763
feat(agents): external NAT agents — backend (register, chat, eval, health)#763marcusds wants to merge 7 commits into
Conversation
📝 WalkthroughWalkthroughChangesExternal agent support
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant AgentStore
participant ExternalAgent
Client->>Gateway: chat/completions request
Gateway->>AgentStore: load Agent
AgentStore-->>Gateway: external endpoint
Gateway->>ExternalAgent: A2A message/send or message/stream
ExternalAgent-->>Gateway: response text or deltas
Gateway-->>Client: OpenAI JSON or SSE
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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
mutually exclusive config/url requirement with oneOf branches, making one branch
require config and the other require url while preserving the existing name
requirement and property definitions.
In `@plugins/nemo-agents/src/nemo_agents_plugin/a2a.py`:
- Around line 142-149: Update the probe loop around the client.get call to use
httpx.AsyncClient.stream with a GET request and an async context manager,
checking only the response status_code before returning. Preserve the existing
HTTPError handling and path iteration while ensuring the response body is never
fully buffered.
- Around line 276-284: Update the response-reading loop in the async stream
handling around resp.aiter_bytes() to preserve UTF-8 characters split across
chunks: keep incoming data as bytes, use an incremental UTF-8 decoder when
producing text, and only then split and parse SSE lines. Retain the existing
total-byte limit and buffering behavior.
- Around line 92-96: Prevent SSRF for external-agent URLs by validating resolved
destinations against loopback, private, link-local, and other disallowed address
ranges before any request, and by preventing or revalidating every redirect at
the HTTP sink. Apply the shared protection to fetch_agent_card at
plugins/nemo-agents/src/nemo_agents_plugin/a2a.py lines 92-96, send_a2a_message
at lines 208-209, and stream_a2a_message at lines 268-269; update
CreateAgentRequest validation in
plugins/nemo-agents/src/nemo_agents_plugin/schema.py lines 54-66 to enforce the
same destination policy rather than syntax-only http(s) validation.
In `@plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py`:
- Around line 520-540: Update _stream_openai_from_a2a to accept the external
agent name, then log a warning with that name and the caught A2AMessageError in
its exception handler before yielding the embedded SSE error content. Update
every call site, including the additional location noted in the review, to pass
name and preserve the existing streaming response behavior.
- Around line 143-148: Sanitize entity-store failures in both lookup paths: in
plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py lines 143-148 and
563-568, log the caught exception through the existing logging mechanism and
replace the raw str(exc) HTTP 500 detail with a fixed, non-sensitive error
message. Preserve the existing 404 handling for NemoEntityNotFoundError.
- Around line 466-490: Update _conversation_prompt to retain system messages in
the conversation context and require the final relevant message to have role
"user"; return an empty string when the last relevant turn is not user. Preserve
existing single-turn handling and transcript formatting while labeling system
turns appropriately.
🪄 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: 43ecfbbc-2a8b-4c0c-bd45-030e2f74b37d
📒 Files selected for processing (11)
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.py
…alth)
Add a first-class "external" agent that runs outside NeMo Platform: registered
by URL, discovered via its A2A card, reachable without a NAT deployment.
Entity + create:
- Agent gains source (managed|external), endpoint, card
- POST /agents takes config XOR url; url fetches the A2A card (timeout +
size-capped, generic error to avoid a port-scan oracle) and stores an
external agent; structural URL validation
A2A bridge (dependency-free JSON-RPC in a2a.py):
- chat/completions <-> message/send + message/stream (streaming), reached via
the Studio route and the CLI/SDK agent-name proxy (/-/v1/chat/completions)
- NAT /generate/full <-> message/send, so external agents evaluate through the
existing `nat eval` pipeline with unchanged eval configs
- multi-turn: NAT's A2A executor is message-only, so the OpenAI history is
folded into the message (single turn verbatim; multi-turn as a transcript)
- byte-capped reads; A2A failures logged; targets the vetted registered
endpoint, never the card's self-reported url
Lifecycle + gating:
- POST /agents/{name}/refresh (re-fetch card), GET /agents/{name}/reachability
- external agents cannot be deployed (400); is_external_agent predicate
~100 unit tests (respx + real-socket); ruff + ty clean.
Signed-off-by: mschwab <mschwab@nvidia.com>
Studio support for external agents (registered by URL). Depends on the backend PR (#763): needs the source/endpoint/card Agent fields, config-XOR-url create, and the refresh/reachability endpoints. - Register modal: Connect running agent by endpoint URL (structural validation) - Agents table: External badge; onboarding empty state + NAT docs link - Agent panel: - Workflow tab — managed: config graph + YAML (credentials redacted); external: A2A card skills + endpoint - Chat playground wired to the external chat bridge (streaming, multi-turn) - Details: Source/Endpoint, reachability badge + Refresh card button - Deploy/Logs gated for external; Evaluate available (via bridge) - resolve the panel agent via GET /agents/{name} (not a page-1 list scan) - shared isExternalAgent predicate + ExternalAgentNotice Unit tests for read models + credential redaction; typecheck + lint clean. Signed-off-by: mschwab <mschwab@nvidia.com>
1e75a83 to
4915ed8
Compare
| for path in AGENT_CARD_PATHS: | ||
| url = _card_url(base, path) | ||
| try: | ||
| async with client.stream("GET", url, headers={"Accept": "application/json"}) as resp: |
|
- probe_agent_reachable streams and checks status only (no unbounded body read) - external chat honors the card's capabilities.streaming: a streaming request to a non-streaming agent uses message/send wrapped in one SSE chunk - streaming uses an incremental UTF-8 decoder so a multi-byte char split across network chunks isn't corrupted - streaming primes the first delta so a failure before any token returns 502; a mid-stream failure is logged and ends without a normal "stop" finish - the conversation prompt preserves system messages and anchors on the last user turn (drops trailing assistant) - managed-agent proxy no longer double-fetches the Agent entity Skipped (P2): full AgentCard schema validation — would require taking a2a-sdk as a runtime dep; cards stay loosely validated (operator-trust) and incompatible ones fail at invocation with a clear error. Signed-off-by: mschwab <mschwab@nvidia.com>
4915ed8 to
e098344
Compare
…ize gateway errors - a2a.py: follow_redirects=False on all httpx sinks so a registered external agent URL can't 302 the backend into loopback/metadata endpoints (SSRF). - gateway.py: entity-store lookup failures no longer leak str(exc); log and return a fixed detail, matching the agents.py CRUD routes. - Regression tests for both. Signed-off-by: mschwab <mschwab@nvidia.com>
Signed-off-by: mschwab <mschwab@nvidia.com>
…bing - a2a.py: block external-agent URLs that resolve to private/loopback/link-local/ reserved addresses (e.g. 169.254.169.254 metadata) before any request, in fetch/send/stream/probe. Gated by NEMO_AGENTS_ALLOW_PRIVATE_AGENT_HOSTS for local dev. (py/full-ssrf #4269) - log_utils.scrub(): strip CR/LF from user-controlled values before logging; applied across gateway/agents/a2a logger calls. (py/log-injection #4270-4279) - Tests for the guard (blocks private, allows public) and the scrubber. Signed-off-by: mschwab <mschwab@nvidia.com>
…tion _endpoint_host output feeds logger calls whose URL arg is user-controlled; sanitize at the source so CodeQL clears py/log-injection #4280/#4281. Signed-off-by: mschwab <mschwab@nvidia.com>
Runtime enforces exactly-one-of config/url via model_validator, but the generated OpenAPI schema allowed both/neither. Add json_schema_extra oneOf so generated client contracts match runtime behavior. Signed-off-by: mschwab <mschwab@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/nemo_agents_plugin/a2a.py`:
- Around line 260-261: Remove the endpoint value from the A2AMessageError
messages in both HTTP transport exception handlers, including the handler around
the shown raise and the second handler at the other referenced location.
Preserve the exception type and chained cause while reporting only a generic
connection failure and the safe exception class name, so embedded credentials
cannot reach gateway responses.
In `@plugins/nemo-agents/src/nemo_agents_plugin/api/v2/gateway.py`:
- Around line 636-641: Require the request method to be POST before dispatching
to the external agent in the handler around _external_a2a_endpoint. Guard both
the _external_openai_chat and _serve_external_generate_full branches, returning
the existing method-not-allowed response for GET, HEAD, OPTIONS, and other
non-POST methods while preserving current POST 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: e90057af-c151-4739-a765-ad924d1c91a3
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/.github/workflows/ci.ymlis excluded by!sdk/**
📒 Files selected for processing (14)
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/log_utils.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_entities.pyplugins/nemo-agents/tests/unit/test_gateway.pyplugins/nemo-agents/tests/unit/test_log_utils.py
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/nemo-agents/src/nemo_agents_plugin/schema.py
- plugins/nemo-agents/src/nemo_agents_plugin/api/v2/deployments.py
- plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
- plugins/nemo-agents/src/nemo_agents_plugin/entities.py
- plugins/nemo-agents/openapi/openapi.yaml
| except httpx.HTTPError as exc: | ||
| raise A2AMessageError(f"could not reach agent at {endpoint} ({exc.__class__.__name__})") from exc |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the full endpoint from transport errors.
These exceptions flow into gateway HTTP/SSE responses. An endpoint containing embedded credentials would be disclosed to callers.
Proposed fix
- raise A2AMessageError(f"could not reach agent at {endpoint} ({exc.__class__.__name__})") from exc
+ raise A2AMessageError(f"could not reach agent ({exc.__class__.__name__})") from excApply the same change to both transport handlers.
Also applies to: 349-350
🤖 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 260 - 261,
Remove the endpoint value from the A2AMessageError messages in both HTTP
transport exception handlers, including the handler around the shown raise and
the second handler at the other referenced location. Preserve the exception type
and chained cause while reporting only a generic connection failure and the safe
exception class name, so embedded credentials cannot reach gateway responses.
| endpoint = _external_a2a_endpoint(agent, name) | ||
| if trailing_uri.endswith("chat/completions"): | ||
| body = await _read_json_object(request) | ||
| return await _external_openai_chat(name, endpoint, body, _card_supports_streaming(agent)) | ||
| if trailing_uri.startswith("generate"): | ||
| return await _serve_external_generate_full(name, endpoint, request) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require POST before invoking an external agent.
This handler is also reached through read-scoped GET/HEAD/OPTIONS routes. A GET carrying a JSON body can therefore invoke chat or generation without write scope.
Proposed fix
endpoint = _external_a2a_endpoint(agent, name)
+ if request.method != "POST":
+ raise HTTPException(
+ status_code=405,
+ detail="External agent invocation requires POST.",
+ headers={"Allow": "POST"},
+ )
if trailing_uri.endswith("chat/completions"):📝 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.
| endpoint = _external_a2a_endpoint(agent, name) | |
| if trailing_uri.endswith("chat/completions"): | |
| body = await _read_json_object(request) | |
| return await _external_openai_chat(name, endpoint, body, _card_supports_streaming(agent)) | |
| if trailing_uri.startswith("generate"): | |
| return await _serve_external_generate_full(name, endpoint, request) | |
| endpoint = _external_a2a_endpoint(agent, name) | |
| if request.method != "POST": | |
| raise HTTPException( | |
| status_code=405, | |
| detail="External agent invocation requires POST.", | |
| headers={"Allow": "POST"}, | |
| ) | |
| if trailing_uri.endswith("chat/completions"): | |
| body = await _read_json_object(request) | |
| return await _external_openai_chat(name, endpoint, body, _card_supports_streaming(agent)) | |
| if trailing_uri.startswith("generate"): | |
| return await _serve_external_generate_full(name, endpoint, request) |
🤖 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
636 - 641, Require the request method to be POST before dispatching to the
external agent in the handler around _external_a2a_endpoint. Guard both the
_external_openai_chat and _serve_external_generate_full branches, returning the
existing method-not-allowed response for GET, HEAD, OPTIONS, and other non-POST
methods while preserving current POST behavior.
Summary
Backend for external NAT agents — agents that run outside NeMo Platform, registered by URL and discovered via their A2A agent card, reachable without a NAT deployment. (Studio UI is a separate stacked PR that depends on this.)
What's included
Entity + create
Agentgainssource(managed|external),endpoint,card.POST /agentstakesconfigXORurl; aurlfetches the A2A card (timeout + size-capped; generic error to avoid a port-scan oracle) and stores an external agent. Structural URL validation.A2A bridge (dependency-free JSON-RPC,
a2a.py)chat/completions↔message/send+message/stream(streaming), reachable via the Studio route and the CLI/SDK agent-name proxy (/-/v1/chat/completions)./generate/full↔message/send, so external agents evaluate through the existingnat evalpipeline with unchanged eval configs.Lifecycle + gating
POST /agents/{name}/refresh(re-fetch card),GET /agents/{name}/reachability(liveness probe).is_external_agentpredicate.Testing
~100 unit tests (respx + real-socket), ruff + ty clean. Live E2E against a real
nat a2a serveagent (chat + fullnat evalthrough the bridge, accuracy 0.79).Follow-ups (not in this PR)
Auth to external agents (would mirror IGW ModelProvider secret ref + header template); SSRF is platform-wide operator-trust (no per-feature guard); Python SDK (Stainless) regen at release; docs.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes