From c242c1463ca04862e6e4b5781edb0b47675750da Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Thu, 30 Jul 2026 16:05:08 -0700 Subject: [PATCH 1/4] feat(nemo-agents): stage Fabric agent.yaml artifacts into Docker/K8s deployments Download the agent-spec fileset at deploy time and materialize sibling files (skills, prompts, etc.) under /workspace so Fabric relative paths resolve in containers the same way they do locally. Closes AIRCORE-966. Signed-off-by: Tyler Bray --- .../src/nemo_agents_plugin/runner/backend.py | 1 + .../nemo_agents_plugin/runner/controller.py | 1 + .../runner/deployments_backend.py | 72 +++++- .../runner/fabric_artifact_staging.py | 162 ++++++++++++++ .../nemo_agents_plugin/runner/in_memory.py | 3 +- .../unit/test_fabric_artifact_staging.py | 206 ++++++++++++++++++ .../tests/unit/test_runner_deployments.py | 191 +++++++++++++++- 7 files changed, 623 insertions(+), 13 deletions(-) create mode 100644 plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py create mode 100644 plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/runner/backend.py b/plugins/nemo-agents/src/nemo_agents_plugin/runner/backend.py index 654f9e6313..e8a78e0ddb 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/runner/backend.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/runner/backend.py @@ -93,6 +93,7 @@ async def create_deployment( config: dict[str, Any], port: int, *, + agent: str = "", image: str | None = None, deployment_mode: DeploymentMode = "subprocess", created_by: str | None = None, diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py b/plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py index c0079eaad6..ad7cf15969 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py @@ -186,6 +186,7 @@ async def _start_deployment(self, dep: AgentDeployment) -> None: name=dep.name, config=dep.config, port=port, + agent=dep.agent, image=dep.image or None, deployment_mode=dep.deployment_mode, created_by=dep.created_by, diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py b/plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py index 33c1023266..864853686b 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py @@ -14,7 +14,9 @@ from __future__ import annotations import asyncio +import base64 import copy +import json import logging import shlex import time @@ -34,6 +36,10 @@ ) from nemo_agents_plugin.fabric.gateway_credentials import platform_gateway_credential_env from nemo_agents_plugin.runner.backend import DeploymentInfo, ExternalLog, LogLocation, RunnerBackend +from nemo_agents_plugin.runner.fabric_artifact_staging import ( + FabricArtifactStagingError, + stage_fabric_spec_config_files, +) from nemo_agents_plugin.utils import get_base_url, get_internal_base_url from nemo_deployments_plugin.auth_proxy import auth_proxy_port from nemo_deployments_plugin.entities import ( @@ -64,6 +70,7 @@ _NAT_CONFIG_YAML_ENV = "NAT_CONFIG_YAML" _AGENT_CONFIG_YAML_ENV = "AGENT_CONFIG_YAML" _AGENT_CONFIG_PATH_ENV = "AGENT_CONFIG_PATH" +_STAGED_CONFIG_FILES_ENV = "STAGED_CONFIG_FILES_B64_JSON" _FABRIC_SERVER_MODULE = "nemo_agents_plugin.fabric.server" _AUTH_PROXY_IDENTITY = "agents" @@ -243,6 +250,18 @@ def _materialize_config_and_exec(*, config_path: str, yaml_env: str, argv: list[ return [f'mkdir -p "$(dirname {quoted_path})" && printf "%s" "${yaml_env}" > {quoted_path} && exec {quoted_argv}'] +def _materialize_staged_config_files_and_exec(*, env_name: str, argv: list[str]) -> list[str]: + """Return ``sh -c`` args that write staged ``config_files`` from *env_name*, then exec *argv*.""" + quoted_argv = " ".join(shlex.quote(arg) for arg in argv) + inline_python = ( + "import base64,json,os,pathlib;" + f"data=json.loads(os.environ[{json.dumps(env_name)}]);" + "[(pathlib.Path(p).parent.mkdir(parents=True,exist_ok=True)," + "pathlib.Path(p).write_bytes(base64.b64decode(b))) for p,b in data.items()]" + ) + return [f"python -c {shlex.quote(inline_python)} && exec {quoted_argv}"] + + def executor_for_mode(config: DeploymentsRunnerConfig, mode: DeploymentMode) -> str | None: """Resolve the named deployments-plugin executor for *mode*.""" if mode == "docker": @@ -291,6 +310,7 @@ def build_deployment_config( labels: dict[str, str] | None = None, auth_proxy_identity: str | None = None, auth_proxy_on_behalf_of: str | None = None, + config_files: list[ConfigFile] | None = None, ) -> DeploymentConfig: """Compile an agent into a long-running ``DeploymentConfig`` (Always). @@ -314,6 +334,7 @@ def build_deployment_config( is_fabric = _is_fabric_agent_config(agent_config) config_yaml = yaml.safe_dump(agent_config, sort_keys=False) config_path = _fabric_config_mount_path(config_mount_path) if is_fabric else config_mount_path + resolved_config_files = config_files or [ConfigFile(path=config_path, content=config_yaml)] env = [ EnvVar(name="NMP_WORKSPACE", value=workspace), EnvVar(name="NMP_AGENT_NAME", value=name), @@ -371,14 +392,27 @@ def build_deployment_config( ] if mode == "docker": - # Docker backend does not mount config_files; materialize the YAML from env. - env.append(EnvVar(name=config_yaml_env, value=config_yaml)) - command = ["sh", "-c"] - args = _materialize_config_and_exec( - config_path=config_path, - yaml_env=config_yaml_env, - argv=[*server_command, *server_args], - ) + # Docker backend does not mount config_files; materialize staged files from env. + if len(resolved_config_files) == 1: + single = resolved_config_files[0] + env.append(EnvVar(name=config_yaml_env, value=single.content)) + command = ["sh", "-c"] + args = _materialize_config_and_exec( + config_path=single.path, + yaml_env=config_yaml_env, + argv=[*server_command, *server_args], + ) + else: + payload = { + config_file.path: base64.b64encode(config_file.content.encode("utf-8")).decode("ascii") + for config_file in resolved_config_files + } + env.append(EnvVar(name=_STAGED_CONFIG_FILES_ENV, value=json.dumps(payload, separators=(",", ":")))) + command = ["sh", "-c"] + args = _materialize_staged_config_files_and_exec( + env_name=_STAGED_CONFIG_FILES_ENV, + argv=[*server_command, *server_args], + ) else: command = server_command args = server_args @@ -412,9 +446,7 @@ def build_deployment_config( ).model_copy( update={ "init_containers": init_containers, - "config_files": [ - ConfigFile(path=config_path, content=config_yaml), - ], + "config_files": resolved_config_files, "restart_policy": "Always", "auth_proxy_sidecar": auth_proxy_identity is not None, "auth_proxy_sidecar_identity": auth_proxy_identity, @@ -443,6 +475,7 @@ async def create_deployment( config: dict[str, Any], port: int, *, + agent: str = "", image: str | None = None, deployment_mode: DeploymentMode = "docker", created_by: str | None = None, @@ -519,6 +552,22 @@ async def create_deployment( if is_fabric: deployment_labels["nemo.agents/runtime"] = "fabric" + staged_config_files: list[ConfigFile] | None = None + if is_fabric and agent: + agent_yaml_path = _fabric_config_mount_path(self._config.config_mount_path) + try: + sdk = get_async_platform_sdk(as_service="agents", internal=True) + staged_config_files = await stage_fabric_spec_config_files( + workspace=workspace, + agent_name=agent, + rewritten_agent_config=config, + agent_yaml_path=agent_yaml_path, + sdk=sdk.files, + ) + except FabricArtifactStagingError as exc: + logger.error("Refusing to deploy Fabric agent %r: %s", name, exc) + return DeploymentInfo(name=name, status="failed", error=str(exc)) + deployment_config = build_deployment_config( name=name, workspace=workspace, @@ -532,6 +581,7 @@ async def create_deployment( labels=deployment_labels, auth_proxy_identity=auth_proxy_identity, auth_proxy_on_behalf_of=auth_proxy_on_behalf_of, + config_files=staged_config_files, ) await entities.create(deployment_config) try: diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py b/plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py new file mode 100644 index 0000000000..4208820ad5 --- /dev/null +++ b/plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py @@ -0,0 +1,162 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Stage Fabric agent-spec fileset artifacts for container deployments.""" + +from __future__ import annotations + +import logging +import tempfile +from pathlib import Path, PurePosixPath +from typing import Any, Protocol + +import yaml +from nemo_agents_plugin.entities import ( + AGENT_CONFIG_FILENAME, + AGENT_SPEC_FILENAME, + agent_spec_fileset_name, +) +from nemo_deployments_plugin.entities import ConfigFile +from nemo_platform import NotFoundError + +logger = logging.getLogger(__name__) + +# Staged artifacts ride to the container inside a ConfigMap (k8s) or a single env +# var (docker); both cap out around 1MiB, so refuse oversized filesets here with a +# clear error instead of letting the container fail to start. +_MAX_STAGED_BYTES = 900_000 + + +class FabricArtifactStagingError(ValueError): + """Raised when Fabric deployment artifact staging cannot satisfy the agent config.""" + + +class _FilesDownloader(Protocol): + async def download( + self, + *, + local_path: str, + fileset: str | None = None, + workspace: str | None = None, + ) -> None: ... + + +async def stage_fabric_spec_config_files( + *, + workspace: str, + agent_name: str, + rewritten_agent_config: dict[str, Any], + agent_yaml_path: str, + sdk: _FilesDownloader, +) -> list[ConfigFile]: + """Download the agent spec fileset and return container ``config_files`` entries. + + When the fileset is unavailable, returns a single inline ``agent.yaml`` entry + (AIRCORE-947 behavior). When available, maps every fileset file under the same + ``base_dir`` as *agent_yaml_path*, substituting the rewritten YAML for + ``agent.yaml``. + """ + config_yaml = yaml.safe_dump(rewritten_agent_config, sort_keys=False) + if not agent_name: + return [ConfigFile(path=agent_yaml_path, content=config_yaml)] + + fileset_name = agent_spec_fileset_name(agent_name) + try: + with tempfile.TemporaryDirectory(prefix=f".fabric-spec-{agent_name}-") as tmp: + tmp_path = Path(tmp) + await sdk.download(local_path=str(tmp_path), fileset=fileset_name, workspace=workspace) + config_files = _collect_staged_config_files( + root=tmp_path, + agent_yaml_path=agent_yaml_path, + rewritten_agent_yaml=config_yaml, + ) + _validate_referenced_skill_paths(rewritten_agent_config, config_files, agent_yaml_path) + _validate_staged_size(config_files, fileset_name) + return config_files + except (FileNotFoundError, NotFoundError) as exc: + logger.info( + "Agent spec fileset %s/%s unavailable (%s); using inline agent.yaml only", + workspace, + fileset_name, + exc, + ) + inline_only = [ConfigFile(path=agent_yaml_path, content=config_yaml)] + # A config that references skills still needs them staged, so an absent + # fileset must fail here rather than at container start. + _validate_referenced_skill_paths(rewritten_agent_config, inline_only, agent_yaml_path) + return inline_only + + +def _collect_staged_config_files( + *, + root: Path, + agent_yaml_path: str, + rewritten_agent_yaml: str, +) -> list[ConfigFile]: + base_dir = PurePosixPath(agent_yaml_path).parent + config_files: list[ConfigFile] = [] + + for path in sorted(root.rglob("*")): + if not path.is_file(): + continue + rel = path.relative_to(root) + if ".." in rel.parts: + raise FabricArtifactStagingError(f"Path escape in agent spec fileset: {rel.as_posix()!r}") + if rel.name == AGENT_SPEC_FILENAME: + continue + container_path = str(base_dir / PurePosixPath(rel.as_posix())) + if rel.name == AGENT_CONFIG_FILENAME and container_path == agent_yaml_path: + content = rewritten_agent_yaml + else: + try: + content = path.read_text(encoding="utf-8") + except UnicodeDecodeError as exc: + raise FabricArtifactStagingError( + f"Agent spec fileset contains non-UTF-8 file {rel.as_posix()!r}; staged artifacts must be text" + ) from exc + config_files.append(ConfigFile(path=container_path, content=content)) + + if not any(cf.path == agent_yaml_path for cf in config_files): + config_files.append(ConfigFile(path=agent_yaml_path, content=rewritten_agent_yaml)) + return config_files + + +def _validate_staged_size(config_files: list[ConfigFile], fileset_name: str) -> None: + total = sum(len(config_file.content.encode("utf-8")) for config_file in config_files) + if total > _MAX_STAGED_BYTES: + raise FabricArtifactStagingError( + f"Agent spec fileset {fileset_name!r} stages {total} bytes across {len(config_files)} files, " + f"exceeding the {_MAX_STAGED_BYTES} byte limit for container config delivery" + ) + + +def _validate_referenced_skill_paths( + agent_config: dict[str, Any], + config_files: list[ConfigFile], + agent_yaml_path: str, +) -> None: + skills = agent_config.get("skills") + if not isinstance(skills, dict): + return + paths = skills.get("paths") + if not isinstance(paths, list) or not paths: + return + + base_dir = PurePosixPath(agent_yaml_path).parent + staged_paths = {PurePosixPath(config_file.path) for config_file in config_files} + + for skill_path in paths: + if not isinstance(skill_path, str) or not skill_path: + continue + rel = PurePosixPath(skill_path) + if rel.is_absolute() or ".." in rel.parts: + raise FabricArtifactStagingError( + f"Invalid skills.paths entry {skill_path!r}: must be a relative path under the agent base directory" + ) + expected = base_dir / rel + prefix = f"{expected}/" + matched = any(str(staged) == str(expected) or str(staged).startswith(prefix) for staged in staged_paths) + if not matched: + raise FabricArtifactStagingError( + f"Referenced skills.paths entry {skill_path!r} was not found in staged agent spec fileset" + ) diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.py b/plugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.py index fe6ab5b442..cb14f429de 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.py @@ -219,6 +219,7 @@ async def create_deployment( config: dict[str, Any], port: int, *, + agent: str = "", image: str | None = None, deployment_mode: DeploymentMode = "subprocess", created_by: str | None = None, @@ -227,7 +228,7 @@ async def create_deployment( # created_by drives on-behalf-of delegation only for container modes (via # the auth-proxy sidecar). Subprocess deployments run in-process on the # platform host and do not use the sidecar, so it does not apply here. - del image, deployment_mode, created_by + del agent, image, deployment_mode, created_by if config.get("config_format") == NEMO_AGENTS_SPEC_CONFIG_FORMAT: return await self._create_fabric_deployment(workspace, name, config, port) diff --git a/plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py b/plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py new file mode 100644 index 0000000000..10827eb7a8 --- /dev/null +++ b/plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py @@ -0,0 +1,206 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for Fabric agent-spec artifact staging.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest +import yaml +from nemo_agents_plugin.runner.fabric_artifact_staging import ( + FabricArtifactStagingError, + stage_fabric_spec_config_files, +) +from nemo_deployments_plugin.entities import ConfigFile +from nemo_platform import NotFoundError + + +def _fabric_config(*, skills_paths: list[str] | None = None) -> dict[str, Any]: + config: dict[str, Any] = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "main", + "harnesses": { + "main": { + "kind": "codex", + "settings": {}, + } + }, + } + if skills_paths is not None: + config["skills"] = {"paths": skills_paths} + return config + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_without_agent_name_returns_inline_yaml() -> None: + config = _fabric_config() + rewritten = {**config, "gateway": "http://example"} + result = await stage_fabric_spec_config_files( + workspace="default", + agent_name="", + rewritten_agent_config=rewritten, + agent_yaml_path="/workspace/agent.yaml", + sdk=AsyncMock(), + ) + assert result == [ + ConfigFile(path="/workspace/agent.yaml", content=yaml.safe_dump(rewritten, sort_keys=False)), + ] + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_missing_fileset_falls_back() -> None: + config = _fabric_config() + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=FileNotFoundError("missing")) + + result = await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=config, + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + assert len(result) == 1 + assert result[0].path == "/workspace/agent.yaml" + sdk.download.assert_awaited_once() + await_args = sdk.download.await_args + assert await_args is not None + assert await_args.kwargs["fileset"] == "fabric-agent-spec" + assert await_args.kwargs["workspace"] == "default" + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_not_found_error_falls_back() -> None: + config = _fabric_config() + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=NotFoundError("missing fileset", response=MagicMock(), body=None)) + + result = await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=config, + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + assert len(result) == 1 + assert result[0].path == "/workspace/agent.yaml" + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_stages_sibling_artifacts() -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + root = Path(local_path) + (root / "agent.yaml").write_text("stale: true\n", encoding="utf-8") + (root / "prompts").mkdir() + (root / "prompts" / "system.md").write_text("You are helpful.\n", encoding="utf-8") + skill_dir = root / "skills" / "review" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# Review\n", encoding="utf-8") + (root / "AGENT-SPEC.md").write_text("# Spec\n", encoding="utf-8") + + rewritten = _fabric_config(skills_paths=["skills/review"]) + rewritten["models"] = {"default": {"provider": "openai", "model": "gpt", "settings": {"base_url": "http://igw"}}} + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + result = await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=rewritten, + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + by_path = {item.path: item.content for item in result} + assert "/workspace/agent.yaml" in by_path + assert "/workspace/prompts/system.md" in by_path + assert "/workspace/skills/review/SKILL.md" in by_path + assert "AGENT-SPEC.md" not in by_path + loaded = yaml.safe_load(by_path["/workspace/agent.yaml"]) + assert loaded["models"]["default"]["settings"]["base_url"] == "http://igw" + assert "stale" not in by_path["/workspace/agent.yaml"] + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_missing_fileset_rejects_configured_skills() -> None: + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=NotFoundError("missing fileset", response=MagicMock(), body=None)) + + with pytest.raises(FabricArtifactStagingError, match="skills/review"): + await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=_fabric_config(skills_paths=["skills/review"]), + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_rejects_non_utf8_artifact() -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + root = Path(local_path) + (root / "agent.yaml").write_text("name: fabric-agent\n", encoding="utf-8") + (root / "logo.bin").write_bytes(b"\xff\xfe\x00binary") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + with pytest.raises(FabricArtifactStagingError, match="non-UTF-8"): + await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=_fabric_config(), + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_rejects_oversized_fileset() -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + root = Path(local_path) + (root / "agent.yaml").write_text("name: fabric-agent\n", encoding="utf-8") + (root / "huge.md").write_text("x" * 1_000_000, encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + with pytest.raises(FabricArtifactStagingError, match="exceeding"): + await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=_fabric_config(), + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_rejects_missing_skill_path() -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + root = Path(local_path) + (root / "agent.yaml").write_text("name: fabric-agent\n", encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + with pytest.raises(FabricArtifactStagingError, match="skills/review"): + await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=_fabric_config(skills_paths=["skills/review"]), + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) diff --git a/plugins/nemo-agents/tests/unit/test_runner_deployments.py b/plugins/nemo-agents/tests/unit/test_runner_deployments.py index f2a91c5b5f..a67c361453 100644 --- a/plugins/nemo-agents/tests/unit/test_runner_deployments.py +++ b/plugins/nemo-agents/tests/unit/test_runner_deployments.py @@ -23,7 +23,8 @@ rewrite_config_base_urls, rewrite_fabric_config_base_urls, ) -from nemo_deployments_plugin.entities import Deployment, DeploymentConfig +from nemo_agents_plugin.runner.fabric_artifact_staging import FabricArtifactStagingError +from nemo_deployments_plugin.entities import ConfigFile, Deployment, DeploymentConfig from nemo_deployments_plugin.types import Endpoint as PluginEndpoint from nemo_platform_plugin.entities.client import AsyncEntitiesClient from nemo_platform_plugin.entity_client import NemoEntityNotFoundError @@ -373,6 +374,59 @@ def test_build_deployment_config_fabric_direct_endpoint_has_no_placeholder() -> assert not any(e.name in {PLATFORM_IGW_API_KEY_ENV, "OPENAI_API_KEY"} for e in cfg.containers[0].env) +def test_build_deployment_config_fabric_docker_materializes_multiple_config_files() -> None: + staged_files = [ + ConfigFile(path="/workspace/agent.yaml", content="name: fabric-agent\n"), + ConfigFile(path="/workspace/skills/review/SKILL.md", content="# Review\n"), + ConfigFile(path="/workspace/prompts/system.md", content="You are helpful.\n"), + ] + cfg = build_deployment_config( + name="fabric-dep", + workspace="default", + image="fabric-runtime:latest", + port=8000, + agent_config=_FABRIC_AGENT_CONFIG, + platform_base_url="http://host.docker.internal:8080", + config_mount_path="/workspace/config.yaml", + mode="docker", + config_files=staged_files, + ) + container = cfg.containers[0] + assert container.command == ["sh", "-c"] + assert not any(e.name == "AGENT_CONFIG_YAML" for e in container.env) + assert any(e.name == "STAGED_CONFIG_FILES_B64_JSON" for e in container.env) + assert "python -c" in container.args[0] + assert "nemo_agents_plugin.fabric.server" in container.args[0] + assert len(cfg.config_files) == 3 + assert {item.path for item in cfg.config_files} == { + "/workspace/agent.yaml", + "/workspace/skills/review/SKILL.md", + "/workspace/prompts/system.md", + } + + +def test_build_deployment_config_fabric_k8s_mounts_multiple_config_files() -> None: + staged_files = [ + ConfigFile(path="/workspace/agent.yaml", content="name: fabric-agent\n"), + ConfigFile(path="/workspace/skills/review/SKILL.md", content="# Review\n"), + ] + cfg = build_deployment_config( + name="fabric-dep", + workspace="default", + image="fabric-runtime:latest", + port=8000, + agent_config=_FABRIC_AGENT_CONFIG, + platform_base_url="http://nmp-api:8080", + config_mount_path="/workspace/config.yaml", + mode="k8s", + config_files=staged_files, + ) + container = cfg.containers[0] + assert container.command == ["python"] + assert not any(e.name == "STAGED_CONFIG_FILES_B64_JSON" for e in container.env) + assert len(cfg.config_files) == 2 + + def _backend(**deployments_kwargs: Any) -> DeploymentsRunnerBackend: agents = AgentsConfig.model_validate({"deployments": DeploymentsRunnerConfig(**deployments_kwargs)}) return DeploymentsRunnerBackend(agents) @@ -858,3 +912,138 @@ def test_agent_deployment_defaults_are_subprocess() -> None: assert dep.endpoints == [] assert dep.image == "" assert dep.plugin_deployment == "" + + +@pytest.mark.asyncio +async def test_create_deployment_fabric_docker_stages_fileset_artifacts() -> None: + backend = _backend(default_image="fabric:latest", default_executor="local-docker") + entities = AsyncMock() + backend._entities = entities + config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "main", + "skills": {"paths": ["skills/review"]}, + "harnesses": {"main": {"kind": "codex", "settings": {}}}, + } + staged_files = [ + ConfigFile(path="/workspace/agent.yaml", content=yaml.safe_dump(config, sort_keys=False)), + ConfigFile(path="/workspace/skills/review/SKILL.md", content="# Review\n"), + ] + sdk = MagicMock() + + with ( + patch("nemo_agents_plugin.runner.deployments_backend.get_base_url", return_value="http://localhost:8080"), + patch( + "nemo_agents_plugin.runner.deployments_backend.get_async_platform_sdk", + return_value=sdk, + ), + patch( + "nemo_agents_plugin.runner.deployments_backend.stage_fabric_spec_config_files", + new_callable=AsyncMock, + return_value=staged_files, + ) as mock_stage, + ): + info = await backend.create_deployment( + workspace="default", + name="fabric-dep", + config=config, + port=0, + deployment_mode="docker", + agent="fabric-agent", + ) + + assert info.status == "starting" + mock_stage.assert_awaited_once() + created_config = entities.create.await_args_list[0].args[0] + assert len(created_config.config_files) == 2 + assert any(e.name == "STAGED_CONFIG_FILES_B64_JSON" for e in created_config.containers[0].env) + + +@pytest.mark.asyncio +async def test_create_deployment_fabric_k8s_stages_fileset_artifacts() -> None: + backend = _backend( + default_image="fabric:latest", + default_executor="k8s", + k8s_internal_base_url="http://nmp-api:8080", + ) + entities = AsyncMock() + backend._entities = entities + config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "main", + "skills": {"paths": ["skills/review"]}, + "harnesses": {"main": {"kind": "codex", "settings": {}}}, + } + staged_files = [ + ConfigFile(path="/workspace/agent.yaml", content=yaml.safe_dump(config, sort_keys=False)), + ConfigFile(path="/workspace/skills/review/SKILL.md", content="# Review\n"), + ] + sdk = MagicMock() + + with ( + patch("nemo_agents_plugin.runner.deployments_backend.get_base_url", return_value="http://localhost:8080"), + patch( + "nemo_agents_plugin.runner.deployments_backend.get_async_platform_sdk", + return_value=sdk, + ), + patch( + "nemo_agents_plugin.runner.deployments_backend.stage_fabric_spec_config_files", + new_callable=AsyncMock, + return_value=staged_files, + ), + ): + info = await backend.create_deployment( + workspace="default", + name="fabric-dep", + config=config, + port=0, + deployment_mode="k8s", + agent="fabric-agent", + ) + + assert info.status == "starting" + created_config = entities.create.await_args_list[0].args[0] + assert len(created_config.config_files) == 2 + assert created_config.containers[0].command == ["python"] + + +@pytest.mark.asyncio +async def test_create_deployment_fabric_staging_error_fails_before_entity_create() -> None: + backend = _backend(default_image="fabric:latest", default_executor="local-docker") + entities = AsyncMock() + backend._entities = entities + config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "main", + "skills": {"paths": ["skills/review"]}, + "harnesses": {"main": {"kind": "codex", "settings": {}}}, + } + sdk = MagicMock() + + with ( + patch("nemo_agents_plugin.runner.deployments_backend.get_base_url", return_value="http://localhost:8080"), + patch( + "nemo_agents_plugin.runner.deployments_backend.get_async_platform_sdk", + return_value=sdk, + ), + patch( + "nemo_agents_plugin.runner.deployments_backend.stage_fabric_spec_config_files", + new_callable=AsyncMock, + side_effect=FabricArtifactStagingError("missing skills/review"), + ), + ): + info = await backend.create_deployment( + workspace="default", + name="fabric-dep", + config=config, + port=0, + deployment_mode="docker", + agent="fabric-agent", + ) + + assert info.status == "failed" + assert "skills/review" in info.error + entities.create.assert_not_called() From 387e6c83c30155ba0a9719815696a693c2a3d0dc Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Fri, 31 Jul 2026 13:29:34 -0700 Subject: [PATCH 2/4] feat(nemo-agents): upload agent-spec fileset on Fabric CLI create Pair Fabric agent create/delete with the conventional {agent}-spec fileset so Docker/K8s staging has artifacts to download. Roll back the agent entity if upload fails; best-effort delete the fileset on agent delete. Closes the producer gap flagged on #1007. Signed-off-by: Tyler Bray --- .../nemo-agents/src/nemo_agents_plugin/cli.py | 91 ++++++++++- plugins/nemo-agents/tests/unit/test_cli.py | 148 ++++++++++++++++++ .../tests/unit/test_cli_delete_undeploy.py | 137 ++++++++++++---- 3 files changed, 343 insertions(+), 33 deletions(-) diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/cli.py b/plugins/nemo-agents/src/nemo_agents_plugin/cli.py index afbfcd9c38..815614330a 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/cli.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/cli.py @@ -64,6 +64,7 @@ CONTAINER_DEPLOYMENT_MODES, NAT_WORKFLOW_CONFIG_FORMAT, NEMO_AGENTS_SPEC_CONFIG_FORMAT, + agent_spec_fileset_name, ) from nemo_agents_plugin.leaderboard.cli import register_leaderboard_commands from nemo_agents_plugin.usage.cli import register_usage_commands @@ -784,6 +785,36 @@ def create( "config_format": config_format, } resp = _api_request("POST", base_url, f"/apis/agents/v2/workspaces/{workspace}/agents", json_body=payload) + if config_format == NEMO_AGENTS_SPEC_CONFIG_FORMAT: + try: + _upload_agent_spec_fileset( + agent_name=name, + workspace=workspace, + agent_root=agent_config.parent, + base_url=base_url, + ) + except Exception as exc: + typer.echo( + f"Error: failed to upload agent spec fileset for {name!r}: {exc}", + err=True, + ) + try: + _delete_agent_and_spec_fileset( + agent_name=name, + workspace=workspace, + base_url=base_url, + ) + except typer.Exit: + logger.error( + "Failed to roll back agent %r after fileset upload failure", + name, + ) + except Exception: + logger.exception( + "Failed to roll back agent %r after fileset upload failure", + name, + ) + raise typer.Exit(code=1) from exc typer.echo(json.dumps(resp, indent=2)) @app.command(name="list", rich_help_panel="Agent Resources (requires running cluster)") @@ -840,7 +871,7 @@ def delete( base_url = _resolve_base_url(base_url) if not yes: typer.confirm(f"Delete agent '{name}'?", abort=True) - _api_request("DELETE", base_url, f"/apis/agents/v2/workspaces/{workspace}/agents/{name}") + _delete_agent_and_spec_fileset(agent_name=name, workspace=workspace, base_url=base_url) typer.echo(f"Agent '{name}' deleted.") @app.command(rich_help_panel="Agent Resources (requires running cluster)") @@ -1535,6 +1566,64 @@ def _api_request(method: str, base_url: str, path: str, *, json_body: dict[str, raise typer.Exit(code=1) +def _platform_sdk(base_url: str) -> Any: + """Return an auth-aware platform SDK client for fileset upload/delete.""" + from nemo_platform import NeMoPlatform + + headers = _resolve_context_headers() + if headers: + return NeMoPlatform(base_url=base_url, default_headers=headers) + return NeMoPlatform(base_url=base_url) + + +def _upload_agent_spec_fileset( + *, + agent_name: str, + workspace: str, + agent_root: Path, + base_url: str, +) -> None: + """Upload *agent_root* into the conventional ``{agent}-spec`` fileset. + + *agent_root* is ``agent.yaml``'s parent directory (Fabric ``base_dir``). + Agent YAML must live in a dedicated agent root so sibling artifacts + (skills, prompts) upload without shipping an unrelated checkout tree. + """ + from nemo_agents_plugin.jobs.fileset_io import upload_to_fileset + + upload_to_fileset( + agent_root, + fileset=agent_spec_fileset_name(agent_name), + workspace=workspace, + sdk=_platform_sdk(base_url), + ) + + +def _delete_agent_spec_fileset(*, agent_name: str, workspace: str, base_url: str) -> None: + """Best-effort delete of the conventional ``{agent}-spec`` fileset.""" + from nemo_platform import NotFoundError + + fileset_name = agent_spec_fileset_name(agent_name) + try: + _platform_sdk(base_url).files.filesets.delete(name=fileset_name, workspace=workspace) + except NotFoundError: + logger.info("Agent spec fileset %s/%s already absent", workspace, fileset_name) + + +def _delete_agent_and_spec_fileset(*, agent_name: str, workspace: str, base_url: str) -> None: + """Delete the agent entity, then best-effort remove its spec fileset.""" + _api_request("DELETE", base_url, f"/apis/agents/v2/workspaces/{workspace}/agents/{agent_name}") + try: + _delete_agent_spec_fileset(agent_name=agent_name, workspace=workspace, base_url=base_url) + except Exception: + logger.warning( + "Agent %r deleted but failed to remove spec fileset %r", + agent_name, + agent_spec_fileset_name(agent_name), + exc_info=True, + ) + + def _load_yaml(path: Path) -> dict: with open(path, encoding="utf-8") as fh: return yaml.safe_load(fh) diff --git a/plugins/nemo-agents/tests/unit/test_cli.py b/plugins/nemo-agents/tests/unit/test_cli.py index 76eb40c622..2798e7795f 100644 --- a/plugins/nemo-agents/tests/unit/test_cli.py +++ b/plugins/nemo-agents/tests/unit/test_cli.py @@ -208,6 +208,7 @@ def handler(req: httpx.Request) -> httpx.Response: with ( _install_mock_transport(handler), patch("nemo_agents_plugin.fabric.validation.validate_platform_agent_config", _validate_platform_agent_config), + patch("nemo_agents_plugin.cli._upload_agent_spec_fileset") as mock_upload, ): result = CliRunner().invoke( app, @@ -220,6 +221,153 @@ def handler(req: httpx.Request) -> httpx.Response: assert captured["validated_config"]["config_format"] == "nemo-agents-spec-v1" assert sent["config"] == normalized_config assert sent["config_format"] == "nemo-agents-spec-v1" + mock_upload.assert_called_once_with( + agent_name="fabric-agent", + workspace="default", + agent_root=tmp_path, + base_url="http://test", + ) + + +def test_create_fabric_uploads_agent_spec_fileset(tmp_path) -> None: + config = tmp_path / "agent.yaml" + config.write_text( + "\n".join( + [ + "config_format: nemo-agents-spec-v1", + "name: fabric-agent", + "default_harness: hermes", + "harnesses:", + " hermes:", + " kind: hermes", + "", + ] + ) + ) + normalized_config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "hermes", + "harnesses": {"hermes": {"kind": "hermes", "settings": {}}}, + "environment": {"provider": "local"}, + } + + async def _validate_platform_agent_config(config_dict: dict[str, Any], *, base_dir: Path): + del config_dict, base_dir + return type("ValidationResult", (), {"agent_config": _ValidatedAgentConfig(normalized_config)})() + + def handler(req: httpx.Request) -> httpx.Response: + assert req.method == "POST" + return httpx.Response(200, json={"name": "fabric-agent"}) + + uploaded: dict[str, Any] = {} + + def fake_upload(local_dir: Path, *, fileset: str, workspace: str, sdk: Any) -> None: + uploaded["local_dir"] = local_dir + uploaded["fileset"] = fileset + uploaded["workspace"] = workspace + uploaded["sdk_base_url"] = sdk.base_url + + app = AgentsCLI().get_cli() + with ( + _install_mock_transport(handler), + patch("nemo_agents_plugin.fabric.validation.validate_platform_agent_config", _validate_platform_agent_config), + patch("nemo_agents_plugin.jobs.fileset_io.upload_to_fileset", fake_upload), + patch("nemo_agents_plugin.cli._platform_sdk") as mock_sdk, + ): + mock_sdk.return_value = type("SDK", (), {"base_url": "http://test"})() + result = CliRunner().invoke( + app, + ["create", "--name", "fabric-agent", "--agent-config", str(config), "--base-url", "http://test"], + ) + + assert result.exit_code == 0, result.stderr + assert uploaded["local_dir"] == tmp_path + assert uploaded["fileset"] == "fabric-agent-spec" + assert uploaded["workspace"] == "default" + + +def test_create_fabric_rolls_back_agent_when_fileset_upload_fails(tmp_path) -> None: + config = tmp_path / "agent.yaml" + config.write_text( + "\n".join( + [ + "config_format: nemo-agents-spec-v1", + "name: fabric-agent", + "default_harness: hermes", + "harnesses:", + " hermes:", + " kind: hermes", + "", + ] + ) + ) + normalized_config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "hermes", + "harnesses": {"hermes": {"kind": "hermes", "settings": {}}}, + "environment": {"provider": "local"}, + } + methods: list[str] = [] + + async def _validate_platform_agent_config(config_dict: dict[str, Any], *, base_dir: Path): + del config_dict, base_dir + return type("ValidationResult", (), {"agent_config": _ValidatedAgentConfig(normalized_config)})() + + def handler(req: httpx.Request) -> httpx.Response: + methods.append(req.method) + if req.method == "POST": + return httpx.Response(200, json={"name": "fabric-agent"}) + if req.method == "DELETE": + return httpx.Response(204) + raise AssertionError(f"unexpected {req.method}") + + app = AgentsCLI().get_cli() + with ( + _install_mock_transport(handler), + patch("nemo_agents_plugin.fabric.validation.validate_platform_agent_config", _validate_platform_agent_config), + patch( + "nemo_agents_plugin.cli._upload_agent_spec_fileset", + side_effect=RuntimeError("upload boom"), + ), + patch("nemo_agents_plugin.cli._delete_agent_spec_fileset") as mock_delete_fileset, + ): + result = CliRunner().invoke( + app, + ["create", "--name", "fabric-agent", "--agent-config", str(config), "--base-url", "http://test"], + ) + + assert result.exit_code == 1 + assert "failed to upload agent spec fileset" in result.stderr + assert methods == ["POST", "DELETE"] + mock_delete_fileset.assert_called_once_with( + agent_name="fabric-agent", + workspace="default", + base_url="http://test", + ) + + +def test_create_nat_does_not_upload_agent_spec_fileset(tmp_path) -> None: + config = tmp_path / "agent.yml" + config.write_text("llms:\n llm:\n _type: openai\n model_name: nvidia-nemotron-3-super-v3\n") + + def handler(req: httpx.Request) -> httpx.Response: + assert req.method == "POST" + return httpx.Response(200, json={"name": "calc"}) + + app = AgentsCLI().get_cli() + with ( + _install_mock_transport(handler), + patch("nemo_agents_plugin.cli._upload_agent_spec_fileset") as mock_upload, + patch("nemo_agents_plugin.utils.get_default_model", return_value="nvidia-nemotron-3-super-v3"), + ): + result = CliRunner().invoke( + app, ["create", "--name", "calc", "--agent-config", str(config), "--base-url", "http://test"] + ) + + assert result.exit_code == 0, result.stderr + mock_upload.assert_not_called() def test_create_rejects_unsupported_config_format(tmp_path) -> None: diff --git a/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py b/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py index 3bd914224f..bee2e8084f 100644 --- a/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py +++ b/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py @@ -8,12 +8,14 @@ - Abort on user decline - ``--yes`` / ``-y`` skips the prompt - ``--all`` works as an alias for ``--agent`` on ``undeploy`` +- Agent delete also best-effort removes the conventional ``{agent}-spec`` fileset """ from __future__ import annotations -from unittest.mock import patch +from unittest.mock import MagicMock, patch +import httpx import pytest from nemo_agents_plugin.cli import AgentsCLI from typer.testing import CliRunner @@ -29,6 +31,17 @@ def app(): return AgentsCLI().get_cli() +def _install_mock_transport(handler): + transport = httpx.MockTransport(handler) + real_client = httpx.Client + + def _factory(*args, **kwargs): + kwargs["transport"] = transport + return real_client(*args, **kwargs) + + return patch(f"{_PATCH_PREFIX}.httpx.Client", _factory) + + # --------------------------------------------------------------------------- # nemo agents delete # --------------------------------------------------------------------------- @@ -37,16 +50,20 @@ def app(): class TestDeleteConfirmation: def test_delete_prompts_when_no_yes_flag(self, app) -> None: """Without --yes, the user is prompted; answering 'y' proceeds.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._delete_agent_and_spec_fileset") as mock_delete: result = runner.invoke(app, ["delete", "my-agent"], input="y\n") assert result.exit_code == 0, result.output - mock_delete.assert_called_once() + mock_delete.assert_called_once_with( + agent_name="my-agent", + workspace="default", + base_url=mock_delete.call_args.kwargs["base_url"], + ) assert "deleted" in result.output.lower() def test_delete_aborts_when_user_declines(self, app) -> None: """Without --yes, answering 'n' aborts without calling the API.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._delete_agent_and_spec_fileset") as mock_delete: result = runner.invoke(app, ["delete", "my-agent"], input="n\n") assert result.exit_code != 0 @@ -55,13 +72,56 @@ def test_delete_aborts_when_user_declines(self, app) -> None: @pytest.mark.parametrize("flag", ["--yes", "-y"]) def test_delete_skips_prompt_with_yes_flag(self, app, flag: str) -> None: """--yes and -y both skip the confirmation prompt.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._delete_agent_and_spec_fileset") as mock_delete: result = runner.invoke(app, ["delete", "my-agent", flag]) assert result.exit_code == 0, result.output mock_delete.assert_called_once() assert "deleted" in result.output.lower() + def test_delete_removes_agent_then_best_effort_fileset(self, app) -> None: + methods: list[str] = [] + + def handler(req: httpx.Request) -> httpx.Response: + methods.append(req.method) + assert req.url.path.endswith("/agents/my-agent") + return httpx.Response(204) + + with ( + _install_mock_transport(handler), + patch(f"{_PATCH_PREFIX}._delete_agent_spec_fileset") as mock_fileset, + ): + result = runner.invoke(app, ["delete", "my-agent", "--yes", "--base-url", "http://test"]) + + assert result.exit_code == 0, result.output + assert methods == ["DELETE"] + mock_fileset.assert_called_once_with( + agent_name="my-agent", + workspace="default", + base_url="http://test", + ) + + def test_delete_succeeds_when_fileset_already_absent(self, app) -> None: + from nemo_platform import NotFoundError + + def handler(req: httpx.Request) -> httpx.Response: + assert req.method == "DELETE" + return httpx.Response(204) + + filesets = MagicMock() + filesets.delete.side_effect = NotFoundError("missing", response=MagicMock(), body=None) + sdk = MagicMock() + sdk.files.filesets = filesets + + with ( + _install_mock_transport(handler), + patch(f"{_PATCH_PREFIX}._platform_sdk", return_value=sdk), + ): + result = runner.invoke(app, ["delete", "my-agent", "--yes", "--base-url", "http://test"]) + + assert result.exit_code == 0, result.output + filesets.delete.assert_called_once_with(name="my-agent-spec", workspace="default") + # --------------------------------------------------------------------------- # nemo agents undeploy @@ -76,27 +136,29 @@ def _mock_deployments_response(deployments: list[dict]) -> dict: class TestUndeployConfirmation: def test_undeploy_single_prompts_confirmation(self, app) -> None: """Undeploying a single deployment prompts for confirmation.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._api_request") as mock_request: result = runner.invoke(app, ["undeploy", "dep-1"], input="y\n") assert result.exit_code == 0, result.output - mock_delete.assert_called_once() + mock_request.assert_called_once() + assert mock_request.call_args.args[0] == "DELETE" def test_undeploy_single_aborts_on_decline(self, app) -> None: """Declining the prompt does not call the API.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._api_request") as mock_request: result = runner.invoke(app, ["undeploy", "dep-1"], input="n\n") assert result.exit_code != 0 - mock_delete.assert_not_called() + mock_request.assert_not_called() def test_undeploy_single_yes_skips_prompt(self, app) -> None: """--yes skips the confirmation for single deployment undeploy.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._api_request") as mock_request: result = runner.invoke(app, ["undeploy", "dep-1", "--yes"]) assert result.exit_code == 0, result.output - mock_delete.assert_called_once() + mock_request.assert_called_once() + assert mock_request.call_args.args[0] == "DELETE" def test_undeploy_by_agent_prompts_with_count(self, app) -> None: """Undeploying by --agent lists deployments, shows count in prompt, and deletes on 'y'.""" @@ -106,14 +168,17 @@ def test_undeploy_by_agent_prompts_with_count(self, app) -> None: {"name": "dep-2", "agent": "my-agent"}, ] ) - with ( - patch(f"{_PATCH_PREFIX}._api_get", return_value=deps), - patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete, - ): + + def _request(method: str, *_args, **_kwargs): + if method == "GET": + return deps + return None + + with patch(f"{_PATCH_PREFIX}._api_request", side_effect=_request) as mock_request: result = runner.invoke(app, ["undeploy", "--agent", "my-agent"], input="y\n") assert result.exit_code == 0, result.output - assert mock_delete.call_count == 2 + assert sum(1 for call in mock_request.call_args_list if call.args[0] == "DELETE") == 2 def test_undeploy_by_agent_aborts_on_decline(self, app) -> None: """Declining the prompt after listing does not delete anything.""" @@ -123,14 +188,17 @@ def test_undeploy_by_agent_aborts_on_decline(self, app) -> None: {"name": "dep-2", "agent": "my-agent"}, ] ) - with ( - patch(f"{_PATCH_PREFIX}._api_get", return_value=deps), - patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete, - ): + + def _request(method: str, *_args, **_kwargs): + if method == "GET": + return deps + raise AssertionError("should not DELETE after decline") + + with patch(f"{_PATCH_PREFIX}._api_request", side_effect=_request) as mock_request: result = runner.invoke(app, ["undeploy", "--agent", "my-agent"], input="n\n") assert result.exit_code != 0 - mock_delete.assert_not_called() + assert all(call.args[0] != "DELETE" for call in mock_request.call_args_list) def test_undeploy_all_flag_works_as_agent_alias(self, app) -> None: """--all is an alias for --agent on undeploy.""" @@ -139,14 +207,17 @@ def test_undeploy_all_flag_works_as_agent_alias(self, app) -> None: {"name": "dep-1", "agent": "my-agent"}, ] ) - with ( - patch(f"{_PATCH_PREFIX}._api_get", return_value=deps), - patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete, - ): + + def _request(method: str, *_args, **_kwargs): + if method == "GET": + return deps + return None + + with patch(f"{_PATCH_PREFIX}._api_request", side_effect=_request) as mock_request: result = runner.invoke(app, ["undeploy", "--all", "my-agent", "--yes"]) assert result.exit_code == 0, result.output - mock_delete.assert_called_once() + assert sum(1 for call in mock_request.call_args_list if call.args[0] == "DELETE") == 1 # --------------------------------------------------------------------------- @@ -157,24 +228,26 @@ def test_undeploy_all_flag_works_as_agent_alias(self, app) -> None: class TestDeploymentsDeleteConfirmation: def test_deployments_delete_prompts_without_yes(self, app) -> None: """deployments delete prompts for confirmation.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._api_request") as mock_request: result = runner.invoke(app, ["deployments", "delete", "dep-1"], input="y\n") assert result.exit_code == 0, result.output - mock_delete.assert_called_once() + mock_request.assert_called_once() + assert mock_request.call_args.args[0] == "DELETE" def test_deployments_delete_aborts_on_decline(self, app) -> None: """Declining aborts without calling the API.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._api_request") as mock_request: result = runner.invoke(app, ["deployments", "delete", "dep-1"], input="n\n") assert result.exit_code != 0 - mock_delete.assert_not_called() + mock_request.assert_not_called() def test_deployments_delete_skips_with_yes(self, app) -> None: """--yes skips the confirmation prompt.""" - with patch(f"{_PATCH_PREFIX}._api_delete") as mock_delete: + with patch(f"{_PATCH_PREFIX}._api_request") as mock_request: result = runner.invoke(app, ["deployments", "delete", "dep-1", "--yes"]) assert result.exit_code == 0, result.output - mock_delete.assert_called_once() + mock_request.assert_called_once() + assert mock_request.call_args.args[0] == "DELETE" From 2dfec4462938164a71b3fcbe10431b37f5e48a7c Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Fri, 31 Jul 2026 13:42:30 -0700 Subject: [PATCH 3/4] fix(nemo-agents): bound agent root before spec fileset upload The spec fileset upload is recursive with no server-side filtering, so an agent.yaml sitting in a source checkout would ship the whole tree and only fail later when the ConfigMap/env payload is assembled. Check file count and total size up front against limits shared with the staging consumer. Signed-off-by: Tyler Bray --- .../nemo-agents/src/nemo_agents_plugin/cli.py | 32 ++++++++++++++ .../src/nemo_agents_plugin/entities.py | 10 +++++ .../runner/fabric_artifact_staging.py | 10 ++--- plugins/nemo-agents/tests/unit/test_cli.py | 42 ++++++++++++++++++- .../tests/unit/test_cli_delete_undeploy.py | 4 +- 5 files changed, 88 insertions(+), 10 deletions(-) diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/cli.py b/plugins/nemo-agents/src/nemo_agents_plugin/cli.py index 815614330a..5e69ae4abd 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/cli.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/cli.py @@ -62,6 +62,8 @@ ) from nemo_agents_plugin.entities import ( CONTAINER_DEPLOYMENT_MODES, + MAX_AGENT_SPEC_STAGED_BYTES, + MAX_AGENT_SPEC_STAGED_FILES, NAT_WORKFLOW_CONFIG_FORMAT, NEMO_AGENTS_SPEC_CONFIG_FORMAT, agent_spec_fileset_name, @@ -1576,6 +1578,35 @@ def _platform_sdk(base_url: str) -> Any: return NeMoPlatform(base_url=base_url) +def _check_agent_root_bounds(agent_root: Path) -> None: + """Reject an agent root too large to deliver into a container deployment. + + The upload is recursive with no server-side filtering, so an ``agent.yaml`` + sitting in a source checkout would ship the whole tree and then fail at + container start when the ConfigMap/env payload is built. Fail here instead, + naming the limit that was exceeded. + """ + total_bytes = 0 + file_count = 0 + for path in agent_root.rglob("*"): + if not path.is_file() or path.is_symlink(): + continue + file_count += 1 + total_bytes += path.stat().st_size + if file_count > MAX_AGENT_SPEC_STAGED_FILES: + raise ValueError( + f"agent directory {str(agent_root)!r} holds more than " + f"{MAX_AGENT_SPEC_STAGED_FILES} files; point --agent-config at a " + "directory containing only the agent's own artifacts" + ) + if total_bytes > MAX_AGENT_SPEC_STAGED_BYTES: + raise ValueError( + f"agent directory {str(agent_root)!r} exceeds the " + f"{MAX_AGENT_SPEC_STAGED_BYTES} byte limit for container config delivery; " + "point --agent-config at a directory containing only the agent's own artifacts" + ) + + def _upload_agent_spec_fileset( *, agent_name: str, @@ -1591,6 +1622,7 @@ def _upload_agent_spec_fileset( """ from nemo_agents_plugin.jobs.fileset_io import upload_to_fileset + _check_agent_root_bounds(agent_root) upload_to_fileset( agent_root, fileset=agent_spec_fileset_name(agent_name), diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/entities.py b/plugins/nemo-agents/src/nemo_agents_plugin/entities.py index 5203b315ba..d3eab6ccfe 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/entities.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/entities.py @@ -89,6 +89,16 @@ class Endpoint(BaseModel): NEMO_AGENTS_SPEC_CONFIG_FORMAT = "nemo-agents-spec-v1" """Canonical format tag for the Platform-owned agent.yaml spec format.""" +# Container deployments deliver the spec fileset through a ConfigMap (k8s) or a +# single env var (docker), both of which cap out around 1MiB. Bound the tree at +# both ends of the pipe so an agent root pointed at a whole checkout fails at +# upload time with a clear message instead of at container start. +MAX_AGENT_SPEC_STAGED_BYTES = 900_000 +"""Maximum total bytes an agent spec fileset may contribute to a deployment.""" + +MAX_AGENT_SPEC_STAGED_FILES = 500 +"""Maximum number of files an agent spec fileset may contain.""" + def agent_spec_fileset_name(agent_name: str) -> str: """Return the conventional fileset name holding an agent's spec.""" diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py b/plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py index 4208820ad5..5ededa0877 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/runner/fabric_artifact_staging.py @@ -14,6 +14,7 @@ from nemo_agents_plugin.entities import ( AGENT_CONFIG_FILENAME, AGENT_SPEC_FILENAME, + MAX_AGENT_SPEC_STAGED_BYTES, agent_spec_fileset_name, ) from nemo_deployments_plugin.entities import ConfigFile @@ -21,11 +22,6 @@ logger = logging.getLogger(__name__) -# Staged artifacts ride to the container inside a ConfigMap (k8s) or a single env -# var (docker); both cap out around 1MiB, so refuse oversized filesets here with a -# clear error instead of letting the container fail to start. -_MAX_STAGED_BYTES = 900_000 - class FabricArtifactStagingError(ValueError): """Raised when Fabric deployment artifact staging cannot satisfy the agent config.""" @@ -123,10 +119,10 @@ def _collect_staged_config_files( def _validate_staged_size(config_files: list[ConfigFile], fileset_name: str) -> None: total = sum(len(config_file.content.encode("utf-8")) for config_file in config_files) - if total > _MAX_STAGED_BYTES: + if total > MAX_AGENT_SPEC_STAGED_BYTES: raise FabricArtifactStagingError( f"Agent spec fileset {fileset_name!r} stages {total} bytes across {len(config_files)} files, " - f"exceeding the {_MAX_STAGED_BYTES} byte limit for container config delivery" + f"exceeding the {MAX_AGENT_SPEC_STAGED_BYTES} byte limit for container config delivery" ) diff --git a/plugins/nemo-agents/tests/unit/test_cli.py b/plugins/nemo-agents/tests/unit/test_cli.py index 2798e7795f..c988770b9c 100644 --- a/plugins/nemo-agents/tests/unit/test_cli.py +++ b/plugins/nemo-agents/tests/unit/test_cli.py @@ -12,7 +12,12 @@ import httpx import pytest -from nemo_agents_plugin.cli import AgentsCLI +from nemo_agents_plugin.cli import ( + MAX_AGENT_SPEC_STAGED_BYTES, + MAX_AGENT_SPEC_STAGED_FILES, + AgentsCLI, + _check_agent_root_bounds, +) from typer.testing import CliRunner @@ -285,6 +290,41 @@ def fake_upload(local_dir: Path, *, fileset: str, workspace: str, sdk: Any) -> N assert uploaded["local_dir"] == tmp_path assert uploaded["fileset"] == "fabric-agent-spec" assert uploaded["workspace"] == "default" + assert uploaded["sdk_base_url"] == "http://test" + + +def test_check_agent_root_bounds_allows_small_agent_root(tmp_path) -> None: + (tmp_path / "agent.yaml").write_text("name: a\n") + (tmp_path / "skills").mkdir() + (tmp_path / "skills" / "SKILL.md").write_text("# skill\n") + + _check_agent_root_bounds(tmp_path) + + +def test_check_agent_root_bounds_rejects_oversized_agent_root(tmp_path) -> None: + (tmp_path / "big.bin").write_bytes(b"x" * (MAX_AGENT_SPEC_STAGED_BYTES + 1)) + + with pytest.raises(ValueError, match="byte limit for container config delivery"): + _check_agent_root_bounds(tmp_path) + + +def test_check_agent_root_bounds_rejects_too_many_files(tmp_path) -> None: + for index in range(MAX_AGENT_SPEC_STAGED_FILES + 1): + (tmp_path / f"f{index}.txt").write_text("x") + + with pytest.raises(ValueError, match="more than"): + _check_agent_root_bounds(tmp_path) + + +def test_check_agent_root_bounds_skips_symlinks(tmp_path) -> None: + outside = tmp_path.parent / "outside.bin" + outside.write_bytes(b"x" * (MAX_AGENT_SPEC_STAGED_BYTES + 1)) + agent_root = tmp_path / "agent" + agent_root.mkdir() + (agent_root / "agent.yaml").write_text("name: a\n") + (agent_root / "link.bin").symlink_to(outside) + + _check_agent_root_bounds(agent_root) def test_create_fabric_rolls_back_agent_when_fileset_upload_fails(tmp_path) -> None: diff --git a/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py b/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py index bee2e8084f..2b87ad1854 100644 --- a/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py +++ b/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py @@ -13,7 +13,7 @@ from __future__ import annotations -from unittest.mock import MagicMock, patch +from unittest.mock import ANY, MagicMock, patch import httpx import pytest @@ -57,7 +57,7 @@ def test_delete_prompts_when_no_yes_flag(self, app) -> None: mock_delete.assert_called_once_with( agent_name="my-agent", workspace="default", - base_url=mock_delete.call_args.kwargs["base_url"], + base_url=ANY, ) assert "deleted" in result.output.lower() From a3ca948fbca54ba2a13418c9c6111937fe84fb57 Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Mon, 3 Aug 2026 10:06:23 -0700 Subject: [PATCH 4/4] fix(nemo-agents): preserve spec fileset on delete, reject symlinked artifacts Agent delete removed the whole {agent}-spec fileset, but that fileset is the canonical home of AGENT-SPEC.md: nemo-spec writes it before the agent exists and nemo-build-agent reads it on every rebuild. Delete the agent entity only, in both the delete command and the create-failure rollback, and surface a rollback failure to the user instead of only logging it. Reject symlinks in the agent root rather than skipping them. fsspec enumerates symlinks during upload, so a skipped one both staged content from outside the agent root and evaded the file-count and byte limits. Signed-off-by: Tyler Bray --- .../nemo-agents/src/nemo_agents_plugin/cli.py | 58 +++++++------- plugins/nemo-agents/tests/unit/test_cli.py | 77 +++++++++++++++++-- .../tests/unit/test_cli_delete_undeploy.py | 42 +++------- 3 files changed, 108 insertions(+), 69 deletions(-) diff --git a/plugins/nemo-agents/src/nemo_agents_plugin/cli.py b/plugins/nemo-agents/src/nemo_agents_plugin/cli.py index 5e69ae4abd..dc5e9c6c31 100644 --- a/plugins/nemo-agents/src/nemo_agents_plugin/cli.py +++ b/plugins/nemo-agents/src/nemo_agents_plugin/cli.py @@ -801,21 +801,23 @@ def create( err=True, ) try: - _delete_agent_and_spec_fileset( + _delete_agent_entity( agent_name=name, workspace=workspace, base_url=base_url, ) - except typer.Exit: - logger.error( - "Failed to roll back agent %r after fileset upload failure", - name, - ) + # ``typer.Exit`` subclasses ``Exception``, so this also covers the + # exit raised by ``_api_request`` on an HTTP error. except Exception: logger.exception( "Failed to roll back agent %r after fileset upload failure", name, ) + typer.echo( + f"Error: failed to roll back agent {name!r}; it may still exist on the " + f"platform. Remove it with `nemo agents delete {name}`.", + err=True, + ) raise typer.Exit(code=1) from exc typer.echo(json.dumps(resp, indent=2)) @@ -873,7 +875,7 @@ def delete( base_url = _resolve_base_url(base_url) if not yes: typer.confirm(f"Delete agent '{name}'?", abort=True) - _delete_agent_and_spec_fileset(agent_name=name, workspace=workspace, base_url=base_url) + _delete_agent_entity(agent_name=name, workspace=workspace, base_url=base_url) typer.echo(f"Agent '{name}' deleted.") @app.command(rich_help_panel="Agent Resources (requires running cluster)") @@ -1585,11 +1587,22 @@ def _check_agent_root_bounds(agent_root: Path) -> None: sitting in a source checkout would ship the whole tree and then fail at container start when the ConfigMap/env payload is built. Fail here instead, naming the limit that was exceeded. + + Symlinks are rejected rather than skipped: the upload enumerates them and + ships their target content, so skipping one here would both stage files from + outside *agent_root* and let them evade the limits below. """ total_bytes = 0 file_count = 0 for path in agent_root.rglob("*"): - if not path.is_file() or path.is_symlink(): + if path.is_symlink(): + raise ValueError( + f"agent directory {str(agent_root)!r} contains symlink " + f"{path.relative_to(agent_root).as_posix()!r}; the fileset upload follows " + "symlinks, which would stage content from outside the agent directory. " + "Replace it with a regular file or move the target inside the agent directory" + ) + if not path.is_file(): continue file_count += 1 total_bytes += path.stat().st_size @@ -1631,29 +1644,16 @@ def _upload_agent_spec_fileset( ) -def _delete_agent_spec_fileset(*, agent_name: str, workspace: str, base_url: str) -> None: - """Best-effort delete of the conventional ``{agent}-spec`` fileset.""" - from nemo_platform import NotFoundError +def _delete_agent_entity(*, agent_name: str, workspace: str, base_url: str) -> None: + """Delete the agent entity, leaving the ``{agent}-spec`` fileset in place. - fileset_name = agent_spec_fileset_name(agent_name) - try: - _platform_sdk(base_url).files.filesets.delete(name=fileset_name, workspace=workspace) - except NotFoundError: - logger.info("Agent spec fileset %s/%s already absent", workspace, fileset_name) - - -def _delete_agent_and_spec_fileset(*, agent_name: str, workspace: str, base_url: str) -> None: - """Delete the agent entity, then best-effort remove its spec fileset.""" + The fileset outlives the agent on purpose: it is the canonical home of + ``AGENT-SPEC.md`` (see ``agent_spec_file_ref``), which ``nemo-spec`` writes + before the agent exists and ``nemo-build-agent`` reads on every rebuild. + Deleting the fileset here would destroy that durable contract, so the + executable artifacts it also carries are left behind instead. + """ _api_request("DELETE", base_url, f"/apis/agents/v2/workspaces/{workspace}/agents/{agent_name}") - try: - _delete_agent_spec_fileset(agent_name=agent_name, workspace=workspace, base_url=base_url) - except Exception: - logger.warning( - "Agent %r deleted but failed to remove spec fileset %r", - agent_name, - agent_spec_fileset_name(agent_name), - exc_info=True, - ) def _load_yaml(path: Path) -> dict: diff --git a/plugins/nemo-agents/tests/unit/test_cli.py b/plugins/nemo-agents/tests/unit/test_cli.py index c988770b9c..b810d35a35 100644 --- a/plugins/nemo-agents/tests/unit/test_cli.py +++ b/plugins/nemo-agents/tests/unit/test_cli.py @@ -316,7 +316,7 @@ def test_check_agent_root_bounds_rejects_too_many_files(tmp_path) -> None: _check_agent_root_bounds(tmp_path) -def test_check_agent_root_bounds_skips_symlinks(tmp_path) -> None: +def test_check_agent_root_bounds_rejects_file_symlink(tmp_path) -> None: outside = tmp_path.parent / "outside.bin" outside.write_bytes(b"x" * (MAX_AGENT_SPEC_STAGED_BYTES + 1)) agent_root = tmp_path / "agent" @@ -324,7 +324,21 @@ def test_check_agent_root_bounds_skips_symlinks(tmp_path) -> None: (agent_root / "agent.yaml").write_text("name: a\n") (agent_root / "link.bin").symlink_to(outside) - _check_agent_root_bounds(agent_root) + with pytest.raises(ValueError, match="contains symlink 'link.bin'"): + _check_agent_root_bounds(agent_root) + + +def test_check_agent_root_bounds_rejects_directory_symlink(tmp_path) -> None: + outside = tmp_path.parent / "outside" + outside.mkdir() + (outside / "a.txt").write_text("x") + agent_root = tmp_path / "agent" + agent_root.mkdir() + (agent_root / "agent.yaml").write_text("name: a\n") + (agent_root / "linkdir").symlink_to(outside, target_is_directory=True) + + with pytest.raises(ValueError, match="contains symlink 'linkdir'"): + _check_agent_root_bounds(agent_root) def test_create_fabric_rolls_back_agent_when_fileset_upload_fails(tmp_path) -> None: @@ -371,7 +385,7 @@ def handler(req: httpx.Request) -> httpx.Response: "nemo_agents_plugin.cli._upload_agent_spec_fileset", side_effect=RuntimeError("upload boom"), ), - patch("nemo_agents_plugin.cli._delete_agent_spec_fileset") as mock_delete_fileset, + patch("nemo_agents_plugin.cli._platform_sdk") as mock_sdk, ): result = CliRunner().invoke( app, @@ -380,12 +394,61 @@ def handler(req: httpx.Request) -> httpx.Response: assert result.exit_code == 1 assert "failed to upload agent spec fileset" in result.stderr + # Rollback removes the agent entity only; the spec fileset is durable and may + # already hold an AGENT-SPEC.md written before this agent existed. assert methods == ["POST", "DELETE"] - mock_delete_fileset.assert_called_once_with( - agent_name="fabric-agent", - workspace="default", - base_url="http://test", + mock_sdk.assert_not_called() + + +def test_create_fabric_reports_rollback_failure(tmp_path) -> None: + config = tmp_path / "agent.yaml" + config.write_text( + "\n".join( + [ + "config_format: nemo-agents-spec-v1", + "name: fabric-agent", + "default_harness: hermes", + "harnesses:", + " hermes:", + " kind: hermes", + "", + ] + ) ) + normalized_config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "hermes", + "harnesses": {"hermes": {"kind": "hermes", "settings": {}}}, + "environment": {"provider": "local"}, + } + + async def _validate_platform_agent_config(config_dict: dict[str, Any], *, base_dir: Path): + del config_dict, base_dir + return type("ValidationResult", (), {"agent_config": _ValidatedAgentConfig(normalized_config)})() + + def handler(req: httpx.Request) -> httpx.Response: + if req.method == "POST": + return httpx.Response(200, json={"name": "fabric-agent"}) + return httpx.Response(500, json={"detail": "delete exploded"}) + + app = AgentsCLI().get_cli() + with ( + _install_mock_transport(handler), + patch("nemo_agents_plugin.fabric.validation.validate_platform_agent_config", _validate_platform_agent_config), + patch( + "nemo_agents_plugin.cli._upload_agent_spec_fileset", + side_effect=RuntimeError("upload boom"), + ), + ): + result = CliRunner().invoke( + app, + ["create", "--name", "fabric-agent", "--agent-config", str(config), "--base-url", "http://test"], + ) + + assert result.exit_code == 1 + assert "failed to roll back agent 'fabric-agent'" in result.stderr + assert "nemo agents delete fabric-agent" in result.stderr def test_create_nat_does_not_upload_agent_spec_fileset(tmp_path) -> None: diff --git a/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py b/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py index 2b87ad1854..df7c01c49f 100644 --- a/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py +++ b/plugins/nemo-agents/tests/unit/test_cli_delete_undeploy.py @@ -8,12 +8,12 @@ - Abort on user decline - ``--yes`` / ``-y`` skips the prompt - ``--all`` works as an alias for ``--agent`` on ``undeploy`` -- Agent delete also best-effort removes the conventional ``{agent}-spec`` fileset +- Agent delete leaves the durable ``{agent}-spec`` fileset in place """ from __future__ import annotations -from unittest.mock import ANY, MagicMock, patch +from unittest.mock import ANY, patch import httpx import pytest @@ -50,7 +50,7 @@ def _factory(*args, **kwargs): class TestDeleteConfirmation: def test_delete_prompts_when_no_yes_flag(self, app) -> None: """Without --yes, the user is prompted; answering 'y' proceeds.""" - with patch(f"{_PATCH_PREFIX}._delete_agent_and_spec_fileset") as mock_delete: + with patch(f"{_PATCH_PREFIX}._delete_agent_entity") as mock_delete: result = runner.invoke(app, ["delete", "my-agent"], input="y\n") assert result.exit_code == 0, result.output @@ -63,7 +63,7 @@ def test_delete_prompts_when_no_yes_flag(self, app) -> None: def test_delete_aborts_when_user_declines(self, app) -> None: """Without --yes, answering 'n' aborts without calling the API.""" - with patch(f"{_PATCH_PREFIX}._delete_agent_and_spec_fileset") as mock_delete: + with patch(f"{_PATCH_PREFIX}._delete_agent_entity") as mock_delete: result = runner.invoke(app, ["delete", "my-agent"], input="n\n") assert result.exit_code != 0 @@ -72,14 +72,15 @@ def test_delete_aborts_when_user_declines(self, app) -> None: @pytest.mark.parametrize("flag", ["--yes", "-y"]) def test_delete_skips_prompt_with_yes_flag(self, app, flag: str) -> None: """--yes and -y both skip the confirmation prompt.""" - with patch(f"{_PATCH_PREFIX}._delete_agent_and_spec_fileset") as mock_delete: + with patch(f"{_PATCH_PREFIX}._delete_agent_entity") as mock_delete: result = runner.invoke(app, ["delete", "my-agent", flag]) assert result.exit_code == 0, result.output mock_delete.assert_called_once() assert "deleted" in result.output.lower() - def test_delete_removes_agent_then_best_effort_fileset(self, app) -> None: + def test_delete_removes_agent_but_preserves_spec_fileset(self, app) -> None: + """The ``{agent}-spec`` fileset is durable: it holds ``AGENT-SPEC.md``.""" methods: list[str] = [] def handler(req: httpx.Request) -> httpx.Response: @@ -89,38 +90,13 @@ def handler(req: httpx.Request) -> httpx.Response: with ( _install_mock_transport(handler), - patch(f"{_PATCH_PREFIX}._delete_agent_spec_fileset") as mock_fileset, + patch(f"{_PATCH_PREFIX}._platform_sdk") as mock_sdk, ): result = runner.invoke(app, ["delete", "my-agent", "--yes", "--base-url", "http://test"]) assert result.exit_code == 0, result.output assert methods == ["DELETE"] - mock_fileset.assert_called_once_with( - agent_name="my-agent", - workspace="default", - base_url="http://test", - ) - - def test_delete_succeeds_when_fileset_already_absent(self, app) -> None: - from nemo_platform import NotFoundError - - def handler(req: httpx.Request) -> httpx.Response: - assert req.method == "DELETE" - return httpx.Response(204) - - filesets = MagicMock() - filesets.delete.side_effect = NotFoundError("missing", response=MagicMock(), body=None) - sdk = MagicMock() - sdk.files.filesets = filesets - - with ( - _install_mock_transport(handler), - patch(f"{_PATCH_PREFIX}._platform_sdk", return_value=sdk), - ): - result = runner.invoke(app, ["delete", "my-agent", "--yes", "--base-url", "http://test"]) - - assert result.exit_code == 0, result.output - filesets.delete.assert_called_once_with(name="my-agent-spec", workspace="default") + mock_sdk.assert_not_called() # ---------------------------------------------------------------------------