feat(nemo-agents): support for nemo agents run and nemo agents deploy for Fabric backed agents - #909
Merged
mmogallapalli merged 16 commits intoJul 28, 2026
Conversation
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
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 centralizes agent-config validation and deployment resolution, adds a session-based Fabric HTTP server and runtime lifecycle management, integrates local Fabric subprocess deployments, and routes the CLI between NAT and Fabric servers. ChangesAgent configuration and Fabric serving
Sequence Diagram(s)sequenceDiagram
participant OpenAIClient
participant FabricServingApp
participant FabricSessionManager
participant FabricRuntime
OpenAIClient->>FabricServingApp: POST /v1/chat/completions
FabricServingApp->>FabricSessionManager: resolve_session and invoke_session
FabricSessionManager->>FabricRuntime: invoke_fabric_runtime
FabricRuntime-->>FabricSessionManager: FabricRuntimeResult
FabricSessionManager-->>FabricServingApp: normalized result
FabricServingApp-->>OpenAIClient: ChatCompletionResponse and session ID
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (4)
plugins/nemo-agents/src/nemo_agents_plugin/fabric/server.py (1)
170-182: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReject
stream=trueexplicitly.Streaming is out of scope, but the field is accepted and ignored, so OpenAI-compatible clients get a non-streaming body they won't parse. Return 400 instead.
♻️ Proposed change
) -> ChatCompletionResponse: + if request.stream: + raise HTTPException(status_code=400, detail="Streaming responses are not supported.") try:🤖 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/fabric/server.py` around lines 170 - 182, Update chat_completions to validate request.stream before resolving the session or processing the invocation; when streaming is requested, raise HTTPException with status 400, while preserving the existing non-streaming session and response flow.plugins/nemo-agents/src/nemo_agents_plugin/fabric/session_registry.py (1)
12-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
Runtimenormally rather than underTYPE_CHECKING.
session_manager.pyline 24 already importsnemo_fabricat module scope with thetysuppression, so the guard isn't needed here.As per coding guidelines: "do not import those types only under
TYPE_CHECKING; import them normally when possible".♻️ Proposed change
from dataclasses import dataclass, field -from typing import TYPE_CHECKING -if TYPE_CHECKING: - from nemo_fabric import Runtime +# CI type-checks this plugin via ty extra-paths without installing nemo-agents deps. +from nemo_fabric import Runtime # ty: ignore[unresolved-import]🤖 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/fabric/session_registry.py` around lines 12 - 23, Update the Runtime import used by FabricRuntimeSession to import Runtime normally at module scope instead of guarding it with TYPE_CHECKING; remove the now-unused TYPE_CHECKING import while preserving the session_id and runtime annotations.Source: Coding guidelines
plugins/nemo-agents/src/nemo_agents_plugin/fabric/session_manager.py (2)
50-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the concrete
Fabrictype instead ofAny.
Fabricis already imported normally at line 24, so the injection point can be typed precisely.As per coding guidelines: "prefer concrete type hints over string-based type hints, and do not import those types only under
TYPE_CHECKING".♻️ Proposed change
- fabric: Any | None = None, + fabric: Fabric | None = None,🤖 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/fabric/session_manager.py` at line 50, Update the fabric parameter or attribute declaration in the session manager to use the imported concrete Fabric type instead of Any, preserving the existing optional None default and avoiding string-based or TYPE_CHECKING-only typing.Source: Coding guidelines
133-140: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winShutdown aborts on the first unexpected stop error.
stop_sessiononly swallowsFabricSessionStopError; anything else propagates out ofgather, leaving the remaining stop coroutines un-awaited and runtimes leaked during shutdown.♻️ Proposed change
- await asyncio.gather(*(stop_session(session) for session in sessions)) + await asyncio.gather(*(stop_session(session) for session in sessions), return_exceptions=True)🤖 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/fabric/session_manager.py` around lines 133 - 140, Update the local stop_session helper in the session shutdown flow to catch unexpected exceptions as well as FabricSessionStopError, logging them without re-raising so asyncio.gather can complete all session stops. Preserve the existing session-specific error logging and return len(sessions) after every stop coroutine has been awaited.
🤖 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/fabric/environment.py`:
- Around line 16-19: Constrain workspace handling in the environment setup flow:
resolve the configured workspace against base_dir, reject absolute paths and any
resolved path that escapes base_dir (including traversal such as ../../x), and
only then create the directory. Add tests covering both absolute and traversal
configurations.
In `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/runtime.py`:
- Around line 100-103: Update the TimeoutError handling in runtime.invoke and
run_fabric_agent_once so timeout_seconds=None produces a valid timeout message
instead of applying numeric formatting to None. Build the timeout text
conditionally for configured versus unset deadlines, then raise
FabricRuntimeTimeoutError with that text while preserving the original error as
the cause.
In `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/server.py`:
- Around line 158-161: Update the shutdown cleanup in the surrounding server
lifecycle method so exceptions or cancellation from cleanup_task do not prevent
session_manager.close_all_sessions() from running. Ensure close_all_sessions()
is always awaited after signaling cleanup_shutdown, while preserving the cleanup
task’s existing exception behavior where appropriate.
---
Nitpick comments:
In `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/server.py`:
- Around line 170-182: Update chat_completions to validate request.stream before
resolving the session or processing the invocation; when streaming is requested,
raise HTTPException with status 400, while preserving the existing non-streaming
session and response flow.
In `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/session_manager.py`:
- Line 50: Update the fabric parameter or attribute declaration in the session
manager to use the imported concrete Fabric type instead of Any, preserving the
existing optional None default and avoiding string-based or TYPE_CHECKING-only
typing.
- Around line 133-140: Update the local stop_session helper in the session
shutdown flow to catch unexpected exceptions as well as FabricSessionStopError,
logging them without re-raising so asyncio.gather can complete all session
stops. Preserve the existing session-specific error logging and return
len(sessions) after every stop coroutine has been awaited.
In `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/session_registry.py`:
- Around line 12-23: Update the Runtime import used by FabricRuntimeSession to
import Runtime normally at module scope instead of guarding it with
TYPE_CHECKING; remove the now-unused TYPE_CHECKING import while preserving the
session_id and runtime annotations.
🪄 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: e7d3e9a2-999d-4276-b6a8-9212cd7b0aa9
📒 Files selected for processing (19)
plugins/nemo-agents/src/nemo_agents_plugin/agent_config_formats.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/fabric/environment.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/invocation.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/runtime.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/server.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/serving_models.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/session_manager.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/session_registry.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.pyplugins/nemo-agents/tests/unit/test_agent_config_formats.pyplugins/nemo-agents/tests/unit/test_controller.pyplugins/nemo-agents/tests/unit/test_fabric_runtime.pyplugins/nemo-agents/tests/unit/test_fabric_server.pyplugins/nemo-agents/tests/unit/test_fabric_serving_models.pyplugins/nemo-agents/tests/unit/test_fabric_session_manager.pyplugins/nemo-agents/tests/unit/test_fabric_session_registry.pyplugins/nemo-agents/tests/unit/test_runner_in_memory.py
💤 Files with no reviewable changes (1)
- plugins/nemo-agents/tests/unit/test_controller.py
Contributor
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pr_description.md (1)
1-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftKeep this page within one Diataxis type.
The page mixes explanation, reference content, and validation/how-to material. Reframe or split it into one quadrant, add prerequisites before the main content, and add a
Next Stepssection with cross-links.As per coding guidelines, each documentation page should fit ONE Diataxis quadrant, list prerequisites at the top, and include a
Next Stepssection at the end.🤖 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 `@pr_description.md` around lines 1 - 147, Rework pr_description.md to fit a single Diataxis type, removing or relocating mixed design, reference, and validation/how-to material as appropriate. Add a Prerequisites section near the beginning before the main content, and append a Next Steps section containing links to related documentation or follow-up topics.Source: Coding guidelines
🤖 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 `@pr_description.md`:
- Line 1: Update the first heading in pr_description.md from level 2 to level 1
by changing Summary to use a single leading hash, preserving the heading text.
---
Nitpick comments:
In `@pr_description.md`:
- Around line 1-147: Rework pr_description.md to fit a single Diataxis type,
removing or relocating mixed design, reference, and validation/how-to material
as appropriate. Add a Prerequisites section near the beginning before the main
content, and append a Next Steps section containing links to related
documentation or follow-up topics.
🪄 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: 79bf39b3-69a5-4c53-bd15-b7d07990fb66
📒 Files selected for processing (8)
plugins/nemo-agents/src/nemo_agents_plugin/fabric/environment.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/runtime.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/server.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/session_registry.pyplugins/nemo-agents/tests/unit/test_fabric_invocation.pyplugins/nemo-agents/tests/unit/test_fabric_runtime.pyplugins/nemo-agents/tests/unit/test_fabric_server.pypr_description.md
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/nemo-agents/src/nemo_agents_plugin/fabric/environment.py
- plugins/nemo-agents/src/nemo_agents_plugin/fabric/session_registry.py
- plugins/nemo-agents/src/nemo_agents_plugin/fabric/server.py
- plugins/nemo-agents/tests/unit/test_fabric_server.py
- plugins/nemo-agents/src/nemo_agents_plugin/fabric/runtime.py
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
mmogallapalli
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/cli.py`:
- Line 220: Update the configuration-loading flow around _load_yaml and
config_format to validate that the loaded YAML value is a mapping before calling
.get(); treat None or other invalid values as an invalid configuration and exit
cleanly. Preserve the existing default config_format for valid mappings, and add
a test covering an empty YAML file.
🪄 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: ebed3ba3-3379-4706-b60d-e71013c9e379
📒 Files selected for processing (2)
plugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/tests/unit/test_cli.py
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
mmogallapalli
requested review from
AnuradhaKaruppiah,
benmccown,
mikeknep and
tylersbray
July 27, 2026 19:04
tylersbray
approved these changes
Jul 28, 2026
tylersbray
left a comment
tylersbray
left a comment
Contributor
There was a problem hiding this comment.
Nice foundation, looks good.
mikeknep
reviewed
Jul 28, 2026
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
mikeknep
approved these changes
Jul 28, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 28, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 28, 2026
mmogallapalli
deleted the
mmogallapall/aircore-932-define-and-implement-managed-fabric-runtimesession-lifecycle
branch
July 28, 2026 22:01
This was referenced Jul 29, 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
This PR implements the first Platform-managed serving lifecycle for Fabric-backed NeMo Agents as part of AIRCORE-932.
It adds a local FastAPI serving process that creates one Fabric runtime per logical session. The same server can be started directly through
nemo agents runfor foreground development or throughnemo agents deployfor Platform-managed serving:Fabric continues to own harness execution and the runtime
start/invoke/stoplifecycle. NeMo Platform owns the multi-user server, logical session identity, runtime registry, request routing, concurrency policy, expiration, and cleanup.Changes
GET /healthPOST /v1/chat/completionsDELETE /v1/sessions/{session_id}nemo agents runto dispatch by agent config format:nemo-agents-spec-v1starts the Platform-owned Fabric server.nat-workflow-v1continues to runnat start fastapi.FabricConfigtranslation and runtime startup.nemo-agents-spec-v1agents by:agent.yamlunder the Platform agent workspace.nat-workflow-v1.Design Choices
Platform owns sessions; Fabric owns runtimes
A Platform session is a logical conversation rather than an HTTP connection. Platform maintains the
session_id -> FabricRuntimemapping, while Fabric remains unaware of users, HTTP routing, and other runtimes.This keeps the integration aligned with Fabric's public lifecycle contract instead of adding lifecycle behavior inside adapters.
Runtimes are created lazily
Server startup loads and validates the reusable Platform agent definition but does not create a Fabric runtime. A complete
FabricConfigis translated when the first request opens a logical session, and that config is bound to the resulting runtime.This avoids allocating harness resources for sessions that never invoke the agent and leaves room for future per-session policy, environment, and profile resolution.
Session identity uses a response header
The first request may omit
X-Nemo-Session-Id. Platform generates an opaque session ID and returns it in that response header. Later requests provide the same header to reuse the runtime.Using a header keeps the request body compatible with the OpenAI chat-completions shape. Supplying an unknown or closed session ID returns
404; it does not silently create a replacement runtime.One runtime processes one turn at a time
Invocations for the same session are serialized with a per-session lock because ordered turns share harness state. Different sessions may run concurrently, subject to a server-wide semaphore. The initial default permits eight concurrent invocations.
The runtime owns conversation state
Each HTTP request passes the current user message to the existing runtime. Prior turns are not replayed from the HTTP payload because the Fabric runtime and selected harness adapter own the session's conversation state.
Cleanup is explicit and bounded
Clients can close sessions explicitly. The server also expires idle sessions after 30 minutes, checks every five minutes, and drains all remaining runtimes during shutdown. Runtime registration failures also stop any runtime that was already started.
Local deployment builds on the existing runner
The first implementation uses the existing in-memory subprocess backend rather than introducing a second process-management system. Fabric and NAT deployments therefore share port allocation, health polling, logs, status transitions, termination, and filesystem cleanup.
Docker/Kubernetes runtime placement and distributed session ownership remain separate follow-up work.
runanddeployshare the Fabric servernemo agents runstarts the Fabric server in the foreground from a localagent.yaml, with the caller choosing the host and port and stopping it withCtrl-C. It does not require a running Platform or create an Agent or AgentDeployment entity.nemo agents deploystarts the same server through the existing Platform runner. Platform allocates the port, tracks deployment status, routes requests through the gateway, exposes logs, and owns process cleanup.Keeping both paths on the same server implementation provides the Fabric equivalent of the existing NAT
run/deploysplit without duplicating runtime or session behavior.Agent formats share a small internal protocol
Agent creation and deployment now resolve behavior through config-format handlers. This keeps
nat-workflow-v1as the default and addsnemo-agents-spec-v1without spreading format-specific branches through the API layer.Error Behavior
404503502504502Errors for an existing session preserve the session ID header where appropriate.
Out of Scope
Validation
Focused branch coverage:
This includes agent config handling, format dispatch, Fabric translation/validation, one-shot and active-runtime invocation, serving routes, session registry/manager behavior, deployment APIs, controller behavior, and the in-memory runner.
Additional nested NeMo Agents suites:
Focused CLI validation, including NAT and Fabric
nemo agents rundispatch:Repository Python style and formatting:
Manual end-to-end validation:
nemo-agents-spec-v1Agent fromagent.yaml.pending -> starting -> running.X-Nemo-Session-Id.204.404.Summary by CodeRabbit