diff --git a/.github/workflows/ci_python.yml b/.github/workflows/ci_python.yml index 694aa4e2..49d893ad 100644 --- a/.github/workflows/ci_python.yml +++ b/.github/workflows/ci_python.yml @@ -76,10 +76,11 @@ jobs: toolchain: stable cache: false - - name: Install just + # Remove nemo-relay once we are using 0.7 (and thus the Python package for nemo-relay-cli) + - name: Install test tools uses: taiki-e/install-action@c070f87102a1c75b3183910f391c1cb887fe13c8 # v2.77.6 with: - tool: just@1.50.0 + tool: just@1.50.0,nemo-relay-cli@0.6.0 - name: Set up uv uses: astral-sh/setup-uv@cec208311dfd045dd5311c1add060b2062131d57 # v8 diff --git a/adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py b/adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py index 75222fc8..6567cb3a 100644 --- a/adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py +++ b/adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py @@ -72,11 +72,15 @@ def render_relay_hooks( if agent not in ("claude", "codex"): raise ValueError(f"unsupported NeMo Relay hook agent {agent!r}") - executable_arg = ( - subprocess.list2cmdline([str(executable)]) - if platform == "win32" - else shlex.quote(str(executable)) - ) + if platform == "win32": + executable_path = str(executable).replace("\\", "/") + executable_arg = ( + f'"{executable_path}"' + if agent == "claude" + else subprocess.list2cmdline([executable_path]) + ) + else: + executable_arg = shlex.quote(str(executable)) command = f"{executable_arg} hook-forward {agent}" hooks: dict[str, list[dict[str, Any]]] = {} for event in RELAY_HOOK_EVENTS[agent]: diff --git a/tests/_utils/utils.py b/tests/_utils/utils.py index 6b3c10ec..9b7ca63b 100644 --- a/tests/_utils/utils.py +++ b/tests/_utils/utils.py @@ -6,6 +6,63 @@ from pathlib import Path from typing import Any + +def atof_records(output: Mapping[str, Any]) -> list[dict[str, Any]]: + atof_path = next( + Path(artifact["path"]) + for artifact in output["relay_artifacts"] + if artifact["kind"] == "atof" + ) + return [ + json.loads(line) for line in atof_path.read_text(encoding="utf-8").splitlines() + ] + + +def assert_atof_skill_selection( + output: Mapping[str, Any], expected_skill: str | None +) -> None: + records = atof_records(output) + tool_records = [record for record in records if record["category"] == "tool"] + llm_end_records = [ + record + for record in records + if record["category"] == "llm" and record["scope_category"] == "end" + ] + serialized_tool_records = json.dumps(tool_records).replace("\\\\", "/") + serialized_skill_calls = json.dumps(tool_records + llm_end_records).replace( + "\\\\", "/" + ) + loaded_skills = { + skill + for skill in ("default", "alternate") + if f"{skill} skill loaded" in serialized_tool_records + or f"/{skill}/SKILL.md" in serialized_skill_calls + } + claude_skill_ends = [ + record + for record in tool_records + if record["name"] == "Skill" and record["scope_category"] == "end" + ] + assert all(record["data"]["success"] for record in claude_skill_ends) + loaded_skills.update(record["data"]["commandName"] for record in claude_skill_ends) + + expected_skills = {expected_skill} if expected_skill is not None else set() + assert loaded_skills == expected_skills + + +def assert_atof_model(output: Mapping[str, Any], expected_model: str) -> None: + events = atof_records(output) + llm_starts = [ + record + for record in events + if record["category"] == "llm" and record["scope_category"] == "start" + ] + assert llm_starts, events + assert {record["data"]["content"]["model"] for record in llm_starts} == { + expected_model + } + + def _relay_event_total_tokens(event: dict) -> int: profile = event.get("category_profile") or {} annotated = profile.get("annotated_response") or {} @@ -19,13 +76,8 @@ def assert_semantic_relay_artifacts( ) -> None: """Assert Relay artifacts contain model, usage, and agent-response semantics.""" - artifacts = { - item["kind"]: Path(item["path"]) for item in output["relay_artifacts"] - } - events = [ - json.loads(line) - for line in artifacts["atof"].read_text(encoding="utf-8").splitlines() - ] + artifacts = {item["kind"]: Path(item["path"]) for item in output["relay_artifacts"]} + events = atof_records(output) llm_starts = [ event for event in events @@ -33,8 +85,7 @@ def assert_semantic_relay_artifacts( ] assert llm_starts, events assert all( - isinstance(event.get("data", {}).get("content"), dict) - for event in llm_starts + isinstance(event.get("data", {}).get("content"), dict) for event in llm_starts ) assert all(event["data"]["content"].get("model") for event in llm_starts) @@ -57,6 +108,8 @@ def assert_semantic_relay_artifacts( assert any( expected_response.lower() in message.lower() for message in agent_messages ), agent_messages + + def assert_relay_disabled_native_observability(result: dict): """Assert telemetry-off runs still surface native harness evidence.""" diff --git a/tests/adapters/test_adapters_common_relay_hooks.py b/tests/adapters/test_adapters_common_relay_hooks.py index b8e5d4fa..8a39a7bc 100644 --- a/tests/adapters/test_adapters_common_relay_hooks.py +++ b/tests/adapters/test_adapters_common_relay_hooks.py @@ -49,7 +49,10 @@ ) def test_render_relay_hooks_matches_relay_agent_contract(agent, expected_events): executable = Path("/opt/nvidia relay/bin/nemo-relay") - quoted_executable = f'"{executable}"' if platform == "win32" else f"'{executable}'" + executable_path = str(executable).replace("\\", "/") + quoted_executable = ( + f'"{executable_path}"' if platform == "win32" else f"'{executable}'" + ) hooks = relay_hooks.render_relay_hooks(agent, executable)["hooks"] @@ -83,11 +86,20 @@ def test_render_relay_hooks_rejects_unsupported_agent(): ) -def test_render_relay_hooks_uses_windows_command_quoting(monkeypatch): - executable = Path(r"C:\Program Files\NVIDIA\nemo-relay.exe") +@pytest.mark.parametrize( + ("agent", "expected_executable"), + [ + ("claude", '"C:/Users/runneradmin/.cargo/bin/nemo-relay.exe"'), + ("codex", "C:/Users/runneradmin/.cargo/bin/nemo-relay.exe"), + ], +) +def test_render_relay_hooks_uses_windows_agent_command_format( + monkeypatch, agent, expected_executable +): + executable = Path(r"C:\Users\runneradmin\.cargo\bin\nemo-relay.exe") monkeypatch.setattr(relay_hooks, "platform", "win32") - hooks = relay_hooks.render_relay_hooks("claude", executable)["hooks"] + hooks = relay_hooks.render_relay_hooks(agent, executable)["hooks"] command = hooks["SessionStart"][0]["hooks"][0]["command"] - assert command == f'"{executable}" hook-forward claude' + assert command == f"{expected_executable} hook-forward {agent}" diff --git a/tests/adapters/test_claude_adapter.py b/tests/adapters/test_claude_adapter.py index 4ecbdc8d..9ab7ca86 100644 --- a/tests/adapters/test_claude_adapter.py +++ b/tests/adapters/test_claude_adapter.py @@ -6,6 +6,7 @@ import asyncio import json import os +import sys import tomllib from collections.abc import AsyncIterator from collections.abc import Callable @@ -393,11 +394,15 @@ def test_prepare_claude_relay_writes_gateway_config_and_complete_hook_plugin( "PostCompact", "SessionEnd", } + executable_arg = str(executable) + if sys.platform == "win32": + executable_arg = executable_arg.replace("\\", "/") + executable_arg = f'"{executable_arg}"' assert hooks["SessionStart"][0] == { "hooks": [ { "type": "command", - "command": f"{executable} hook-forward claude", + "command": f"{executable_arg} hook-forward claude", "timeout": 30, } ] diff --git a/tests/adapters/test_codex_adapter.py b/tests/adapters/test_codex_adapter.py index d47df4ae..798191ce 100644 --- a/tests/adapters/test_codex_adapter.py +++ b/tests/adapters/test_codex_adapter.py @@ -4,6 +4,8 @@ import asyncio import json import os +import subprocess +import sys from pathlib import Path from types import SimpleNamespace from typing import Any @@ -880,9 +882,12 @@ def test_relay_uses_gateway_and_request_scoped_sdk_config( assert config["features"]["web_search"] is False assert config["openai_base_url"] == gateway.url assert "model_providers" not in config + executable_arg = str(executable) + if sys.platform == "win32": + executable_arg = subprocess.list2cmdline([executable_arg.replace("\\", "/")]) assert config["hooks"]["SessionStart"][0]["hooks"][0] == { "type": "command", - "command": f"{executable} hook-forward codex", + "command": f"{executable_arg} hook-forward codex", "timeout": 30, } assert output["relay_runtime"] == { diff --git a/tests/conftest.py b/tests/conftest.py index f68f47af..67bf22e4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -51,16 +51,29 @@ def restore_environ_fixture(): if key not in orig_vars: del os.environ[key] + @pytest.fixture(name="repo_root", scope="session") def repo_root_fixture() -> Path: return CUR_DIR.parent.resolve() + +@pytest.fixture(name="default_skill") +def default_skill_fixture() -> Path: + return CUR_DIR / "fixtures" / "default" + + +@pytest.fixture(name="alternate_skill") +def alternate_skill_fixture() -> Path: + return CUR_DIR / "fixtures" / "alternate" + + @pytest.fixture(name="hermes_shim_agent_dir_src", scope="session") def hermes_shim_agent_dir_src_fixture() -> Path: agent_dir = CUR_DIR / "fixtures" / "hermes-shim-agent" assert agent_dir.exists(), f"Missing Hermes shim agent directory: {agent_dir}" return agent_dir + def _copy_agent_dir(src_dir: Path, tmp_path: Path, agent_name: str) -> Path: """ Creates a temporary copy of the specified agent directory for testing. @@ -72,6 +85,7 @@ def _copy_agent_dir(src_dir: Path, tmp_path: Path, agent_name: str) -> Path: assert agent_dir.exists(), f"Missing {agent_name} directory: {agent_dir}" return agent_dir.resolve() + @pytest.fixture(name="hermes_shim_agent_dir") def hermes_shim_agent_dir_fixture( hermes_shim_agent_dir_src: Path, @@ -80,6 +94,7 @@ def hermes_shim_agent_dir_fixture( """Creates a temporary copy of the Hermes shim agent directory.""" return _copy_agent_dir(hermes_shim_agent_dir_src, tmp_path, "hermes-shim-agent") + @pytest.fixture(name="code_review_agent_dir") def code_review_agent_dir_fixture(repo_root: Path, tmp_path: Path) -> Path: """ @@ -95,13 +110,16 @@ def code_review_agent_dir_fixture(repo_root: Path, tmp_path: Path) -> Path: @pytest.fixture(name="api_server") def api_server_fixture(unused_tcp_port: int) -> Iterator[str]: from _utils.mock_api_server import mock_api_server + with mock_api_server(unused_tcp_port) as base_url: yield base_url + @pytest.fixture(name="nemo_relay") def nemo_relay_fixture() -> types.ModuleType: return pytest.importorskip("nemo_relay", reason="nemo-relay extra is required") + @pytest.fixture(name="mock_nvidia_api_key") def mock_nvidia_api_key_fixture() -> str: nak = "test123" diff --git a/tests/e2e/test_claude.py b/tests/e2e/test_claude.py index 8ed32539..5443182c 100644 --- a/tests/e2e/test_claude.py +++ b/tests/e2e/test_claude.py @@ -13,7 +13,11 @@ import pytest import requests -from _utils.utils import assert_semantic_relay_artifacts +from _utils.utils import ( + assert_atof_model, + assert_atof_skill_selection, + assert_semantic_relay_artifacts, +) from nemo_fabric import ( EnvironmentConfig, Fabric, @@ -243,6 +247,64 @@ async def test_mcp_stdio_transport(api_server, tmp_path, enabled): assert "America/Los_Angeles" in str(tool_results[0]["content"]) +@pytest.mark.usefixtures("nemo_relay") +@pytest.mark.parametrize("skill", ["default", "alternate", None]) +async def test_skill_selection( + api_server, tmp_path, skill, default_skill, alternate_skill +): + config = fabric_config(tmp_path, relay=True) + config.models["default"].provider = "fabric-test" + config.models["default"].model = "fabric-echo" + config.models["default"].api_key_env = "FABRIC_TEST_API_KEY" + config.models["default"].base_url = f"{api_server}/v1" + config.environment.env["FABRIC_TEST_API_KEY"] = "test" + config.add_skill_path(default_skill) + + if skill == "alternate" or skill is None: + config.remove_skill_path(default_skill) + + if skill == "alternate": + config.add_skill_path(alternate_skill) + + if skill is not None: + scenario_response = requests.post( + f"{api_server}/_scenario", + json={ + "tool_call": { + "name": "Skill", + "arguments": {"skill": skill}, + } + }, + timeout=5, + ) + scenario_response.raise_for_status() + + result = await Fabric().run( + config, + base_dir=tmp_path, + input=f"Use the {skill} skill." if skill else "Reply without using a skill.", + ) + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_skill_selection(result["output"], skill) + + +@pytest.mark.usefixtures("nemo_relay") +@pytest.mark.parametrize("model", ["m1", "m2"]) +async def test_model_selection(api_server, tmp_path, model): + config = fabric_config(tmp_path, relay=True) + config.models["default"].provider = "fabric-test" + config.models["default"].model = model + config.models["default"].api_key_env = "FABRIC_TEST_API_KEY" + config.models["default"].base_url = f"{api_server}/v1" + config.environment.env["FABRIC_TEST_API_KEY"] = "test" + + result = await Fabric().run(config, base_dir=tmp_path, input="Reply with hello.") + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_model(result["output"], model) + + @pytest.mark.skipif( sys.platform in {"darwin", "win32"}, reason="the mock Relay gateway is not supported on macOS or Windows", diff --git a/tests/e2e/test_codex.py b/tests/e2e/test_codex.py index 3d1809c4..fc308646 100644 --- a/tests/e2e/test_codex.py +++ b/tests/e2e/test_codex.py @@ -17,7 +17,101 @@ import pytest import requests -from _utils.utils import assert_semantic_relay_artifacts +from _utils.utils import ( + assert_atof_model, + assert_atof_skill_selection, + assert_semantic_relay_artifacts, +) + + +def _mock_codex_config(api_server, tmp_path): + from examples.code_review_agent import codex_config, with_relay + + config = with_relay(codex_config()) + config.models["default"].provider = "fabric-test" + config.models["default"].api_key_env = "FABRIC_TEST_API_KEY" + config.models["default"].base_url = f"{api_server}/v1" + config.environment.workspace = tmp_path + config.environment.artifacts = tmp_path / "artifacts" + config.environment.env["FABRIC_TEST_API_KEY"] = "test" + config.runtime.artifacts = tmp_path / "artifacts" + return config + + +def _skill_tool_call(selected_skill, workdir): + if sys.platform == "win32": + return { + "name": "shell_command", + "arguments": { + "command": f'Get-Content -Raw "{selected_skill / "SKILL.md"}"', + "workdir": str(workdir), + }, + } + return { + "name": "exec_command", + "arguments": { + "cmd": f"cat {selected_skill / 'SKILL.md'}", + "workdir": str(workdir), + }, + } + + +def test_skill_tool_call_uses_classic_shell_on_windows(monkeypatch, tmp_path): + monkeypatch.setattr(sys, "platform", "win32") + + tool_call = _skill_tool_call(tmp_path / "default", tmp_path) + + assert tool_call["name"] == "shell_command" + assert set(tool_call["arguments"]) == {"command", "workdir"} + + +@pytest.mark.usefixtures("nemo_relay") +@pytest.mark.parametrize("skill", ["default", "alternate", None]) +async def test_skill_selection( + api_server, tmp_path, skill, default_skill, alternate_skill +): + from nemo_fabric import Fabric + + config = _mock_codex_config(api_server, tmp_path) + config.models["default"].model = "fabric-echo" + config.add_skill_path(default_skill) + + if skill == "alternate" or skill is None: + config.remove_skill_path(default_skill) + if skill == "alternate": + config.add_skill_path(alternate_skill) + + if skill is not None: + selected_skill = default_skill if skill == "default" else alternate_skill + scenario_response = requests.post( + f"{api_server}/_scenario", + json={"tool_call": _skill_tool_call(selected_skill, tmp_path)}, + timeout=5, + ) + scenario_response.raise_for_status() + + result = await Fabric().run( + config, + base_dir=tmp_path, + input=f"Use the {skill} skill." if skill else "Reply without using a skill.", + ) + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_skill_selection(result["output"], skill) + + +@pytest.mark.usefixtures("nemo_relay") +@pytest.mark.parametrize("model", ["m1", "m2"]) +async def test_model_selection(api_server, tmp_path, model): + from nemo_fabric import Fabric + + config = _mock_codex_config(api_server, tmp_path) + config.models["default"].model = model + + result = await Fabric().run(config, base_dir=tmp_path, input="Reply with hello.") + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_model(result["output"], model) @pytest.mark.parametrize("enabled", [True, False]) diff --git a/tests/e2e/test_deepagents.py b/tests/e2e/test_deepagents.py index cb5e7531..a29258de 100644 --- a/tests/e2e/test_deepagents.py +++ b/tests/e2e/test_deepagents.py @@ -17,6 +17,7 @@ import pytest import requests +from _utils.utils import assert_atof_model, assert_atof_skill_selection @pytest.mark.usefixtures("mock_nvidia_api_key") @@ -174,6 +175,88 @@ async def test_mcp_stdio_transport(api_server, tmp_path, enabled): assert "America/Los_Angeles" in str(tool_results[0]["content"]) +@pytest.mark.usefixtures("mock_nvidia_api_key", "nemo_relay") +@pytest.mark.parametrize("skill", ["default", "alternate", None]) +async def test_skill_selection( + api_server, tmp_path, skill, default_skill, alternate_skill +): + pytest.importorskip("deepagents") + from examples.code_review_agent import deepagents_config, with_relay + from nemo_fabric import EnvironmentConfig, Fabric, RuntimeConfig + + config = with_relay(deepagents_config()) + config.models["default"].model = "fabric-echo" + config.models["default"].base_url = f"{api_server}/v1" + config.environment = EnvironmentConfig( + provider="local", + workspace=default_skill.parents[2], + artifacts=tmp_path / "artifacts", + ) + config.runtime = RuntimeConfig( + input_schema="chat", + output_schema="message", + artifacts=tmp_path / "artifacts", + ) + config.add_skill_path(default_skill) + + if skill == "alternate" or skill is None: + config.remove_skill_path(default_skill) + + if skill == "alternate": + config.add_skill_path(alternate_skill) + + if skill is not None: + selected_skill = default_skill if skill == "default" else alternate_skill + skill_file = selected_skill.relative_to(default_skill.parents[2]) / "SKILL.md" + scenario_response = requests.post( + f"{api_server}/_scenario", + json={ + "tool_call": { + "name": "read_file", + "arguments": {"file_path": f"/{skill_file}"}, + } + }, + timeout=5, + ) + scenario_response.raise_for_status() + + result = await Fabric().run( + config, + base_dir=tmp_path, + input=f"Use the {skill} skill." if skill else "Reply without using a skill.", + ) + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_skill_selection(result["output"], skill) + + +@pytest.mark.usefixtures("mock_nvidia_api_key", "nemo_relay") +@pytest.mark.parametrize("model", ["m1", "m2"]) +async def test_model_selection(api_server, tmp_path, model): + pytest.importorskip("deepagents") + from examples.code_review_agent import deepagents_config, with_relay + from nemo_fabric import EnvironmentConfig, Fabric, RuntimeConfig + + config = with_relay(deepagents_config()) + config.models["default"].model = model + config.models["default"].base_url = f"{api_server}/v1" + config.environment = EnvironmentConfig( + provider="local", + workspace=tmp_path, + artifacts=tmp_path / "artifacts", + ) + config.runtime = RuntimeConfig( + input_schema="chat", + output_schema="message", + artifacts=tmp_path / "artifacts", + ) + + result = await Fabric().run(config, base_dir=tmp_path, input="Reply with hello.") + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_model(result["output"], model) + + @pytest.fixture(name="_require_integration") def _require_integration_fixture() -> None: if os.environ.get("RUN_FABRIC_DEEPAGENTS_INTEGRATION") != "1": diff --git a/tests/e2e/test_hermes_e2e.py b/tests/e2e/test_hermes_e2e.py index 6a630bde..54c3a25e 100644 --- a/tests/e2e/test_hermes_e2e.py +++ b/tests/e2e/test_hermes_e2e.py @@ -15,6 +15,7 @@ import pytest import requests import yaml +from _utils.utils import assert_atof_model, assert_atof_skill_selection from examples.code_review_agent import ( hermes_config, @@ -201,6 +202,73 @@ async def test_mcp_stdio_transport( assert "America/Los_Angeles" in tool_end["data"] +@pytest.mark.usefixtures("mock_nvidia_api_key", "nemo_relay") +@pytest.mark.parametrize("skill", ["default", "alternate", None]) +async def test_skill_selection( + code_review_agent_dir: Path, + api_server: str, + skill: str | None, + default_skill: Path, + alternate_skill: Path, +): + os.environ["ADAPTER_PYTHON"] = sys.executable + config = with_relay(hermes_config()) + config.models["default"].model = "fabric-echo" + config.models["default"].base_url = f"{api_server}/v1" + config.tools.enabled = None + config.add_skill_path(default_skill) + + if skill == "alternate" or skill is None: + config.remove_skill_path(default_skill) + + if skill == "alternate": + config.add_skill_path(alternate_skill) + + if skill is not None: + scenario_response = requests.post( + f"{api_server}/_scenario", + json={ + "tool_call": { + "name": "skill_view", + "arguments": {"name": skill}, + } + }, + timeout=5, + ) + scenario_response.raise_for_status() + + result = await Fabric().run( + config, + base_dir=code_review_agent_dir, + input=f"Use the {skill} skill." if skill else "Reply without using a skill.", + ) + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_skill_selection(result["output"], skill) + + +@pytest.mark.usefixtures("mock_nvidia_api_key", "nemo_relay") +@pytest.mark.parametrize("model", ["m1", "m2"]) +async def test_model_selection( + code_review_agent_dir: Path, + api_server: str, + model: str, +): + os.environ["ADAPTER_PYTHON"] = sys.executable + config = with_relay(hermes_config()) + config.models["default"].model = model + config.models["default"].base_url = f"{api_server}/v1" + + result = await Fabric().run( + config, + base_dir=code_review_agent_dir, + input="Reply with hello.", + ) + + assert result["status"] == "succeeded", result.to_mapping() + assert_atof_model(result["output"], model) + + class TestHermesE2E: """End-to-end Hermes relay assertions.""" diff --git a/tests/fixtures/alternate/SKILL.md b/tests/fixtures/alternate/SKILL.md new file mode 100644 index 00000000..3ff02400 --- /dev/null +++ b/tests/fixtures/alternate/SKILL.md @@ -0,0 +1,9 @@ +--- +name: alternate +description: Use this skill when asked to load the alternate test skill. +license: Apache-2.0 +--- + +# Alternate test skill + +Reply with `alternate skill loaded`. diff --git a/tests/fixtures/default/SKILL.md b/tests/fixtures/default/SKILL.md new file mode 100644 index 00000000..f28d50d9 --- /dev/null +++ b/tests/fixtures/default/SKILL.md @@ -0,0 +1,9 @@ +--- +name: default +description: Use this skill when asked to load the default test skill. +license: Apache-2.0 +--- + +# Default test skill + +Reply with `default skill loaded`.