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 b59b7a1e86..59ed0e885b 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 @@ -1,12 +1,16 @@ # 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.""" +"""Stage Fabric agent-spec fileset artifacts for container and subprocess deployments.""" from __future__ import annotations +import asyncio import logging +import posixpath +import shutil import tempfile +from collections.abc import Collection from pathlib import Path, PurePosixPath from typing import Any, Protocol @@ -38,6 +42,136 @@ async def download( ) -> None: ... +async def stage_fabric_spec_dir( + *, + workspace: str, + agent_name: str, + agent_config: dict[str, Any], + base_dir: Path, + sdk: _FilesDownloader | None, +) -> None: + """Download the agent spec fileset into *base_dir* for subprocess deployments. + + Mirrors :func:`stage_fabric_spec_config_files` for a runtime that reads the + agent root off local disk rather than through a ConfigMap. An unavailable + fileset is not fatal — config-only agents deploy from ``agent.yaml`` alone — + but a config referencing skills still needs them staged. + + ``AGENT-SPEC.md`` is dropped after download, matching the container path, + so both runtimes see the same tree. + + The container byte cap does not apply here: it bounds ConfigMap and env + delivery, neither of which is in this path. What lands on the platform host + is bounded by ``_check_agent_root_bounds`` at CLI upload time only — a + fileset written straight through the files API is unbounded here. That is + the intended trust boundary for local subprocess mode, not an oversight. + + *base_dir* is reused when the controller restarts a subprocess deployment, + so previously staged files are cleared first. Otherwise a file dropped from + the fileset would survive locally, and stale skills could satisfy validation + for a deployment that staged nothing. + """ + preserved = _runtime_dir_names(agent_config) + await asyncio.to_thread(_clear_staged_tree, base_dir, preserved) + + if agent_name and sdk is not None: + fileset_name = agent_spec_fileset_name(agent_name) + try: + await sdk.download(local_path=str(base_dir), fileset=fileset_name, workspace=workspace) + except (FileNotFoundError, PlatformNotFoundError, PluginClientNotFoundError) as exc: + logger.info( + "Agent spec fileset %s/%s unavailable (%s); using inline agent.yaml only", + workspace, + fileset_name, + exc, + ) + + staged = await asyncio.to_thread(_collect_staged_dir, base_dir, preserved) + validate_referenced_skill_paths(agent_config, staged) + + +def _runtime_dir_names(agent_config: dict[str, Any]) -> set[str]: + """Return top-level names under the agent root that the runtime owns, not staging. + + ``environment.workspace`` and ``environment.artifacts`` hold a live agent's + output (Relay telemetry among it) and resolve relative to ``agent.yaml``, so + restaging must not delete them. + """ + environment = agent_config.get("environment") + if not isinstance(environment, dict): + return set() + + names: set[str] = set() + for key in ("workspace", "artifacts"): + value = environment.get(key) + if not isinstance(value, str) or not value: + continue + rel = PurePosixPath(posixpath.normpath(value)) + if rel.is_absolute(): + continue + # normpath resolves "foo/../workspace" to "workspace"; a leading ".." escapes base_dir. + if rel.parts and rel.parts[0] != "..": + names.add(rel.parts[0]) + return names + + +def _clear_staged_tree(base_dir: Path, preserved: set[str]) -> None: + """Remove previously staged content, leaving *preserved* runtime directories. + + Deletion failures are fatal: a surviving skill tree would otherwise satisfy + validation for a deployment that staged nothing. + """ + if not base_dir.is_dir(): + return + for child in base_dir.iterdir(): + if child.name in preserved: + continue + try: + if child.is_dir() and not child.is_symlink(): + shutil.rmtree(child) + else: + child.unlink(missing_ok=True) + except OSError as exc: + raise FabricArtifactStagingError( + f"Failed to remove stale agent spec artifact {child.name!r} from {base_dir}: {exc}" + ) from exc + + +def _collect_staged_dir(base_dir: Path, preserved: set[str]) -> set[PurePosixPath]: + """Return staged files relative to *base_dir*, rejecting anything that escapes it. + + ``_collect_staged_config_files`` refuses ``..`` in a staged path; this path + writes to the platform host's filesystem rather than an inert ConfigMap, so + it keeps the same guard. Symlinks are the reachable form here: the download + lands real files, but a link inside the tree would resolve outside it. + ``AGENT-SPEC.md`` is removed for parity with the container path. + + *preserved* runtime directories are skipped: a running agent owns their + contents, symlinks included, and staging neither writes nor validates them. + """ + resolved_base = base_dir.resolve() + staged: set[PurePosixPath] = set() + for path in base_dir.rglob("*"): + rel = path.relative_to(base_dir) + if rel.parts[0] in preserved: + continue + if path.is_symlink() or not path.resolve().is_relative_to(resolved_base): + raise FabricArtifactStagingError( + f"Staged agent spec path {rel.as_posix()!r} escapes the agent base directory {base_dir}" + ) + if not path.is_file(): + continue + if path.name == AGENT_SPEC_FILENAME: + path.unlink() + continue + staged.add(PurePosixPath(rel.as_posix())) + return staged + + +def _staged_relative_paths(base_dir: Path) -> set[PurePosixPath]: + return {PurePosixPath(path.relative_to(base_dir).as_posix()) for path in base_dir.rglob("*") if path.is_file()} + + async def stage_fabric_spec_config_files( *, workspace: str, @@ -67,7 +201,10 @@ async def stage_fabric_spec_config_files( agent_yaml_path=agent_yaml_path, rewritten_agent_yaml=config_yaml, ) - _validate_referenced_skill_paths(rewritten_agent_config, config_files, agent_yaml_path) + # Validate against what the fileset delivered, not config_files, which + # carries the generated agent.yaml whether or not the fileset supplied one. + delivered = {path for path in _staged_relative_paths(tmp_path) if path.name != AGENT_SPEC_FILENAME} + validate_referenced_skill_paths(rewritten_agent_config, delivered) _validate_staged_size(config_files, fileset_name) return config_files except (FileNotFoundError, PlatformNotFoundError, PluginClientNotFoundError) as exc: @@ -77,11 +214,11 @@ async def stage_fabric_spec_config_files( 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 + # fileset must fail here rather than at container start. Nothing was + # staged from the fileset, so no skills.paths entry can resolve. + validate_referenced_skill_paths(rewritten_agent_config, set()) + return [ConfigFile(path=agent_yaml_path, content=config_yaml)] def _collect_staged_config_files( @@ -127,11 +264,16 @@ def _validate_staged_size(config_files: list[ConfigFile], fileset_name: str) -> ) -def _validate_referenced_skill_paths( +def validate_referenced_skill_paths( agent_config: dict[str, Any], - config_files: list[ConfigFile], - agent_yaml_path: str, + staged_paths: Collection[PurePosixPath], ) -> None: + """Ensure every ``skills.paths`` entry resolves to staged content. + + *staged_paths* holds the staged files relative to the agent base directory, + so container ``config_files`` and an on-disk base directory validate against + one definition of "the skill is present". + """ skills = agent_config.get("skills") if not isinstance(skills, dict): return @@ -139,9 +281,6 @@ def _validate_referenced_skill_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 @@ -150,9 +289,12 @@ def _validate_referenced_skill_paths( 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) + # "." normalizes to zero parts — the agent root itself, which packaging accepts. + rel_parts = tuple(part for part in rel.parts if part != ".") + if rel_parts: + matched = any(staged.parts[: len(rel_parts)] == rel_parts for staged in staged_paths) + else: + matched = bool(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 cb14f429de..6e8966727e 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 @@ -40,6 +40,8 @@ from nemo_agents_plugin.entities import AGENT_CONFIG_FILENAME, NEMO_AGENTS_SPEC_CONFIG_FORMAT, DeploymentMode from nemo_agents_plugin.fabric.gateway_credentials import platform_gateway_credential_env from nemo_agents_plugin.runner.backend import DeploymentInfo, LocalLog, LogLocation, NotYetAvailable, RunnerBackend +from nemo_agents_plugin.runner.fabric_artifact_staging import stage_fabric_spec_dir +from nemo_platform_plugin.sdk_provider import get_async_platform_sdk # Match characters not safe for filesystem paths. Deployment names are # normally URL-safe identifiers, but we sanitise defensively to ensure we @@ -228,9 +230,9 @@ 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 agent, image, deployment_mode, created_by + del 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) + return await self._create_fabric_deployment(workspace, name, config, port, agent=agent) key = (workspace, name) config_path = await asyncio.to_thread(self._write_config, workspace, name, config) @@ -268,12 +270,15 @@ async def _create_fabric_deployment( name: str, config: dict[str, Any], port: int, + *, + agent: str = "", ) -> DeploymentInfo: """Validate and start a Platform-owned Fabric-backed agent server.""" key = (workspace, name) base_dir = self._fabric_base_dir_for(workspace, name) await asyncio.to_thread(base_dir.mkdir, parents=True, exist_ok=True) try: + await self._stage_agent_spec(workspace, agent, config, base_dir) config_path = await asyncio.to_thread(self._write_fabric_config, base_dir, config) await validate_platform_agent_config(config, base_dir=base_dir) log_path = self.log_path_for(workspace, name) @@ -406,6 +411,23 @@ def _write_fabric_config(self, base_dir: Path, config: dict[str, Any]) -> Path: """Write a Platform-owned agent config into its deployment directory.""" return _write_yaml_config(base_dir / AGENT_CONFIG_FILENAME, config) + async def _stage_agent_spec( + self, + workspace: str, + agent: str, + config: dict[str, Any], + base_dir: Path, + ) -> None: + """Deliver the agent's spec fileset (skills, MCP servers, prompts) into *base_dir*.""" + sdk = get_async_platform_sdk(as_service="agents", internal=True) if agent else None + await stage_fabric_spec_dir( + workspace=workspace, + agent_name=agent, + agent_config=config, + base_dir=base_dir, + sdk=sdk.files if sdk else None, + ) + def _spawn( self, name: str, diff --git a/plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py b/plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py index eeb86b4c89..6aa70766e1 100644 --- a/plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py +++ b/plugins/nemo-agents/tests/unit/test_fabric_artifact_staging.py @@ -5,9 +5,9 @@ from __future__ import annotations -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch import httpx import pytest @@ -15,6 +15,8 @@ from nemo_agents_plugin.runner.fabric_artifact_staging import ( FabricArtifactStagingError, stage_fabric_spec_config_files, + stage_fabric_spec_dir, + validate_referenced_skill_paths, ) from nemo_deployments_plugin.entities import ConfigFile from nemo_platform import NotFoundError @@ -229,3 +231,364 @@ async def _fake_download(*, local_path: str, fileset: str | None, workspace: str agent_yaml_path="/workspace/agent.yaml", sdk=sdk, ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_stages_sibling_artifacts(tmp_path: Path) -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + root = Path(local_path) + (root / "mcps").mkdir() + (root / "mcps" / "calculator.py").write_text("print(1)\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") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(skills_paths=["skills/review"]), + base_dir=tmp_path, + sdk=sdk, + ) + + assert (tmp_path / "mcps" / "calculator.py").read_text(encoding="utf-8") == "print(1)\n" + assert (tmp_path / "skills" / "review" / "SKILL.md").exists() + await_args = sdk.download.await_args + assert await_args is not None + assert await_args.kwargs["fileset"] == "fabric-agent-spec" + assert await_args.kwargs["local_path"] == str(tmp_path) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_missing_fileset_is_not_fatal(tmp_path: Path) -> None: + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=NotFoundError("missing fileset", response=MagicMock(), body=None)) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(), + base_dir=tmp_path, + sdk=sdk, + ) + + assert list(tmp_path.iterdir()) == [] + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_missing_fileset_rejects_configured_skills(tmp_path: Path) -> None: + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=FileNotFoundError("missing")) + + with pytest.raises(FabricArtifactStagingError, match="skills/review"): + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(skills_paths=["skills/review"]), + base_dir=tmp_path, + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_without_downloader_skips_download(tmp_path: Path) -> None: + await stage_fabric_spec_dir( + workspace="default", + agent_name="", + agent_config=_fabric_config(), + base_dir=tmp_path, + sdk=None, + ) + + assert list(tmp_path.iterdir()) == [] + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_allows_oversized_tree(tmp_path: Path) -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + (Path(local_path) / "huge.md").write_text("x" * 1_000_000, encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(), + base_dir=tmp_path, + sdk=sdk, + ) + + assert (tmp_path / "huge.md").exists() + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_removes_files_dropped_from_fileset(tmp_path: Path) -> None: + (tmp_path / "skills").mkdir() + (tmp_path / "skills" / "STALE.md").write_text("removed upstream\n", encoding="utf-8") + (tmp_path / "agent.yaml").write_text("stale: true\n", encoding="utf-8") + + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + (Path(local_path) / "mcps").mkdir() + (Path(local_path) / "mcps" / "calculator.py").write_text("print(1)\n", encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(), + base_dir=tmp_path, + sdk=sdk, + ) + + assert not (tmp_path / "skills").exists() + assert not (tmp_path / "agent.yaml").exists() + assert (tmp_path / "mcps" / "calculator.py").exists() + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_missing_fileset_does_not_accept_stale_skills(tmp_path: Path) -> None: + skill_dir = tmp_path / "skills" / "review" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# Stale\n", encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=FileNotFoundError("missing")) + + with pytest.raises(FabricArtifactStagingError, match="skills/review"): + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(skills_paths=["skills/review"]), + base_dir=tmp_path, + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_preserves_runtime_directories(tmp_path: Path) -> None: + (tmp_path / "artifacts").mkdir() + (tmp_path / "artifacts" / "events.atof.jsonl").write_text("{}\n", encoding="utf-8") + (tmp_path / "workspace").mkdir() + (tmp_path / "workspace" / "scratch.txt").write_text("keep\n", encoding="utf-8") + (tmp_path / "STALE.md").write_text("drop\n", encoding="utf-8") + + config = _fabric_config() + config["environment"] = {"workspace": "./workspace", "artifacts": "./artifacts"} + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=FileNotFoundError("missing")) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=config, + base_dir=tmp_path, + sdk=sdk, + ) + + assert (tmp_path / "artifacts" / "events.atof.jsonl").exists() + assert (tmp_path / "workspace" / "scratch.txt").exists() + assert not (tmp_path / "STALE.md").exists() + + +def test_validate_referenced_skill_paths_accepts_agent_root() -> None: + validate_referenced_skill_paths(_fabric_config(skills_paths=["."]), {PurePosixPath("SKILL.md")}) + + +def test_validate_referenced_skill_paths_rejects_agent_root_when_nothing_staged() -> None: + with pytest.raises(FabricArtifactStagingError, match=r"\."): + validate_referenced_skill_paths(_fabric_config(skills_paths=["."]), set()) + + +def test_validate_referenced_skill_paths_does_not_match_sibling_prefix() -> None: + with pytest.raises(FabricArtifactStagingError, match="skills/review"): + validate_referenced_skill_paths( + _fabric_config(skills_paths=["skills/review"]), + {PurePosixPath("skills/review-notes/SKILL.md")}, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_preserves_runtime_dir_through_parent_traversal(tmp_path: Path) -> None: + (tmp_path / "workspace").mkdir() + (tmp_path / "workspace" / "scratch.txt").write_text("keep\n", encoding="utf-8") + (tmp_path / "foo").mkdir() + (tmp_path / "foo" / "stale.txt").write_text("drop\n", encoding="utf-8") + + config = _fabric_config() + config["environment"] = {"workspace": "foo/../workspace", "artifacts": "../outside"} + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=FileNotFoundError("missing")) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=config, + base_dir=tmp_path, + sdk=sdk, + ) + + assert (tmp_path / "workspace" / "scratch.txt").exists() + assert not (tmp_path / "foo").exists() + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_fails_when_stale_tree_cannot_be_removed(tmp_path: Path) -> None: + (tmp_path / "skills").mkdir() + (tmp_path / "skills" / "SKILL.md").write_text("# Stale\n", encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock() + + with patch( + "nemo_agents_plugin.runner.fabric_artifact_staging.shutil.rmtree", + side_effect=PermissionError("read-only"), + ): + with pytest.raises(FabricArtifactStagingError, match="stale agent spec artifact"): + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(), + base_dir=tmp_path, + sdk=sdk, + ) + + sdk.download.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_missing_fileset_rejects_agent_root_skill() -> None: + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=NotFoundError("missing fileset", response=MagicMock(), body=None)) + + with pytest.raises(FabricArtifactStagingError, match=r"skills\.paths entry|was not found"): + await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=_fabric_config(skills_paths=["."]), + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_accepts_agent_root_skill_from_fileset() -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + (Path(local_path) / "SKILL.md").write_text("# Root skill\n", encoding="utf-8") + + 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=_fabric_config(skills_paths=["."]), + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + assert "/workspace/SKILL.md" in {item.path for item in result} + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_config_files_agent_spec_md_alone_does_not_satisfy_root_skill() -> None: + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + (Path(local_path) / "AGENT-SPEC.md").write_text("# Spec\n", encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + with pytest.raises(FabricArtifactStagingError, match="was not found"): + await stage_fabric_spec_config_files( + workspace="default", + agent_name="fabric-agent", + rewritten_agent_config=_fabric_config(skills_paths=["."]), + agent_yaml_path="/workspace/agent.yaml", + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_drops_agent_spec_markdown(tmp_path: 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-SPEC.md").write_text("# Spec\n", encoding="utf-8") + (root / "mcps").mkdir() + (root / "mcps" / "calculator.py").write_text("print(1)\n", encoding="utf-8") + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(), + base_dir=tmp_path, + sdk=sdk, + ) + + assert not (tmp_path / "AGENT-SPEC.md").exists() + assert (tmp_path / "mcps" / "calculator.py").exists() + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_rejects_symlink_escaping_base_dir(tmp_path: Path) -> None: + outside = tmp_path / "outside" + outside.mkdir() + (outside / "secret.txt").write_text("nope\n", encoding="utf-8") + base_dir = tmp_path / "base" + base_dir.mkdir() + + async def _fake_download(*, local_path: str, fileset: str | None, workspace: str | None) -> None: + del fileset, workspace + (Path(local_path) / "escape").symlink_to(outside) + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=_fake_download) + + with pytest.raises(FabricArtifactStagingError, match="escapes the agent base directory"): + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=_fabric_config(), + base_dir=base_dir, + sdk=sdk, + ) + + +@pytest.mark.asyncio +async def test_stage_fabric_spec_dir_allows_symlinks_inside_runtime_directories(tmp_path: Path) -> None: + outside = tmp_path / "outside" + outside.mkdir() + base_dir = tmp_path / "base" + (base_dir / "artifacts").mkdir(parents=True) + (base_dir / "artifacts" / "link").symlink_to(outside) + + config = _fabric_config() + config["environment"] = {"workspace": "./workspace", "artifacts": "./artifacts"} + + sdk = AsyncMock() + sdk.download = AsyncMock(side_effect=FileNotFoundError("missing")) + + await stage_fabric_spec_dir( + workspace="default", + agent_name="fabric-agent", + agent_config=config, + base_dir=base_dir, + sdk=sdk, + ) + + assert (base_dir / "artifacts" / "link").is_symlink() diff --git a/plugins/nemo-agents/tests/unit/test_runner_in_memory.py b/plugins/nemo-agents/tests/unit/test_runner_in_memory.py index 92dff03557..b607dd10ca 100644 --- a/plugins/nemo-agents/tests/unit/test_runner_in_memory.py +++ b/plugins/nemo-agents/tests/unit/test_runner_in_memory.py @@ -26,12 +26,13 @@ from pathlib import Path from types import SimpleNamespace from typing import Any -from unittest.mock import ANY, patch +from unittest.mock import ANY, MagicMock, patch import pytest import yaml from nemo_agents_plugin.config import AgentsConfig, ControllerConfig from nemo_agents_plugin.runner.backend import DeploymentInfo +from nemo_agents_plugin.runner.fabric_artifact_staging import FabricArtifactStagingError from nemo_agents_plugin.runner.in_memory import InMemoryRunnerBackend, _resolve_nat_bin from nemo_platform_plugin.config import Configuration, nmp_user_data_dir @@ -529,3 +530,137 @@ def test_resolve_nat_bin_falls_back_to_container_path(monkeypatch: pytest.Monkey monkeypatch.setattr(sys, "executable", str(fake_python)) assert _resolve_nat_bin() == "/app/.venv/bin/nat" + + +@pytest.mark.asyncio +async def test_create_deployment_stages_agent_spec_fileset_into_base_dir(tmp_path: Path) -> None: + backend = _backend(tmp_path) + config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "hermes", + "harnesses": {"hermes": {"kind": "hermes"}}, + "models": {"default": {"provider": "openai", "model": "openai/gpt-5.4"}}, + } + staged: list[dict[str, Any]] = [] + + async def _stage_fabric_spec_dir( + *, + workspace: str, + agent_name: str, + agent_config: dict[str, Any], + base_dir: Path, + sdk: Any, + ) -> None: + del sdk + staged.append({"workspace": workspace, "agent_name": agent_name, "base_dir": base_dir}) + assert not (base_dir / "agent.yaml").exists() + (base_dir / "mcps").mkdir() + (base_dir / "mcps" / "calculator.py").write_text("print(1)\n") + assert agent_config == config + + async def _validate_platform_agent_config(config_: dict[str, Any], *, base_dir: Path) -> Any: + del config_, base_dir + return SimpleNamespace(agent_config=SimpleNamespace(name="fabric-agent")) + + fake_process = SimpleNamespace(pid=4343, returncode=None, poll=lambda: None) + + def _spawn_fabric(self_, name, config_path, log_path, port, credential_env=None): # noqa: ANN001 + del self_, name, config_path, port, credential_env + log_path.parent.mkdir(parents=True, exist_ok=True) + log_path.write_text("") + return fake_process + + with ( + patch("nemo_agents_plugin.runner.in_memory.validate_platform_agent_config", _validate_platform_agent_config), + patch("nemo_agents_plugin.runner.in_memory.stage_fabric_spec_dir", _stage_fabric_spec_dir), + patch("nemo_agents_plugin.runner.in_memory.get_async_platform_sdk", MagicMock()), + patch.object(InMemoryRunnerBackend, "_spawn_fabric", _spawn_fabric), + ): + info = await backend.create_deployment("ws", "fabric-dep", config, port=49212, agent="fabric-agent") + + base_dir = Path(info.extra["base_dir"]) + assert staged == [{"workspace": "ws", "agent_name": "fabric-agent", "base_dir": base_dir}] + assert (base_dir / "mcps" / "calculator.py").exists() + assert yaml.safe_load((base_dir / "agent.yaml").read_text()) == config + + +@pytest.mark.asyncio +async def test_create_deployment_cleans_base_dir_when_staging_fails(tmp_path: Path) -> None: + backend = _backend(tmp_path) + config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "hermes", + "harnesses": {"hermes": {"kind": "hermes"}}, + "models": {"default": {"provider": "openai", "model": "openai/gpt-5.4"}}, + } + base_dir = tmp_path / "system" / "ws" / "fabric-dep-fabric" + + async def _stage_fabric_spec_dir(**kwargs: Any) -> None: + del kwargs + raise FabricArtifactStagingError("skills/review missing") + + with ( + patch("nemo_agents_plugin.runner.in_memory.stage_fabric_spec_dir", _stage_fabric_spec_dir), + patch("nemo_agents_plugin.runner.in_memory.get_async_platform_sdk", MagicMock()), + ): + with pytest.raises(FabricArtifactStagingError, match="skills/review"): + await backend.create_deployment("ws", "fabric-dep", config, port=0, agent="fabric-agent") + + assert not base_dir.exists() + assert await backend.get_deployment_status("ws", "fabric-dep") is None + + +@pytest.mark.asyncio +async def test_redeploy_after_crash_does_not_merge_previous_fileset(tmp_path: Path) -> None: + backend = _backend(tmp_path) + config = { + "config_format": "nemo-agents-spec-v1", + "name": "fabric-agent", + "default_harness": "hermes", + "harnesses": {"hermes": {"kind": "hermes"}}, + "models": {"default": {"provider": "openai", "model": "openai/gpt-5.4"}}, + } + + def _sdk_serving(skill_name: str) -> MagicMock: + async def _download(*, local_path: str, fileset: str | None = None, workspace: str | None = None) -> None: + del fileset, workspace + skills = Path(local_path) / "skills" + skills.mkdir(parents=True, exist_ok=True) + (skills / skill_name).write_text("# skill\n") + + sdk = MagicMock() + sdk.files = SimpleNamespace(download=_download) + return sdk + + async def _validate(config_: dict[str, Any], *, base_dir: Path) -> Any: + del config_, base_dir + return SimpleNamespace(agent_config=SimpleNamespace(name="fabric-agent")) + + fake_process = SimpleNamespace(pid=5150, returncode=None, poll=lambda: None) + + def _spawn_fabric(self_, name, config_path, log_path, port, credential_env=None): # noqa: ANN001 + del self_, name, config_path, port, credential_env + log_path.parent.mkdir(parents=True, exist_ok=True) + log_path.write_text("") + return fake_process + + base_dirs: list[Path] = [] + for skill_name in ("old.md", "new.md"): + with ( + patch("nemo_agents_plugin.runner.in_memory.validate_platform_agent_config", _validate), + patch( + "nemo_agents_plugin.runner.in_memory.get_async_platform_sdk", + return_value=_sdk_serving(skill_name), + ), + patch.object(InMemoryRunnerBackend, "_spawn_fabric", _spawn_fabric), + ): + info = await backend.create_deployment("ws", "dep", config, port=49300, agent="fabric-agent") + base_dirs.append(Path(info.extra["base_dir"])) + # Crash case: in-memory state disappears without delete_deployment's rmtree. + backend._processes.pop(("ws", "dep"), None) + backend._deployments.pop(("ws", "dep"), None) + + assert base_dirs[0] == base_dirs[1] + assert sorted(path.name for path in (base_dirs[1] / "skills").iterdir()) == ["new.md"]