feat(studio): revive nemo-agent & wire coding agent UI to use it - #966
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a packaged NeMo Agent with NAT packaging, SDK tools, skills, wrapper compatibility, evaluation assets, deployment URL propagation, and Studio routing with in-memory sessions and NeMo Agent terminology. ChangesNeMo Agent runtime
Deployment base URL propagation
Studio NeMo Agent integration
Sequence Diagram(s)sequenceDiagram
participant Studio
participant NemoAgent
participant PlatformSDK
Studio->>NemoAgent: Send message with session context
NemoAgent->>PlatformSDK: Execute SDK or Studio tool
PlatformSDK-->>NemoAgent: Return tool result
NemoAgent-->>Studio: Return assistant and interaction events
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
|
![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: 11
🧹 Nitpick comments (3)
plugins/nemo-agents/tests/unit/test_runner_deployments.py (1)
138-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover trailing-slash normalization.
The implementation calls
rstrip("/"), but every test supplies an already-normalized URL. Pass a value such as"http://host.docker.internal:8080///"and assert the emitted value is"http://host.docker.internal:8080".🤖 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/tests/unit/test_runner_deployments.py` around lines 138 - 149, Update the test setup for the deployment configuration to pass a trailing-slash URL such as "http://host.docker.internal:8080///" and assert the emitted NMP_BASE_URL value is normalized to "http://host.docker.internal:8080". Keep the existing container and environment assertions unchanged.agents/nemo-agent-local-spec/AGENT-SPEC.md (1)
82-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMove evaluation instructions into a linked how-to.
This reference contract mixes execution steps with reference content. Keep a brief link here; put prerequisites and the runnable evaluation procedure in a separate how-to.
🤖 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 `@agents/nemo-agent-local-spec/AGENT-SPEC.md` around lines 82 - 92, Replace the detailed Evaluation Setup content in AGENT-SPEC.md with a brief link to a separate evaluation how-to. Move the existing test command, NAT workflow details, inputs, scoring configuration, manual Studio validation reference, prerequisites, and runnable evaluation procedure into that how-to, preserving the referenced symbols and commands.Source: Coding guidelines
services/studio/src/nmp/studio/coding_agents.py (1)
1215-1226: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead
Nonecheck.Every branch of the
orchain yields astr(str(request.base_url)is the final fallback), sobase_url is Noneis unreachable. An empty/whitespaceSTUDIO_CODING_AGENT_BASE_URLalready falls through via_trimmed_string.♻️ Proposed simplification
- if base_url is None: # pragma: no cover - Request always has a base URL - raise RuntimeError("Could not determine the NeMo Platform base URL") return (🤖 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 `@services/studio/src/nmp/studio/coding_agents.py` around lines 1215 - 1226, Remove the unreachable None guard from _studio_coding_agent_url, since its base_url fallback chain always produces a string. Keep the existing environment-variable trimming, fallback order, URL construction, and quoting behavior unchanged.
🤖 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 `@agents/nemo-agent-local/src/nemo_agent/nemo-eval-data.json`:
- Around line 3-4: Update the mutation eval around the input_message and
expected_output fixture to make repeated runs isolated: either create a uniquely
scoped workspace and remove it during teardown, or replace the mutation with a
read-only scenario. Ensure reruns do not depend on the persistent fixed name
“test-workspace” or leave resources on the target platform.
In `@agents/nemo-agent-local/src/nemo_agent/nemo-eval.yml`:
- Around line 49-55: Update the bypass-mode caveat near the agent tool
description to state that the current dataset exercises platform resources,
including workspace and model/provider operations, so bypassing the gateway is
not an LLM-only smoke test. Clarify that a separate pure-LLM dataset is required
for a fully self-contained bypass run.
In `@agents/nemo-agent-local/src/nemo_agent/register.py`:
- Around line 580-592: The destructive branch in _direct_response must not call
_delete_fileset directly from regex-matched text. When a Studio session is
available, route the fileset deletion through _call_studio_tool with
"approval_prompt" so the required confirmation occurs; retain the existing
direct behavior only where no Studio session exists.
- Around line 261-284: The approval gate in the tool handler must not depend on
the model-supplied studio_session_id argument. Update the surrounding
registration/invocation flow and approval check to obtain the session ID from
trusted invocation context or environment, then use that injected value for
_call_studio_tool before non-read-only _call_sdk_method operations.
In `@agents/nemo-agent-local/src/nemo_agent/skills/benchmark-execution/SKILL.md`:
- Around line 7-12: Correct the README link in the benchmark-execution skill
description by removing one parent-directory segment so it resolves to
tests/agentic-use/README.md from the skill directory. Verify the resulting
target exists and leave the surrounding execution requirements unchanged.
In `@agents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.md`:
- Around line 16-18: Update the operational guidance around temporary and final
fileset handling to cap retries: retry only once after validating and correcting
parameters, then stop and return the error or request clarification if the
operation remains unsuccessful. Preserve the required fileset creation, cleanup,
verification upload sequence without continuing after an unrecoverable failure.
In `@agents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.md`:
- Around line 9-13: Update the SDK/API operation examples for nemo_api to pass
params as a JSON string rather than a Python dictionary, including properly
quoted JSON object values such as name and description. Preserve the existing
resource and action examples while making every params argument match the
nemo_api signature.
In `@agents/nemo-agent-local/src/nemo_agent/wrapper.py`:
- Around line 389-393: Update NemoAgentWrapper.convert_to_str to handle
dict-backed messages produced by model_construct, using the same normalized
value path as convert_to_chat_response instead of accessing .text directly.
Preserve the empty-message return and ensure the method returns the final
message’s text for both dictionary and BaseMessage representations.
In `@services/studio/src/nmp/studio/coding_agents.py`:
- Line 798: Update the initial items assignment in the surrounding coding-agent
method to declare it as list[dict[str, Any]] immediately, then remove the later
annotation on the same-scope items assignment. Keep subsequent items usage
unchanged.
- Around line 134-135: Bound the in-memory state managed by
_session_conversations and _session_mtimes to prevent indefinite growth. Add
session eviction using an LRU or TTL strategy keyed by _session_mtimes, enforce
a maximum number of retained turns per session, and ensure /history/sessions
only enumerates active retained sessions while preserving current behavior for
non-evicted sessions.
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts`:
- Around line 674-679: Update the interruption error fallback in handleRun to
use the current NeMo Agent user-facing terminology instead of “Connection to
Claude was interrupted.” Keep the existing error-handling behavior unchanged and
update the corresponding fallback in the additional referenced block for
consistency.
---
Nitpick comments:
In `@agents/nemo-agent-local-spec/AGENT-SPEC.md`:
- Around line 82-92: Replace the detailed Evaluation Setup content in
AGENT-SPEC.md with a brief link to a separate evaluation how-to. Move the
existing test command, NAT workflow details, inputs, scoring configuration,
manual Studio validation reference, prerequisites, and runnable evaluation
procedure into that how-to, preserving the referenced symbols and commands.
In `@plugins/nemo-agents/tests/unit/test_runner_deployments.py`:
- Around line 138-149: Update the test setup for the deployment configuration to
pass a trailing-slash URL such as "http://host.docker.internal:8080///" and
assert the emitted NMP_BASE_URL value is normalized to
"http://host.docker.internal:8080". Keep the existing container and environment
assertions unchanged.
In `@services/studio/src/nmp/studio/coding_agents.py`:
- Around line 1215-1226: Remove the unreachable None guard from
_studio_coding_agent_url, since its base_url fallback chain always produces a
string. Keep the existing environment-variable trimming, fallback order, URL
construction, and quoting behavior unchanged.
🪄 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: 42269b64-d99d-485e-9e79-0b8eac7aa5be
⛔ Files ignored due to path filters (1)
agents/nemo-agent-local/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
agents/nemo-agent-local-spec/AGENT-SPEC.mdagents/nemo-agent-local/.dockerignoreagents/nemo-agent-local/Dockerfileagents/nemo-agent-local/pyproject.tomlagents/nemo-agent-local/src/nemo_agent/nemo-agent.remote.ymlagents/nemo-agent-local/src/nemo_agent/nemo-agent.ymlagents/nemo-agent-local/src/nemo_agent/nemo-eval-data.jsonagents/nemo-agent-local/src/nemo_agent/nemo-eval.ymlagents/nemo-agent-local/src/nemo_agent/register.pyagents/nemo-agent-local/src/nemo_agent/skills/.gitkeepagents/nemo-agent-local/src/nemo_agent/skills/auditor/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/benchmark-execution/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/entities/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/evaluator/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/guardrails/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/inference/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/secrets/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.mdagents/nemo-agent-local/src/nemo_agent/wrapper.pyagents/nemo-agent-local/tests/smoke_test.mdagents/nemo-agent-local/tests/test_nemo_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyservices/studio/src/nmp/studio/coding_agent_mcp_tools.pyservices/studio/src/nmp/studio/coding_agents.pyservices/studio/tests/unit/test_coding_agents.pyweb/packages/studio/env/.env.dev.local.sampleweb/packages/studio/src/routes/DashboardLandingRoute/SkillActionSection.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.test.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.tsxweb/packages/studio/src/routes/agents/AgentDeploymentsListRoute/CreateDeploymentModal/index.test.tsxweb/packages/studio/src/routes/agents/AgentDeploymentsListRoute/CreateDeploymentModal/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeChatThread.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeChatThread.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistoryPanelContents.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/SkillsPanelContents.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/helpers.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/stream.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts
dmariali
force-pushed
the
restore-nemo-agent
branch
from
July 29, 2026 16:23
2f78436 to
8af5d65
Compare
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
dmariali
force-pushed
the
restore-nemo-agent
branch
from
July 29, 2026 16:27
8af5d65 to
1e318bf
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/nemo-agents/tests/unit/test_runner_deployments.py (1)
351-355: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover
NMP_BASE_URLin the auth-enabled path.This test verifies only the NAT inference URL rewrite. Since
create_deploymentpassesgatewayasplatform_base_urleven with the auth proxy enabled, SDK calls usingNMP_BASE_URLcould bypass the sidecar or fail authentication. Assert the intended generated environment value here, or explicitly document and test why the direct gateway is correct.🤖 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/tests/unit/test_runner_deployments.py` around lines 351 - 355, Extend the auth-enabled deployment test around create_deployment to inspect the generated environment configuration and assert the intended NMP_BASE_URL value routes through the authentication sidecar. If the direct gateway URL is intentional, document that behavior in the test and assert that value instead; use the existing generated-config symbols rather than adding unrelated coverage.
🤖 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.
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/test_runner_deployments.py`:
- Around line 351-355: Extend the auth-enabled deployment test around
create_deployment to inspect the generated environment configuration and assert
the intended NMP_BASE_URL value routes through the authentication sidecar. If
the direct gateway URL is intentional, document that behavior in the test and
assert that value instead; use the existing generated-config symbols rather than
adding unrelated coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 94e5a67d-e8f8-4195-89bb-fe53fc297f79
📒 Files selected for processing (2)
plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/tests/unit/test_runner_deployments.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
agents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.md (1)
9-13: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExamples still pass
paramsas a dict, not a JSON string.
nemo_api(params=...)expects a JSON string (parsed viajson.loads). These examples still use Python dict literals — same issue flagged previously, not yet fixed.✏️ Proposed fix
- - `nemo_api(resource="workspaces", action="create", params={"name":"...", "description":"..."})` + - `nemo_api(resource="workspaces", action="create", params='{"name":"...", "description":"..."}')` - `nemo_api(resource="workspaces", action="list")` - - `nemo_api(resource="workspaces", action="retrieve", params={"name":"..."})` - - `nemo_api(resource="workspaces", action="delete", params={"name":"..."})` + - `nemo_api(resource="workspaces", action="retrieve", params='{"name":"..."}')` + - `nemo_api(resource="workspaces", action="delete", params='{"name":"..."}')`🤖 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 `@agents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.md` around lines 9 - 13, Update the nemo_api examples in the SDK/API operations section to pass params as JSON-encoded strings rather than Python dict literals, preserving the existing workspace resource, action, and parameter values.services/studio/src/nmp/studio/coding_agents.py (2)
134-135: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftUnbounded in-memory session storage — still no eviction.
_session_conversations/_session_mtimesgrow without bound; retained across process lifetime. Same concern raised previously.🤖 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 `@services/studio/src/nmp/studio/coding_agents.py` around lines 134 - 135, Add bounded eviction for the in-memory _session_conversations and _session_mtimes stores, removing stale or least-recently-used session entries when the configured capacity is exceeded. Ensure eviction removes each session from both mappings together and preserves active session behavior.
798-798: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
itemsredefinition across branches — still unfixed.Line 798 assigns
items = []untyped; the disk-fallback path at line 820 re-declaresitems: list[dict[str, Any]] = []in the same function scope. Same issue raised previously.Also applies to: 820-820
🤖 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 `@services/studio/src/nmp/studio/coding_agents.py` at line 798, Update the function containing the disk-fallback logic so `items` is declared with its intended type only once before the conditional branches; change the later branch assignments, including the fallback near the existing `items: list[dict[str, Any]]` declaration, to reuse that variable without redeclaring it.
🧹 Nitpick comments (3)
agents/nemo-agent-local/tests/test_nemo_agent.py (2)
235-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the missing-
NMP_BASE_URLpath.
_studio_callback_urlraisesRuntimeErrorwhen neitherNMP_BASE_URLnorNEMO_BASE_URLis set, and that raise propagates through every Studio UI tool. Nothing here pins that behavior, andtest_studio_callback_uses_deployment_reachable_base_urlleavesNEMO_BASE_URLuntouched so it also depends on ambient env for the negative case.💚 Proposed test
+ def test_studio_callback_requires_base_url(self, monkeypatch): + monkeypatch.delenv("NMP_BASE_URL", raising=False) + monkeypatch.delenv("NEMO_BASE_URL", raising=False) + + with pytest.raises(RuntimeError, match="NMP_BASE_URL"): + _studio_callback_url("session-id")🤖 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 `@agents/nemo-agent-local/tests/test_nemo_agent.py` around lines 235 - 262, Add a test covering _studio_callback_url when both NMP_BASE_URL and NEMO_BASE_URL are unset, explicitly removing either ambient environment variable with monkeypatch. Assert that the call raises RuntimeError, while keeping test_studio_callback_uses_deployment_reachable_base_url isolated from ambient NEMO_BASE_URL state.
468-481: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the private DeepAgents import Assert against
nemo_agent.register._discover_skillsinstead; keep this only if the test is meant to lock to DeepAgents internals.🤖 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 `@agents/nemo-agent-local/tests/test_nemo_agent.py` around lines 468 - 481, Remove the private DeepAgents import and the direct _list_skills_with_errors assertions from test_create_agent_passes_backend_visible_skills. Instead, use nemo_agent.register._discover_skills to verify the configured skills directory is discovered, while preserving the existing create_deep_agent skills argument assertion.agents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.md (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBenchmark fixtures are baked into a shipped runtime skill.
harbor-test-fileset,harbor-final-fileset, andverify.txtare eval fixtures, but this skill is packaged into the image and loaded for every request. The description ("Use when the task involves ...harbor-test-fileset") plus the "prefer this exact flow" playbook will pull real user fileset tasks toward creating and deleting harbor-named resources. Keep the genericfiles/files.filesetsguidance here and move the harbor sequence into the evaluation assets.Also applies to: 24-34
🤖 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 `@agents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.md` at line 3, Update the skill description and the guidance in its affected sections to remove benchmark-specific names and the harbor-specific exact sequence, retaining only generic files/filesets CRUD and upload/download guidance. Move the `harbor-test-fileset`, `harbor-final-fileset`, and `verify.txt` workflow into the evaluation assets instead of the shipped runtime skill.
🤖 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 `@agents/nemo-agent-local/src/nemo_agent/register.py`:
- Around line 208-223: Constrain _resolve_resource and _call_sdk_method to an
explicit allowlist of documented NeMo SDK resource paths and supported actions,
rejecting any resource or action not listed before attribute traversal or
invocation. Do not permit private attributes or arbitrary kwargs; validate
params against the allowed method parameters while preserving valid documented
API calls.
- Around line 332-339: Update the SSE handling around the response parsing loop
to continue scanning every data line until a decoded dictionary containing a
JSON-RPC result or error is found. Ignore keepalive/non-result payloads and
malformed JSON rather than raising or passing them to _json_rpc_content;
preserve the existing RuntimeError when no valid result is found.
In `@agents/nemo-agent-local/src/nemo_agent/skills/evaluator/SKILL.md`:
- Line 18: Update the evaluator guidance in SKILL.md so a job remaining in
`created` status is reported as observed, along with the absence of a job
controller, rather than being treated as satisfying the instructions. Preserve
the AGENT-SPEC.md verification contract by requiring verification before
claiming mutation or deployment success.
In `@agents/nemo-agent-local/tests/smoke_test.md`:
- Line 18: Update the pytest command in the smoke test instructions to use the
relocated test path agents/nemo-agent-local/tests/test_nemo_agent.py, and update
the related commands on lines 36–37 to reference
agents/nemo-agent-local/src/nemo_agent/register.py instead of the stale
plugins/nemo-agents paths.
In `@agents/nemo-agent-local/tests/test_nemo_agent.py`:
- Around line 510-526: Patch nemo_agent.register._get_client alongside
_resolve_resource in both test_list_resource_names tests and the two
TestDirectFilesetDeleteFastPath tests, returning a mocked client so
_list_resource_names and the delete paths never construct or cache a real
NeMoPlatform. Also add a session-scoped autouse fixture that resets
register._client to None before or after tests to prevent cross-test cache
contamination.
In `@services/studio/src/nmp/studio/coding_agents.py`:
- Around line 1353-1366: Stop exposing raw exception or upstream response
details in the SSE error emitted by the NeMo Agent invocation handler. Update
the exception path around the existing logger.exception and _sse call to log
diagnostic details server-side while sending the client a stable, generic
failure message; preserve the session-specific server logging and error event
behavior.
- Around line 1215-1226: The _studio_coding_agent_url and _invoke_nemo_agent
request flow can derive the destination from the attacker-controlled Host header
while forwarding authorization and cookie credentials. Remove the
request.base_url fallback and require a configured trusted base URL, and update
_coding_agent_request_headers so caller authentication and cookies are not
forwarded to the coding-agent client.post destination.
---
Duplicate comments:
In `@agents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.md`:
- Around line 9-13: Update the nemo_api examples in the SDK/API operations
section to pass params as JSON-encoded strings rather than Python dict literals,
preserving the existing workspace resource, action, and parameter values.
In `@services/studio/src/nmp/studio/coding_agents.py`:
- Around line 134-135: Add bounded eviction for the in-memory
_session_conversations and _session_mtimes stores, removing stale or
least-recently-used session entries when the configured capacity is exceeded.
Ensure eviction removes each session from both mappings together and preserves
active session behavior.
- Line 798: Update the function containing the disk-fallback logic so `items` is
declared with its intended type only once before the conditional branches;
change the later branch assignments, including the fallback near the existing
`items: list[dict[str, Any]]` declaration, to reuse that variable without
redeclaring it.
---
Nitpick comments:
In `@agents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.md`:
- Line 3: Update the skill description and the guidance in its affected sections
to remove benchmark-specific names and the harbor-specific exact sequence,
retaining only generic files/filesets CRUD and upload/download guidance. Move
the `harbor-test-fileset`, `harbor-final-fileset`, and `verify.txt` workflow
into the evaluation assets instead of the shipped runtime skill.
In `@agents/nemo-agent-local/tests/test_nemo_agent.py`:
- Around line 235-262: Add a test covering _studio_callback_url when both
NMP_BASE_URL and NEMO_BASE_URL are unset, explicitly removing either ambient
environment variable with monkeypatch. Assert that the call raises RuntimeError,
while keeping test_studio_callback_uses_deployment_reachable_base_url isolated
from ambient NEMO_BASE_URL state.
- Around line 468-481: Remove the private DeepAgents import and the direct
_list_skills_with_errors assertions from
test_create_agent_passes_backend_visible_skills. Instead, use
nemo_agent.register._discover_skills to verify the configured skills directory
is discovered, while preserving the existing create_deep_agent skills argument
assertion.
🪄 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: fb44ee38-88af-40f7-9817-8d5dce6d78eb
⛔ Files ignored due to path filters (1)
agents/nemo-agent-local/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
agents/nemo-agent-local-spec/AGENT-SPEC.mdagents/nemo-agent-local/.dockerignoreagents/nemo-agent-local/Dockerfileagents/nemo-agent-local/pyproject.tomlagents/nemo-agent-local/src/nemo_agent/nemo-agent.remote.ymlagents/nemo-agent-local/src/nemo_agent/nemo-agent.ymlagents/nemo-agent-local/src/nemo_agent/nemo-eval-data.jsonagents/nemo-agent-local/src/nemo_agent/nemo-eval.ymlagents/nemo-agent-local/src/nemo_agent/register.pyagents/nemo-agent-local/src/nemo_agent/skills/.gitkeepagents/nemo-agent-local/src/nemo_agent/skills/auditor/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/benchmark-execution/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/entities/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/evaluator/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/guardrails/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/inference/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/secrets/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.mdagents/nemo-agent-local/src/nemo_agent/wrapper.pyagents/nemo-agent-local/tests/smoke_test.mdagents/nemo-agent-local/tests/test_nemo_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyservices/studio/src/nmp/studio/coding_agent_mcp_tools.pyservices/studio/src/nmp/studio/coding_agents.pyservices/studio/tests/unit/test_coding_agents.pyweb/packages/studio/env/.env.dev.local.sampleweb/packages/studio/src/routes/DashboardLandingRoute/SkillActionSection.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.test.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.tsxweb/packages/studio/src/routes/agents/AgentDeploymentsListRoute/CreateDeploymentModal/index.test.tsxweb/packages/studio/src/routes/agents/AgentDeploymentsListRoute/CreateDeploymentModal/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeChatThread.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeChatThread.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistoryPanelContents.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/SkillsPanelContents.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/helpers.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/stream.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/SkillsPanelContents.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/stream.ts
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/helpers.ts
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.tsx
- web/packages/studio/src/routes/DashboardLandingRoute/index.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts
- agents/nemo-agent-local/src/nemo_agent/nemo-agent.yml
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/stream.test.ts
- agents/nemo-agent-local/src/nemo_agent/skills/benchmark-execution/SKILL.md
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.test.ts
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeChatThread.test.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeChatThread.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.test.tsx
- web/packages/studio/src/routes/agents/AgentDeploymentsListRoute/CreateDeploymentModal/index.test.tsx
- agents/nemo-agent-local/Dockerfile
- web/packages/studio/src/routes/agents/AgentDeploymentsListRoute/CreateDeploymentModal/index.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.ts
- agents/nemo-agent-local/pyproject.toml
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.test.tsx
- plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
- web/packages/studio/src/routes/DashboardLandingRoute/index.test.tsx
- services/studio/src/nmp/studio/coding_agent_mcp_tools.py
- agents/nemo-agent-local/src/nemo_agent/wrapper.py
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py (1)
400-400: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRoute
NMP_BASE_URLthrough the auth proxy.When platform auth is enabled, the inference config is rewritten to
http://127.0.0.1:<port>at Line 390, butNMP_BASE_URLstill receivesgatewayhere. SDK calls therefore bypass the sidecar and lack its service-principal credentials. Pass the proxy URL asplatform_base_urlin that branch and add an auth-enabled regression test.Proposed fix
+ agent_base_url = gateway if platform_auth_enabled(): auth_proxy_identity = _AUTH_PROXY_IDENTITY - config = rewrite_config_base_urls(config, f"http://127.0.0.1:{auth_proxy_port()}") + agent_base_url = f"http://127.0.0.1:{auth_proxy_port()}" + config = rewrite_config_base_urls(config, agent_base_url) else: config = rewrite_config_base_urls(config, gateway) deployment_config = build_deployment_config( ... - platform_base_url=gateway, + platform_base_url=agent_base_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/src/nemo_agents_plugin/runner/deployments_backend.py` at line 400, Update the deployment configuration branch that rewrites inference access for platform auth to also pass the resulting local auth-proxy URL as platform_base_url instead of gateway. Preserve gateway for the auth-disabled branch, and add a regression test covering that NMP_BASE_URL uses the proxy URL when authentication is enabled.
🧹 Nitpick comments (2)
agents/nemo-agent-local/src/nemo_agent/register.py (1)
299-315: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGate normalizes the action but the call uses the raw string.
normalized_actionis only used for the read-only check;_call_sdk_methodgetsaction. Not a bypass (getattr fails on odd casing/whitespace), but the divergence is fragile — passnormalized_actionthrough.♻️ Proposed change
- result = _call_sdk_method(resolved, action, parsed_params) + result = _call_sdk_method(resolved, normalized_action, parsed_params)🤖 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 `@agents/nemo-agent-local/src/nemo_agent/register.py` around lines 299 - 315, Use the already computed normalized_action consistently in the SDK invocation: update _call_sdk_method in the surrounding registration flow to receive normalized_action instead of the raw action, while preserving the existing read-only check and parameters.services/studio/tests/unit/test_coding_agents.py (1)
1683-1769: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
fake_streamstub across two tests.
fake_stream(lines 1692-1711) is duplicated almost verbatim at lines 1781-1800 in the same file. This PR already had to update both copies in lockstep when the_stream_nemo_agentsignature changed — a good signal to extract a shared fixture/helper to avoid repeating that cost next time.♻️ Suggested fixture extraction
`@pytest.fixture` def capture_stream(monkeypatch: pytest.MonkeyPatch): captured: dict[str, Any] = {} async def fake_stream(session_id, message, agent_url, headers, studio_system_prompt): captured.update( session_id=session_id, message=message, agent_url=agent_url, headers=headers, studio_system_prompt=studio_system_prompt, ) yield coding_agents._sse(json.dumps({"type": "system", "subtype": "init"})) yield coding_agents._sse("", event="done") monkeypatch.setattr(coding_agents, "_stream_nemo_agent", fake_stream) return captured🤖 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 `@services/studio/tests/unit/test_coding_agents.py` around lines 1683 - 1769, Extract the duplicated fake_stream monkeypatch and captured-state setup into a shared pytest fixture near the affected tests, returning the captured dictionary and patching coding_agents._stream_nemo_agent once. Update both tests to use the fixture and remove their local fake_stream implementations while preserving the existing assertions and streamed SSE 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 `@agents/nemo-agent-local/src/nemo_agent/register.py`:
- Around line 620-635: Update the direct fileset deletion flow around
_direct_fileset_delete_name so regex-matched deletes fail closed when
_trusted_studio_session_id(config) is absent, instead of calling _delete_fileset
unapproved. Preserve the existing approval request and denial handling for
trusted Studio sessions.
---
Outside diff comments:
In `@plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py`:
- Line 400: Update the deployment configuration branch that rewrites inference
access for platform auth to also pass the resulting local auth-proxy URL as
platform_base_url instead of gateway. Preserve gateway for the auth-disabled
branch, and add a regression test covering that NMP_BASE_URL uses the proxy URL
when authentication is enabled.
---
Nitpick comments:
In `@agents/nemo-agent-local/src/nemo_agent/register.py`:
- Around line 299-315: Use the already computed normalized_action consistently
in the SDK invocation: update _call_sdk_method in the surrounding registration
flow to receive normalized_action instead of the raw action, while preserving
the existing read-only check and parameters.
In `@services/studio/tests/unit/test_coding_agents.py`:
- Around line 1683-1769: Extract the duplicated fake_stream monkeypatch and
captured-state setup into a shared pytest fixture near the affected tests,
returning the captured dictionary and patching coding_agents._stream_nemo_agent
once. Update both tests to use the fixture and remove their local fake_stream
implementations while preserving the existing assertions and streamed SSE
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: b68a8ee9-57cf-4c6c-95f5-df3c1d16a559
📒 Files selected for processing (14)
agents/nemo-agent-local/src/nemo_agent/nemo-eval-data.jsonagents/nemo-agent-local/src/nemo_agent/nemo-eval.ymlagents/nemo-agent-local/src/nemo_agent/register.pyagents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.mdagents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.mdagents/nemo-agent-local/src/nemo_agent/wrapper.pyagents/nemo-agent-local/tests/test_nemo_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyservices/studio/src/nmp/studio/coding_agents.pyservices/studio/tests/unit/test_coding_agents.pyweb/packages/studio/src/routes/DashboardLandingRoute/index.test.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- agents/nemo-agent-local/src/nemo_agent/skills/workspace/SKILL.md
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts
- agents/nemo-agent-local/src/nemo_agent/nemo-eval.yml
- agents/nemo-agent-local/src/nemo_agent/skills/files/SKILL.md
- agents/nemo-agent-local/src/nemo_agent/wrapper.py
- agents/nemo-agent-local/tests/test_nemo_agent.py
- web/packages/studio/src/routes/DashboardLandingRoute/index.test.tsx
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
htolentino-nvidia
requested changes
Jul 30, 2026
htolentino-nvidia
approved these changes
Jul 30, 2026
This was referenced Jul 30, 2026
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
3 participants
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 by CodeRabbit
docker/k8sruntime mode with required container image validation.