diff --git a/.github/ci-path-filters.yml b/.github/ci-path-filters.yml index 812b9af1b..d0efde810 100644 --- a/.github/ci-path-filters.yml +++ b/.github/ci-path-filters.yml @@ -59,7 +59,6 @@ rust_package: - 'crates/ffi/nemo_relay.h' - 'crates/ffi/src/**' - 'justfile' - - 'packages/cli-bin/**' - 'python/cli-bin/**' - 'rust-toolchain.toml' - 'scripts/package-cli-bin.py' @@ -82,10 +81,8 @@ node_package: - 'justfile' - 'package.json' - 'package-lock.json' - - 'packages/cli-bin/**' - 'pyproject.toml' - 'rust-toolchain.toml' - - 'scripts/package-cli-bin.py' - 'scripts/package-node-bin.py' - 'uv.lock' @@ -132,7 +129,6 @@ dependencies: - 'integrations/**/package.json' - 'package.json' - 'package-lock.json' - - 'packages/cli-bin/**' - 'pyproject.toml' - 'python/cli-bin/**' - 'scripts/licensing/**' diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 6bd1e232b..c45f35cb5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -310,12 +310,10 @@ jobs: cli_binaries=(nemo-relay-cli-*) cli_wheels=(nemo_relay_cli_bin-*.whl) python_api_wheels=(nemo_relay-*.whl) - cli_npm=(nemo-relay-bin-npm-*.tgz) node_npm=(nemo-relay-node-npm-*.tgz) expect_count 7 "CLI binary" "${cli_binaries[@]}" expect_count 7 "CLI wheel" "${cli_wheels[@]}" expect_count 7 "Python API wheel" "${python_api_wheels[@]}" - expect_count 8 "CLI npm" "${cli_npm[@]}" expect_count 8 "Node npm" "${node_npm[@]}" mapfile -t distribution_assets < <( @@ -523,13 +521,6 @@ jobs: name: openclaw-npm path: openclaw-package/ - - name: Download CLI npm artifacts - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 - with: - pattern: cli-npm-package-* - merge-multiple: true - path: cli-packages/ - - name: Select npm dist-tag run: | set -e @@ -558,25 +549,6 @@ jobs: npm publish "./$metapackage" --access public --tag "${NEMO_RELAY_NPM_DIST_TAG}" fi - - name: Publish CLI packages to npm - run: | - set -euo pipefail - version="${{ github.ref_name }}" - for pkg in cli-packages/nemo-relay-bin-npm-{linux-x64,linux-arm64,linux-x64-musl,linux-arm64-musl,darwin-arm64,win32-x64,win32-arm64}-${version}.tgz; do - name="$(tar xOf "$pkg" package/package.json | node -p 'JSON.parse(require("fs").readFileSync(0, "utf8")).name')" - if npm view "${name}@${version}" version --registry https://registry.npmjs.org >/dev/null 2>&1; then - echo "${name} ${version} already exists on npm; skipping" - continue - fi - npm publish "./$pkg" --access public --tag "${NEMO_RELAY_NPM_DIST_TAG}" - done - launcher="cli-packages/nemo-relay-bin-npm-${version}.tgz" - if npm view "nemo-relay-cli-bin@${version}" version --registry https://registry.npmjs.org >/dev/null 2>&1; then - echo "nemo-relay-cli-bin ${version} already exists on npm; skipping" - else - npm publish "./$launcher" --access public --tag "${NEMO_RELAY_NPM_DIST_TAG}" - fi - - name: Publish OpenClaw plugin package to npm run: | set -euo pipefail diff --git a/.github/workflows/ci_rust.yml b/.github/workflows/ci_rust.yml index c76ee9d30..5cdd1c361 100644 --- a/.github/workflows/ci_rust.yml +++ b/.github/workflows/ci_rust.yml @@ -254,6 +254,28 @@ jobs: cache-bin: false save-if: false + - uses: astral-sh/setup-uv@cec208311dfd045dd5311c1add060b2062131d57 # v8 + with: + version: ${{ steps.ci-config.outputs.uv_version }} + + - uses: taiki-e/install-action@c070f87102a1c75b3183910f391c1cb887fe13c8 # v2.77.6 + with: + tool: just@${{ steps.ci-config.outputs.just_version }} + + - name: Set CLI binary version + working-directory: ${{ env.NEMO_RELAY_CI_WORKSPACE }} + env: + REF_NAME: ${{ github.ref_name }} + REF_TYPE: ${{ github.ref_type }} + run: | + set -euo pipefail + version="$REF_NAME" + if [[ "$REF_TYPE" != "tag" ]]; then + version="$(sed -n 's/^version = "\(.*\)"$/\1/p' Cargo.toml | head -n1)+${GIT_COMMIT::8}" + fi + just set-cargo-version "$version" + printf 'NEMO_RELAY_CLI_PACKAGE_VERSION=%s\n' "$version" >> "$GITHUB_ENV" + - name: Install musl tools if: ${{ startsWith(matrix.target, 'x86_64-unknown-linux-musl') || startsWith(matrix.target, 'aarch64-unknown-linux-musl') }} run: | @@ -300,8 +322,17 @@ jobs: docker run --rm \ --volume "${{ env.NEMO_RELAY_CI_WORKSPACE }}:${{ env.NEMO_RELAY_CI_WORKSPACE }}" \ --workdir "${{ env.NEMO_RELAY_CI_WORKSPACE }}" \ + --env NEMO_RELAY_CI_WORKSPACE \ + --env NEMO_RELAY_CLI_PACKAGE_VERSION \ "${{ matrix.runtime_image }}" \ - "${{ env.NEMO_RELAY_CI_WORKSPACE }}/target/${{ matrix.target }}/release/nemo-relay" --version + /bin/sh -ec ' + actual="$("$NEMO_RELAY_CI_WORKSPACE/target/${{ matrix.target }}/release/nemo-relay" --version)" + expected="nemo-relay $NEMO_RELAY_CLI_PACKAGE_VERSION" + if [ "$actual" != "$expected" ]; then + echo "Error: expected CLI version \"$expected\", got \"$actual\"" >&2 + exit 1 + fi + ' - name: Stage CLI binary artifact working-directory: ${{ env.NEMO_RELAY_CI_WORKSPACE }} @@ -327,7 +358,7 @@ jobs: mkdir -p "${NEMO_RELAY_CI_WORKSPACE_TMP}/cli" cp "$source" "${NEMO_RELAY_CI_WORKSPACE_TMP}/cli/${asset}" - - name: Package CLI binary for PyPI and npm + - name: Package CLI binary for PyPI working-directory: ${{ env.NEMO_RELAY_CI_WORKSPACE }} run: | set -euo pipefail @@ -337,22 +368,14 @@ jobs: binary="${binary}.exe" fi source="${NEMO_RELAY_CI_WORKSPACE}/target/${target}/release/${binary}" - version="${{ github.ref_name }}" - if [ "${{ github.ref_type }}" != "tag" ]; then - version="$(sed -n 's/^version = "\(.*\)"$/\1/p' Cargo.toml | head -n1)+${GIT_COMMIT::8}" - fi + version="$NEMO_RELAY_CLI_PACKAGE_VERSION" rm -rf "${NEMO_RELAY_CI_WORKSPACE_TMP}/cli-packages" mkdir -p "${NEMO_RELAY_CI_WORKSPACE_TMP}/cli-packages" - args=( - --binary "$source" - --target "$target" - --version "$version" + python scripts/package-cli-bin.py \ + --binary "$source" \ + --target "$target" \ + --version "$version" \ --output-dir "${NEMO_RELAY_CI_WORKSPACE_TMP}/cli-packages" - ) - if [ "${{ matrix.platform }}" = "linux-amd64" ]; then - args+=(--npm-launcher) - fi - python scripts/package-cli-bin.py "${args[@]}" - name: Install and run CLI wheel if: ${{ !endsWith(matrix.target, '-musl') }} @@ -369,7 +392,12 @@ jobs: venv_cli="${NEMO_RELAY_CI_WORKSPACE_TMP}/cli-wheel-venv/bin/nemo-relay" fi "$venv_python" -m pip install --force-reinstall --no-deps "${wheels[0]}" - "$venv_cli" --version + actual="$("$venv_cli" --version)" + expected="nemo-relay $NEMO_RELAY_CLI_PACKAGE_VERSION" + if [[ "$actual" != "$expected" ]]; then + echo "Error: expected CLI version '$expected', got '$actual'" >&2 + exit 1 + fi - name: Install and run CLI wheel on musllinux if: ${{ endsWith(matrix.target, '-musl') }} @@ -380,13 +408,19 @@ jobs: --volume "${{ env.NEMO_RELAY_CI_WORKSPACE }}:${{ env.NEMO_RELAY_CI_WORKSPACE }}" \ --workdir "${{ env.NEMO_RELAY_CI_WORKSPACE }}" \ --env NEMO_RELAY_CI_WORKSPACE_TMP \ + --env NEMO_RELAY_CLI_PACKAGE_VERSION \ "${{ matrix.runtime_image }}" \ /bin/sh -ec ' wheel="$(find "$NEMO_RELAY_CI_WORKSPACE_TMP/cli-packages" -maxdepth 1 -name "*.whl" -print -quit)" test -n "$wheel" /opt/python/cp311-cp311/bin/python -m venv /tmp/nemo-relay-cli-wheel-venv /tmp/nemo-relay-cli-wheel-venv/bin/python -m pip install --force-reinstall --no-deps "$wheel" - /tmp/nemo-relay-cli-wheel-venv/bin/nemo-relay --version + actual="$(/tmp/nemo-relay-cli-wheel-venv/bin/nemo-relay --version)" + expected="nemo-relay $NEMO_RELAY_CLI_PACKAGE_VERSION" + if [ "$actual" != "$expected" ]; then + echo "Error: expected CLI version \"$expected\", got \"$actual\"" >&2 + exit 1 + fi ' - name: Upload CLI binary artifact @@ -402,10 +436,3 @@ jobs: name: cli-python-wheel-${{ matrix.platform }} path: ${{ env.NEMO_RELAY_CI_WORKSPACE_TMP }}/cli-packages/*.whl if-no-files-found: error - - - name: Upload CLI npm artifact - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: cli-npm-package-${{ matrix.platform }} - path: ${{ env.NEMO_RELAY_CI_WORKSPACE_TMP }}/cli-packages/*.tgz - if-no-files-found: error diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 92153c744..1e89de2c4 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -68,7 +68,7 @@ collect:github-artifacts: fi export GH_TOKEN="${NEMO_RELAY_CI_GITHUB_TOKEN}" - mkdir -p collected/wheels collected/sdists collected/node collected/cli-npm downloaded + mkdir -p collected/wheels collected/sdists collected/node downloaded tag="${CI_COMMIT_TAG:-}" if [ -z "$tag" ]; then @@ -133,7 +133,6 @@ collect:github-artifacts: gh run download "$run_id" --repo "$NEMO_RELAY_CI_GITHUB_REPOSITORY" --pattern 'wheel-*' --dir downloaded/wheels gh run download "$run_id" --repo "$NEMO_RELAY_CI_GITHUB_REPOSITORY" --pattern 'cli-python-wheel-*' --dir downloaded/cli-wheels - gh run download "$run_id" --repo "$NEMO_RELAY_CI_GITHUB_REPOSITORY" --pattern 'cli-npm-package-*' --dir downloaded/cli-npm gh run download "$run_id" --repo "$NEMO_RELAY_CI_GITHUB_REPOSITORY" --name 'plugin-wheel' --dir downloaded/wheels gh run download "$run_id" --repo "$NEMO_RELAY_CI_GITHUB_REPOSITORY" --name 'python-sdist' --dir downloaded/sdists gh run download "$run_id" --repo "$NEMO_RELAY_CI_GITHUB_REPOSITORY" --pattern 'node-npm-package-*' --dir downloaded/node @@ -141,7 +140,6 @@ collect:github-artifacts: find downloaded/wheels -type f -name '*.whl' -exec cp {} collected/wheels/ \; find downloaded/cli-wheels -type f -name '*.whl' -exec cp {} collected/wheels/ \; find downloaded/sdists -type f -name 'nemo_relay-*.tar.gz' -exec cp {} collected/sdists/ \; - find downloaded/cli-npm -type f -name '*.tgz' -exec cp {} collected/cli-npm/ \; find downloaded/node -type f -name '*.tgz' -exec cp {} collected/node/ \; if ! ls collected/wheels/*.whl >/dev/null 2>&1; then @@ -156,10 +154,6 @@ collect:github-artifacts: echo "Error: collected GitHub Node artifacts did not contain any .tgz files." >&2 exit 1 fi - if ! ls collected/cli-npm/*.tgz >/dev/null 2>&1; then - echo "Error: collected GitHub CLI npm artifacts did not contain any .tgz files." >&2 - exit 1 - fi { printf '{\n' @@ -177,7 +171,6 @@ collect:github-artifacts: - collected/wheels/*.whl - collected/sdists/nemo_relay-*.tar.gz - collected/node/*.tgz - - collected/cli-npm/*.tgz - collected/github-run.json publish:artifactory:wheels: @@ -337,8 +330,3 @@ publish:artifactory:npm: npm publish --tag dev "$package" done npm publish --tag dev "collected/node/nemo-relay-node-npm-${CI_COMMIT_TAG}.tgz" - for platform in linux-x64 linux-arm64 linux-x64-musl linux-arm64-musl darwin-arm64 win32-x64 win32-arm64; do - package="collected/cli-npm/nemo-relay-bin-npm-${platform}-${CI_COMMIT_TAG}.tgz" - npm publish --tag dev "$package" - done - npm publish --tag dev "collected/cli-npm/nemo-relay-bin-npm-${CI_COMMIT_TAG}.tgz" diff --git a/README.md b/README.md index e064be81e..94b223fe9 100644 --- a/README.md +++ b/README.md @@ -58,12 +58,6 @@ Install the prebuilt CLI from PyPI: pip install nemo-relay-cli-bin ``` -Install the prebuilt CLI from npm: - -```bash -npm install --global nemo-relay-cli-bin -``` - Python API users can install the matching CLI through the optional extra: ```bash diff --git a/RELEASING.md b/RELEASING.md index f2ce9ba00..cf6d058cb 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -32,13 +32,16 @@ The release pipeline publishes these package surfaces from a tag push: |---|---| | crates.io | `nemo-relay-types`, `nemo-relay-plugin`, `nemo-relay-worker-proto`, `nemo-relay-worker`, `nemo-relay`, `nemo-relay-adaptive`, `nemo-relay-pii-redaction`, `nemo-relay-switchyard`, `nemo-relay-ffi`, `nemo-relay-cli` | | PyPI | `nemo-relay` wheels and source distribution, `nemo-relay-plugin` and `nemo-relay-cli-bin` wheels | -| npm | `nemo-relay-node` and its seven platform packages, `nemo-relay-openclaw`, `nemo-relay-cli-bin`, and its seven platform packages | -| GitHub Releases | CLI binaries, `nemo-relay` and `nemo-relay-cli-bin` wheels, CLI and Node npm tarballs, and checksums | +| npm | `nemo-relay-node` and its seven platform packages, and `nemo-relay-openclaw` | +| GitHub Releases | CLI binaries, `nemo-relay` and `nemo-relay-cli-bin` wheels, Node npm tarballs, and checksums | | Fern | The documentation site | Go remains source-first. There is no separate Go package-manager publication step in the repository release workflow. +The former npm `nemo-relay-cli-bin` metapackage and its seven platform +packages are retired and must not be republished. + The mirrored GitLab pipeline also publishes the same tag's collected package artifacts to NVIDIA Artifactory. It is driven by the tag push, not by a GitLab pipeline schedule. @@ -61,9 +64,8 @@ NeMo Relay versions are anchored on the workspace SemVer in the repository root lock entries and must be updated with it. - `integrations/openclaw/package.json` carries the base npm version for the OpenClaw plugin package and must stay aligned with the same release version. -- `packages/cli-bin/package.json` and `python/cli-bin/pyproject.toml` carry the - base CLI package versions in their registry-specific SemVer and PEP 440 - spellings. The `nemo-relay[cli]` extra must pin the exact PyPI version. +- `python/cli-bin/pyproject.toml` carries the base CLI package version in its + PEP 440 spelling. The `nemo-relay[cli]` extra must pin the exact PyPI version. - The Python package version is derived at packaging time. `pyproject.toml` stays `dynamic = ["version"]` in the repository, and the packaging recipe writes a concrete version into `pyproject.toml` and `crates/python/Cargo.toml` @@ -151,11 +153,6 @@ Before you create a release tag, confirm the following: `nemo-relay-node-linux-x64-musl`, `nemo-relay-node-linux-arm64-musl`, `nemo-relay-node-darwin-arm64`, `nemo-relay-node-win32-x64-msvc`, and `nemo-relay-node-win32-arm64-msvc` - - npm trusted publishers are configured for `nemo-relay-cli-bin` and - `nemo-relay-cli-bin-linux-x64`, `nemo-relay-cli-bin-linux-arm64`, - `nemo-relay-cli-bin-linux-x64-musl`, `nemo-relay-cli-bin-linux-arm64-musl`, - `nemo-relay-cli-bin-darwin-arm64`, `nemo-relay-cli-bin-win32-x64`, and - `nemo-relay-cli-bin-win32-arm64` 5. The GitHub Release entry is ready to become the only canonical release-notes surface. @@ -182,7 +179,7 @@ The helper updates: 4. [`integrations/openclaw/package.json`](integrations/openclaw/package.json) and the `integrations/openclaw` entry in the root [`package-lock.json`](package-lock.json) to the same release version. -5. The Python and npm `nemo-relay-cli-bin` metadata, including the exact +5. The Python `nemo-relay-cli-bin` metadata, including the exact `nemo-relay[cli]` dependency version. Review docs and snippets that mention explicit versions, including: @@ -268,18 +265,15 @@ The release pipeline then: - `package-python-plugin` builds the `nemo-relay-plugin` wheel. - The Rust CLI matrix builds GNU Linux binaries in manylinux containers and musl Linux binaries natively, validates each in its matching container, and - packages each prebuilt binary as a - `nemo-relay-cli-bin` wheel and npm platform package, and creates the npm - launcher package once. + packages each prebuilt binary as a `nemo-relay-cli-bin` wheel. - The distribution release-asset job uploads the CLI binaries, `nemo-relay` - API wheels, CLI wheels, CLI npm packages, and split Node npm packages. + API wheels, CLI wheels, and split Node npm packages. `SHA256SUMS` covers every attached distribution artifact, and raw CLI binaries also receive individual `.sha256` files for installer compatibility. GitHub-facing npm artifacts use - `nemo-relay-bin-npm[--]-.tgz` for the CLI and - `nemo-relay-node-npm[--]-.tgz` for Node.js; their - registry package names remain in the tarball manifests. + `nemo-relay-node-npm[--]-.tgz`; their registry package + names remain in the tarball manifests. 4. Publishes packages from the top-level workflow after the reusable packaging jobs complete: - `publish-rust` stamps Cargo workspace versions from the release tag, then @@ -293,8 +287,8 @@ The release pipeline then: artifacts plus the `nemo-relay-plugin` and `nemo-relay-cli-bin` wheels, then uploads them to PyPI with trusted publishing from the top-level workflow - - `publish-npm` publishes the Node.js and CLI native packages before their - metapackages, then publishes the OpenClaw package, through npm trusted + - `publish-npm` publishes the Node.js native packages before their + metapackage, then publishes the OpenClaw package, through npm trusted publishing from the top-level workflow - Stable tags publish to the npm `latest` dist-tag - Prerelease tags such as `0.1.0-rc.1` publish to the npm `next` @@ -329,8 +323,7 @@ npm trusted publishing has its own registry-side constraints: - Each npm package can only have one trusted publisher configured at a time. - Configure trusted publishers for `nemo-relay-node`, all seven Node platform - packages, `nemo-relay-openclaw`, `nemo-relay-cli-bin`, and all seven CLI - platform packages before pushing a release tag. + packages, and `nemo-relay-openclaw` before pushing a release tag. - npm trusted publishing currently supports GitHub-hosted runners, not self-hosted runners. @@ -363,8 +356,8 @@ After the release is live, verify: are visible on crates.io. 2. The `nemo-relay` and `nemo-relay-cli-bin` wheels are visible on PyPI, and `pip install "nemo-relay[cli]"` exposes `nemo-relay`. -3. The `nemo-relay-node`, its seven platform packages, `nemo-relay-openclaw`, - `nemo-relay-cli-bin`, and its seven platform packages are visible on npm. +3. The `nemo-relay-node`, its seven platform packages, and + `nemo-relay-openclaw` are visible on npm. 4. The Unix and Windows installers resolve the new stable tag and verify matching CLI release asset checksums on their supported platforms. 5. The Fern documentation site shows the expected version and release notes. diff --git a/crates/cli/README.md b/crates/cli/README.md index b2d39ea10..19badc512 100644 --- a/crates/cli/README.md +++ b/crates/cli/README.md @@ -68,12 +68,6 @@ Install the prebuilt CLI from PyPI: pip install nemo-relay-cli-bin ``` -Install the prebuilt CLI from npm: - -```bash -npm install --global nemo-relay-cli-bin -``` - Install the Python API and matching CLI with the optional extra: ```bash diff --git a/crates/cli/src/agents/claude/launch.rs b/crates/cli/src/agents/claude/launch.rs index 4147c7ccb..89ab79e32 100644 --- a/crates/cli/src/agents/claude/launch.rs +++ b/crates/cli/src/agents/claude/launch.rs @@ -7,7 +7,7 @@ use serde_json::{Value, json}; use crate::agents::CodingAgent; use crate::error::CliError; -use crate::hooks::{generated_hooks, transparent_hook_forward_command}; +use crate::hooks::{generated_policy_hooks, transparent_hook_forward_commands}; use crate::process::{PreparedAgentLaunch, insert_after_host}; pub(crate) fn prepare( @@ -61,7 +61,7 @@ pub(crate) fn prepare( })) .map_err(|error| CliError::Launch(error.to_string()))?, )?; - let hook_command = transparent_hook_forward_command( + let hook_commands = transparent_hook_forward_commands( &transparent_hook_executable(), CodingAgent::ClaudeCode, gateway_url, @@ -69,7 +69,7 @@ pub(crate) fn prepare( .map_err(CliError::Launch)?; write_hooks( &root.join("hooks/hooks.json"), - generated_hooks(CodingAgent::ClaudeCode, &hook_command), + generated_policy_hooks(CodingAgent::ClaudeCode, &hook_commands), )?; let settings_path = root.join("settings.json"); let settings = settings_overlay(&launch.argv, launch.host_index, gateway_url)?; diff --git a/crates/cli/src/agents/codex/host.rs b/crates/cli/src/agents/codex/host.rs index 185121ffb..ffaeb4b51 100644 --- a/crates/cli/src/agents/codex/host.rs +++ b/crates/cli/src/agents/codex/host.rs @@ -14,6 +14,7 @@ use toml_edit::{DocumentMut, InlineTable, Item, Table, Value as TomlValue, value use crate::agents::CodingAgent; use crate::configuration::{BOOTSTRAP_CLIENT_TOKEN_HEADER, BootstrapChallengeKey, RELAY_PLUGIN_ID}; +#[cfg(test)] use crate::hooks::generated_hooks; #[cfg(test)] use crate::hooks::merge_hooks; @@ -109,11 +110,11 @@ pub(crate) fn install_codex_with_generation( pub(crate) fn install_codex_with_trust( gateway_url: &str, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, trust_hooks: F, ) -> Result where - F: FnOnce(&Path, &Path, &str) -> Result<(), String>, + F: FnOnce(&Path, &Path, &crate::hooks::GeneratedHookCommands) -> Result<(), String>, { let home = home_dir()?; let codex_dir = codex_home_dir()?; @@ -125,7 +126,7 @@ where let snapshots = codex_install_snapshots(&config_path, &hooks_path)?; let install_result = remove_legacy_codex_hooks(&hooks_path) .and_then(|()| install_codex_config(&config_path, gateway_url)) - .and_then(|()| trust_hooks(&home, &config_path, expected_command)); + .and_then(|()| trust_hooks(&home, &config_path, expected_commands)); if let Err(error) = install_result { return match restore_codex_install_snapshots(&snapshots) { Ok(()) => Err(error), @@ -257,9 +258,9 @@ pub(crate) fn codex_hook_trust_report_with_generation( pub(crate) fn codex_hook_trust_report_with_client( client: &mut dyn CodexHooksClient, cwd: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result { - let hooks = relay_codex_hooks(client, cwd, expected_command)?; + let hooks = relay_codex_hooks(client, cwd, expected_commands)?; Ok(codex_hook_trust_report_for(&hooks)) } @@ -267,9 +268,9 @@ pub(crate) fn auto_trust_codex_hooks( client: &mut dyn CodexHooksClient, cwd: &Path, config_path: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result<(), String> { - let hooks = relay_codex_hooks(client, cwd, expected_command)?; + let hooks = relay_codex_hooks(client, cwd, expected_commands)?; let before = codex_hook_trust_report_for(&hooks); if !before.missing_required.is_empty() || !before.duplicate_required.is_empty() { return Err(format!( @@ -280,7 +281,7 @@ pub(crate) fn auto_trust_codex_hooks( } let state = snapshot_hook_trust_state(config_path, &hooks)?; let trust_result = client.trust_hooks(&hooks).and_then(|()| { - let verified_hooks = relay_codex_hooks(client, cwd, expected_command)?; + let verified_hooks = relay_codex_hooks(client, cwd, expected_commands)?; let verified = codex_hook_trust_report_for(&verified_hooks); let unverified_targets = hooks .iter() @@ -308,7 +309,7 @@ pub(crate) fn auto_trust_codex_hooks( return restore_hook_trust_after_failure( client, cwd, - expected_command, + expected_commands, &hooks, &state, error, @@ -320,21 +321,28 @@ pub(crate) fn auto_trust_codex_hooks( fn relay_codex_hooks( client: &mut dyn CodexHooksClient, cwd: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result, String> { let hooks = relay_codex_plugin_hooks(client, cwd)? .into_iter() - .filter(|hook| hook.command.as_deref() == Some(expected_command)) + .filter(|hook| { + expected_codex_hook_command(expected_commands, &hook.event_name).is_some_and( + |expected| { + hook.command.as_deref() == Some(expected) + || hook.command.as_deref() == expected_commands.legacy() + }, + ) + }) .collect::>(); - validate_loaded_hook_sources(&hooks, expected_command)?; + validate_loaded_hook_sources(&hooks, expected_commands)?; Ok(hooks) } fn validate_loaded_hook_sources( hooks: &[CodexHookMetadata], - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, ) -> Result<(), String> { - let expected = generated_hooks(CodingAgent::Codex, expected_command); + let expected = crate::hooks::generated_policy_hooks(CodingAgent::Codex, expected_commands); let sources = hooks .iter() .map(|hook| hook.source_path.as_str()) @@ -538,7 +546,7 @@ fn verify_restored_hook_trust( fn restore_hook_trust_after_failure( client: &mut dyn CodexHooksClient, cwd: &Path, - expected_command: &str, + expected_commands: &crate::hooks::GeneratedHookCommands, before: &[CodexHookMetadata], state: &[(String, Option)], original_error: String, @@ -548,7 +556,7 @@ fn restore_hook_trust_after_failure( "{original_error}; additionally failed to restore Codex hook trust: {rollback_error}" )); } - let restored = relay_codex_hooks(client, cwd, expected_command).map_err(|rollback_error| { + let restored = relay_codex_hooks(client, cwd, expected_commands).map_err(|rollback_error| { format!( "{original_error}; additionally failed to verify restored Codex hook trust: {rollback_error}" ) @@ -569,14 +577,16 @@ fn restore_hook_trust_after_failure( } #[cfg(test)] -pub(crate) fn expected_plugin_hook_command(plugin_hooks_path: &Path) -> Result { +pub(crate) fn expected_plugin_hook_command( + plugin_hooks_path: &Path, +) -> Result { expected_plugin_hook_command_with_token(plugin_hooks_path, None) } fn expected_plugin_hook_command_with_token( plugin_hooks_path: &Path, generation_token: Option<&str>, -) -> Result { +) -> Result { let relay = current_exe()?; let relay = relay.canonicalize().unwrap_or(relay); let relay = portable_executable_path(relay); @@ -614,9 +624,12 @@ fn plugin_generation_file(plugin_hooks_path: &Path) -> Result { } } -fn validate_plugin_hooks(path: &Path, expected_command: &str) -> Result<(), String> { +fn validate_plugin_hooks( + path: &Path, + expected_commands: &crate::hooks::GeneratedHookCommands, +) -> Result<(), String> { let actual = read_json_object(path)?; - let expected = generated_hooks(CodingAgent::Codex, expected_command); + let expected = crate::hooks::generated_policy_hooks(CodingAgent::Codex, expected_commands); if actual == expected { Ok(()) } else { @@ -674,7 +687,7 @@ fn is_generated_codex_hook_event(event: &str) -> bool { .any(|expected| normalize_hook_event(expected) == normalized) } -fn normalize_hook_event(event: &str) -> String { +pub(crate) fn normalize_hook_event(event: &str) -> String { event .chars() .filter(|character| character.is_ascii_alphanumeric()) @@ -682,6 +695,18 @@ fn normalize_hook_event(event: &str) -> String { .collect() } +pub(crate) fn expected_codex_hook_command<'a>( + commands: &'a crate::hooks::GeneratedHookCommands, + event: &str, +) -> Option<&'a str> { + let normalized = normalize_hook_event(event); + CodingAgent::Codex + .hook_events() + .iter() + .find(|expected| normalize_hook_event(expected) == normalized) + .map(|event| commands.for_event(event)) +} + fn codex_install_snapshots( config_path: &Path, hooks_path: &Path, @@ -1635,7 +1660,7 @@ pub(crate) fn codex_hooks_installed_with_generation( generation_token: Option<&str>, ) -> Result { let value = read_json_object(path)?; - let generated = generated_hooks( + let generated = crate::hooks::generated_policy_hooks( CodingAgent::Codex, &expected_plugin_hook_command_with_token(path, generation_token)?, ); @@ -1660,8 +1685,8 @@ pub(crate) fn codex_plugin_hook_command( relay: &Path, generation: &Path, generation_token: &str, -) -> Result { - crate::hooks::persistent_hook_forward_command( +) -> Result { + crate::hooks::persistent_hook_forward_commands( relay, CodingAgent::Codex, generation, @@ -1675,8 +1700,8 @@ pub(crate) fn codex_plugin_hook_command_for_platform( generation: &Path, generation_token: &str, windows: bool, -) -> String { - crate::hooks::persistent_hook_forward_command_for_platform( +) -> crate::hooks::GeneratedHookCommands { + crate::hooks::persistent_hook_forward_commands_for_platform( relay, CodingAgent::Codex, generation, diff --git a/crates/cli/src/agents/codex/launch.rs b/crates/cli/src/agents/codex/launch.rs index ad20c5cc0..41cfeafe6 100644 --- a/crates/cli/src/agents/codex/launch.rs +++ b/crates/cli/src/agents/codex/launch.rs @@ -8,7 +8,7 @@ use serde_json::Value; use crate::agents::CodingAgent; use crate::configuration::{RELAY_PLUGIN_ID, RELAY_SOURCE_PLUGIN_ID}; use crate::error::CliError; -use crate::hooks::{generated_hooks, transparent_hook_forward_command}; +use crate::hooks::{generated_policy_hooks, transparent_hook_forward_commands}; use crate::process::{PreparedAgentLaunch, insert_after_host}; pub(crate) fn prepare(launch: &mut PreparedAgentLaunch, gateway_url: &str) -> Result<(), CliError> { @@ -26,13 +26,13 @@ pub(crate) fn prepare(launch: &mut PreparedAgentLaunch, gateway_url: &str) -> Re or pass `--openai-base-url` to an upstream that needs no key." ); } - let hook_command = transparent_hook_forward_command( + let hook_commands = transparent_hook_forward_commands( &transparent_hook_executable(), CodingAgent::Codex, gateway_url, ) .map_err(CliError::Launch)?; - let hook_groups = generated_hooks(CodingAgent::Codex, &hook_command); + let hook_groups = generated_policy_hooks(CodingAgent::Codex, &hook_commands); let mut args = vec![ "--config".to_string(), "features.hooks=true".to_string(), diff --git a/crates/cli/src/agents/hermes/config.rs b/crates/cli/src/agents/hermes/config.rs index 95d305fb3..06c3d413b 100644 --- a/crates/cli/src/agents/hermes/config.rs +++ b/crates/cli/src/agents/hermes/config.rs @@ -8,7 +8,7 @@ use std::path::{Path, PathBuf}; use serde_json::{Map, Value, json}; use crate::error::CliError; -use crate::hooks::{generated_hooks, merge_hooks}; +use crate::hooks::{GeneratedHookCommands, generated_policy_hooks, merge_hooks}; pub(super) use crate::mcp::SERVER_NAME as MCP_SERVER_NAME; @@ -32,9 +32,9 @@ pub(crate) fn transparent_config( ) -> Result { let mut root = parse_yaml_object(Some(existing), "Hermes config")?; let owned = owned_install_command(&root, relay, None)?; - strip_owned_hooks(&mut root, owned.as_deref())?; + strip_owned_hooks(&mut root, owned.as_ref())?; remove_owned_mcp(&mut root, owned.is_some())?; - let command = crate::hooks::transparent_hook_forward_command( + let commands = crate::hooks::transparent_hook_forward_commands( relay, crate::agents::CodingAgent::Hermes, gateway_url, @@ -42,7 +42,7 @@ pub(crate) fn transparent_config( .map_err(CliError::Install)?; let mut root = merge_hooks( root, - generated_hooks(crate::agents::CodingAgent::Hermes, &command), + generated_policy_hooks(crate::agents::CodingAgent::Hermes, &commands), )?; let object = root .as_object_mut() @@ -75,12 +75,12 @@ pub(crate) fn transparent_config( serde_yaml::to_string(&root).map_err(|error| CliError::Install(error.to_string())) } -pub(crate) fn persistent_hook_command( +pub(crate) fn persistent_hook_commands( relay: &Path, generation: &Path, generation_token: &str, -) -> Result { - crate::hooks::persistent_hook_forward_command( +) -> Result { + crate::hooks::persistent_hook_forward_commands( relay, crate::agents::CodingAgent::Hermes, generation, @@ -89,13 +89,13 @@ pub(crate) fn persistent_hook_command( } #[cfg(test)] -pub(super) fn persistent_hook_command_for_platform( +pub(super) fn persistent_hook_commands_for_platform( relay: &Path, generation: &Path, generation_token: &str, windows: bool, -) -> String { - crate::hooks::persistent_hook_forward_command_for_platform( +) -> GeneratedHookCommands { + crate::hooks::persistent_hook_forward_commands_for_platform( relay, crate::agents::CodingAgent::Hermes, generation, @@ -107,7 +107,7 @@ pub(super) fn persistent_hook_command_for_platform( pub(super) fn persistent_config( existing: Option<&str>, relay: &Path, - command: &str, + commands: &GeneratedHookCommands, generation: &Path, generation_token: &str, environment: &[String], @@ -123,10 +123,10 @@ pub(super) fn persistent_config( "Hermes MCP server `{MCP_SERVER_NAME}` already exists and is not managed by Relay; rename or remove it before installing the Relay integration" ))); } - strip_owned_hooks(&mut root, owned.as_deref())?; + strip_owned_hooks(&mut root, owned.as_ref())?; root = merge_hooks( root, - generated_hooks(crate::agents::CodingAgent::Hermes, command), + generated_policy_hooks(crate::agents::CodingAgent::Hermes, commands), )?; let servers = object_field_mut(&mut root, "mcp_servers", "mcp_servers")?; servers.insert( @@ -162,7 +162,7 @@ pub(super) fn forwarded_environment_names( pub(super) fn strip_owned_hooks( root: &mut Value, - owned_command: Option<&str>, + owned_commands: Option<&GeneratedHookCommands>, ) -> Result<(), CliError> { let Some(hooks) = root.get_mut("hooks") else { return Ok(()); @@ -180,7 +180,9 @@ pub(super) fn strip_owned_hooks( group .get("command") .and_then(Value::as_str) - .is_none_or(|command| Some(command) != owned_command) + .is_none_or(|command| { + owned_commands.is_none_or(|commands| !commands.contains(command)) + }) }); if groups.is_empty() { empty.push(event.clone()); @@ -221,7 +223,7 @@ pub(super) fn owned_install_command( root: &Value, relay: &Path, expected_generation: Option<&Path>, -) -> Result, CliError> { +) -> Result, CliError> { let Some(server) = root.pointer(&format!("/mcp_servers/{MCP_SERVER_NAME}")) else { return Ok(None); }; @@ -243,15 +245,18 @@ pub(super) fn owned_install_command( && !token.is_empty() && expected_generation.is_none_or(|expected| Path::new(generation) == expected) { - let command = persistent_hook_command(relay, Path::new(generation), token) + let commands = persistent_hook_commands(relay, Path::new(generation), token) .map_err(CliError::Install)?; - return Ok(Some(command)); + return Ok(Some(commands)); } } legacy_owned_command(root, relay) } -fn legacy_owned_command(root: &Value, relay: &Path) -> Result, CliError> { +fn legacy_owned_command( + root: &Value, + relay: &Path, +) -> Result, CliError> { let server = &root["mcp_servers"][MCP_SERVER_NAME]; if server.get("args") != Some(&json!(["mcp", "--agent", "hermes"])) { return Ok(None); @@ -274,7 +279,7 @@ fn legacy_owned_command(root: &Value, relay: &Path) -> Result, Cl } common = Some(commands[0]); } - Ok(common.map(str::to_owned)) + Ok(common.map(GeneratedHookCommands::uniform)) } fn legacy_command_uses_relay(command: &str, relay: &Path) -> bool { diff --git a/crates/cli/src/agents/hermes/integration.rs b/crates/cli/src/agents/hermes/integration.rs index f14457a3f..e1ea8c2b7 100644 --- a/crates/cli/src/agents/hermes/integration.rs +++ b/crates/cli/src/agents/hermes/integration.rs @@ -11,13 +11,13 @@ use std::time::SystemTime; use serde_json::{Map, Value, json}; #[cfg(test)] -use super::config::persistent_hook_command_for_platform; +use super::config::persistent_hook_commands_for_platform; use super::config::{ MCP_SERVER_NAME, expected_mcp_server, forwarded_environment_names, owned_install_command, parse_yaml_object, persistent_config, relay_is_executable, remove_owned_mcp, strip_owned_hooks, user_config_path_with_override, yaml_bytes, }; -pub(crate) use super::config::{persistent_hook_command, transparent_config}; +pub(crate) use super::config::{persistent_hook_commands, transparent_config}; use super::files::{ FileSnapshot, INSTALL_LOCK_TIMEOUT, PersistentPaths, acquire_allowlist_lock, acquire_install_lock, read_optional_utf8, remove_optional_file, replace_optional_file, @@ -27,6 +27,7 @@ use crate::agents::CodingAgent; use crate::bootstrap::DEFAULT_BIND; use crate::error::CliError; use crate::filesystem::atomic_write; +use crate::hooks::GeneratedHookCommands; #[cfg(test)] use crate::installation::generation::GENERATION_FILE_NAME; use crate::installation::generation::{ @@ -166,7 +167,10 @@ fn config_has_managed_state(config: &Value) -> bool { owned_command_from_config(config, None).is_some() } -fn allowlist_has_owned_command(allowlist: &Value, command: Option<&str>) -> bool { +fn allowlist_has_owned_command( + allowlist: &Value, + commands: Option<&GeneratedHookCommands>, +) -> bool { allowlist .get("approvals") .and_then(Value::as_array) @@ -174,16 +178,23 @@ fn allowlist_has_owned_command(allowlist: &Value, command: Option<&str>) -> bool .flatten() .filter_map(|entry| entry.get("command").and_then(Value::as_str)) .any(|candidate| { - command == Some(candidate) - || (command.is_none() && is_persistent_relay_hook_command(candidate)) + commands.is_some_and(|commands| commands.contains(candidate)) + || (commands.is_none() && is_persistent_relay_hook_command(candidate)) }) } fn is_persistent_relay_hook_command(command: &str) -> bool { #[cfg(any(windows, test))] if let Some(arguments) = crate::hooks::decode_windows_hook_command(command) { + let arguments = arguments.as_slice(); + let base = match arguments { + [base @ .., policy] if matches!(policy.as_str(), "--fail-open" | "--fail-closed") => { + base + } + base => base, + }; return matches!( - arguments.as_slice(), + base, [ _, hook_forward, @@ -211,7 +222,10 @@ fn is_persistent_relay_hook_command(command: &str) -> bool { && command.contains("--generation-token") } -fn owned_command_from_config(config: &Value, generation: Option<&Path>) -> Option { +fn owned_command_from_config( + config: &Value, + generation: Option<&Path>, +) -> Option { let relay = config .pointer(&format!("/mcp_servers/{MCP_SERVER_NAME}/command")) .and_then(Value::as_str) @@ -235,9 +249,9 @@ pub(crate) fn diagnose_persistent(config_path: &Path) -> Result )); } let generation = InstallGeneration::capture(paths.generation.clone())?; - let command = persistent_hook_command(&relay, &paths.generation, generation.token())?; - verify_hook_definitions(&config, &command)?; - verify_trust(&paths.allowlist, &command)?; + let commands = persistent_hook_commands(&relay, &paths.generation, generation.token())?; + verify_hook_definitions(&config, &commands)?; + verify_trust(&paths.allowlist, &commands)?; let mcp_env = config["mcp_servers"][MCP_SERVER_NAME] .get("env") @@ -374,20 +388,20 @@ where }; let environment = forwarded_environment_names(environment, plugin_config); let token = uuid::Uuid::now_v7().to_string(); - let command = - persistent_hook_command(relay, &paths.generation, &token).map_err(CliError::Install)?; + let commands = + persistent_hook_commands(relay, &paths.generation, &token).map_err(CliError::Install)?; let config = persistent_config( existing_config.as_deref(), relay, - &command, + &commands, &paths.generation, &token, &environment, )?; let allowlist = trusted_hooks( existing_allowlist.as_deref(), - previous_command.as_deref(), - &command, + previous_command.as_ref(), + &commands, relay, now, )?; @@ -404,7 +418,7 @@ where verify_install( &paths, relay, - &command, + &commands, &environment, &token, generation_transaction, @@ -437,7 +451,7 @@ where .map(|raw| { let mut root = parse_yaml_object(Some(&raw), "Hermes config")?; let owned = owned_command_from_config(&root, Some(&paths.generation)); - strip_owned_hooks(&mut root, owned.as_deref())?; + strip_owned_hooks(&mut root, owned.as_ref())?; remove_owned_mcp(&mut root, owned.is_some())?; if root.as_object().is_some_and(Map::is_empty) { Ok(None) @@ -466,7 +480,12 @@ where entry .get("command") .and_then(Value::as_str) - .is_none_or(|command| Some(command) != owned.as_deref()) + .is_none_or(|command| { + owned.as_ref().map_or_else( + || !is_persistent_relay_hook_command(command), + |commands| !commands.contains(command), + ) + }) }); if approvals.is_empty() { object.remove("approvals"); @@ -485,7 +504,7 @@ where remove_optional_file(&paths.generation)?; replace_optional_file(&paths.allowlist, allowlist.as_deref(), &mut write)?; replace_optional_file(&paths.config, config.as_deref(), &mut write)?; - verify_uninstall(&paths, owned.as_deref()) + verify_uninstall(&paths, owned.as_ref()) })(); if let Err(error) = result { return rollback_error("uninstall", error, &snapshots, &mut write); @@ -520,7 +539,7 @@ where fn verify_install( paths: &PersistentPaths, relay: &Path, - command: &str, + commands: &GeneratedHookCommands, environment: &[String], token: &str, generation_transaction: Option<&GenerationRetirement>, @@ -532,8 +551,8 @@ fn verify_install( if config.pointer("/mcp_servers/nemo-relay") != Some(&expected) { return Err("Hermes MCP server did not persist exactly".into()); } - verify_hook_definitions(&config, command)?; - verify_trust(&paths.allowlist, command)?; + verify_hook_definitions(&config, commands)?; + verify_trust(&paths.allowlist, commands)?; let actual_token = match generation_transaction { Some(transaction) => transaction.active_visible_token()?, @@ -547,7 +566,7 @@ fn verify_install( Ok(()) } -fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> { +fn verify_hook_definitions(config: &Value, commands: &GeneratedHookCommands) -> Result<(), String> { for event in CodingAgent::Hermes.hook_events() { let groups = config .pointer(&format!("/hooks/{event}")) @@ -555,7 +574,9 @@ fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> .ok_or_else(|| format!("Hermes hook {event} is missing"))?; let matching = groups .iter() - .filter(|group| group.get("command").and_then(Value::as_str) == Some(command)) + .filter(|group| { + group.get("command").and_then(Value::as_str) == Some(commands.for_event(event)) + }) .count(); if matching != 1 { return Err(format!( @@ -573,9 +594,12 @@ fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> .as_array() .ok_or_else(|| format!("Hermes {event} hooks must be an array"))?; if !CodingAgent::Hermes.hook_events().contains(&event.as_str()) - && groups - .iter() - .any(|group| group.get("command").and_then(Value::as_str) == Some(command)) + && groups.iter().any(|group| { + group + .get("command") + .and_then(Value::as_str) + .is_some_and(|command| commands.contains(command)) + }) { return Err("Hermes config contains an unexpected Relay hook handler".into()); } @@ -583,7 +607,10 @@ fn verify_hook_definitions(config: &Value, command: &str) -> Result<(), String> Ok(()) } -fn verify_uninstall(paths: &PersistentPaths, owned_command: Option<&str>) -> Result<(), String> { +fn verify_uninstall( + paths: &PersistentPaths, + owned_commands: Option<&GeneratedHookCommands>, +) -> Result<(), String> { if paths.generation.exists() { return Err("Hermes MCP generation fence still exists".into()); } @@ -596,7 +623,7 @@ fn verify_uninstall(paths: &PersistentPaths, owned_command: Option<&str>) -> Res if let Some(raw) = read_optional_utf8(&paths.allowlist).map_err(|error| error.to_string())? { let allowlist = parse_json_object(Some(&raw), "Hermes shell-hook allowlist") .map_err(|e| e.to_string())?; - if allowlist_has_owned_command(&allowlist, owned_command) { + if allowlist_has_owned_command(&allowlist, owned_commands) { return Err("managed Hermes Relay trust approval still exists".into()); } } diff --git a/crates/cli/src/agents/hermes/trust.rs b/crates/cli/src/agents/hermes/trust.rs index 494c26c53..4dd529dff 100644 --- a/crates/cli/src/agents/hermes/trust.rs +++ b/crates/cli/src/agents/hermes/trust.rs @@ -12,11 +12,12 @@ use serde_json::{Value, json}; use crate::agents::CodingAgent; use crate::error::CliError; +use crate::hooks::GeneratedHookCommands; pub(super) fn trusted_hooks( existing: Option<&str>, - previous_command: Option<&str>, - command: &str, + previous_commands: Option<&GeneratedHookCommands>, + commands: &GeneratedHookCommands, relay: &Path, now: SystemTime, ) -> Result { @@ -34,7 +35,9 @@ pub(super) fn trusted_hooks( entry .get("command") .and_then(Value::as_str) - .is_none_or(|candidate| Some(candidate) != previous_command) + .is_none_or(|candidate| { + previous_commands.is_none_or(|commands| !commands.contains(candidate)) + }) }); let approved_at = timestamp(now); let script_mtime_at_approval = fs::metadata(relay) @@ -44,7 +47,7 @@ pub(super) fn trusted_hooks( approvals.extend(CodingAgent::Hermes.hook_events().iter().map(|event| { json!({ "event": event, - "command": command, + "command": commands.for_event(event), "approved_at": approved_at, "script_mtime_at_approval": script_mtime_at_approval, }) @@ -56,7 +59,10 @@ fn timestamp(time: SystemTime) -> String { DateTime::::from(time).to_rfc3339_opts(SecondsFormat::Micros, true) } -pub(super) fn verify_trust(allowlist_path: &Path, command: &str) -> Result<(), String> { +pub(super) fn verify_trust( + allowlist_path: &Path, + commands: &GeneratedHookCommands, +) -> Result<(), String> { let raw = fs::read_to_string(allowlist_path) .map_err(|error| format!("failed to read {}: {error}", allowlist_path.display()))?; let allowlist = @@ -70,7 +76,8 @@ pub(super) fn verify_trust(allowlist_path: &Path, command: &str) -> Result<(), S .iter() .filter(|entry| { entry.get("event").and_then(Value::as_str) == Some(event) - && entry.get("command").and_then(Value::as_str) == Some(command) + && entry.get("command").and_then(Value::as_str) + == Some(commands.for_event(event)) }) .count(); if matching != 1 { @@ -80,14 +87,19 @@ pub(super) fn verify_trust(allowlist_path: &Path, command: &str) -> Result<(), S } } for entry in approvals { - if entry.get("command").and_then(Value::as_str) != Some(command) { + let Some(command) = entry.get("command").and_then(Value::as_str) else { + continue; + }; + if !commands.contains(command) { continue; } let event = entry .get("event") .and_then(Value::as_str) .ok_or_else(|| "Hermes Relay hook approval is missing its event".to_string())?; - if !CodingAgent::Hermes.hook_events().contains(&event) { + if !CodingAgent::Hermes.hook_events().contains(&event) + || command != commands.for_event(event) + { return Err("Hermes allowlist contains an unexpected Relay hook approval".into()); } } diff --git a/crates/cli/src/agents/mod.rs b/crates/cli/src/agents/mod.rs index 41cd5f72a..376387797 100644 --- a/crates/cli/src/agents/mod.rs +++ b/crates/cli/src/agents/mod.rs @@ -200,13 +200,13 @@ impl crate::installation::marketplace::MarketplaceHost for CodingAgent { generation_fence: &std::path::Path, generation_token: &str, ) -> Result { - let command = crate::hooks::persistent_hook_forward_command( + let commands = crate::hooks::persistent_hook_forward_commands( relay, self, generation_fence, generation_token, )?; - Ok(crate::hooks::generated_hooks(self, &command)) + Ok(crate::hooks::generated_policy_hooks(self, &commands)) } fn plugin_registration_args(self, plugin_id: &str) -> Vec { diff --git a/crates/cli/src/bootstrap/mod.rs b/crates/cli/src/bootstrap/mod.rs index fbd9142d4..79e8aad02 100644 --- a/crates/cli/src/bootstrap/mod.rs +++ b/crates/cli/src/bootstrap/mod.rs @@ -38,7 +38,7 @@ use state::{BOOTSTRAP_STATE_DIR_ENV, state_dir as bootstrap_state_dir}; pub(crate) const DEFAULT_BIND: &str = "127.0.0.1:47632"; pub(crate) const DEFAULT_URL: &str = "http://127.0.0.1:47632"; pub(crate) const HEALTHZ_TIMEOUT: Duration = Duration::from_millis(500); -pub(crate) const BOOTSTRAP_PROTOCOL_VERSION: u64 = 2; +pub(crate) const BOOTSTRAP_PROTOCOL_VERSION: u64 = 3; pub(super) const BOOTSTRAP_LOCK_TIMEOUT: Duration = Duration::from_secs(20); const BOOTSTRAP_START_TIMEOUT: Duration = Duration::from_secs(10); diff --git a/crates/cli/src/commands/hook_forward.rs b/crates/cli/src/commands/hook_forward.rs index 9b767570f..14fdf5770 100644 --- a/crates/cli/src/commands/hook_forward.rs +++ b/crates/cli/src/commands/hook_forward.rs @@ -45,8 +45,11 @@ pub(crate) struct HookForwardCommand { #[arg(long, value_enum)] pub(crate) gateway_mode: Option, /// Return a failure when the payload cannot be delivered or Relay rejects it. - #[arg(long)] + #[arg(long, conflicts_with = "fail_open")] pub(crate) fail_closed: bool, + /// Allow the coding agent to continue when the payload cannot be delivered. + #[arg(long, conflicts_with = "fail_closed")] + pub(crate) fail_open: bool, } impl HookForwardCommand { @@ -61,7 +64,13 @@ impl HookForwardCommand { profile: self.profile, session_metadata: self.session_metadata, gateway_mode: self.gateway_mode.map(Into::into), - fail_closed: self.fail_closed, + failure_policy: if self.fail_closed { + crate::hooks::HookFailurePolicy::FailClosed + } else if self.fail_open { + crate::hooks::HookFailurePolicy::FailOpen + } else { + crate::hooks::HookFailurePolicy::Default + }, } } } diff --git a/crates/cli/src/hooks/delivery.rs b/crates/cli/src/hooks/delivery.rs index b8db65620..cb9cf566f 100644 --- a/crates/cli/src/hooks/delivery.rs +++ b/crates/cli/src/hooks/delivery.rs @@ -29,8 +29,7 @@ pub(crate) async fn hook_forward(command: HookForwardRequest) -> Result<(), CliE return Ok(()); } validate_optional_json("session metadata", command.session_metadata.as_deref())?; - let fail_closed = - command.fail_closed || std::env::var("NEMO_RELAY_FAIL_CLOSED").ok().as_deref() == Some("1"); + let fail_closed = command.failure_policy.fail_closed(); let destination = hook_destination(&command); let persistent = match persistent_gateway(&destination) { Ok(persistent) => persistent, diff --git a/crates/cli/src/hooks/encoding.rs b/crates/cli/src/hooks/encoding.rs index 1ac762c65..f9eaf5113 100644 --- a/crates/cli/src/hooks/encoding.rs +++ b/crates/cli/src/hooks/encoding.rs @@ -12,22 +12,70 @@ use crate::agents::CodingAgent; #[cfg(any(windows, test))] use base64::Engine; +#[cfg(test)] pub(crate) fn generated_hooks(agent: CodingAgent, command: &str) -> Value { + generated_policy_hooks(agent, &GeneratedHookCommands::uniform(command)) +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GeneratedHookCommands { + fail_open: String, + fail_closed: String, + legacy: Option, +} + +impl GeneratedHookCommands { + pub(crate) fn new(fail_open: impl Into, fail_closed: impl Into) -> Self { + Self { + fail_open: fail_open.into(), + fail_closed: fail_closed.into(), + legacy: None, + } + } + + pub(crate) fn uniform(command: impl Into) -> Self { + let command = command.into(); + Self::new(command.clone(), command) + } + + pub(crate) fn for_event(&self, event: &str) -> &str { + if event_requires_fail_closed(event) { + &self.fail_closed + } else { + &self.fail_open + } + } + + pub(crate) fn contains(&self, command: &str) -> bool { + command == self.fail_open + || command == self.fail_closed + || self.legacy.as_deref() == Some(command) + } + + pub(crate) fn legacy(&self) -> Option<&str> { + self.legacy.as_deref() + } +} + +pub(crate) fn generated_policy_hooks( + agent: CodingAgent, + commands: &GeneratedHookCommands, +) -> Value { if agent.uses_direct_hook_entries() { - direct_hooks(agent.hook_events(), command) + direct_hooks(agent.hook_events(), commands) } else { - grouped_hooks(agent.hook_events(), command) + grouped_hooks(agent.hook_events(), commands) } } /// Canonical persistent hook command used by every supported host. -pub(crate) fn persistent_hook_forward_command( +pub(crate) fn persistent_hook_forward_commands( relay: &Path, agent: CodingAgent, generation_file: &Path, generation_token: &str, -) -> Result { - hook_command( +) -> Result { + hook_commands( relay, &persistent_hook_arguments(agent, generation_file, generation_token), ) @@ -35,22 +83,22 @@ pub(crate) fn persistent_hook_forward_command( /// Canonical transparent hook command. It embeds the process-private dynamic gateway so hook hosts /// that filter inherited environment variables cannot redirect delivery to the fixed endpoint. -pub(crate) fn transparent_hook_forward_command( +pub(crate) fn transparent_hook_forward_commands( relay: &Path, agent: CodingAgent, gateway_url: &str, -) -> Result { - hook_command(relay, &transparent_hook_arguments(agent, gateway_url)) +) -> Result { + hook_commands(relay, &transparent_hook_arguments(agent, gateway_url)) } #[cfg(test)] -pub(crate) fn transparent_hook_forward_command_for_platform( +pub(crate) fn transparent_hook_forward_commands_for_platform( relay: &Path, agent: CodingAgent, gateway_url: &str, windows: bool, -) -> String { - hook_command_for_platform( +) -> GeneratedHookCommands { + hook_commands_for_platform( relay, &transparent_hook_arguments(agent, gateway_url), windows, @@ -58,14 +106,14 @@ pub(crate) fn transparent_hook_forward_command_for_platform( } #[cfg(test)] -pub(crate) fn persistent_hook_forward_command_for_platform( +pub(crate) fn persistent_hook_forward_commands_for_platform( relay: &Path, agent: CodingAgent, generation_file: &Path, generation_token: &str, windows: bool, -) -> String { - hook_command_for_platform( +) -> GeneratedHookCommands { + hook_commands_for_platform( relay, &persistent_hook_arguments(agent, generation_file, generation_token), windows, @@ -99,6 +147,45 @@ pub(super) fn persistent_hook_arguments( ] } +fn hook_commands(relay: &Path, arguments: &[String]) -> Result { + let mut commands = GeneratedHookCommands::new( + hook_command(relay, &with_failure_policy(arguments, "--fail-open"))?, + hook_command(relay, &with_failure_policy(arguments, "--fail-closed"))?, + ); + commands.legacy = Some(hook_command(relay, arguments)?); + Ok(commands) +} + +#[cfg(test)] +fn hook_commands_for_platform( + relay: &Path, + arguments: &[String], + windows: bool, +) -> GeneratedHookCommands { + let mut commands = GeneratedHookCommands::new( + hook_command_for_platform( + relay, + &with_failure_policy(arguments, "--fail-open"), + windows, + ), + hook_command_for_platform( + relay, + &with_failure_policy(arguments, "--fail-closed"), + windows, + ), + ); + commands.legacy = Some(hook_command_for_platform(relay, arguments, windows)); + commands +} + +fn with_failure_policy(arguments: &[String], policy: &str) -> Vec { + arguments + .iter() + .cloned() + .chain(std::iter::once(policy.to_string())) + .collect() +} + pub(super) fn hook_command(relay: &Path, arguments: &[String]) -> Result { #[cfg(windows)] { @@ -302,14 +389,14 @@ pub(super) fn parse_powershell_single_quoted_arguments(mut raw: &str) -> Option< (!arguments.is_empty()).then_some(arguments) } -pub(super) fn direct_hooks(events: &[&str], command: &str) -> Value { +fn direct_hooks(events: &[&str], commands: &GeneratedHookCommands) -> Value { let hooks: serde_json::Map = events .iter() .map(|event| { ( (*event).to_string(), json!([{ - "command": command, + "command": commands.for_event(event), "timeout": 30 }]), ) @@ -321,7 +408,7 @@ pub(super) fn direct_hooks(events: &[&str], command: &str) -> Value { // Generates hook groups for Claude/Codex events and adds a wildcard matcher to tool events when // the target agent requires matcher-scoped tool hooks. Non-tool events omit matchers so they fire // for the full lifecycle. -pub(super) fn grouped_hooks(events: &[&str], command: &str) -> Value { +fn grouped_hooks(events: &[&str], commands: &GeneratedHookCommands) -> Value { let hooks: serde_json::Map = events .iter() .map(|event| { @@ -333,7 +420,7 @@ pub(super) fn grouped_hooks(events: &[&str], command: &str) -> Value { "hooks".into(), json!([{ "type": "command", - "command": command, + "command": commands.for_event(event), "timeout": 30 }]), ); @@ -354,3 +441,7 @@ pub(crate) fn event_matches_tools(event: &str) -> bool { "PreToolUse" | "PostToolUse" | "PostToolUseFailure" | "PermissionRequest" ) } + +pub(crate) fn event_requires_fail_closed(event: &str) -> bool { + matches!(event, "PreToolUse" | "PermissionRequest" | "pre_tool_call") +} diff --git a/crates/cli/src/hooks/mod.rs b/crates/cli/src/hooks/mod.rs index 6389f3c2e..d21efe041 100644 --- a/crates/cli/src/hooks/mod.rs +++ b/crates/cli/src/hooks/mod.rs @@ -23,18 +23,19 @@ pub(crate) use destination::{ pub(crate) use encoding::decode_windows_hook_command; #[cfg(all(test, windows))] pub(crate) use encoding::windows_powershell_path; -#[cfg(test)] pub(crate) use encoding::{ - encoded_windows_hook_command, event_matches_tools, - persistent_hook_forward_command_for_platform, transparent_hook_forward_command_for_platform, + GeneratedHookCommands, generated_policy_hooks, persistent_hook_forward_commands, + transparent_hook_forward_commands, }; +#[cfg(test)] pub(crate) use encoding::{ - generated_hooks, persistent_hook_forward_command, transparent_hook_forward_command, + encoded_windows_hook_command, event_matches_tools, event_requires_fail_closed, generated_hooks, + persistent_hook_forward_commands_for_platform, transparent_hook_forward_commands_for_platform, }; pub(crate) use merging::merge_hooks; #[cfg(test)] pub(crate) use response::{handle_hook_forward_status, handle_verified_hook_forward_response}; -pub(crate) use types::{GatewayMode, HookForwardRequest}; +pub(crate) use types::{GatewayMode, HookFailurePolicy, HookForwardRequest}; #[cfg(test)] use serde_json::json; diff --git a/crates/cli/src/hooks/types.rs b/crates/cli/src/hooks/types.rs index 9959b309d..b11713112 100644 --- a/crates/cli/src/hooks/types.rs +++ b/crates/cli/src/hooks/types.rs @@ -16,7 +16,24 @@ pub(crate) struct HookForwardRequest { pub(crate) profile: Option, pub(crate) session_metadata: Option, pub(crate) gateway_mode: Option, - pub(crate) fail_closed: bool, + pub(crate) failure_policy: HookFailurePolicy, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum HookFailurePolicy { + Default, + FailOpen, + FailClosed, +} + +impl HookFailurePolicy { + pub(crate) fn fail_closed(self) -> bool { + match self { + Self::Default => std::env::var("NEMO_RELAY_FAIL_CLOSED").ok().as_deref() == Some("1"), + Self::FailOpen => false, + Self::FailClosed => true, + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 4fce1eea1..7a59b9ced 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -24,6 +24,7 @@ fn gateway_bin() -> &'static str { } const ACTIVE_GENERATION_TOKEN: &str = "active-generation"; +const BOOTSTRAP_PROTOCOL_VERSION: u64 = 3; const SIDECAR_PUBLICATION_TIMEOUT: Duration = Duration::from_secs(30); fn write_active_generation(temp: &std::path::Path) -> std::path::PathBuf { @@ -639,8 +640,9 @@ fn cli_mcp_starts_gateway_even_when_stdio_closes_before_request() { fn cli_mcp_rejects_an_unauthenticated_transparent_gateway() { let temp = tempfile::tempdir().unwrap(); let body = format!( - r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":2,"instance_id":"transparent"}}"#, - env!("CARGO_PKG_VERSION") + r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":{},"instance_id":"transparent"}}"#, + env!("CARGO_PKG_VERSION"), + BOOTSTRAP_PROTOCOL_VERSION, ); let (gateway_url, received) = spawn_single_request_server(200, &body); let mut child = Command::new(gateway_bin()) @@ -786,17 +788,23 @@ fn assert_hermes_install_config(config_path: &std::path::Path, hermes_home: &std .unwrap() .contains("not-written-to-config") ); - let command = config["hooks"]["on_session_start"][0]["command"] - .as_str() - .unwrap(); - assert!(command.contains("hook-forward hermes")); let approvals: serde_json::Value = serde_json::from_str( &std::fs::read_to_string(hermes_home.join("shell-hooks-allowlist.json")).unwrap(), ) .unwrap(); let approvals = approvals["approvals"].as_array().unwrap(); assert_eq!(approvals.len(), 13); - assert!(approvals.iter().all(|entry| entry["command"] == command)); + for approval in approvals { + let event = approval["event"].as_str().unwrap(); + let command = approval["command"].as_str().unwrap(); + assert!(command.contains("hook-forward hermes")); + assert_eq!(approval["command"], config["hooks"][event][0]["command"]); + if event == "pre_tool_call" { + assert!(command.ends_with(" --fail-closed")); + } else { + assert!(command.ends_with(" --fail-open")); + } + } } #[cfg(unix)] @@ -1127,8 +1135,9 @@ fn write_fake_bootstrap_health( let nonce = bootstrap_request_header(request, "x-nemo-relay-bootstrap-nonce").unwrap(); let proof_header = fake_bootstrap_proof_header(proof, key_path, fingerprint, nonce); let body = format!( - r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":2,"instance_id":"test-instance"}}"#, - env!("CARGO_PKG_VERSION") + r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":{},"instance_id":"test-instance"}}"#, + env!("CARGO_PKG_VERSION"), + BOOTSTRAP_PROTOCOL_VERSION, ); stream .write_all( @@ -1171,8 +1180,9 @@ fn handle_fake_bootstrap_tunnel( let health = read_http_request(&mut stream); requests.lock().unwrap().push(health); let body = format!( - r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":2,"instance_id":"test-instance"}}"#, - env!("CARGO_PKG_VERSION") + r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":{},"instance_id":"test-instance"}}"#, + env!("CARGO_PKG_VERSION"), + BOOTSTRAP_PROTOCOL_VERSION, ); stream .write_all( @@ -1314,6 +1324,60 @@ fn cli_codex_hook_launch_resolution_error_respects_forwarding_policy() { } } +#[test] +fn cli_hook_forward_explicit_policy_overrides_the_environment() { + let temp = tempfile::tempdir().unwrap(); + let generation = write_active_generation(temp.path()); + for (policy, environment, succeeds) in [ + ("--fail-open", Some("1"), true), + ("--fail-closed", None, false), + ] { + let mut command = Command::new(gateway_bin()); + command + .args([ + "hook-forward", + "codex", + "--gateway-url", + "http://127.0.0.1:1", + "--generation-file", + ]) + .arg(&generation) + .arg("--generation-token") + .arg(ACTIVE_GENERATION_TOKEN) + .arg(policy) + .env("HOME", temp.path()) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("XDG_RUNTIME_DIR", temp.path().join("runtime")) + .env("TMPDIR", temp.path()) + .env("NEMO_RELAY_PLUGIN_IDLE_TIMEOUT_SECS", "not-a-number") + .env_remove("NEMO_RELAY_FAIL_CLOSED") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if let Some(value) = environment { + command.env("NEMO_RELAY_FAIL_CLOSED", value); + } + let mut child = command.spawn().unwrap(); + child.stdin.take().unwrap().write_all(b"{}").unwrap(); + let output = wait_child_with_output(child); + assert_eq!(output.status.success(), succeeds, "{policy}"); + assert!( + String::from_utf8_lossy(&output.stderr).contains("NEMO_RELAY_PLUGIN_IDLE_TIMEOUT_SECS") + ); + } +} + +#[test] +fn cli_hook_forward_rejects_conflicting_failure_policies() { + let output = Command::new(gateway_bin()) + .args(["hook-forward", "codex", "--fail-open", "--fail-closed"]) + .output() + .unwrap(); + + assert!(!output.status.success()); + assert!(String::from_utf8_lossy(&output.stderr).contains("cannot be used with")); +} + #[test] fn cli_codex_hook_launch_resolution_error_retains_default_payload_cap() { const DEFAULT_HOOK_PAYLOAD_BYTES: usize = 20 * 1024 * 1024; @@ -1635,7 +1699,7 @@ fn cli_mcp_clients_share_gateway_until_final_idle_shutdown() { let health = relay_health(address); assert_eq!(health["service"], "nemo-relay"); assert_eq!(health["version"], env!("CARGO_PKG_VERSION")); - assert_eq!(health["bootstrap_protocol"], 2); + assert_eq!(health["bootstrap_protocol"], BOOTSTRAP_PROTOCOL_VERSION); assert!( health["instance_id"] .as_str() @@ -4500,8 +4564,9 @@ fn write_phase_health_response( .ok_or_else(|| "health probe omitted its nonce".to_string())?; let proof = fake_bootstrap_proof(key, fingerprint, nonce); let body = format!( - r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":2,"instance_id":"phase-health"}}"#, - env!("CARGO_PKG_VERSION") + r#"{{"status":"ok","service":"nemo-relay","version":"{}","bootstrap_protocol":{},"instance_id":"phase-health"}}"#, + env!("CARGO_PKG_VERSION"), + BOOTSTRAP_PROTOCOL_VERSION, ); stream .write_all( diff --git a/crates/cli/tests/coverage/agents/hermes_tests.rs b/crates/cli/tests/coverage/agents/hermes_tests.rs index eae5ee906..ede96d7fc 100644 --- a/crates/cli/tests/coverage/agents/hermes_tests.rs +++ b/crates/cli/tests/coverage/agents/hermes_tests.rs @@ -118,16 +118,20 @@ fn hook_command_round_trips_paths_and_platform_metacharacters() { let relay = Path::new("/tmp/NeMo $Relay`test'/bin/nemo-relay"); let generation = Path::new("/tmp/generation"); assert_eq!( - persistent_hook_command_for_platform(relay, generation, TEST_GENERATION_TOKEN, false), - "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:47632 --generation-file /tmp/generation --generation-token test-generation" + persistent_hook_commands_for_platform(relay, generation, TEST_GENERATION_TOKEN, false) + .for_event("on_session_start"), + "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:47632 --generation-file /tmp/generation --generation-token test-generation --fail-open" ); assert_eq!( - crate::hooks::decode_windows_hook_command(&persistent_hook_command_for_platform( - Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), - Path::new(r"C:\Temp\generation"), - TEST_GENERATION_TOKEN, - true, - )) + crate::hooks::decode_windows_hook_command( + persistent_hook_commands_for_platform( + Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), + Path::new(r"C:\Temp\generation"), + TEST_GENERATION_TOKEN, + true, + ) + .for_event("pre_tool_call") + ) .unwrap(), vec![ r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe", @@ -139,25 +143,29 @@ fn hook_command_round_trips_paths_and_platform_metacharacters() { r"C:\Temp\generation", "--generation-token", TEST_GENERATION_TOKEN, + "--fail-closed", ] ); assert_eq!( - crate::hooks::transparent_hook_forward_command_for_platform( + crate::hooks::transparent_hook_forward_commands_for_platform( relay, CodingAgent::Hermes, "http://127.0.0.1:1234", false, - ), - "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run" + ) + .for_event("on_session_start"), + "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run --fail-open" ); - let encoded = persistent_hook_command_for_platform( + let encoded = persistent_hook_commands_for_platform( Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), Path::new(r"C:\Temp\generation"), TEST_GENERATION_TOKEN, true, ); - assert!(is_persistent_relay_hook_command(&encoded)); - let encoded_codex = crate::hooks::persistent_hook_forward_command_for_platform( + assert!(is_persistent_relay_hook_command( + encoded.for_event("pre_tool_call") + )); + let encoded_codex = crate::hooks::persistent_hook_forward_commands_for_platform( Path::new(r"C:\Program Files\NeMo 100%\bin\nemo-relay.exe"), CodingAgent::Codex, Path::new(r"C:\Temp\generation"), @@ -165,7 +173,9 @@ fn hook_command_round_trips_paths_and_platform_metacharacters() { true, ); assert_ne!(encoded, encoded_codex); - assert!(!is_persistent_relay_hook_command(&encoded_codex)); + assert!(!is_persistent_relay_hook_command( + encoded_codex.for_event("PreToolUse") + )); } #[test] @@ -200,7 +210,7 @@ fn persistent_config_migrates_owned_state_and_preserves_unrelated_config() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let legacy_command = format!("{} hook-forward hermes", relay.display()); let mut legacy_hooks = serde_json::Map::new(); for event in CodingAgent::Hermes.hook_events() { @@ -264,7 +274,7 @@ fn persistent_config_migrates_owned_state_and_preserves_unrelated_config() { ); assert_eq!( merged["hooks"]["on_session_start"][1]["command"], - json!(command) + json!(command.for_event("on_session_start")) ); assert_eq!( merged["hooks"]["custom_event"][0]["command"], @@ -275,7 +285,7 @@ fn persistent_config_migrates_owned_state_and_preserves_unrelated_config() { assert_eq!( groups .iter() - .filter(|group| group["command"] == json!(command)) + .filter(|group| group["command"] == json!(command.for_event(event))) .count(), 1, "event {event}" @@ -288,7 +298,7 @@ fn persistent_config_rejects_a_foreign_server_with_the_reserved_name() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let existing = r#" model: keep-me mcp_servers: @@ -317,7 +327,7 @@ fn manual_same_named_mcp_and_hooks_are_never_claimed() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let manual = serde_yaml::to_string(&json!({ "mcp_servers": { MCP_SERVER_NAME: {"command": relay, "args": ["mcp"], "env": {"CUSTOM": "keep"}} @@ -373,21 +383,15 @@ fn modern_mcp_generation_proves_ownership_independently_of_hook_completeness() { let mut root = persistent_config( None, &relay, - &persistent_hook_command(&relay, &generation, "hook-token").unwrap(), + &persistent_hook_commands(&relay, &generation, "hook-token").unwrap(), &generation, "mcp-token", &[], ) .unwrap(); assert_eq!( - owned_install_command(&root, &relay, Some(&generation)) - .unwrap() - .as_deref(), - Some( - persistent_hook_command(&relay, &generation, "mcp-token") - .unwrap() - .as_str() - ) + owned_install_command(&root, &relay, Some(&generation)).unwrap(), + Some(persistent_hook_commands(&relay, &generation, "mcp-token").unwrap()) ); root["mcp_servers"][MCP_SERVER_NAME]["command"] = json!(temp.path().join("other/nemo-relay")); @@ -398,6 +402,43 @@ fn modern_mcp_generation_proves_ownership_independently_of_hook_completeness() { ); } +#[test] +fn persistent_config_migrates_modern_single_command_hooks_to_explicit_policies() { + let temp = tempfile::tempdir().unwrap(); + let relay = relay_binary(temp.path()); + let generation = temp.path().join(GENERATION_FILE_NAME); + let commands = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let mut installed = persistent_config( + None, + &relay, + &commands, + &generation, + TEST_GENERATION_TOKEN, + &[], + ) + .unwrap(); + let legacy = commands.legacy().unwrap(); + for event in CodingAgent::Hermes.hook_events() { + installed["hooks"][event][0]["command"] = json!(legacy); + } + + let migrated = persistent_config( + Some(&serde_yaml::to_string(&installed).unwrap()), + &relay, + &commands, + &generation, + TEST_GENERATION_TOKEN, + &[], + ) + .unwrap(); + + for event in CodingAgent::Hermes.hook_events() { + let hooks = migrated["hooks"][event].as_array().unwrap(); + assert_eq!(hooks.len(), 1, "event {event}"); + assert_eq!(hooks[0]["command"], json!(commands.for_event(event))); + } +} + #[test] fn foreign_reserved_server_aborts_install_before_any_file_changes() { let temp = tempfile::tempdir().unwrap(); @@ -425,7 +466,7 @@ fn trusted_hooks_migrates_only_relay_approvals_and_records_every_event() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let existing = json!({ "schema": 7, "approvals": [ @@ -435,9 +476,10 @@ fn trusted_hooks_migrates_only_relay_approvals_and_records_every_event() { ] }); let now = UNIX_EPOCH + Duration::from_secs(1_700_000_000); + let legacy = crate::hooks::GeneratedHookCommands::uniform("nemo-relay hook-forward hermes"); let merged = trusted_hooks( Some(&serde_json::to_string(&existing).unwrap()), - Some("nemo-relay hook-forward hermes"), + Some(&legacy), &command, &relay, now, @@ -455,7 +497,10 @@ fn trusted_hooks_migrates_only_relay_approvals_and_records_every_event() { for event in CodingAgent::Hermes.hook_events() { let entries = approvals .iter() - .filter(|entry| entry["event"] == json!(event) && entry["command"] == json!(command)) + .filter(|entry| { + entry["event"] == json!(event) + && entry["command"] == json!(command.for_event(event)) + }) .collect::>(); assert_eq!(entries.len(), 1, "event {event}"); assert_eq!( @@ -471,7 +516,7 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let mut config = persistent_config( None, &relay, @@ -481,7 +526,8 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { &[], ) .unwrap(); - config["hooks"]["unexpected_event"] = json!([{"command": command, "timeout": 30}]); + config["hooks"]["unexpected_event"] = + json!([{"command": command.for_event("on_session_start"), "timeout": 30}]); let error = verify_hook_definitions(&config, &command).unwrap_err(); assert!(error.contains("unexpected Relay hook")); let mut malformed = persistent_config( @@ -493,14 +539,15 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { &[], ) .unwrap(); - malformed["hooks"]["unexpected_event"] = json!({"command": command}); + malformed["hooks"]["unexpected_event"] = + json!({"command": command.for_event("on_session_start")}); let error = verify_hook_definitions(&malformed, &command).unwrap_err(); assert!(error.contains("must be an array")); let mut allowlist = trusted_hooks(None, None, &command, &relay, UNIX_EPOCH).unwrap(); allowlist["approvals"].as_array_mut().unwrap().push(json!({ "event": "unexpected_event", - "command": command, + "command": command.for_event("on_session_start"), "approved_at": "1970-01-01T00:00:00.000000Z" })); let path = temp.path().join("shell-hooks-allowlist.json"); @@ -513,12 +560,25 @@ fn verification_rejects_relay_handlers_and_approvals_on_unexpected_events() { .as_array_mut() .unwrap() .push(json!({ - "command": command, + "command": command.for_event("on_session_start"), "approved_at": "1970-01-01T00:00:00.000000Z" })); std::fs::write(&path, serde_json::to_vec(&missing_event).unwrap()).unwrap(); let error = verify_trust(&path, &command).unwrap_err(); assert!(error.contains("missing its event")); + + let mut wrong_policy = trusted_hooks(None, None, &command, &relay, UNIX_EPOCH).unwrap(); + wrong_policy["approvals"] + .as_array_mut() + .unwrap() + .push(json!({ + "event": "pre_tool_call", + "command": command.for_event("on_session_start"), + "approved_at": "1970-01-01T00:00:00.000000Z" + })); + std::fs::write(&path, serde_json::to_vec(&wrong_policy).unwrap()).unwrap(); + let error = verify_trust(&path, &command).unwrap_err(); + assert!(error.contains("unexpected Relay hook approval")); } #[test] @@ -526,7 +586,7 @@ fn hermes_structure_and_trust_validation_cover_exact_failure_shapes() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); let generation = temp.path().join(GENERATION_FILE_NAME); - let command = persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + let command = persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let error = trusted_hooks( Some(r#"{"approvals": {}}"#), @@ -617,13 +677,15 @@ fn install_is_verified_idempotent_and_rotates_the_generation() { let config = yaml(&paths.config); let second_command = - persistent_hook_command(&relay, &paths.generation, &second_generation).unwrap(); + persistent_hook_commands(&relay, &paths.generation, &second_generation).unwrap(); assert_eq!( config["hooks"]["on_session_start"] .as_array() .unwrap() .iter() - .filter(|group| group["command"] == json!(second_command)) + .filter(|group| { + group["command"] == json!(second_command.for_event("on_session_start")) + }) .count(), 1 ); @@ -954,6 +1016,41 @@ fn uninstall_removes_only_relay_owned_hermes_state() { assert_eq!(allowlist["approvals"][0]["command"], json!("custom-hook")); } +#[test] +fn uninstall_removes_orphaned_generated_approval_without_config() { + let temp = tempfile::tempdir().unwrap(); + let relay = relay_binary(temp.path()); + let paths = paths(&temp.path().join("hermes")); + let commands = + persistent_hook_commands(&relay, &paths.generation, TEST_GENERATION_TOKEN).unwrap(); + std::fs::create_dir_all(paths.allowlist.parent().unwrap()).unwrap(); + std::fs::write( + &paths.allowlist, + serde_json::to_vec(&json!({ + "approvals": [ + { + "event": "pre_tool_call", + "command": commands.for_event("pre_tool_call") + }, + { + "event": "custom_event", + "command": "custom-hook" + } + ] + })) + .unwrap(), + ) + .unwrap(); + + let removed = uninstall_persistent_with(paths.clone(), atomic_write).unwrap(); + + assert_eq!(removed, vec![paths.allowlist.clone()]); + assert_eq!( + json_file(&paths.allowlist)["approvals"], + json!([{"event": "custom_event", "command": "custom-hook"}]) + ); +} + #[test] fn uninstall_rolls_back_every_file_when_commit_fails() { let temp = tempfile::tempdir().unwrap(); @@ -1084,11 +1181,11 @@ fn persistent_state_detection_recognizes_each_relay_owned_surface() { &roots[2].config, serde_yaml::to_string(&json!({ "hooks": { - "on_session_start": [{"command": persistent_hook_command( + "on_session_start": [{"command": persistent_hook_commands( &relay, &roots[2].generation, TEST_GENERATION_TOKEN - ).unwrap()}] + ).unwrap().for_event("on_session_start")}] } })) .unwrap(), @@ -1099,11 +1196,11 @@ fn persistent_state_detection_recognizes_each_relay_owned_surface() { serde_json::to_vec(&json!({ "approvals": [{ "event": "on_session_start", - "command": persistent_hook_command( + "command": persistent_hook_commands( &relay, &roots[3].generation, TEST_GENERATION_TOKEN - ).unwrap() + ).unwrap().for_event("on_session_start") }] })) .unwrap(), @@ -1124,7 +1221,7 @@ fn persistent_state_detection_recognizes_each_relay_owned_surface() { fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() { let temp = tempfile::tempdir().unwrap(); let relay = relay_binary(temp.path()); - let command = crate::hooks::transparent_hook_forward_command( + let command = crate::hooks::transparent_hook_forward_commands( &relay, CodingAgent::Hermes, "http://127.0.0.1:1234", @@ -1132,7 +1229,7 @@ fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() .unwrap(); let generation = temp.path().join(GENERATION_FILE_NAME); let persistent_command = - persistent_hook_command(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); + persistent_hook_commands(&relay, &generation, TEST_GENERATION_TOKEN).unwrap(); let mut existing = persistent_config( None, &relay, @@ -1172,7 +1269,7 @@ fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() groups .iter() .filter_map(|group| group.get("command").and_then(Value::as_str)) - .filter(|candidate| **candidate == command) + .filter(|candidate| *candidate == command.for_event(event)) .count(), 1, "event {event}" @@ -1180,7 +1277,7 @@ fn transparent_config_suppresses_only_the_managed_mcp_and_uses_one_relay_hook() assert!( groups .iter() - .any(|group| group["command"] == json!(command)) + .any(|group| group["command"] == json!(command.for_event(event))) ); } assert!( @@ -1345,21 +1442,18 @@ fn hermes_uninstall_and_verification_reject_malformed_or_residual_state() { .unwrap(); let config = yaml(&hermes_paths.config); - let command = config["hooks"]["on_session_start"][0]["command"] - .as_str() - .unwrap() - .to_string(); let token = InstallGeneration::capture(hermes_paths.generation.clone()) .unwrap() .token() .to_owned(); + let command = persistent_hook_commands(&relay, &hermes_paths.generation, &token).unwrap(); let expected_environment = forwarded_environment_names(&[], None); let mut duplicate_hook = config.clone(); duplicate_hook["hooks"]["on_session_start"] .as_array_mut() .unwrap() - .push(json!({"command": command})); + .push(json!({"command": command.for_event("on_session_start")})); let error = verify_hook_definitions(&duplicate_hook, &command).unwrap_err(); assert!( error.contains("exactly one trusted Relay handler"), @@ -1406,15 +1500,12 @@ fn hermes_uninstall_and_verification_reject_malformed_or_residual_state() { atomic_write, ) .unwrap(); - let config = yaml(&hermes_paths.config); - let command = config["hooks"]["on_session_start"][0]["command"] - .as_str() - .unwrap() - .to_string(); let expected_token = InstallGeneration::capture(hermes_paths.generation.clone()) .unwrap() .token() .to_owned(); + let command = + persistent_hook_commands(&relay, &hermes_paths.generation, &expected_token).unwrap(); crate::installation::generation::write_new_generation(&hermes_paths.generation).unwrap(); let error = verify_install( &hermes_paths, @@ -1447,18 +1538,18 @@ fn hermes_uninstall_verifier_identifies_each_residual_owned_surface() { install_persistent_with(paths.clone(), &relay, &[], None, UNIX_EPOCH, atomic_write).unwrap(); let command = owned_command_from_config(&yaml(&paths.config), Some(&paths.generation)); - let error = verify_uninstall(&paths, command.as_deref()).unwrap_err(); + let error = verify_uninstall(&paths, command.as_ref()).unwrap_err(); assert!(error.contains("generation fence still exists"), "{error}"); std::fs::remove_file(&paths.generation).unwrap(); - let error = verify_uninstall(&paths, command.as_deref()).unwrap_err(); + let error = verify_uninstall(&paths, command.as_ref()).unwrap_err(); assert!( error.contains("managed Hermes Relay config still exists"), "{error}" ); std::fs::remove_file(&paths.config).unwrap(); - let error = verify_uninstall(&paths, command.as_deref()).unwrap_err(); + let error = verify_uninstall(&paths, command.as_ref()).unwrap_err(); assert!( error.contains("managed Hermes Relay trust approval still exists"), "{error}" diff --git a/crates/cli/tests/coverage/agents/plugin_host_tests.rs b/crates/cli/tests/coverage/agents/plugin_host_tests.rs index c44f13e99..9d6b15447 100644 --- a/crates/cli/tests/coverage/agents/plugin_host_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_host_tests.rs @@ -86,7 +86,7 @@ impl CodexHooksClient for FakeCodexHooksClient { } } -fn expected_plugin_command() -> String { +fn expected_plugin_command() -> crate::hooks::GeneratedHookCommands { let relay = current_exe().unwrap(); let relay = relay.canonicalize().unwrap_or(relay); let relay = portable_executable_path(relay); @@ -108,7 +108,7 @@ fn write_plugin_generation_for_hooks(path: &Path) { .unwrap(); } -fn expected_plugin_command_for_hooks(path: &Path) -> String { +fn expected_plugin_command_for_hooks(path: &Path, event: &str) -> String { fs::read_to_string(path) .ok() .and_then(|raw| serde_json::from_str::(&raw).ok()) @@ -116,8 +116,9 @@ fn expected_plugin_command_for_hooks(path: &Path) -> String { value .get("hooks")? .as_object()? - .values() - .next()? + .iter() + .find(|(name, _)| normalize_hook_event(name) == normalize_hook_event(event))? + .1 .as_array()? .first()? .get("hooks")? @@ -127,7 +128,7 @@ fn expected_plugin_command_for_hooks(path: &Path) -> String { .as_str() .map(str::to_owned) }) - .unwrap_or_else(expected_plugin_command) + .unwrap_or_else(|| expected_plugin_command().for_event(event).to_owned()) } fn empty_codex_hooks_client() -> FakeCodexHooksClient { @@ -143,7 +144,7 @@ fn write_plugin_hooks(plugin_root: &Path) -> PathBuf { write_plugin_generation_for_hooks(&path); fs::write( &path, - serde_json::to_vec_pretty(&generated_hooks( + serde_json::to_vec_pretty(&crate::hooks::generated_policy_hooks( CodingAgent::Codex, &expected_plugin_hook_command(&path).unwrap(), )) @@ -170,9 +171,9 @@ fn codex_hook_metadata( fs::create_dir_all(hooks_path.parent().unwrap()).unwrap(); fs::write( &hooks_path, - serde_json::to_vec_pretty(&generated_hooks( + serde_json::to_vec_pretty(&crate::hooks::generated_policy_hooks( CodingAgent::Codex, - &expected_plugin_command_for_hooks(&hooks_path), + &expected_plugin_command(), )) .unwrap(), ) @@ -182,7 +183,7 @@ fn codex_hook_metadata( key: key.into(), event_name: event_name.into(), handler_type: "command".into(), - command: Some(expected_plugin_command_for_hooks(&hooks_path)), + command: Some(expected_plugin_command_for_hooks(&hooks_path, event_name)), source_path: hooks_path.display().to_string(), source: "plugin".into(), plugin_id: Some(CODEX_PLUGIN_ID.into()), @@ -949,8 +950,13 @@ fn codex_auto_trust_rejects_modified_loaded_plugin_hook_file() { let config_path = dir.path().join("config.toml"); fs::write(&config_path, "").unwrap(); let mut hooks = required_codex_hook_metadata(&reported_hooks_path, "untrusted", true); + let expected = expected_plugin_command(); for hook in &mut hooks { - hook.command = Some(expected_plugin_command()); + hook.command = Some( + expected_codex_hook_command(&expected, &hook.event_name) + .unwrap() + .to_owned(), + ); } let mut client = FakeCodexHooksClient { hook_lists: VecDeque::from([Ok(hooks)]), @@ -1018,6 +1024,33 @@ fn codex_hook_trust_report_distinguishes_modified_disabled_and_missing_hooks() { ); } +#[test] +fn codex_hook_trust_report_recognizes_legacy_generated_hooks_as_stale() { + let dir = tempdir().unwrap(); + let hooks_path = dir.path().join(".codex").join("hooks.json"); + let expected = expected_plugin_command(); + let legacy = expected.legacy().unwrap().to_owned(); + let mut hooks = required_codex_hook_metadata(&hooks_path, "trusted", true); + for hook in &mut hooks { + hook.command = Some(legacy.clone()); + } + fs::write( + &hooks_path, + serde_json::to_vec_pretty(&generated_hooks(CodingAgent::Codex, &legacy)).unwrap(), + ) + .unwrap(); + let mut client = FakeCodexHooksClient { + hook_lists: VecDeque::from([Ok(hooks)]), + ..FakeCodexHooksClient::default() + }; + + let error = codex_hook_trust_report_with_client(&mut client, dir.path(), &expected) + .expect_err("legacy generated hooks must require reinstall"); + + assert!(error.contains("loaded modified Relay hooks"), "{error}"); + assert!(error.contains("install codex --force"), "{error}"); +} + #[test] fn codex_hook_state_key_path_quotes_arbitrary_hook_identity() { assert_eq!( @@ -1896,7 +1929,11 @@ fn codex_setup_can_validate_hooks_while_installer_holds_the_generation_lock() { fs::create_dir_all(hooks_path.parent().unwrap()).unwrap(); fs::write( &hooks_path, - serde_json::to_vec_pretty(&generated_hooks(CodingAgent::Codex, &command)).unwrap(), + serde_json::to_vec_pretty(&crate::hooks::generated_policy_hooks( + CodingAgent::Codex, + &command, + )) + .unwrap(), ) .unwrap(); let _transaction = @@ -2000,7 +2037,7 @@ fn codex_setup_uses_plugin_hooks_without_writing_user_hooks() { DEFAULT_URL, &expected_plugin_command(), |_home, _config, command| { - assert_eq!(command, expected_plugin_command()); + assert_eq!(command, &expected_plugin_command()); Ok(()) }, ) @@ -2876,12 +2913,10 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { r#""C:\Program Files\NeMo 100%%cd:~,%\bin\nemo-relay.exe""# ); assert_eq!( - crate::hooks::decode_windows_hook_command(&codex_plugin_hook_command_for_platform( - &relay, - &generation, - "test-generation", - true, - )) + crate::hooks::decode_windows_hook_command( + codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", true,) + .for_event("PreToolUse") + ) .unwrap(), vec![ relay.display().to_string(), @@ -2893,6 +2928,7 @@ fn windows_shell_argument_quoting_and_hook_encoding_preserve_paths() { generation.display().to_string(), "--generation-token".into(), "test-generation".into(), + "--fail-closed".into(), ] ); assert_eq!( @@ -2913,7 +2949,10 @@ fn generated_windows_hook_command_executes_exact_arguments() { let marker = temp.path().join("hook-ran.txt"); let input_marker = temp.path().join("hook-input.txt"); let generation = temp.path().join("Generation & %USERPROFILE%"); - let command = codex_plugin_hook_command(&relay, &generation, "test-generation").unwrap(); + let command = codex_plugin_hook_command(&relay, &generation, "test-generation") + .unwrap() + .for_event("PreToolUse") + .to_owned(); let mut child = std::process::Command::new("cmd.exe") .arg("/C") .arg(&command) @@ -2950,7 +2989,10 @@ fn generated_windows_hook_command_propagates_the_relay_exit_code() { let relay = temp.path().join("relay failure.exe"); compile_windows_hook_test_relay(&relay); let generation = temp.path().join("generation"); - let command = codex_plugin_hook_command(&relay, &generation, "test-generation").unwrap(); + let command = codex_plugin_hook_command(&relay, &generation, "test-generation") + .unwrap() + .for_event("PreToolUse") + .to_owned(); let status = std::process::Command::new("cmd.exe") .arg("/C") @@ -2990,8 +3032,9 @@ fn posix_shell_argument_quoting_and_hook_encoding_preserve_paths() { "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay'" ); assert_eq!( - codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", false), - "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:47632 --generation-file '/tmp/NeMo $Relay`test'\\''/plugin/.nemo-relay-generation' --generation-token test-generation" + codex_plugin_hook_command_for_platform(&relay, &generation, "test-generation", false) + .for_event("SessionStart"), + "'/tmp/NeMo $Relay`test'\\''/bin/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:47632 --generation-file '/tmp/NeMo $Relay`test'\\''/plugin/.nemo-relay-generation' --generation-token test-generation --fail-open" ); assert_eq!(shell_quote_arg_for_platform("", false), "''"); assert_eq!( @@ -3780,7 +3823,7 @@ fn plugin_host_entrypoints_reject_unsupported_agents_and_report_json() { "SessionStart": [{ "hooks": [{ "type": "command", - "command": expected_plugin_command(), + "command": expected_plugin_command().for_event("SessionStart"), "timeout": 30 }] }] diff --git a/crates/cli/tests/coverage/agents/plugin_install_tests.rs b/crates/cli/tests/coverage/agents/plugin_install_tests.rs index 8af743d26..d20daa95b 100644 --- a/crates/cli/tests/coverage/agents/plugin_install_tests.rs +++ b/crates/cli/tests/coverage/agents/plugin_install_tests.rs @@ -1420,13 +1420,14 @@ fn plugin_manifests_and_hooks_use_path_based_relay_command() { ) .unwrap()["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!( - crate::hooks::persistent_hook_forward_command( + crate::hooks::persistent_hook_forward_commands( Path::new("/bin/nemo-relay"), CodingAgent::Codex, &generation_fence, TEST_GENERATION_TOKEN, ) .unwrap() + .for_event("SessionStart") ) ); assert_eq!( @@ -1438,13 +1439,14 @@ fn plugin_manifests_and_hooks_use_path_based_relay_command() { ) .unwrap()["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!( - crate::hooks::persistent_hook_forward_command( + crate::hooks::persistent_hook_forward_commands( Path::new("/bin/nemo-relay"), CodingAgent::ClaudeCode, &generation_fence, TEST_GENERATION_TOKEN, ) .unwrap() + .for_event("SessionStart") ) ); } @@ -1471,13 +1473,14 @@ fn relay_identity_prefers_the_path_resolved_executable() { ) .unwrap()["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!( - crate::hooks::persistent_hook_forward_command( + crate::hooks::persistent_hook_forward_commands( &relay, CodingAgent::Codex, &generation, TEST_GENERATION_TOKEN, ) .unwrap() + .for_event("SessionStart") ) ); assert_eq!( diff --git a/crates/cli/tests/coverage/shared/bootstrap_tests.rs b/crates/cli/tests/coverage/shared/bootstrap_tests.rs index 24f8feaf4..15ab05263 100644 --- a/crates/cli/tests/coverage/shared/bootstrap_tests.rs +++ b/crates/cli/tests/coverage/shared/bootstrap_tests.rs @@ -88,17 +88,21 @@ fn compatible_gateway_is_reused_without_starting_another_process() { #[test] fn foreign_and_incompatible_listeners_are_never_adopted() { crate::test_support::enable_operational_logs(); + let incompatible = format!( + "{{\"status\":\"incompatible\",\"service\":\"nemo-relay\",\"version\":\"other\",\"bootstrap_protocol\":{BOOTSTRAP_PROTOCOL_VERSION},\"instance_id\":\"other\"}}" + ); + let previous_protocol = format!( + "{{\"status\":\"ok\",\"service\":\"nemo-relay\",\"version\":\"{}\",\"bootstrap_protocol\":{},\"instance_id\":\"previous\"}}", + env!("CARGO_PKG_VERSION"), + BOOTSTRAP_PROTOCOL_VERSION - 1, + ); for (status, body, expected) in [ - ("200 OK", "{}", "not a compatible"), - ( - "409 Conflict", - "{\"status\":\"incompatible\",\"service\":\"nemo-relay\",\"version\":\"other\",\"bootstrap_protocol\":2,\"instance_id\":\"other\"}", - "different version", - ), + ("200 OK", "{}".to_string(), "not a compatible"), + ("409 Conflict", incompatible, "different version"), + ("200 OK", previous_protocol, "not a compatible"), ] { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let address = listener.local_addr().unwrap(); - let body = body.to_string(); listener.set_nonblocking(true).unwrap(); let (stop_tx, stop_rx) = std::sync::mpsc::channel(); let server = std::thread::spawn(move || { diff --git a/crates/cli/tests/coverage/shared/installer_tests.rs b/crates/cli/tests/coverage/shared/installer_tests.rs index f77f7d74d..40d82f12d 100644 --- a/crates/cli/tests/coverage/shared/installer_tests.rs +++ b/crates/cli/tests/coverage/shared/installer_tests.rs @@ -92,7 +92,7 @@ async fn transparent_hook_delivery_authenticates_the_wrapper_gateway() { profile: None, session_metadata: None, gateway_mode: None, - fail_closed: true, + failure_policy: HookFailurePolicy::FailClosed, }; let gateway = transparent_gateway_spec(&gateway_url).unwrap(); @@ -329,42 +329,39 @@ fn helper_formatting_and_headers_cover_optional_paths() { #[test] fn generated_hook_dispatch_covers_all_agents() { - for agent in [ - CodingAgent::ClaudeCode, - CodingAgent::Codex, - CodingAgent::Hermes, - ] { - assert!(generated_hooks(agent, "cmd")["hooks"].is_object()); - } + assert_generated_hook_policies(); assert_eq!( - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( Path::new("nemo-relay"), CodingAgent::Hermes, "http://127.0.0.1:1234", false, - ), - "nemo-relay hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run" + ) + .for_event("on_session_start"), + "nemo-relay hook-forward hermes --gateway-url http://127.0.0.1:1234 --transparent-run --fail-open" ); assert_eq!( - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( Path::new("/abs/path/to/nemo-relay"), CodingAgent::Codex, "http://127.0.0.1:1234", false, - ), - "/abs/path/to/nemo-relay hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run" + ) + .for_event("PreToolUse"), + "/abs/path/to/nemo-relay hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run --fail-closed" ); let relay = Path::new("/opt/NeMo Relay's & tools/nemo-relay"); assert_eq!( - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( relay, CodingAgent::Codex, "http://127.0.0.1:1234", false - ), - r#"'/opt/NeMo Relay'\''s & tools/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run"# + ) + .for_event("SessionStart"), + r#"'/opt/NeMo Relay'\''s & tools/nemo-relay' hook-forward codex --gateway-url http://127.0.0.1:1234 --transparent-run --fail-open"# ); - let native = transparent_hook_forward_command( + let native = transparent_hook_forward_commands( Path::new("nemo-relay"), CodingAgent::Hermes, "http://127.0.0.1:1234", @@ -372,7 +369,7 @@ fn generated_hook_dispatch_covers_all_agents() { .unwrap(); if cfg!(windows) { assert_eq!( - decode_windows_hook_command(&native).unwrap(), + decode_windows_hook_command(native.for_event("on_session_start")).unwrap(), vec![ String::from("nemo-relay"), String::from("hook-forward"), @@ -380,12 +377,13 @@ fn generated_hook_dispatch_covers_all_agents() { String::from("--gateway-url"), String::from("http://127.0.0.1:1234"), String::from("--transparent-run"), + String::from("--fail-open"), ] ); } else { assert_eq!( native, - transparent_hook_forward_command_for_platform( + transparent_hook_forward_commands_for_platform( Path::new("nemo-relay"), CodingAgent::Hermes, "http://127.0.0.1:1234", @@ -393,12 +391,13 @@ fn generated_hook_dispatch_covers_all_agents() { ) ); } - let windows = transparent_hook_forward_command_for_platform( + let windows = transparent_hook_forward_commands_for_platform( relay, CodingAgent::ClaudeCode, "http://127.0.0.1:1234", true, ); + let windows = windows.for_event("PreToolUse"); let (launcher, encoded) = windows.rsplit_once(' ').unwrap(); assert_eq!( launcher, @@ -412,7 +411,7 @@ fn generated_hook_dispatch_covers_all_agents() { || matches!(character, '+' | '/' | '=')) ); assert_eq!( - decode_windows_hook_command(&windows).unwrap(), + decode_windows_hook_command(windows).unwrap(), vec![ relay.display().to_string(), "hook-forward".into(), @@ -420,6 +419,7 @@ fn generated_hook_dispatch_covers_all_agents() { "--gateway-url".into(), "http://127.0.0.1:1234".into(), "--transparent-run".into(), + "--fail-closed".into(), ] ); assert!(decode_windows_hook_command("powershell.exe -EncodedCommand invalid").is_none()); @@ -446,6 +446,38 @@ fn generated_hook_dispatch_covers_all_agents() { assert!(error.contains("shorten the Relay or plugin installation path")); } +fn assert_generated_hook_policies() { + for agent in [ + CodingAgent::ClaudeCode, + CodingAgent::Codex, + CodingAgent::Hermes, + ] { + assert!(generated_hooks(agent, "cmd")["hooks"].is_object()); + let commands = GeneratedHookCommands::new("cmd --fail-open", "cmd --fail-closed"); + let generated = generated_policy_hooks(agent, &commands); + for event in agent.hook_events() { + let command = if agent.uses_direct_hook_entries() { + generated["hooks"][event][0]["command"].as_str() + } else { + generated["hooks"][event][0]["hooks"][0]["command"].as_str() + } + .unwrap(); + assert_eq!( + command, + commands.for_event(event), + "unexpected policy for {} {event}", + agent.label() + ); + assert_eq!( + command.ends_with("--fail-closed"), + event_requires_fail_closed(event), + "unexpected enforcement classification for {} {event}", + agent.label() + ); + } + } +} + #[test] fn codex_generation_uses_exactly_the_supported_hook_schema() { let generated = generated_hooks(CodingAgent::Codex, "cmd"); @@ -521,34 +553,46 @@ fn packaged_plugin_hooks_use_expected_forwarding_commands() { assert_eq!( claude["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!(format!( - "nemo-relay hook-forward claude --gateway-url {} --forward-only", + "nemo-relay hook-forward claude --gateway-url {} --forward-only --fail-open", crate::bootstrap::DEFAULT_URL )) ); assert_eq!( codex["hooks"]["SessionStart"][0]["hooks"][0]["command"], json!(format!( - "nemo-relay hook-forward codex --gateway-url {} --forward-only", + "nemo-relay hook-forward codex --gateway-url {} --forward-only --fail-open", crate::bootstrap::DEFAULT_URL )) ); assert_eq!( claude["hooks"], - generated_hooks( + generated_policy_hooks( CodingAgent::ClaudeCode, - &format!( - "nemo-relay hook-forward claude --gateway-url {} --forward-only", - crate::bootstrap::DEFAULT_URL + &GeneratedHookCommands::new( + format!( + "nemo-relay hook-forward claude --gateway-url {} --forward-only --fail-open", + crate::bootstrap::DEFAULT_URL + ), + format!( + "nemo-relay hook-forward claude --gateway-url {} --forward-only --fail-closed", + crate::bootstrap::DEFAULT_URL + ), ), )["hooks"] ); assert_eq!( codex["hooks"], - generated_hooks( + generated_policy_hooks( CodingAgent::Codex, - &format!( - "nemo-relay hook-forward codex --gateway-url {} --forward-only", - crate::bootstrap::DEFAULT_URL + &GeneratedHookCommands::new( + format!( + "nemo-relay hook-forward codex --gateway-url {} --forward-only --fail-open", + crate::bootstrap::DEFAULT_URL + ), + format!( + "nemo-relay hook-forward codex --gateway-url {} --forward-only --fail-closed", + crate::bootstrap::DEFAULT_URL + ), ), )["hooks"] ); diff --git a/crates/cli/tests/fixtures/windows_hook_relay.rs b/crates/cli/tests/fixtures/windows_hook_relay.rs index e8a03b5d0..cedd3c885 100644 --- a/crates/cli/tests/fixtures/windows_hook_relay.rs +++ b/crates/cli/tests/fixtures/windows_hook_relay.rs @@ -16,6 +16,7 @@ fn main() { generation, OsString::from("--generation-token"), OsString::from("test-generation"), + OsString::from("--fail-closed"), ]; let actual = std::env::args_os().skip(1).collect::>(); if actual != expected { diff --git a/crates/core/src/api/shared.rs b/crates/core/src/api/shared.rs index 2f823a27d..dadd33c69 100644 --- a/crates/core/src/api/shared.rs +++ b/crates/core/src/api/shared.rs @@ -220,6 +220,11 @@ pub(crate) fn metadata_with_otel_error(metadata: Option, error: &FlowError metadata .entry("error.type".to_string()) .or_insert_with(|| Json::String(error.otel_error_type().to_string())); + if let Some(exception_type) = error.exception_type() { + metadata + .entry("exception.type".to_string()) + .or_insert_with(|| Json::String(exception_type.to_string())); + } } metadata } diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index 270f9737a..23ed1c01a 100644 --- a/crates/core/src/error.rs +++ b/crates/core/src/error.rs @@ -129,6 +129,15 @@ pub enum FlowError { /// An internal runtime error (e.g., lock poisoning). #[error("internal error: {0}")] Internal(String), + + /// An exception raised by a language-binding callback. + #[error("internal error: {message}")] + CallbackException { + /// Original binding-rendered exception message. + message: String, + /// Original language exception class name. + exception_type: String, + }, } /// A specialized [`Result`](std::result::Result) type for NeMo Relay operations. @@ -158,7 +167,15 @@ impl FlowError { UpstreamFailureClass::InvalidRequest => "invalid_request", UpstreamFailureClass::Other => "upstream_error", }, - Self::Internal(_) => "internal_error", + Self::Internal(_) | Self::CallbackException { .. } => "internal_error", + } + } + + /// Returns the originating language exception class, when available. + pub(crate) fn exception_type(&self) -> Option<&str> { + match self { + Self::CallbackException { exception_type, .. } => Some(exception_type), + _ => None, } } } diff --git a/crates/core/src/logging/mod.rs b/crates/core/src/logging/mod.rs index 9b3c7528c..4d06f5806 100644 --- a/crates/core/src/logging/mod.rs +++ b/crates/core/src/logging/mod.rs @@ -13,7 +13,7 @@ mod sink; use std::io::{self, Write}; use std::path::Path; -use std::sync::{Arc, Mutex, MutexGuard}; +use std::sync::{Arc, Mutex, MutexGuard, Weak}; use spdlog::sink::Sink; use spdlog::{Logger, ThreadPool}; @@ -33,6 +33,8 @@ use sink::log_level_filter; pub(crate) use format::format_event_for_test; static LOGGER_LIFECYCLE_LOCK: Mutex<()> = Mutex::new(()); +static DEFAULT_LOGGING_RUNTIME: Mutex> = Mutex::new(None); +static ACTIVE_RELAY_LOGGER: Mutex>> = Mutex::new(None); fn lock_logger_lifecycle() -> MutexGuard<'static, ()> { LOGGER_LIFECYCLE_LOCK @@ -44,6 +46,32 @@ fn log_crate_proxy_is_installed() -> bool { std::ptr::addr_eq(log::logger(), spdlog::log_crate_proxy() as &dyn log::Log) } +fn active_relay_logger_exists() -> bool { + ACTIVE_RELAY_LOGGER + .lock() + .unwrap_or_else(|error| error.into_inner()) + .as_ref() + .is_some_and(|logger| logger.upgrade().is_some()) +} + +fn set_active_relay_logger(logger: &Arc) { + *ACTIVE_RELAY_LOGGER + .lock() + .unwrap_or_else(|error| error.into_inner()) = Some(Arc::downgrade(logger)); +} + +fn clear_active_relay_logger(logger: &Arc) { + let mut active = ACTIVE_RELAY_LOGGER + .lock() + .unwrap_or_else(|error| error.into_inner()); + if active + .as_ref() + .is_some_and(|current| Weak::ptr_eq(current, &Arc::downgrade(logger))) + { + *active = None; + } +} + fn install_log_crate_proxy() -> Result<()> { match spdlog::init_log_crate_proxy() { Ok(()) => Ok(()), @@ -75,17 +103,22 @@ impl LoggingRuntime { /// opened. Dropping the returned runtime flushes sinks and detaches its logger from the /// process-global `log` proxy when it is still installed. pub fn configure(config: LoggingConfig) -> Result { - let root_relay_id = Uuid::now_v7().to_string(); - let (logger, thread_pools) = build_logger(&config, root_relay_id.clone())?; - // Install once per process. Subsequent calls (tests / re-entry) reuse the proxy and swap // the receiver logger. A different global logger would prevent Relay sinks from receiving // `log` facade records, so fail instead of returning a nonfunctional runtime. let _lifecycle = lock_logger_lifecycle(); + Self::configure_with_lifecycle_lock(config) + } + + fn configure_with_lifecycle_lock(config: LoggingConfig) -> Result { + let root_relay_id = Uuid::now_v7().to_string(); + let (logger, thread_pools) = build_logger(&config, root_relay_id.clone())?; + install_log_crate_proxy()?; spdlog::log_crate_proxy().set_logger(Some(Arc::clone(&logger))); spdlog::log_crate_proxy().set_filter(None); log::set_max_level(log_level_filter(config.level)); + set_active_relay_logger(&logger); log::info!( target: "nemo_relay.logging", @@ -154,7 +187,10 @@ impl Drop for LoggingRuntime { if let Some(logger) = detached && !Arc::ptr_eq(&logger, &self.logger) { - spdlog::log_crate_proxy().set_logger(Some(logger)); + spdlog::log_crate_proxy().set_logger(Some(Arc::clone(&logger))); + set_active_relay_logger(&logger); + } else { + clear_active_relay_logger(&self.logger); } } } @@ -170,6 +206,51 @@ pub fn init_logging(config: &LoggingConfig) -> Result { LoggingRuntime::configure(config.clone()) } +/// Installs and retains the default process-wide logging runtime for a language binding. +/// +/// Configuration is resolved from the supported logging environment variables, with built-in +/// defaults when none are present. Repeated initialization in the same linked runtime is a no-op. +#[doc(hidden)] +pub fn initialize_default_logging() -> Result<()> { + let mut runtime = DEFAULT_LOGGING_RUNTIME.lock().map_err(|error| { + FlowError::Internal(format!("default logging runtime lock poisoned: {error}")) + })?; + if runtime.is_none() { + let config = LoggingConfig::from_environment()?; + let uses_default_config = config.is_none(); + let _lifecycle = lock_logger_lifecycle(); + if uses_default_config && active_relay_logger_exists() { + return Ok(()); + } + match LoggingRuntime::configure_with_lifecycle_lock(config.unwrap_or_default()) { + Ok(configured) => *runtime = Some(configured), + // Language bindings initialize logging automatically. When Relay was not explicitly + // configured, defer to an application logger that already owns the process facade. + Err(FlowError::AlreadyExists(_)) if uses_default_config => {} + Err(error) => return Err(error), + } + } + Ok(()) +} + +/// Shuts down and releases the default process-wide logging runtime for a language binding. +/// +/// Repeated shutdown in the same linked runtime is a no-op. The runtime is removed from shared +/// state before its sinks are drained so shutdown does not hold the default-runtime lock. +#[doc(hidden)] +pub fn shutdown_default_logging() -> Result<()> { + let runtime = DEFAULT_LOGGING_RUNTIME + .lock() + .map_err(|error| { + FlowError::Internal(format!("default logging runtime lock poisoned: {error}")) + })? + .take(); + if let Some(runtime) = runtime { + runtime.shutdown(); + } + Ok(()) +} + #[cfg(test)] #[path = "../../tests/coverage/logging_tests.rs"] mod tests; diff --git a/crates/core/src/observability/otel.rs b/crates/core/src/observability/otel.rs index 362ea5ca3..5a80992de 100644 --- a/crates/core/src/observability/otel.rs +++ b/crates/core/src/observability/otel.rs @@ -948,6 +948,8 @@ pub(super) struct ActiveSpan { span_context: SpanContext, start_model_name: Option, projected_attributes: Vec, + descendant_error_type: Option, + descendant_exception_type: Option, } pub(super) struct OtelEventProcessor { @@ -1151,12 +1153,15 @@ impl OtelEventProcessor { span_context, start_model_name, projected_attributes, + descendant_error_type: None, + descendant_exception_type: None, }, ); } fn process_end(&mut self, event: &Event) { let Some(mut active_span) = self.active_spans.remove(&event.uuid()) else { + self.propagate_suppressed_error_metadata(event); return; }; self.record_completed_span_context(event.uuid(), active_span.span_context.clone()); @@ -1167,6 +1172,36 @@ impl OtelEventProcessor { OpenTelemetryType::GenAi => super::otel_genai::end_attributes(event), OpenTelemetryType::OpenInference => super::openinference::end_attributes(event), }; + let is_error = metadata_string(event, "otel.status_code") == Some("ERROR"); + let explicit_error_type = metadata_string(event, "error.type"); + let error_type = is_error.then(|| { + explicit_error_type + .map(ToOwned::to_owned) + .or(active_span.descendant_error_type.take()) + .unwrap_or_else(|| "_OTHER".to_string()) + }); + let exception_type = is_error + .then(|| { + metadata_string(event, "exception.type") + .map(ToOwned::to_owned) + .or(active_span.descendant_exception_type.take()) + }) + .flatten(); + if matches!( + self.otel_type, + OpenTelemetryType::Full | OpenTelemetryType::GenAi + ) && let Some(error_type) = error_type.as_ref() + { + attributes.retain(|attribute| attribute.key.as_str() != "error.type"); + attributes.push(KeyValue::new("error.type", error_type.clone())); + } + if let Some(exception_type) = exception_type.as_ref() { + active_span.span.add_event_with_timestamp( + "exception", + to_system_time(*event.timestamp()), + vec![KeyValue::new("exception.type", exception_type.clone())], + ); + } let end_model_name = model_name_for_llm_event(event).or_else(|| active_span.start_model_name.take()); if self.otel_type == OpenTelemetryType::Full @@ -1187,12 +1222,40 @@ impl OtelEventProcessor { &self.attribute_mappings, )); } + if is_error && let Some(parent_span) = self.find_parent_span_mut(event) { + if parent_span.descendant_error_type.is_none() { + parent_span.descendant_error_type = error_type; + } + if parent_span.descendant_exception_type.is_none() { + parent_span.descendant_exception_type = exception_type; + } + } active_span.span.set_attributes(attributes); active_span .span .end_with_timestamp(to_system_time(*event.timestamp())); } + fn propagate_suppressed_error_metadata(&mut self, event: &Event) { + if self.otel_type != OpenTelemetryType::GenAi + || !self.suppressed_parent_contexts.contains_key(&event.uuid()) + || metadata_string(event, "otel.status_code") != Some("ERROR") + { + return; + } + let error_type = metadata_string(event, "error.type").map(ToOwned::to_owned); + let exception_type = metadata_string(event, "exception.type").map(ToOwned::to_owned); + let Some(parent_span) = self.find_parent_span_mut(event) else { + return; + }; + if parent_span.descendant_error_type.is_none() { + parent_span.descendant_error_type = error_type; + } + if parent_span.descendant_exception_type.is_none() { + parent_span.descendant_exception_type = exception_type; + } + } + fn process_mark(&mut self, event: &Event) { if self.otel_type == OpenTelemetryType::GenAi { return; @@ -1310,9 +1373,16 @@ impl OtelEventProcessor { } fn parent_span_uuid(&self, event: &Event) -> Option { - event - .parent_uuid() - .filter(|uuid| self.active_spans.contains_key(uuid)) + let parent_uuid = event.parent_uuid()?; + if self.active_spans.contains_key(&parent_uuid) { + return Some(parent_uuid); + } + let suppressed_parent = self.suppressed_parent_contexts.get(&parent_uuid)?; + self.active_spans.iter().find_map(|(uuid, active_span)| { + (active_span.span_context.trace_id() == suppressed_parent.trace_id() + && active_span.span_context.span_id() == suppressed_parent.span_id()) + .then_some(*uuid) + }) } fn find_parent_span(&self, event: &Event) -> Option<&ActiveSpan> { @@ -1368,6 +1438,10 @@ impl OtelEventProcessor { } } +fn metadata_string<'a>(event: &'a Event, key: &str) -> Option<&'a str> { + event.metadata()?.get(key)?.as_str() +} + fn span_kind(event: &Event) -> SpanKind { match semantic_scope_type(event) { Some(ScopeType::Llm) => SpanKind::Client, diff --git a/crates/core/src/observability/plugin_component.rs b/crates/core/src/observability/plugin_component.rs index b622f3016..32101a29c 100644 --- a/crates/core/src/observability/plugin_component.rs +++ b/crates/core/src/observability/plugin_component.rs @@ -21,6 +21,7 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::future::Future; +use std::net::IpAddr; use std::panic::{AssertUnwindSafe, catch_unwind}; use std::path::{Component, Path, PathBuf}; use std::pin::Pin; @@ -62,8 +63,9 @@ use crate::observability::{ use crate::plugin::{ ATIF_RUNTIME_DELIVERY_FAILURE_MARKER, ConfigDiagnostic, ConfigPolicy, DiagnosticLevel, OTEL_RUNTIME_DELIVERY_FAILURE_MARKER, Plugin, PluginComponentSpec, PluginError, - PluginRegistration, PluginRegistrationContext, Result as PluginResult, UnsupportedBehavior, - apply_global_config_policy, deregister_plugin, register_builtin_plugin, + PluginRegistration, PluginRegistrationCleanupOutcome, PluginRegistrationContext, + Result as PluginResult, UnsupportedBehavior, apply_global_config_policy, deregister_plugin, + register_builtin_plugin, }; use crate::plugin::{RuntimeDiagnostic, record_active_plugin_runtime_diagnostic}; @@ -891,41 +893,62 @@ fn register_atif_dispatcher( ); ctx.register_subscriber("atif", dispatcher)?; let shutdown_storage = Arc::clone(&storage); - ctx.add_registration(PluginRegistration::new( + ctx.add_registration(PluginRegistration::new_with_outcome( "observability", ctx.qualify_name("atif.shutdown"), Box::new(move || { - let work = { - let mut guard = manager.lock().map_err(|err| { - PluginError::Internal(format!("ATIF dispatcher lock poisoned: {err}")) - })?; - guard.flush_open_agents() - }; - for (scope_uuid, name) in work.scope_subscribers { - deregister_atif_shutdown_subscriber(&scope_uuid, &name)?; - } - for export in work.exports { - let write = prepare_atif_shutdown_file(&export, Arc::clone(&manager)) - .map_err(observability_registration_error)?; - let agent_uuid = write.agent_uuid; - let targets = { - let guard = manager.lock().map_err(|err| { + let work = match (|| -> PluginResult<_> { + let work = { + let mut guard = manager.lock().map_err(|err| { PluginError::Internal(format!("ATIF dispatcher lock poisoned: {err}")) })?; - guard.sink_targets() + guard.flush_open_agents() }; - let results = write_atif(&write, shutdown_storage.as_slice(), &targets); - let mut guard = manager.lock().map_err(|err| { - PluginError::Internal(format!("ATIF dispatcher lock poisoned: {err}")) - })?; - let _ = guard.complete_scope_write(agent_uuid, results); + for (scope_uuid, name) in &work.scope_subscribers { + deregister_atif_shutdown_subscriber(scope_uuid, name)?; + } + Ok(work) + })() { + Ok(work) => work, + Err(error) => return PluginRegistrationCleanupOutcome::NotRemoved(error), + }; + + let delivery = (|| -> PluginResult<()> { + for export in work.exports { + let write = prepare_atif_shutdown_file(&export, Arc::clone(&manager)) + .map_err(observability_registration_error)?; + let agent_uuid = write.agent_uuid; + let targets = { + let guard = manager.lock().map_err(|err| { + PluginError::Internal(format!("ATIF dispatcher lock poisoned: {err}")) + })?; + guard.sink_targets() + }; + let results = write_atif(&write, shutdown_storage.as_slice(), &targets); + let mut guard = manager.lock().map_err(|err| { + PluginError::Internal(format!("ATIF dispatcher lock poisoned: {err}")) + })?; + let _ = guard.complete_scope_write(agent_uuid, results); + } + Ok(()) + })(); + if let Err(error) = delivery { + return PluginRegistrationCleanupOutcome::RemovedWithError(error); + } + let guard = match manager.lock() { + Ok(guard) => guard, + Err(error) => { + return PluginRegistrationCleanupOutcome::RemovedWithError( + PluginError::Internal(format!("ATIF dispatcher lock poisoned: {error}")), + ); + } + }; + match guard.last_error_result() { + Ok(()) => PluginRegistrationCleanupOutcome::Removed, + Err(error) => PluginRegistrationCleanupOutcome::RemovedWithError( + observability_registration_error(error), + ), } - let guard = manager.lock().map_err(|err| { - PluginError::Internal(format!("ATIF dispatcher lock poisoned: {err}")) - })?; - guard - .last_error_result() - .map_err(observability_registration_error) }), )); Ok(()) @@ -998,10 +1021,20 @@ fn register_opentelemetry( // Retain the subscribers as long as the registered fan-out callback exists. // Their tracer providers and exporter runtimes must outlive event delivery. let delivery_subscribers = subscribers.clone(); - ctx.add_registration(PluginRegistration::new( + ctx.add_registration(PluginRegistration::new_with_outcome( "observability", ctx.qualify_name("opentelemetry.shutdown"), - Box::new(move || shutdown_opentelemetry_subscribers(&subscribers).map_or(Ok(()), Err)), + Box::new( + move || match shutdown_opentelemetry_subscribers(&subscribers) { + None => PluginRegistrationCleanupOutcome::Removed, + Some(OpenTelemetryShutdownFailure::Delivery(error)) => { + PluginRegistrationCleanupOutcome::RemovedWithError(error) + } + Some(OpenTelemetryShutdownFailure::Other(error)) => { + PluginRegistrationCleanupOutcome::NotRemoved(error) + } + }, + ), )); ctx.register_subscriber( "opentelemetry", @@ -1058,9 +1091,14 @@ fn build_opentelemetry_subscribers( Ok(subscribers) } +enum OpenTelemetryShutdownFailure { + Delivery(PluginError), + Other(PluginError), +} + fn shutdown_opentelemetry_subscribers( subscribers: &[Arc], -) -> Option { +) -> Option { let mut errors = Vec::new(); if let Err(error) = flush_subscribers() { errors.push(crate::observability::otel::OpenTelemetryError::Core(error)); @@ -1085,7 +1123,12 @@ fn shutdown_opentelemetry_subscribers( } else { format!("OpenTelemetry shutdown failures: {summary}") }; - Some(PluginError::RegistrationFailed(message)) + let error = PluginError::RegistrationFailed(message); + Some(if all_delivery_failures { + OpenTelemetryShutdownFailure::Delivery(error) + } else { + OpenTelemetryShutdownFailure::Other(error) + }) } fn shutdown_opentelemetry_providers( @@ -1623,16 +1666,31 @@ fn render_atif_filename( })?; let expression = rendered[selector_start..end].to_string(); let (selector, fallback) = parse_atif_metadata_expression(&expression)?; - let value = selector - .split('.') - .fold(metadata, |value, segment| value?.get(segment)) - .and_then(Json::as_str) - .or(fallback) - .ok_or_else(|| { + let mut resolved = metadata; + for segment in selector.split('.') { + resolved = match resolved { + Some(Json::Object(object)) => object.get(segment), + None | Some(Json::Null) => break, + Some(_) => { + return Err(format!( + "filename_template placeholder '{{metadata.{selector}}}' traversed a non-object value" + )); + } + }; + } + let value = match resolved { + Some(Json::String(value)) => value.as_str(), + None | Some(Json::Null) => fallback.ok_or_else(|| { format!( "filename_template placeholder '{{metadata.{selector}}}' must resolve to a string" ) - })?; + })?, + Some(_) => { + return Err(format!( + "filename_template placeholder '{{metadata.{selector}}}' resolved to a non-string value" + )); + } + }; if !is_safe_atif_metadata_path(value) { return Err(format!( "metadata path '{selector}' must be a path-safe relative fragment" @@ -2377,6 +2435,23 @@ struct OpenTelemetryDestinationCollision { message: String, } +#[derive(Debug, PartialEq, Eq)] +enum OpenTelemetryDestinationKey { + Url { + scheme: String, + host: String, + port: Option, + path: String, + query: Option, + }, + Raw(String), +} + +struct OpenTelemetryDestination { + key: OpenTelemetryDestinationKey, + display: String, +} + fn validate_distinct_opentelemetry_destinations( endpoints: &[OpenTelemetryEndpointConfig], ) -> PluginResult<()> { @@ -2398,7 +2473,7 @@ fn opentelemetry_destination_collision_errors( let endpoint_destination = opentelemetry_destination(endpoint); let other_destination = opentelemetry_destination(other); if endpoint.transport == other.transport - && endpoint_destination == other_destination + && endpoint_destination.key == other_destination.key && endpoint.otel_type != other.otel_type { errors.push(OpenTelemetryDestinationCollision { @@ -2408,7 +2483,7 @@ fn opentelemetry_destination_collision_errors( opentelemetry_type_name(other.otel_type), opentelemetry_type_name(endpoint.otel_type), endpoint.transport, - endpoint_destination, + endpoint_destination.display, ), }); } @@ -2417,13 +2492,94 @@ fn opentelemetry_destination_collision_errors( errors } -fn opentelemetry_destination(endpoint: &OpenTelemetryEndpointConfig) -> Cow<'_, str> { +fn opentelemetry_destination(endpoint: &OpenTelemetryEndpointConfig) -> OpenTelemetryDestination { let configured_endpoint = endpoint.endpoint.trim(); - if endpoint.transport == "http_binary" { + let effective_endpoint = if endpoint.transport == "http_binary" { resolve_http_trace_endpoint(configured_endpoint) } else { Cow::Borrowed(configured_endpoint) + }; + canonicalize_opentelemetry_destination(&effective_endpoint) +} + +fn canonicalize_opentelemetry_destination(endpoint: &str) -> OpenTelemetryDestination { + let Ok(url) = reqwest::Url::parse(endpoint) else { + return raw_opentelemetry_destination(endpoint); + }; + if !matches!(url.scheme(), "http" | "https") { + return raw_opentelemetry_destination(endpoint); + } + let Some(url_host) = url.host_str() else { + return raw_opentelemetry_destination(endpoint); + }; + + let scheme = url.scheme().to_string(); + let host = canonical_opentelemetry_host(url_host); + let port = url.port_or_known_default(); + let path = normalize_opentelemetry_path(url.path()); + let query = url.query().map(str::to_string); + let display = format!( + "{scheme}://{host}{}{path}{}", + port.map(|port| format!(":{port}")).unwrap_or_default(), + query + .as_deref() + .map(|query| format!("?{query}")) + .unwrap_or_default(), + ); + OpenTelemetryDestination { + key: OpenTelemetryDestinationKey::Url { + scheme, + host, + port, + path, + query, + }, + display, + } +} + +fn raw_opentelemetry_destination(endpoint: &str) -> OpenTelemetryDestination { + OpenTelemetryDestination { + key: OpenTelemetryDestinationKey::Raw(endpoint.to_string()), + display: endpoint.to_string(), + } +} + +fn canonical_opentelemetry_host(host: &str) -> String { + let domain = host.strip_suffix('.').unwrap_or(host); + let unbracketed = host + .strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host); + let is_loopback_domain = domain == "localhost" || domain.ends_with(".localhost"); + let is_loopback_address = unbracketed + .parse::() + .is_ok_and(|address| address.is_loopback()); + if is_loopback_domain || is_loopback_address { + "".to_string() + } else { + host.to_string() + } +} + +fn normalize_opentelemetry_path(path: &str) -> String { + let mut normalized = String::with_capacity(path.len()); + let mut previous_was_slash = false; + for character in path.chars() { + if character == '/' { + if !previous_was_slash { + normalized.push(character); + } + previous_was_slash = true; + } else { + normalized.push(character); + previous_was_slash = false; + } + } + while normalized.len() > 1 && normalized.ends_with('/') { + normalized.pop(); } + normalized } const fn opentelemetry_type_name(otel_type: OpenTelemetryType) -> &'static str { diff --git a/crates/core/src/plugin.rs b/crates/core/src/plugin.rs index 9fbce2c81..335ec7afd 100644 --- a/crates/core/src/plugin.rs +++ b/crates/core/src/plugin.rs @@ -337,7 +337,13 @@ pub struct PluginRegistration { pub kind: String, /// Runtime-qualified registration name. pub name: String, - deregister: Box Result<()> + Send>, + deregister: Box PluginRegistrationCleanupOutcome + Send>, +} + +pub(crate) enum PluginRegistrationCleanupOutcome { + Removed, + RemovedWithError(PluginError), + NotRemoved(PluginError), } impl fmt::Debug for PluginRegistration { @@ -354,7 +360,22 @@ impl PluginRegistration { pub fn new( kind: impl Into, name: impl Into, - deregister: Box Result<()> + Send>, + mut deregister: Box Result<()> + Send>, + ) -> Self { + Self { + kind: kind.into(), + name: name.into(), + deregister: Box::new(move || match deregister() { + Ok(()) => PluginRegistrationCleanupOutcome::Removed, + Err(error) => PluginRegistrationCleanupOutcome::NotRemoved(error), + }), + } + } + + pub(crate) fn new_with_outcome( + kind: impl Into, + name: impl Into, + deregister: Box PluginRegistrationCleanupOutcome + Send>, ) -> Self { Self { kind: kind.into(), @@ -1558,12 +1579,39 @@ async fn initialize_plugins_exact_inner( }; if let Some(mut previous_state) = previous { - let teardown_errors = rollback_registrations_checked(&mut previous_state.registrations); - if !teardown_errors.is_empty() { - record_rollback_failures(rollback_failures.as_ref(), teardown_errors.clone()); + // Keep the previous report installed while teardown callbacks run so + // runtime diagnostics emitted by teardown remain observable. + { + let mut guard = ACTIVE_PLUGIN_CONFIGURATION.lock().map_err(|err| { + PluginError::Internal(format!("active plugin configuration lock poisoned: {err}")) + })?; + *guard = Some(ActivePluginConfiguration { + config: previous_state.config.clone(), + report: previous_state.report.clone(), + registrations: Vec::new(), + }); + } + let teardown = rollback_registrations_checked(&mut previous_state.registrations); + let teardown_report = ACTIVE_PLUGIN_CONFIGURATION + .lock() + .map_err(|err| { + PluginError::Internal(format!("active plugin configuration lock poisoned: {err}")) + })? + .take() + .map(|state| state.report); + if !teardown.errors.is_empty() { + if let Some(report) = + teardown_report.filter(|report| !report.runtime_diagnostics.is_empty()) + && let Ok(mut guard) = LAST_FAILED_RUNTIME_DIAGNOSTICS_REPORT.lock() + { + *guard = Some(report); + } + if !teardown.callbacks_cleared { + record_rollback_failures(rollback_failures.as_ref(), teardown.errors.clone()); + } return Err(PluginError::RegistrationFailed(format!( "previous plugin configuration could not be cleared: {}", - teardown_errors.join("; ") + teardown.errors.join("; ") ))); } match initialize_plugin_components_catching_panics( @@ -2123,7 +2171,7 @@ fn clear_plugin_configuration_inner() -> PluginHostClearOutcome { }; // Keep the report installed while callbacks run so runtime diagnostics // emitted by teardown work can be recorded against it. - let deregistration_errors = registrations + let deregistration = registrations .as_mut() .map(rollback_registrations_checked) .unwrap_or_default(); @@ -2138,16 +2186,10 @@ fn clear_plugin_configuration_inner() -> PluginHostClearOutcome { }; } }; - // Runtime delivery failures are reported by an otherwise successful - // deregistration callback. They must propagate without treating callback - // removal itself as unsafe. - let callbacks_cleared = deregistration_errors - .iter() - .all(|error| is_runtime_delivery_failure(error)); - let deregistration_error = (!deregistration_errors.is_empty()).then(|| { + let deregistration_error = (!deregistration.errors.is_empty()).then(|| { PluginError::RegistrationFailed(format!( "plugin teardown failed: {}", - deregistration_errors.join("; ") + deregistration.errors.join("; ") )) }); let result = match (flush_error, deregistration_error) { @@ -2170,19 +2212,10 @@ fn clear_plugin_configuration_inner() -> PluginHostClearOutcome { } PluginHostClearOutcome { result, - callbacks_cleared, + callbacks_cleared: deregistration.callbacks_cleared, } } -fn is_runtime_delivery_failure(error: &str) -> bool { - [ - ATIF_RUNTIME_DELIVERY_FAILURE_MARKER, - OTEL_RUNTIME_DELIVERY_FAILURE_MARKER, - ] - .iter() - .any(|marker| error.contains(&format!("registration failed: {marker}:"))) -} - pub(crate) fn plugin_configuration_is_active() -> Result { ACTIVE_PLUGIN_CONFIGURATION .lock() @@ -2342,26 +2375,53 @@ pub fn rollback_registrations(registrations: &mut Vec) { let _ = rollback_registrations_checked(registrations); } -fn rollback_registrations_checked(registrations: &mut Vec) -> Vec { - let mut errors = Vec::new(); +struct PluginRollbackOutcome { + errors: Vec, + callbacks_cleared: bool, +} + +impl Default for PluginRollbackOutcome { + fn default() -> Self { + Self { + errors: Vec::new(), + callbacks_cleared: true, + } + } +} + +fn rollback_registrations_checked( + registrations: &mut Vec, +) -> PluginRollbackOutcome { + let mut outcome = PluginRollbackOutcome::default(); for registration in registrations.iter_mut().rev() { - let failure = match catch_unwind(AssertUnwindSafe(|| (registration.deregister)())) { - Ok(Ok(())) => None, - Ok(Err(error)) => Some(error.to_string()), - Err(payload) => Some(format!( - "deregistration panicked: {}", - panic_payload_message(payload) - )), - }; - if let Some(error) = failure { - errors.push(format!( - "{} registration '{}' could not be removed: {error}", - registration.kind, registration.name - )); + match catch_unwind(AssertUnwindSafe(|| (registration.deregister)())) { + Ok(PluginRegistrationCleanupOutcome::Removed) => {} + Ok(PluginRegistrationCleanupOutcome::RemovedWithError(error)) => { + outcome.errors.push(format!( + "{} registration '{}' reported a delivery failure: {error}", + registration.kind, registration.name + )); + } + Ok(PluginRegistrationCleanupOutcome::NotRemoved(error)) => { + outcome.callbacks_cleared = false; + outcome.errors.push(format!( + "{} registration '{}' could not be removed: {error}", + registration.kind, registration.name + )); + } + Err(payload) => { + outcome.callbacks_cleared = false; + outcome.errors.push(format!( + "{} registration '{}' could not be removed: deregistration panicked: {}", + registration.kind, + registration.name, + panic_payload_message(payload) + )); + } } } registrations.clear(); - errors + outcome } fn panic_payload_message(payload: Box) -> String { @@ -2444,8 +2504,10 @@ impl PendingPluginRegistrations { impl Drop for PendingPluginRegistrations { fn drop(&mut self) { - let errors = rollback_registrations_checked(&mut self.registrations); - record_rollback_failures(self.rollback_failures.as_ref(), errors); + let outcome = rollback_registrations_checked(&mut self.registrations); + if !outcome.callbacks_cleared { + record_rollback_failures(self.rollback_failures.as_ref(), outcome.errors); + } } } @@ -2469,8 +2531,10 @@ impl PendingPluginRegistrationContext { impl Drop for PendingPluginRegistrationContext { fn drop(&mut self) { - let errors = rollback_registrations_checked(&mut self.context.registrations); - record_rollback_failures(self.rollback_failures.as_ref(), errors); + let outcome = rollback_registrations_checked(&mut self.context.registrations); + if !outcome.callbacks_cleared { + record_rollback_failures(self.rollback_failures.as_ref(), outcome.errors); + } } } diff --git a/crates/core/src/plugin/dynamic/native.rs b/crates/core/src/plugin/dynamic/native.rs index 3b0124b5b..7fe3d1998 100644 --- a/crates/core/src/plugin/dynamic/native.rs +++ b/crates/core/src/plugin/dynamic/native.rs @@ -1293,7 +1293,9 @@ fn status_from_flow_error(err: FlowError) -> NemoRelayStatus { FlowError::InvalidArgument(_) => NemoRelayStatus::InvalidArg, FlowError::ScopeStackEmpty => NemoRelayStatus::ScopeStackEmpty, FlowError::GuardrailRejected(_) => NemoRelayStatus::GuardrailRejected, - FlowError::Upstream(_) | FlowError::Internal(_) => NemoRelayStatus::Internal, + FlowError::Upstream(_) | FlowError::Internal(_) | FlowError::CallbackException { .. } => { + NemoRelayStatus::Internal + } } } diff --git a/crates/core/tests/coverage/error_tests.rs b/crates/core/tests/coverage/error_tests.rs index 55e823a72..5c4f02356 100644 --- a/crates/core/tests/coverage/error_tests.rs +++ b/crates/core/tests/coverage/error_tests.rs @@ -104,6 +104,12 @@ fn otel_error_type_maps_internal_failures_to_generic_code() { FlowError::Internal("application callback failed".into()).otel_error_type(), "internal_error" ); + let external = FlowError::CallbackException { + message: "ValueError: boom".into(), + exception_type: "ValueError".into(), + }; + assert_eq!(external.otel_error_type(), "internal_error"); + assert_eq!(external.exception_type(), Some("ValueError")); } #[test] diff --git a/crates/core/tests/coverage/logging_tests.rs b/crates/core/tests/coverage/logging_tests.rs index 430c4f3b6..6ecf30e6a 100644 --- a/crates/core/tests/coverage/logging_tests.rs +++ b/crates/core/tests/coverage/logging_tests.rs @@ -4,7 +4,7 @@ use crate::logging::{ FileLogRotationConfig, FileLogSinkConfig, LogFormat, LogLevel, LogSinkConfig, LoggingConfig, LoggingRuntime, MAX_FILE_SINK_QUEUE_ENTRIES, MAX_FILE_SINK_RETAINED_FILES, build_logger, - format_event_for_test, init_logging, + format_event_for_test, init_logging, initialize_default_logging, shutdown_default_logging, }; use opentelemetry::trace::{Span as _, Tracer as _, TracerProvider as _}; use opentelemetry_sdk::error::OTelSdkResult; @@ -192,6 +192,105 @@ queue_capacity = 16 assert_eq!(record["fields"]["source"], "toml"); } +#[test] +fn default_logging_runtime_initializes_once_and_shuts_down_idempotently() { + let temp = tempfile::tempdir().unwrap(); + let config_path = temp.path().join("logging.toml"); + let log_path = temp.path().join("relay.log.jsonl"); + std::fs::write( + &config_path, + format!( + r#" +[logging] +level = "info" +stderr_format = "human" +flush_interval_millis = 0 + +[[logging.sinks]] +path = {} +level = "info" +format = "jsonl" +queue_capacity = 16 +"#, + toml_basic_string(log_path.to_string_lossy().as_ref()) + ), + ) + .unwrap(); + let _environment = LoggingEnvScope::set(&[ + ("NEMO_RELAY_LOG", None), + ("NEMO_RELAY_LOG_STDERR_FORMAT", None), + ("NEMO_RELAY_LOG_CONFIG_PATH", Some(config_path.as_os_str())), + ]); + + shutdown_default_logging().unwrap(); + initialize_default_logging().unwrap(); + initialize_default_logging().unwrap(); + shutdown_default_logging().unwrap(); + shutdown_default_logging().unwrap(); + + let records = std::fs::read_to_string(log_path) + .unwrap() + .lines() + .map(|line| serde_json::from_str::(line).expect("valid JSONL lifecycle record")) + .collect::>(); + assert_eq!( + records + .iter() + .filter(|record| record["event"] == "logging_initialized") + .count(), + 1 + ); + assert_eq!( + records + .iter() + .filter(|record| record["event"] == "logging_shutdown_started") + .count(), + 1 + ); +} + +#[test] +fn implicit_default_logging_preserves_an_existing_relay_logger() { + let _environment = LoggingEnvScope::set(&[ + ("NEMO_RELAY_LOG", None), + ("NEMO_RELAY_LOG_STDERR_FORMAT", None), + ("NEMO_RELAY_LOG_CONFIG_PATH", None), + ]); + shutdown_default_logging().unwrap(); + let host_runtime = init_logging(&default_config()).unwrap(); + + initialize_default_logging().unwrap(); + shutdown_default_logging().unwrap(); + + let receiver = spdlog::log_crate_proxy().swap_logger(None); + let preserves_host_logger = receiver + .as_ref() + .is_some_and(|receiver| Arc::ptr_eq(receiver, &host_runtime.logger)); + spdlog::log_crate_proxy().set_logger(receiver); + assert!( + preserves_host_logger, + "implicit binding startup must preserve the host Relay logger" + ); + drop(host_runtime); +} + +#[test] +fn default_logging_runtime_rejects_invalid_environment() { + let _environment = LoggingEnvScope::set(&[ + ("NEMO_RELAY_LOG", Some(OsStr::new(""))), + ("NEMO_RELAY_LOG_STDERR_FORMAT", None), + ("NEMO_RELAY_LOG_CONFIG_PATH", None), + ]); + + shutdown_default_logging().unwrap(); + let error = initialize_default_logging().unwrap_err().to_string(); + + assert!( + error.contains("NEMO_RELAY_LOG must not be empty"), + "{error}" + ); +} + #[test] fn logging_config_from_environment_resolves_direct_settings() { let _environment = LoggingEnvScope::set(&[ @@ -1581,6 +1680,17 @@ fn configure_rejects_preinstalled_foreign_logger() { .contains("process-global log facade is already initialized by another logger"), "{error}" ); + initialize_default_logging() + .expect("unconfigured default logging should defer to the foreign logger"); + unsafe { std::env::set_var("NEMO_RELAY_LOG", "info") }; + let error = initialize_default_logging() + .expect_err("explicit default logging should reject the foreign logger"); + assert!( + error + .to_string() + .contains("process-global log facade is already initialized by another logger"), + "{error}" + ); return; } @@ -1589,6 +1699,9 @@ fn configure_rejects_preinstalled_foreign_logger() { let output = std::process::Command::new(std::env::current_exe().unwrap()) .args(["--exact", test_name, "--nocapture"]) .env(FOREIGN_LOGGER_CHILD_ENV, "1") + .env_remove("NEMO_RELAY_LOG") + .env_remove("NEMO_RELAY_LOG_STDERR_FORMAT") + .env_remove("NEMO_RELAY_LOG_CONFIG_PATH") .output() .expect("foreign logger child test should start"); diff --git a/crates/core/tests/integration/atif_storage_tests.rs b/crates/core/tests/integration/atif_storage_tests.rs index 09d3d9f04..84a1083b0 100644 --- a/crates/core/tests/integration/atif_storage_tests.rs +++ b/crates/core/tests/integration/atif_storage_tests.rs @@ -23,7 +23,8 @@ use nemo_relay::api::scope::{PopScopeParams, PushScopeParams, ScopeType, pop_sco use nemo_relay::api::subscriber::flush_subscribers; use nemo_relay::observability::plugin_component::OBSERVABILITY_PLUGIN_KIND; use nemo_relay::plugin::{ - PluginComponentSpec, PluginConfig, clear_plugin_configuration, initialize_plugins, + PluginComponentSpec, PluginConfig, active_plugin_report, clear_plugin_configuration, + initialize_plugins, }; use object_store::{ObjectStore, ObjectStoreExt as _}; use serde_json::{Value as Json, json}; @@ -497,13 +498,18 @@ fn atif_storage_posts_trajectory_to_http_endpoints() { fn atif_storage_http_non_2xx_retries_on_the_next_trajectory() { let _guard = PLUGIN_TEST_LOCK.lock().unwrap(); reset_runtime(); + let recovery_directory = tempfile::tempdir().expect("create recovery directory"); let mut server = start_http_server(2, vec![("/fail", 500)]); // SAFETY: this uniquely named env var is only touched by this test. unsafe { std::env::set_var("NEMO_RELAY_ATIF_HTTP_TEST_TOKEN", "Bearer test-token"); } - let config = build_http_observability_config(&[format!("{}/fail", server.base_url)]); + let mut config = build_http_observability_config(&[format!("{}/fail", server.base_url)]); + config.components[0].config["atif"] + .as_object_mut() + .expect("ATIF config should be an object") + .insert("output_directory".into(), json!(recovery_directory.path())); futures::executor::block_on(initialize_plugins(config)) .expect("observability plugin should initialize with HTTP storage"); @@ -529,6 +535,15 @@ fn atif_storage_http_non_2xx_retries_on_the_next_trajectory() { .expect("pop second agent scope"); flush_subscribers().expect("HTTP upload subscriber should flush after failure"); + let report = active_plugin_report().expect("active plugin report should remain readable"); + let diagnostic = report + .runtime_diagnostics + .iter() + .find(|diagnostic| diagnostic.code == "atif.remote_delivery_failed") + .expect("failed uploads should be visible before teardown"); + assert_eq!(diagnostic.field.as_deref(), Some("storage[0]")); + assert_eq!(diagnostic.count, 2); + server.stop(); { let requests = server.received.lock().unwrap(); @@ -557,6 +572,19 @@ fn atif_storage_http_non_2xx_retries_on_the_next_trajectory() { teardown.to_string().contains("atif.remote_delivery_failed"), "teardown should identify the failed remote destination: {teardown}" ); + assert!( + !teardown.to_string().contains("could not be removed"), + "delivery failure should not imply a leaked registration: {teardown}" + ); + let retained_report = + active_plugin_report().expect("failed teardown should retain the plugin report"); + let retained_diagnostic = retained_report + .runtime_diagnostics + .iter() + .find(|diagnostic| diagnostic.code == "atif.remote_delivery_failed") + .expect("failed upload diagnostic should remain readable after teardown"); + assert_eq!(retained_diagnostic.field.as_deref(), Some("storage[0]")); + assert_eq!(retained_diagnostic.count, 2); // SAFETY: cleanup of test-only env var. unsafe { std::env::remove_var("NEMO_RELAY_ATIF_HTTP_TEST_TOKEN"); diff --git a/crates/core/tests/unit/observability/otel_tests.rs b/crates/core/tests/unit/observability/otel_tests.rs index 88bf5ecea..1d8172947 100644 --- a/crates/core/tests/unit/observability/otel_tests.rs +++ b/crates/core/tests/unit/observability/otel_tests.rs @@ -528,6 +528,27 @@ fn make_end_event( ) } +fn make_end_event_with_metadata( + uuid: Uuid, + parent_uuid: Option, + name: &str, + scope_type: ScopeType, + metadata: Json, +) -> Event { + Event::Scope(ScopeEvent::new( + BaseEvent::builder() + .parent_uuid_opt(parent_uuid) + .uuid(uuid) + .name(name) + .metadata(metadata) + .build(), + ScopeCategory::End, + Vec::new(), + EventCategory::from(scope_type), + None, + )) +} + fn make_scope_event( scope_category: ScopeCategory, uuid: Uuid, @@ -1581,6 +1602,295 @@ fn gen_ai_end_projection_preserves_explicit_error_type() { ); } +#[test] +fn failed_descendant_classification_and_exception_propagate_to_agent_span() { + for otel_type in [OpenTelemetryType::Full, OpenTelemetryType::GenAi] { + let (provider, exporter) = make_provider(); + let mut processor = + OtelEventProcessor::new_with_mark_projection_and_exclusions_and_mappings( + provider, + "error-propagation-test".to_string(), + otel_type, + MarkProjection::default(), + default_mark_exclude_names(), + Vec::new(), + ); + let agent_uuid = Uuid::now_v7(); + let function_uuid = Uuid::now_v7(); + let llm_uuid = Uuid::now_v7(); + processor.process(&make_start_event( + agent_uuid, + None, + "agent", + ScopeType::Agent, + None, + )); + let llm_parent_uuid = if otel_type == OpenTelemetryType::GenAi { + processor.process(&make_start_event( + function_uuid, + Some(agent_uuid), + "function", + ScopeType::Function, + None, + )); + function_uuid + } else { + agent_uuid + }; + processor.process(&make_start_event( + llm_uuid, + Some(llm_parent_uuid), + "chat", + ScopeType::Llm, + None, + )); + processor.process(&make_end_event_with_metadata( + llm_uuid, + Some(llm_parent_uuid), + "chat", + ScopeType::Llm, + json!({ + "otel.status_code": "ERROR", + "otel.status_description": "internal error: ValueError: boom", + "error.type": "internal_error", + "exception.type": "ValueError", + }), + )); + if otel_type == OpenTelemetryType::GenAi { + processor.process(&make_end_event_with_metadata( + function_uuid, + Some(agent_uuid), + "function", + ScopeType::Function, + json!({ + "otel.status_code": "ERROR", + "otel.status_description": "internal error: ValueError: boom", + }), + )); + } + processor.process(&make_end_event_with_metadata( + agent_uuid, + None, + "agent", + ScopeType::Agent, + json!({ + "otel.status_code": "ERROR", + "otel.status_description": "internal error: ValueError: boom", + }), + )); + processor.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + assert_eq!(spans.len(), 2); + for span in &spans { + assert_eq!( + attr_map(&span.attributes).get("error.type"), + Some(&"internal_error".to_string()) + ); + let exception = span + .events + .events + .iter() + .find(|event| event.name.as_ref() == "exception") + .expect("expected exception event"); + assert_eq!( + attr_map(&exception.attributes).get("exception.type"), + Some(&"ValueError".to_string()) + ); + } + } +} + +#[test] +fn suppressed_function_error_propagates_to_agent_span() { + let (provider, exporter) = make_provider(); + let mut processor = OtelEventProcessor::new_with_mark_projection_and_exclusions_and_mappings( + provider, + "suppressed-error-propagation-test".to_string(), + OpenTelemetryType::GenAi, + MarkProjection::default(), + default_mark_exclude_names(), + Vec::new(), + ); + let agent_uuid = Uuid::now_v7(); + let function_uuid = Uuid::now_v7(); + processor.process(&make_start_event( + agent_uuid, + None, + "agent", + ScopeType::Agent, + None, + )); + processor.process(&make_start_event( + function_uuid, + Some(agent_uuid), + "function", + ScopeType::Function, + None, + )); + processor.process(&make_end_event_with_metadata( + function_uuid, + Some(agent_uuid), + "function", + ScopeType::Function, + json!({ + "otel.status_code": "ERROR", + "error.type": "internal_error", + "exception.type": "ValueError", + }), + )); + processor.process(&make_end_event_with_metadata( + agent_uuid, + None, + "agent", + ScopeType::Agent, + json!({"otel.status_code": "ERROR"}), + )); + processor.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + assert_eq!(spans.len(), 1); + let agent_span = &spans[0]; + assert_eq!( + attr_map(&agent_span.attributes).get("error.type"), + Some(&"internal_error".to_string()) + ); + let exception = agent_span + .events + .events + .iter() + .find(|event| event.name.as_ref() == "exception") + .expect("expected propagated exception event"); + assert_eq!( + attr_map(&exception.attributes).get("exception.type"), + Some(&"ValueError".to_string()) + ); +} + +#[test] +fn suppressed_parent_error_propagation_isolated_by_trace_id() { + let (provider, exporter) = make_provider(); + let mut processor = OtelEventProcessor::new_with_mark_projection_and_exclusions_and_mappings( + provider, + "error-trace-isolation-test".to_string(), + OpenTelemetryType::GenAi, + MarkProjection::default(), + default_mark_exclude_names(), + Vec::new(), + ); + let shared_span_id = [0xAB; 8]; + let mut first_agent_bytes = [0x11; 16]; + first_agent_bytes[8..].copy_from_slice(&shared_span_id); + let mut second_agent_bytes = [0x22; 16]; + second_agent_bytes[8..].copy_from_slice(&shared_span_id); + let cases = [ + ( + Uuid::from_bytes(first_agent_bytes), + Uuid::now_v7(), + Uuid::now_v7(), + "first_error", + "FirstException", + ), + ( + Uuid::from_bytes(second_agent_bytes), + Uuid::now_v7(), + Uuid::now_v7(), + "second_error", + "SecondException", + ), + ]; + assert_eq!( + relay_span_id(cases[0].0), + relay_span_id(cases[1].0), + "fixture agents must share a span ID" + ); + assert_ne!( + relay_trace_id(cases[0].0), + relay_trace_id(cases[1].0), + "fixture agents must belong to different traces" + ); + + for (agent_uuid, function_uuid, llm_uuid, _, _) in cases { + processor.process(&make_start_event( + agent_uuid, + None, + "agent", + ScopeType::Agent, + None, + )); + processor.process(&make_start_event( + function_uuid, + Some(agent_uuid), + "function", + ScopeType::Function, + None, + )); + processor.process(&make_start_event( + llm_uuid, + Some(function_uuid), + "chat", + ScopeType::Llm, + None, + )); + } + for (agent_uuid, function_uuid, llm_uuid, error_type, exception_type) in cases { + processor.process(&make_end_event_with_metadata( + llm_uuid, + Some(function_uuid), + "chat", + ScopeType::Llm, + json!({ + "otel.status_code": "ERROR", + "error.type": error_type, + "exception.type": exception_type, + }), + )); + processor.process(&make_end_event_with_metadata( + function_uuid, + Some(agent_uuid), + "function", + ScopeType::Function, + json!({"otel.status_code": "ERROR"}), + )); + } + for (agent_uuid, _, _, _, _) in cases { + processor.process(&make_end_event_with_metadata( + agent_uuid, + None, + "agent", + ScopeType::Agent, + json!({"otel.status_code": "ERROR"}), + )); + } + processor.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + assert_eq!(spans.len(), 4); + for (agent_uuid, _, _, error_type, exception_type) in cases { + let agent_span = spans + .iter() + .find(|span| { + span.span_context.trace_id() == relay_trace_id(agent_uuid) + && span.parent_span_id == SpanId::INVALID + }) + .expect("expected agent span for trace"); + assert_eq!( + attr_map(&agent_span.attributes).get("error.type"), + Some(&error_type.to_string()) + ); + let exception = agent_span + .events + .events + .iter() + .find(|event| event.name.as_ref() == "exception") + .expect("expected propagated exception event"); + assert_eq!( + attr_map(&exception.attributes).get("exception.type"), + Some(&exception_type.to_string()) + ); + } +} + #[test] fn gen_ai_projection_prefers_standard_names_and_normalized_provider_details() { let agent = make_start_event( diff --git a/crates/core/tests/unit/observability/plugin_component_tests.rs b/crates/core/tests/unit/observability/plugin_component_tests.rs index f6e4eb362..abddf29be 100644 --- a/crates/core/tests/unit/observability/plugin_component_tests.rs +++ b/crates/core/tests/unit/observability/plugin_component_tests.rs @@ -2447,6 +2447,35 @@ fn atif_metadata_template_values_must_be_safe_path_fragments() { .prepare_destination("session-1", Some(&non_string)) .is_err() ); + let dispatcher_with_fallback = AtifDispatcher::new(AtifSectionConfig { + filename_template: "{metadata.artifact_path:-unassigned}/trajectory-{session_id}.json" + .to_string(), + ..AtifSectionConfig::default() + }); + let error = dispatcher_with_fallback + .prepare_destination("session-1", Some(&non_string)) + .unwrap_err(); + assert!(error.contains("resolved to a non-string value"), "{error}"); + let nested_non_string = json!({"artifact": 123}); + let nested_dispatcher = AtifDispatcher::new(AtifSectionConfig { + filename_template: "{metadata.artifact.path:-unassigned}/trajectory-{session_id}.json" + .to_string(), + ..AtifSectionConfig::default() + }); + let error = nested_dispatcher + .prepare_destination("session-1", Some(&nested_non_string)) + .unwrap_err(); + assert!(error.contains("traversed a non-object value"), "{error}"); + let nested_null = json!({"artifact": null}); + let destination = nested_dispatcher + .prepare_destination("session-1", Some(&nested_null)) + .unwrap(); + assert_eq!(destination.0, "unassigned/trajectory-session-1.json"); + let nested_string = json!({"artifact": {"path": "tenant-a/team_1"}}); + let destination = nested_dispatcher + .prepare_destination("session-1", Some(&nested_string)) + .unwrap(); + assert_eq!(destination.0, "tenant-a/team_1/trajectory-session-1.json"); for template in [ "/tmp/trajectory-{session_id}.json", @@ -2732,7 +2761,140 @@ fn opentelemetry_endpoints_fan_out_to_heterogeneous_and_repeated_types() { } #[test] -fn opentelemetry_rejects_different_projection_types_at_the_same_effective_destination() { +fn opentelemetry_rejects_canonical_equivalent_destinations() { + for (first, second) in [ + ( + "http://collector.example/v1/traces", + "http://collector.example:80/v1/traces", + ), + ( + "https://collector.example/v1/traces", + "https://collector.example:443/v1/traces", + ), + ( + "HTTP://COLLECTOR.EXAMPLE/v1/traces", + "http://collector.example/v1/traces", + ), + ( + "http://collector.example//v1///traces", + "http://collector.example/v1/traces/", + ), + ("http://localhost/v1/traces", "http://LOCALHOST/v1/traces"), + ("http://localhost/v1/traces", "http://localhost./v1/traces"), + ( + "http://localhost/v1/traces", + "http://agent.localhost/v1/traces", + ), + ("http://localhost/v1/traces", "http://127.0.0.2/v1/traces"), + ("http://localhost/v1/traces", "http://127.1/v1/traces"), + ("http://localhost/v1/traces", "http://[::1]/v1/traces"), + ] { + let config = plugin_config(json!({ + "opentelemetry": { + "enabled": true, + "endpoints": [ + {"type": "full", "endpoint": first}, + {"type": "gen_ai", "endpoint": second} + ] + } + })); + + let report = validate_plugin_config(&config); + assert!( + report.diagnostics.iter().any(|diagnostic| { + diagnostic.code == "observability.unsafe_otel_destination_collision" + }), + "expected equivalent destinations {first:?} and {second:?} to collide" + ); + } + + let grpc = plugin_config(json!({ + "opentelemetry": { + "enabled": true, + "endpoints": [ + { + "type": "full", + "transport": "grpc", + "endpoint": "https://collector.example" + }, + { + "type": "gen_ai", + "transport": "grpc", + "endpoint": "https://collector.example:443/" + } + ] + } + })); + assert!(validate_plugin_config(&grpc).has_errors()); +} + +#[test] +fn opentelemetry_allows_distinct_canonical_destinations() { + for (first, second) in [ + ( + "http://collector.example:4318/v1/traces", + "http://collector.example:4319/v1/traces", + ), + ( + "http://collector.example:443/v1/traces", + "https://collector.example/v1/traces", + ), + ( + "http://collector.example/v1/traces", + "http://collector.example/custom/traces", + ), + ( + "http://collector.example/v1/traces", + "http://collector.example/v1%2Ftraces", + ), + ( + "http://collector.example/v1/traces?tenant=one", + "http://collector.example/v1/traces?tenant=two", + ), + ( + "http://localhost.example/v1/traces", + "http://localhost/v1/traces", + ), + ("http://[::2]/v1/traces", "http://localhost/v1/traces"), + ] { + let config = plugin_config(json!({ + "opentelemetry": { + "enabled": true, + "endpoints": [ + {"type": "full", "endpoint": first}, + {"type": "gen_ai", "endpoint": second} + ] + } + })); + + assert!( + !validate_plugin_config(&config).has_errors(), + "expected distinct destinations {first:?} and {second:?} to remain valid" + ); + } + + let different_transports = plugin_config(json!({ + "opentelemetry": { + "enabled": true, + "endpoints": [ + { + "type": "full", + "transport": "http_binary", + "endpoint": "http://collector.example/v1/traces" + }, + { + "type": "gen_ai", + "transport": "grpc", + "endpoint": "http://collector.example/v1/traces" + } + ] + } + })); + assert!(!validate_plugin_config(&different_transports).has_errors()); +} + +#[test] +fn opentelemetry_rejects_canonical_collision_during_validation_and_activation() { let _guard = crate::observability::test_mutex().lock().unwrap(); reset_runtime(); let config = plugin_config(json!({ @@ -2740,8 +2902,8 @@ fn opentelemetry_rejects_different_projection_types_at_the_same_effective_destin "opentelemetry": { "enabled": true, "endpoints": [ - {"type": "full", "endpoint": " http://127.0.0.1:4318 "}, - {"type": "gen_ai", "endpoint": "http://127.0.0.1:4318/v1/traces"} + {"type": "full", "endpoint": " http://LOCALHOST:80//v1///traces/ "}, + {"type": "gen_ai", "endpoint": "http://127.1/v1/traces"} ] } })); @@ -2755,7 +2917,7 @@ fn opentelemetry_rejects_different_projection_types_at_the_same_effective_destin && diagnostic.message.contains("endpoints[1] (gen_ai)") && diagnostic .message - .contains("http://127.0.0.1:4318/v1/traces") + .contains("http://:80/v1/traces") })); assert!(futures::executor::block_on(initialize_plugins_exact(config)).is_err()); assert!( @@ -2773,8 +2935,8 @@ fn opentelemetry_allows_repeated_projection_types_at_the_same_destination() { "opentelemetry": { "enabled": true, "endpoints": [ - {"type": "full", "endpoint": "http://127.0.0.1:4318/v1/traces"}, - {"type": "full", "endpoint": "http://127.0.0.1:4318/v1/traces"} + {"type": "full", "endpoint": "http://LOCALHOST:80//v1///traces/"}, + {"type": "full", "endpoint": "http://127.1/v1/traces"} ] } })); @@ -2904,9 +3066,13 @@ fn opentelemetry_shutdown_helper_retains_every_endpoint_failure() { }) .collect::>(); - let error = shutdown_opentelemetry_subscribers(&subscribers) - .expect("mixed endpoint shutdown failures should be reported") - .to_string(); + let OpenTelemetryShutdownFailure::Other(error) = + shutdown_opentelemetry_subscribers(&subscribers) + .expect("mixed endpoint shutdown failures should be reported") + else { + panic!("mixed endpoint shutdown failures must retain the registration failure outcome"); + }; + let error = error.to_string(); assert_eq!(dropped_calls.load(Ordering::SeqCst), 1); assert_eq!(timeout_calls.load(Ordering::SeqCst), 1); diff --git a/crates/core/tests/unit/plugin_tests.rs b/crates/core/tests/unit/plugin_tests.rs index 816f2ab8d..815239734 100644 --- a/crates/core/tests/unit/plugin_tests.rs +++ b/crates/core/tests/unit/plugin_tests.rs @@ -1477,6 +1477,37 @@ fn test_pending_registration_records_rollback_failures() { assert!(failures[0].contains("rollback remained registered")); } +#[test] +fn test_pending_rollbacks_ignore_delivery_only_errors() { + let failures = Arc::new(Mutex::new(Vec::new())); + let delivery_error = || { + PluginRegistrationCleanupOutcome::RemovedWithError(PluginError::RegistrationFailed( + "delivery failed".into(), + )) + }; + { + let mut pending = PendingPluginRegistrations::new(Some(Arc::clone(&failures))); + pending.extend(vec![PluginRegistration::new_with_outcome( + "fixture", + "delivery-only-registration", + Box::new(delivery_error), + )]); + } + { + let mut pending = + PendingPluginRegistrationContext::new("fixture.".into(), Some(Arc::clone(&failures))); + pending + .context + .add_registration(PluginRegistration::new_with_outcome( + "fixture", + "delivery-only-context-registration", + Box::new(delivery_error), + )); + } + + assert!(failures.lock().unwrap().is_empty()); +} + #[test] fn test_checked_teardown_reports_unremoved_registrations() { let _guard = lock_runtime_owner(); @@ -1506,13 +1537,41 @@ fn test_checked_teardown_reports_unremoved_registrations() { } #[test] -fn test_teardown_runtime_diagnostics_remain_in_the_plugin_report() { +fn test_teardown_marker_text_does_not_imply_successful_removal() { let _guard = lock_runtime_owner(); reset_global(); store_active_plugin_configuration( PluginConfig::default(), ConfigReport::default(), vec![PluginRegistration::new( + "fixture", + "stale-marker-callback", + Box::new(|| { + Err(PluginError::RegistrationFailed(format!( + "unrelated failure mentioning {}", + crate::plugin::ATIF_RUNTIME_DELIVERY_FAILURE_MARKER + ))) + }), + )], + ) + .unwrap(); + + let outcome = clear_plugin_configuration_inner(); + assert!(!outcome.callbacks_cleared); + let error = outcome.result.unwrap_err().to_string(); + assert!(error.contains("stale-marker-callback"), "{error}"); + assert!(error.contains("could not be removed"), "{error}"); + reset_global(); +} + +#[test] +fn test_teardown_runtime_diagnostics_remain_in_the_plugin_report() { + let _guard = lock_runtime_owner(); + reset_global(); + store_active_plugin_configuration( + PluginConfig::default(), + ConfigReport::default(), + vec![PluginRegistration::new_with_outcome( "fixture", "atif-shutdown", Box::new(|| { @@ -1524,10 +1583,11 @@ fn test_teardown_runtime_diagnostics_remain_in_the_plugin_report() { session_id: Some("session-123".into()), count: 1, }); - Err(PluginError::RegistrationFailed(format!( + let error = PluginError::RegistrationFailed(format!( "{}: atif.remote_delivery_failed (1)", crate::plugin::ATIF_RUNTIME_DELIVERY_FAILURE_MARKER - ))) + )); + PluginRegistrationCleanupOutcome::RemovedWithError(error) }), )], ) @@ -1535,7 +1595,9 @@ fn test_teardown_runtime_diagnostics_remain_in_the_plugin_report() { let outcome = clear_plugin_configuration_inner(); assert!(outcome.callbacks_cleared); - assert!(outcome.result.is_err()); + let error = outcome.result.unwrap_err().to_string(); + assert!(error.contains("atif.remote_delivery_failed"), "{error}"); + assert!(!error.contains("could not be removed"), "{error}"); let report = active_plugin_report().expect("failed teardown should retain its report"); assert_eq!(report.runtime_diagnostics.len(), 1); let diagnostic = &report.runtime_diagnostics[0]; @@ -1556,14 +1618,16 @@ fn test_opentelemetry_delivery_failure_allows_later_plugin_configuration() { store_active_plugin_configuration( PluginConfig::default(), ConfigReport::default(), - vec![PluginRegistration::new( + vec![PluginRegistration::new_with_outcome( "fixture", "opentelemetry-shutdown", Box::new(|| { - Err(PluginError::RegistrationFailed(format!( - "{}: otel.spans_dropped (2)", - crate::plugin::OTEL_RUNTIME_DELIVERY_FAILURE_MARKER - ))) + PluginRegistrationCleanupOutcome::RemovedWithError(PluginError::RegistrationFailed( + format!( + "{}: otel.spans_dropped (2)", + crate::plugin::OTEL_RUNTIME_DELIVERY_FAILURE_MARKER + ), + )) }), )], ) @@ -1582,14 +1646,16 @@ fn test_mixed_opentelemetry_shutdown_failure_blocks_later_configuration() { store_active_plugin_configuration( PluginConfig::default(), ConfigReport::default(), - vec![PluginRegistration::new( + vec![PluginRegistration::new_with_outcome( "fixture", "opentelemetry-shutdown", Box::new(|| { - Err(PluginError::RegistrationFailed(format!( - "OpenTelemetry shutdown failures: provider error: {}: otel.spans_dropped (2); endpoint shutdown timed out", - crate::plugin::OTEL_RUNTIME_DELIVERY_FAILURE_MARKER - ))) + PluginRegistrationCleanupOutcome::NotRemoved(PluginError::RegistrationFailed( + format!( + "OpenTelemetry shutdown failures: provider error: {}: otel.spans_dropped (2); endpoint shutdown timed out", + crate::plugin::OTEL_RUNTIME_DELIVERY_FAILURE_MARKER + ), + )) }), )], ) @@ -1602,6 +1668,72 @@ fn test_mixed_opentelemetry_shutdown_failure_blocks_later_configuration() { reset_global(); } +#[test] +fn test_replacement_teardown_runtime_diagnostics_remain_in_the_plugin_report() { + let _guard = lock_runtime_owner(); + reset_global(); + store_active_plugin_configuration( + PluginConfig::default(), + ConfigReport::default(), + vec![PluginRegistration::new_with_outcome( + "fixture", + "atif-shutdown", + Box::new(|| { + record_active_plugin_runtime_diagnostic(RuntimeDiagnostic { + code: "atif.remote_delivery_failed".into(), + component: "observability".into(), + field: Some("storage[0]".into()), + message: "HTTP 500".into(), + session_id: Some("session-123".into()), + count: 1, + }); + PluginRegistrationCleanupOutcome::RemovedWithError(PluginError::RegistrationFailed( + "ATIF delivery failed".into(), + )) + }), + )], + ) + .unwrap(); + + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let error = runtime + .block_on(initialize_plugins_exact(PluginConfig::default())) + .unwrap_err(); + assert!( + error.to_string().contains("ATIF delivery failed"), + "{error}" + ); + assert!( + error + .to_string() + .contains("fixture registration 'atif-shutdown' reported a delivery failure"), + "{error}" + ); + assert!( + !plugin_configuration_is_active().unwrap(), + "a replacement aborted by delivery failure must not leave a configuration active" + ); + let report = active_plugin_report().expect("failed replacement should retain its report"); + assert_eq!(report.runtime_diagnostics.len(), 1); + assert_eq!( + report.runtime_diagnostics[0].code, + "atif.remote_delivery_failed" + ); + assert_eq!( + report.runtime_diagnostics[0].field.as_deref(), + Some("storage[0]") + ); + assert_eq!(report.runtime_diagnostics[0].count, 1); + + runtime + .block_on(initialize_plugins_exact(PluginConfig::default())) + .expect("delivery-only teardown errors must not block a later initialization"); + reset_global(); +} + #[test] fn test_legacy_clear_retains_mutation_owner_after_incomplete_teardown() { let _guard = lock_runtime_owner(); @@ -2207,11 +2339,18 @@ fn test_plugin_registration_context_maps_deregistration_errors() { set_conflicting_runtime_owner_for_tests(); for (registration, expected) in registrations.iter_mut().zip(expected_messages) { match (registration.deregister)() { - Err(PluginError::RegistrationFailed(message)) => { + PluginRegistrationCleanupOutcome::NotRemoved(PluginError::RegistrationFailed( + message, + )) => { assert!(message.contains(expected), "{message}"); } - Err(other) => panic!("unexpected deregistration failure: {other}"), - Ok(()) => panic!("expected deregistration to fail"), + PluginRegistrationCleanupOutcome::NotRemoved(other) => { + panic!("unexpected deregistration failure: {other}") + } + PluginRegistrationCleanupOutcome::Removed + | PluginRegistrationCleanupOutcome::RemovedWithError(_) => { + panic!("expected deregistration to fail") + } } } diff --git a/crates/core/tests/unit/shared_tests.rs b/crates/core/tests/unit/shared_tests.rs index 2249ca9f7..69793a38f 100644 --- a/crates/core/tests/unit/shared_tests.rs +++ b/crates/core/tests/unit/shared_tests.rs @@ -142,6 +142,30 @@ fn test_metadata_with_otel_error_adds_structured_error_type() { .unwrap(); assert_eq!(explicit_metadata["error.type"], json!("provider_timeout")); + + let external_metadata = metadata_with_otel_error( + None, + &FlowError::CallbackException { + message: "ValueError: boom".into(), + exception_type: "ValueError".into(), + }, + ) + .unwrap(); + assert_eq!(external_metadata["error.type"], json!("internal_error")); + assert_eq!(external_metadata["exception.type"], json!("ValueError")); + + let explicit_exception_metadata = metadata_with_otel_error( + Some(json!({"exception.type": "CallerException"})), + &FlowError::CallbackException { + message: "ValueError: boom".into(), + exception_type: "ValueError".into(), + }, + ) + .unwrap(); + assert_eq!( + explicit_exception_metadata["exception.type"], + json!("CallerException") + ); } #[test] diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index 9a17fe485..112286a8b 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -445,6 +445,23 @@ typedef char *(*NemoRelayToolExecInterceptCb)(void *user_data, */ typedef char *(*NemoRelayToolExecCb)(void *user_data, const char *args_json); +/** + * Initializes the Go binding runtime and installs default operational logging. + * + * Logging configuration is resolved from `NEMO_RELAY_LOG`, + * `NEMO_RELAY_LOG_STDERR_FORMAT`, or `NEMO_RELAY_LOG_CONFIG_PATH`, with built-in defaults when + * none are set. Repeated initialization is a no-op. + */ +NemoRelayStatus nemo_relay_initialize_default_logging(void); + +/** + * Shuts down and releases the default operational logging runtime. + * + * Pending file-sink records are drained before this function returns. Repeated shutdown is a + * no-op. + */ +NemoRelayStatus nemo_relay_shutdown_default_logging(void); + /** * Run the registered tool request intercept chain on the given arguments. * diff --git a/crates/ffi/src/api/mod.rs b/crates/ffi/src/api/mod.rs index cd08d8d03..848ad7243 100644 --- a/crates/ffi/src/api/mod.rs +++ b/crates/ffi/src/api/mod.rs @@ -99,6 +99,35 @@ fn tokio_runtime() -> &'static Runtime { }) } +/// Initializes the Go binding runtime and installs default operational logging. +/// +/// Logging configuration is resolved from `NEMO_RELAY_LOG`, +/// `NEMO_RELAY_LOG_STDERR_FORMAT`, or `NEMO_RELAY_LOG_CONFIG_PATH`, with built-in defaults when +/// none are set. Repeated initialization is a no-op. +#[unsafe(no_mangle)] +pub extern "C" fn nemo_relay_initialize_default_logging() -> NemoRelayStatus { + clear_last_error(); + let result = nemo_relay::shared_runtime::initialize_shared_runtime_binding("go") + .and_then(|()| nemo_relay::logging::initialize_default_logging()); + match result { + Ok(()) => NemoRelayStatus::Ok, + Err(error) => status_from_error(&error), + } +} + +/// Shuts down and releases the default operational logging runtime. +/// +/// Pending file-sink records are drained before this function returns. Repeated shutdown is a +/// no-op. +#[unsafe(no_mangle)] +pub extern "C" fn nemo_relay_shutdown_default_logging() -> NemoRelayStatus { + clear_last_error(); + match nemo_relay::logging::shutdown_default_logging() { + Ok(()) => NemoRelayStatus::Ok, + Err(error) => status_from_error(&error), + } +} + fn block_on_sync_ffi(future: F) -> FlowResult where T: Send, diff --git a/crates/ffi/src/error.rs b/crates/ffi/src/error.rs index 207197123..3bfa7a4f2 100644 --- a/crates/ffi/src/error.rs +++ b/crates/ffi/src/error.rs @@ -120,7 +120,9 @@ impl From<&FlowError> for NemoRelayStatus { FlowError::InvalidArgument(_) => NemoRelayStatus::InvalidArg, FlowError::ScopeStackEmpty => NemoRelayStatus::ScopeStackEmpty, FlowError::GuardrailRejected(_) => NemoRelayStatus::GuardrailRejected, - FlowError::Upstream(_) | FlowError::Internal(_) => NemoRelayStatus::Internal, + FlowError::Upstream(_) + | FlowError::Internal(_) + | FlowError::CallbackException { .. } => NemoRelayStatus::Internal, } } } diff --git a/crates/ffi/tests/coverage/error_tests.rs b/crates/ffi/tests/coverage/error_tests.rs index fe2792666..cc5587d47 100644 --- a/crates/ffi/tests/coverage/error_tests.rs +++ b/crates/ffi/tests/coverage/error_tests.rs @@ -70,6 +70,13 @@ fn test_status_from_error_maps_variants_and_sets_message() { FlowError::Internal("boom".into()), NemoRelayStatus::Internal, ), + ( + FlowError::CallbackException { + message: "callback boom".into(), + exception_type: "ValueError".into(), + }, + NemoRelayStatus::Internal, + ), (FlowError::ScopeStackEmpty, NemoRelayStatus::ScopeStackEmpty), ]; diff --git a/crates/ffi/tests/unit/api_tests.rs b/crates/ffi/tests/unit/api_tests.rs index 6f9eac085..e99c1b5ca 100644 --- a/crates/ffi/tests/unit/api_tests.rs +++ b/crates/ffi/tests/unit/api_tests.rs @@ -243,6 +243,19 @@ unsafe fn fresh_scope_stack() -> *mut FfiScopeStack { stack } +#[test] +fn default_logging_shutdown_is_idempotent() { + let _guard = lock_unpoisoned(&TEST_MUTEX); + assert_status!( + api::nemo_relay_shutdown_default_logging(), + NemoRelayStatus::Ok + ); + assert_status!( + api::nemo_relay_shutdown_default_logging(), + NemoRelayStatus::Ok + ); +} + #[test] fn propagation_context_json_round_trips_through_the_ffi() { let _guard = lock_unpoisoned(&TEST_MUTEX); diff --git a/crates/node/src/api/mod.rs b/crates/node/src/api/mod.rs index 9daec5dad..c81958ef8 100644 --- a/crates/node/src/api/mod.rs +++ b/crates/node/src/api/mod.rs @@ -16,7 +16,7 @@ use std::pin::Pin; use std::ptr; use std::sync::Arc; use std::sync::Mutex as StdMutex; -use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use std::task::{Context, Poll}; use chrono::{DateTime, Utc}; @@ -88,6 +88,29 @@ use crate::promise_call::with_publication_callback_context; use crate::stream::LlmStream; use crate::types::{LlmHandle, ScopeHandle, ScopeStack, ScopeType, ToolHandle}; +static NODE_ENVIRONMENT_COUNT: AtomicUsize = AtomicUsize::new(0); +static NODE_ENVIRONMENT_LIFECYCLE_LOCK: StdMutex<()> = StdMutex::new(()); + +fn register_node_environment() -> FlowResult<()> { + let _guard = NODE_ENVIRONMENT_LIFECYCLE_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + nemo_relay::logging::initialize_default_logging()?; + NODE_ENVIRONMENT_COUNT.fetch_add(1, Ordering::AcqRel); + Ok(()) +} + +fn cleanup_node_environment() { + let _guard = NODE_ENVIRONMENT_LIFECYCLE_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if NODE_ENVIRONMENT_COUNT.fetch_sub(1, Ordering::AcqRel) == 1 + && let Err(error) = nemo_relay::logging::shutdown_default_logging() + { + eprintln!("nemo-relay: operational logging shutdown failed: {error}"); + } +} + fn effective_scope_context( env: &Env, ) -> napi::Result<( @@ -127,7 +150,12 @@ fn init() { #[cfg(not(test))] #[napi_derive::module_exports] -fn install_well_known_symbol_methods(exports: JsObject, env: Env) -> napi::Result<()> { +fn install_well_known_symbol_methods(exports: JsObject, mut env: Env) -> napi::Result<()> { + register_node_environment().map_err(to_napi_err)?; + if let Err(error) = env.add_env_cleanup_hook((), |_| cleanup_node_environment()) { + cleanup_node_environment(); + return Err(error); + } let activation: JsFunction = exports.get_named_property("DynamicPluginActivation")?; let activation = activation.coerce_to_object()?; let mut prototype: JsObject = activation.get_named_property("prototype")?; diff --git a/crates/node/src/callable.rs b/crates/node/src/callable.rs index b6052059a..e807979be 100644 --- a/crates/node/src/callable.rs +++ b/crates/node/src/callable.rs @@ -163,6 +163,8 @@ struct MiddlewareCallbackResult { value: Json, #[serde(default)] error: String, + #[serde(default, rename = "exceptionType")] + exception_type: String, } /// Wrap a middleware callback so exceptions cross the N-API boundary as data. @@ -207,11 +209,16 @@ pub(crate) fn unwrap_middleware_result(value: Json, error_prefix: &str) -> Resul })?; if result.ok { Ok(result.value) - } else { + } else if result.exception_type.is_empty() { Err(FlowError::Internal(format!( "{error_prefix}: {}", result.error ))) + } else { + Err(FlowError::CallbackException { + message: format!("{error_prefix}: {}", result.error), + exception_type: result.exception_type, + }) } } diff --git a/crates/node/src/callback_factory.rs b/crates/node/src/callback_factory.rs index cb7988849..0990d343e 100644 --- a/crates/node/src/callback_factory.rs +++ b/crates/node/src/callback_factory.rs @@ -165,16 +165,26 @@ const CALLBACK_FACTORIES_SOURCE: &str = r#"(() => { }, (error) => { settlePublication(); let message = 'unknown error'; + let exceptionType = 'Error'; try { if (typeof error === 'string') { message = error; } else if (error === null || (typeof error !== 'object' && typeof error !== 'function')) { message = String(error); - } else if (error != null && typeof error.message === 'string') { - message = error.message; + } else if (error != null) { + const errorMessage = error.message; + if (typeof errorMessage === 'string') { + message = errorMessage; + } } } catch {} - reject(message); + try { + const errorName = error?.name; + if (typeof errorName === 'string' && errorName.length > 0) { + exceptionType = errorName; + } + } catch {} + reject(message, exceptionType); }); }; eventSanitizerContext.run(token, invoke); @@ -188,10 +198,20 @@ const CALLBACK_FACTORIES_SOURCE: &str = r#"(() => { return { ok: true, value: jsonValue(value === undefined ? null : value) }; } catch (error) { let message = 'JavaScript callback failed'; + let exceptionType = 'Error'; try { - message = String(error?.message ?? error); + const errorMessage = error?.message; + if (typeof errorMessage === 'string') { + message = errorMessage; + } } catch {} - return { ok: false, error: message }; + try { + const errorName = error?.name; + if (typeof errorName === 'string' && errorName.length > 0) { + exceptionType = errorName; + } + } catch {} + return { ok: false, error: message, exceptionType }; } }; }, @@ -212,11 +232,18 @@ const CALLBACK_FACTORIES_SOURCE: &str = r#"(() => { ) { if (error != null) { let message = 'unknown error'; + let exceptionType = 'Error'; try { message = String(error?.message ?? error); } catch {} + try { + const errorName = error?.name; + if (typeof errorName === 'string' && errorName.length > 0) { + exceptionType = errorName; + } + } catch {} if (typeof reject === 'function') { - reject(message); + reject(message, exceptionType); } return; } diff --git a/crates/node/src/promise_call.rs b/crates/node/src/promise_call.rs index 892bd6627..7832468d5 100644 --- a/crates/node/src/promise_call.rs +++ b/crates/node/src/promise_call.rs @@ -334,7 +334,11 @@ fn build_completion_unknowns( let message = ctx .get::(0) .unwrap_or_else(|_| "unknown error".to_string()); - completion.send(Err(FlowError::Internal(message))); + let exception_type = ctx.get::(1).unwrap_or_else(|_| "Error".to_string()); + completion.send(Err(FlowError::CallbackException { + message, + exception_type, + })); ctx.env.get_undefined() })?; diff --git a/crates/node/tests/llm_tests.mjs b/crates/node/tests/llm_tests.mjs index a2254af50..a65e3f702 100644 --- a/crates/node/tests/llm_tests.mjs +++ b/crates/node/tests/llm_tests.mjs @@ -90,6 +90,9 @@ function sparseArray() { function unprintableError() { const error = new Error('sanitize request guardrail failed'); Object.defineProperties(error, { + name: { + value: 'GetterError', + }, message: { get() { throw new Error('message getter boom'); @@ -285,11 +288,11 @@ describe('LLM execute', () => { await assert.rejects( () => - llmCallExecuteAsync( + llmCallExecute( 'exec_status_error_llm', makeNative(), - async () => { - throw new Error('llm status failure'); + () => { + throw unprintableError(); }, null, null, @@ -299,7 +302,7 @@ describe('LLM execute', () => { }, null, ), - /llm status failure/, + /JavaScript callback failed/, ); await flushSubscribers(); @@ -320,8 +323,9 @@ describe('LLM execute', () => { assert.ok(errorEnd, 'expected failed llm end event'); assert.equal(errorEnd.metadata.caller, 'node-llm-error'); assert.equal(errorEnd.metadata['otel.status_code'], 'ERROR'); - assert.match(errorEnd.metadata['otel.status_description'], /llm status failure/); + assert.match(errorEnd.metadata['otel.status_description'], /JavaScript callback failed/); assert.equal(errorEnd.metadata['error.type'], 'internal_error'); + assert.equal(errorEnd.metadata['exception.type'], 'GetterError'); } finally { deregisterSubscriber('node_llm_status_metadata_sub'); } diff --git a/crates/node/tests/logging_tests.mjs b/crates/node/tests/logging_tests.mjs new file mode 100644 index 000000000..2e4c8170b --- /dev/null +++ b/crates/node/tests/logging_tests.mjs @@ -0,0 +1,121 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const packageDirectory = fileURLToPath(new URL('..', import.meta.url)); +const loggingEnvironmentNames = ['NEMO_RELAY_LOG', 'NEMO_RELAY_LOG_STDERR_FORMAT', 'NEMO_RELAY_LOG_CONFIG_PATH']; + +function requireBinding(loggingEnvironment, source = "require('./index.js')") { + const environment = { ...process.env }; + for (const name of loggingEnvironmentNames) { + delete environment[name]; + } + Object.assign(environment, loggingEnvironment); + return spawnSync(process.execPath, ['-e', source], { + cwd: packageDirectory, + encoding: 'utf8', + env: environment, + }); +} + +describe('operational logging', () => { + it('initializes from the logging environment', () => { + const result = requireBinding({ + NEMO_RELAY_LOG: 'info', + NEMO_RELAY_LOG_STDERR_FORMAT: 'jsonl', + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stderr, /"event":"logging_initialized"/); + }); + + it('rejects an invalid logging environment', () => { + const result = requireBinding({ NEMO_RELAY_LOG: '' }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /NEMO_RELAY_LOG must not be empty/); + }); + + it('flushes file sinks during environment cleanup', () => { + const directory = mkdtempSync(join(tmpdir(), 'nemo-relay-node-logging-')); + try { + const configPath = join(directory, 'logging.toml'); + const logPath = join(directory, 'operational.jsonl'); + writeFileSync( + configPath, + `[logging] +level = "info" +stderr_format = "human" +flush_interval_millis = 0 + +[[logging.sinks]] +path = ${JSON.stringify(logPath)} +level = "info" +format = "jsonl" +queue_capacity = 16 +`, + ); + + const result = requireBinding({ NEMO_RELAY_LOG_CONFIG_PATH: configPath }); + + assert.equal(result.status, 0, result.stderr); + assert.match(readFileSync(logPath, 'utf8'), /"event":"logging_shutdown_started"/); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + + it('keeps logging active while another Node environment remains', () => { + const directory = mkdtempSync(join(tmpdir(), 'nemo-relay-node-worker-logging-')); + try { + const configPath = join(directory, 'logging.toml'); + const logPath = join(directory, 'operational.jsonl'); + writeFileSync( + configPath, + `[logging] +level = "info" +stderr_format = "human" +flush_interval_millis = 0 + +[[logging.sinks]] +path = ${JSON.stringify(logPath)} +level = "info" +format = "jsonl" +queue_capacity = 16 +`, + ); + const workerSource = `require(${JSON.stringify(join(packageDirectory, 'index.js'))})`; + const source = ` +const { Worker } = require('node:worker_threads'); +const relay = require('./index.js'); +const worker = new Worker(${JSON.stringify(workerSource)}, { + eval: true, +}); +worker.once('error', (error) => { + console.error(error); + process.exitCode = 1; +}); +worker.once('exit', (code) => { + if (code !== 0) process.exitCode = code; + relay.deregisterPlugin('adaptive'); +}); +`; + + const result = requireBinding({ NEMO_RELAY_LOG_CONFIG_PATH: configPath }, source); + + assert.equal(result.status, 0, result.stderr); + const output = readFileSync(logPath, 'utf8'); + assert.match(output, /"event":"plugin_deregistered"/); + assert.match(output, /"event":"logging_shutdown_started"/); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); +}); diff --git a/crates/node/tests/tools_tests.mjs b/crates/node/tests/tools_tests.mjs index 29a262e91..e1cc7c6a1 100644 --- a/crates/node/tests/tools_tests.mjs +++ b/crates/node/tests/tools_tests.mjs @@ -380,11 +380,11 @@ describe('Tool execute', () => { await assert.rejects( () => - toolCallExecuteAsync( + toolCallExecute( 'exec_status_error_tool', {}, - async () => { - throw new Error('tool status failure'); + () => { + throw new TypeError('tool status failure'); }, null, null, @@ -416,6 +416,7 @@ describe('Tool execute', () => { assert.equal(errorEnd.metadata['otel.status_code'], 'ERROR'); assert.match(errorEnd.metadata['otel.status_description'], /tool status failure/); assert.equal(errorEnd.metadata['error.type'], 'internal_error'); + assert.equal(errorEnd.metadata['exception.type'], 'TypeError'); } finally { deregisterSubscriber('node_tool_status_metadata_sub'); } diff --git a/crates/python/src/lib.rs b/crates/python/src/lib.rs index 84ebb2295..a5f7ed04b 100644 --- a/crates/python/src/lib.rs +++ b/crates/python/src/lib.rs @@ -50,6 +50,11 @@ fn _native(m: &Bound<'_, PyModule>) -> PyResult<()> { "failed to initialize NeMo Relay runtime ownership: {e}" )) })?; + nemo_relay::logging::initialize_default_logging().map_err(|e| { + pyo3::exceptions::PyRuntimeError::new_err(format!( + "failed to initialize NeMo Relay operational logging: {e}" + )) + })?; register_adaptive_component().map_err(|e| { pyo3::exceptions::PyRuntimeError::new_err(format!( "failed to register adaptive plugin component: {e}" diff --git a/crates/python/src/py_api/mod.rs b/crates/python/src/py_api/mod.rs index d09933a0f..54f801702 100644 --- a/crates/python/src/py_api/mod.rs +++ b/crates/python/src/py_api/mod.rs @@ -62,6 +62,12 @@ fn to_py_err(e: FlowError) -> PyErr { PyErr::new::(e.to_string()) } +#[pyfunction(name = "_shutdown_default_logging")] +fn py_shutdown_default_logging(py: Python<'_>) -> PyResult<()> { + py.detach(nemo_relay::logging::shutdown_default_logging) + .map_err(to_py_err) +} + fn python_event_loop_running(py: Python<'_>) -> PyResult { match py.import("asyncio")?.call_method0("get_running_loop") { Ok(_) => Ok(true), @@ -2083,6 +2089,8 @@ fn scope_deregister_subscriber(scope_uuid: &str, name: &str) -> PyResult { /// Register all API functions into the given `PyModule`. pub fn register(m: &Bound<'_, PyModule>) -> PyResult<()> { + m.add_function(wrap_pyfunction!(py_shutdown_default_logging, m)?)?; + // Scope stack creation / binding / query m.add_function(wrap_pyfunction!(create_scope_stack, m)?)?; m.add_function(wrap_pyfunction!(capture_propagation_context, m)?)?; diff --git a/crates/python/src/py_callable.rs b/crates/python/src/py_callable.rs index 50772a3e6..b73a1dacf 100644 --- a/crates/python/src/py_callable.rs +++ b/crates/python/src/py_callable.rs @@ -59,6 +59,20 @@ use crate::py_types::{ type PyValueFuture = Pin>> + Send>>; +fn python_callback_error(error: PyErr) -> FlowError { + let exception_type = Python::attach(|py| { + error + .get_type(py) + .getattr("__name__") + .and_then(|name| name.extract::()) + .unwrap_or_else(|_| "Exception".to_string()) + }); + FlowError::CallbackException { + message: error.to_string(), + exception_type, + } +} + struct CancellablePyFuture { inner: PyValueFuture, scheduled: Arc>, @@ -296,9 +310,7 @@ async fn resolve_json_or_future( match outcome? { Ok(json) => Ok(json), Err(future) => { - let py_result = future - .await - .map_err(|e| FlowError::Internal(e.to_string()))?; + let py_result = future.await.map_err(python_callback_error)?; Python::attach(|py| { py_to_json(py_result.bind(py)) .map_err(|e: PyErr| FlowError::Internal(e.to_string())) @@ -505,7 +517,7 @@ async fn resolve_py_object_or_future( ) -> FlowResult> { match outcome? { Ok(value) => Ok(value), - Err(future) => future.await.map_err(|e| FlowError::Internal(e.to_string())), + Err(future) => future.await.map_err(python_callback_error), } } @@ -573,7 +585,7 @@ async fn await_async_iter_task_result(task: Py) -> FlowResult context.call_method1("run", (callback.bind(py), py_args)), None => callback.bind(py).call1((py_args,)), } - .map_err(|error| FlowError::Internal(error.to_string()))?; + .map_err(python_callback_error)?; split_json_or_future_with_locals(py, result.unbind(), task_locals.as_ref()) })) .await @@ -1264,7 +1276,7 @@ pub fn wrap_py_llm_stream_exec_intercept_fn( } None => callback.bind(py).call1((py_req, py_next)), } - .map_err(|e: PyErr| FlowError::Internal(e.to_string()))?; + .map_err(python_callback_error)?; let outcome = split_py_object_or_future_with_locals( py, result.unbind(), @@ -1508,7 +1520,7 @@ pub fn wrap_py_llm_exec_fn( Some(context) => context.call_method1("run", (callback.bind(py), py_req)), None => callback.bind(py).call1((py_req,)), } - .map_err(|error| FlowError::Internal(error.to_string()))?; + .map_err(python_callback_error)?; split_json_or_future_with_locals(py, result.unbind(), task_locals.as_ref()) })) .await @@ -1546,7 +1558,7 @@ pub fn wrap_py_llm_stream_exec_fn( Some(context) => context.call_method1("run", (callback.bind(py), py_req)), None => callback.bind(py).call1((py_req,)), } - .map_err(|error| FlowError::Internal(error.to_string()))?; + .map_err(python_callback_error)?; let outcome = split_py_object_or_future_with_locals( py, result.unbind(), @@ -1566,7 +1578,7 @@ pub fn wrap_py_llm_stream_exec_fn( /// The collector is invoked with each intercepted chunk (after stream response /// intercepts have been applied). It receives a single JSON-converted Python /// object argument. If the Python callable raises an exception, it is converted -/// to a `FlowError::Internal` and returned as `Err`, which terminates the +/// to a `FlowError::CallbackException` and returned as `Err`, which terminates the /// stream. If the callable returns normally (including `None`), the collector /// returns `Ok(())`. pub fn wrap_py_collector_fn( @@ -1578,7 +1590,7 @@ pub fn wrap_py_collector_fn( .map_err(|e| FlowError::Internal(format!("collector json_to_py failed: {e}")))?; py_fn .call1(py, (py_chunk,)) - .map_err(|e| FlowError::Internal(format!("Python collector error: {e}")))?; + .map_err(python_callback_error)?; Ok(()) }) }) diff --git a/crates/python/src/py_plugin.rs b/crates/python/src/py_plugin.rs index 281ae0eca..ec11ce32e 100644 --- a/crates/python/src/py_plugin.rs +++ b/crates/python/src/py_plugin.rs @@ -933,16 +933,20 @@ impl PluginConfigurationClearState { let result = std::panic::catch_unwind(clear_plugin_configuration) .map_err(|_| PluginTeardownError::runtime("plugin teardown task panicked")) .and_then(|result| result.map_err(PluginTeardownError::from_plugin_error)); - clear_state.completion.finish(result); + clear_state.finish(result); }); if let Err(error) = spawn { - self.completion - .finish(Err(PluginTeardownError::runtime(format!( - "failed to start plugin teardown task: {error}" - )))); + self.finish(Err(PluginTeardownError::runtime(format!( + "failed to start plugin teardown task: {error}" + )))); } } + fn finish(self: &Arc, result: PluginTeardownResult) { + reset_plugin_configuration_clear_state_if(self); + self.completion.finish(result); + } + async fn wait_for_clear(&self) -> PluginTeardownResult { self.completion.wait("plugin teardown").await } @@ -965,6 +969,15 @@ fn reset_plugin_configuration_clear_state() { Arc::new(PluginConfigurationClearState::new()); } +fn reset_plugin_configuration_clear_state_if(completed: &Arc) { + let mut current = PLUGIN_CONFIGURATION_CLEAR_STATE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if Arc::ptr_eq(¤t, completed) { + *current = Arc::new(PluginConfigurationClearState::new()); + } +} + #[pymethods] impl PyPluginHostActivation { /// Return the activation report captured during initialization. diff --git a/crates/python/tests/coverage/py_plugin_coverage_tests.rs b/crates/python/tests/coverage/py_plugin_coverage_tests.rs index 88127cdea..b8818295b 100644 --- a/crates/python/tests/coverage/py_plugin_coverage_tests.rs +++ b/crates/python/tests/coverage/py_plugin_coverage_tests.rs @@ -165,6 +165,7 @@ fn async_clear_binding_completes_on_python_event_loop() { let _python = crate::test_support::init_python_test(); let _plugin_test_state = lock_plugin_test_state_for_tests(); Python::attach(|py| { + let first_clear_state = plugin_configuration_clear_state(); let module = PyModule::new(py, "_plugin_async_clear").unwrap(); register(&module).unwrap(); let helpers = load_module( @@ -174,6 +175,19 @@ async def clear(module): await module.clear_plugin_configuration_async() "#, ); + with_event_loop(py, |event_loop| { + let clear = helpers + .getattr("clear") + .unwrap() + .call1((module.clone(),)) + .unwrap(); + event_loop + .call_method1("run_until_complete", (clear,)) + .unwrap(); + }); + let second_clear_state = plugin_configuration_clear_state(); + assert!(!Arc::ptr_eq(&first_clear_state, &second_clear_state)); + with_event_loop(py, |event_loop| { let clear = helpers.getattr("clear").unwrap().call1((module,)).unwrap(); event_loop @@ -184,6 +198,22 @@ async def clear(module): }); } +#[test] +fn stale_async_clear_completion_keeps_the_newer_state() { + let _plugin_test_state = lock_plugin_test_state_for_tests(); + let older = plugin_configuration_clear_state(); + let newer = Arc::new(PluginConfigurationClearState::new()); + *PLUGIN_CONFIGURATION_CLEAR_STATE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = Arc::clone(&newer); + + reset_plugin_configuration_clear_state_if(&older); + + let newer_state_remained_current = Arc::ptr_eq(&plugin_configuration_clear_state(), &newer); + reset_plugin_configuration_clear_state(); + assert!(newer_state_remained_current); +} + #[test] fn plugin_context_registers_all_runtime_hooks_and_drains_registrations() { let _python = crate::test_support::init_python_test(); diff --git a/crates/switchyard/README.md b/crates/switchyard/README.md index ba50b355f..0d8dece3b 100644 --- a/crates/switchyard/README.md +++ b/crates/switchyard/README.md @@ -9,14 +9,19 @@ SPDX-License-Identifier: Apache-2.0 # NeMo Relay Switchyard Plugin +> **Deprecated:** The experimental `nemo-relay-switchyard` plugin will be +> removed in NeMo Relay 0.8 and replaced by a Switchyard-owned native plugin. +> The NeMo Relay 0.8 documentation will include an updated configuration guide +> and migration plan when the replacement is available. + `nemo-relay-switchyard` is NeMo Relay's experimental integration with the [NVIDIA NeMo Switchyard](https://github.com/NVIDIA-NeMo/Switchyard) Decision API. It adds routing-aware LLM execution intercepts to the Relay runtime while preserving Relay ownership of provider credentials, target bindings, dispatch, retries, fallbacks, and observability. -NeMo Relay 0.6.0 uses a separately running Switchyard Decision API from the -`topic/nemo-relay-integration` branch. +NeMo Relay 0.6.0 and 0.7.0 use a separately running Switchyard Decision API +from the `topic/nemo-relay-integration` branch. Install it from crates.io, or build it from the NeMo Relay source checkout with the optional CLI feature while the Switchyard Decision API contract and diff --git a/docs/about-nemo-relay/release-notes/index.mdx b/docs/about-nemo-relay/release-notes/index.mdx index 408d20151..6d9ed0ae7 100644 --- a/docs/about-nemo-relay/release-notes/index.mdx +++ b/docs/about-nemo-relay/release-notes/index.mdx @@ -32,6 +32,10 @@ compatibility information applies to the current 0.7 prerelease. ### Highlights +- Generated coding-agent enforcement hooks now fail closed when Relay cannot + start, authenticate, evaluate, or deliver a response. Lifecycle and + after-the-fact hooks explicitly fail open. Reinstall each coding-agent + integration with `nemo-relay install --force` after upgrading. - LLM observability sanitizers now receive the active request or response codec for each managed call. Sanitizers can normalize built-in, runtime-registered, and opaque codec payloads without changing the @@ -98,6 +102,12 @@ their values cannot be isolated between endpoints. ### Fixed Known Issues in 0.7 +- Embedded Python hosts can use + `plugin.load_dynamic_plugin_activation_specs(path)` to convert the standard + `[[plugins.dynamic]]` records in one explicitly selected `plugins.toml` into + the activation specs accepted by `initialize_with_dynamic_plugins()`. This + scoped 0.7 compatibility helper removes host-side TOML and manifest parsing; + a future unified file-backed initializer is expected to replace it. - Programmatically declared plugin components now apply their `enabled` value over discovered file configuration. When code re-enables a component that a discovered file disabled, initialization reports a warning that names the diff --git a/docs/configure-plugins/about.mdx b/docs/configure-plugins/about.mdx index b96315a2f..6e1de0772 100644 --- a/docs/configure-plugins/about.mdx +++ b/docs/configure-plugins/about.mdx @@ -33,9 +33,10 @@ entries in `plugins.toml`. Guardrails-backed policy checks. - [PII Redaction](/configure-plugins/pii-redaction/about) sanitizes sensitive data in observability payloads. -- [Switchyard (Experimental)](/configure-plugins/switchyard/about) routes - requests using decisions from a separately running Switchyard Decision API - service. +- [Switchyard (Deprecated)](/configure-plugins/switchyard/about) routes requests + using decisions from a separately running Switchyard Decision API service. + The experimental plugin will be removed in NeMo Relay 0.8 and replaced by a + Switchyard-owned native plugin. - [Model Pricing](/configure-plugins/model-pricing) configures catalog sources for cost estimates on managed LLM responses. diff --git a/docs/configure-plugins/observability/atif.mdx b/docs/configure-plugins/observability/atif.mdx index dec1ad05c..077d11c2d 100644 --- a/docs/configure-plugins/observability/atif.mdx +++ b/docs/configure-plugins/observability/atif.mdx @@ -89,18 +89,20 @@ With scope metadata `{"atif_prefix":"tenant-a/session-123"}`, Relay writes still contain `{session_id}`. Use `:-` to provide a literal fallback when metadata can be absent, for example -`{metadata.atif_prefix:-unassigned}`. Relay also uses the fallback when the -metadata value is not a string. +`{metadata.atif_prefix:-unassigned}`. Relay uses the fallback when the selected +metadata field is absent or `null`. A present non-string value is rejected +instead of being routed through the fallback. Each metadata placeholder must resolve to a string containing a non-empty, relative path fragment. Slash-separated segments can contain ASCII letters, digits, `-`, `_`, `.`, and `~`. Relay rejects empty segments, `.` and `..` segments, absolute paths, backslashes, spaces, and other characters. If a -placeholder has no fallback and is missing or non-string, or if its resolved -value is unsafe, Relay skips that trajectory. It records a runtime diagnostic -in `plugin.report()` and causes plugin teardown to fail. Literal template text -must also be a relative, traversal-free path. The rendered filename applies to -local, S3, and HTTP storage in the same way as a static filename. +placeholder is missing without a fallback, is present but non-string, or +resolves to an unsafe value, Relay skips that trajectory. It records a runtime +diagnostic in `plugin.report()` and causes plugin teardown to fail. Literal +template text must also be a relative, traversal-free path. The rendered +filename applies to local, S3, and HTTP storage in the same way as a static +filename. The CLI gateway parses `x-nemo-relay-session-metadata` as JSON and merges it into the top-level scope metadata: @@ -245,10 +247,15 @@ delivery diagnostic and retries that destination for each later trajectory. Other destinations continue to receive writes. When every remote destination fails for one trajectory, Relay writes a local recovery copy under `output_directory`; a successful remote destination does not create that copy. -The diagnostic remains visible in `plugin.report()` and is retained there after -a failed teardown. Teardown also reports the degraded delivery, even when the -local recovery write succeeds. Fatal dispatcher failures, such as trajectory -serialization failures, are also reported during teardown. +The diagnostic becomes visible in `plugin.report()` after subscriber delivery +is flushed and is retained there after a failed teardown. Teardown also reports +the degraded delivery, even when the local recovery write succeeds. This error +reports delivery degradation, not a registration leak; callbacks have already +been removed. If the failure is still pending when initialization replaces the +configuration, that replacement returns the delivery error and leaves no +configuration active; a subsequent clear or initialization is safe. Fatal +dispatcher failures, such as trajectory serialization failures, are also +reported during teardown. ## Expected Output diff --git a/docs/configure-plugins/observability/configuration.mdx b/docs/configure-plugins/observability/configuration.mdx index 2cf596424..88b8b61ca 100644 --- a/docs/configure-plugins/observability/configuration.mdx +++ b/docs/configure-plugins/observability/configuration.mdx @@ -71,8 +71,12 @@ endpoint and transport: Relay rejects that configuration because their deterministic trace and span IDs would collide at the receiver. For `http_binary`, this comparison uses the effective trace destination, so a bare URL and the same URL with `/v1/traces` also -collide. All endpoints are constructed before the plugin -registers its fan-out subscriber. +collide. The comparison realizes HTTP port `80` and HTTPS port `443`, collapses +repeated path slashes, ignores a non-root trailing slash, and treats standardized +loopback forms (`localhost`, names under `.localhost`, `127.0.0.0/8`, and `::1`) +as the same host without resolving DNS. Query strings remain part of the +destination. All endpoints are constructed before the plugin registers its +fan-out subscriber. ## Multi-Endpoint Lifecycle diff --git a/docs/configure-plugins/observability/opentelemetry.mdx b/docs/configure-plugins/observability/opentelemetry.mdx index 7b21893f2..edfc432d7 100644 --- a/docs/configure-plugins/observability/opentelemetry.mdx +++ b/docs/configure-plugins/observability/opentelemetry.mdx @@ -31,7 +31,11 @@ compliant trace and span IDs from Relay lifecycle UUIDs, so endpoints that receive the same event stream use the same identifiers and parentage. Different endpoint types must therefore use independent OTLP destinations; configuring them with the same endpoint and transport is rejected to prevent identifier -collisions at the receiver. +collisions at the receiver. Duplicate detection compares canonical destinations: +HTTP and HTTPS default ports are realized, repeated and trailing path slashes +are normalized, and standardized loopback hosts such as `localhost`, names +under `.localhost`, `127.0.0.0/8`, and `::1` are equivalent. Relay does not use +DNS resolution for this comparison, and query strings remain significant. Rooted Relay propagation continues the Relay-derived trace across the import boundary. Rootless propagation retains Relay event parentage but starts a new OpenTelemetry trace from the first local event. Carry W3C `traceparent` and @@ -206,14 +210,23 @@ For managed LLM, tool, and stream failures, NeMo Relay maps structured | Upstream invalid request | `invalid_request` | | Other upstream failure | `upstream_error` | | `Internal` | `internal_error` | +| Binding callback exception | `internal_error` | External application and callback exceptions that do not have a more specific -`FlowError` classification are represented as `Internal` and emit -`internal_error`. NeMo Relay does not inspect error messages to recover Python, -JavaScript, or application-defined exception class names. When no structured -`FlowError` is available, such as a cancellation or dropped execution, the -GenAI projection emits `_OTHER`. Caller-provided `error.type` metadata takes -precedence over the derived mapping. +`FlowError` classification emit `internal_error`. Python and JavaScript callback +boundaries also preserve the exception class separately, and both the `full` +and `gen_ai` projections emit an `exception` span event with `exception.type`. +NeMo Relay does not inspect error messages to recover exception class names. +When an errored parent span has no useful classification of its own, it +inherits the failed descendant's `error.type` and exception type. When no +structured `FlowError` is available, such as a cancellation or dropped +execution, the projection emits `_OTHER`. Caller-provided `error.type` and +`exception.type` metadata take precedence over values derived from `FlowError`. + +`FlowError` is an exhaustive Rust enum. Rust callers upgrading to this release +must handle the new `CallbackException` variant in exhaustive matches. It maps +to the same internal status as `Internal`, while retaining `exception_type` for +observability projection. ## Direct Subscriber diff --git a/docs/configure-plugins/plugin-configuration-files.mdx b/docs/configure-plugins/plugin-configuration-files.mdx index e5a486c9f..203360961 100644 --- a/docs/configure-plugins/plugin-configuration-files.mdx +++ b/docs/configure-plugins/plugin-configuration-files.mdx @@ -163,6 +163,50 @@ manifest’s optional static JSON Schema before you enable or run the plugin. Us dynamic-plugin lifecycle. Refer to [Configure Discoverable Plugins](/configure-plugins/discoverable-plugins) for manifest, trust, and policy requirements. +### Embedded Python Compatibility Helper + +Python hosts that already own plugin activation can convert the dynamic records +from one explicitly selected file into the activation specs accepted by the +0.7 host API: + +```python +import asyncio + +from nemo_relay import plugin + + +async def main() -> None: + dynamic_plugins = plugin.load_dynamic_plugin_activation_specs( + "path/to/plugins.toml" + ) + activation = await plugin.initialize_with_dynamic_plugins({}, dynamic_plugins) + async with activation: + # Run your host application while dynamic plugins are active. + ... + + +asyncio.run(main()) +``` + +The helper resolves each manifest relative to `plugins.toml` and reads the +plugin ID and execution lane from the manifest. It does not perform discovery, +consult CLI lifecycle state, provision a Python worker environment, or change +enablement. + +On success, the helper returns every `[[plugins.dynamic]]` declaration in file +order; it does not skip invalid entries. A missing `plugins.toml` or referenced +manifest raises `FileNotFoundError`. Malformed TOML, invalid records or required +manifest fields, duplicate plugin IDs, and non-JSON configuration raise +`ValueError`. The helper does not apply an optional manifest-declared static +JSON Schema. + +Passing the result to `initialize_with_dynamic_plugins()` is explicit consent +to load those trusted native libraries or worker processes. + +This helper is a 0.7 compatibility surface for embedded integrations and is +planned for deprecation after Relay provides a unified file-backed +initializer. Keep its use localized so migration is straightforward. + The runtime reads only files named `plugins.toml` during default discovery. ## Runtime Discovery diff --git a/docs/configure-plugins/switchyard/about.mdx b/docs/configure-plugins/switchyard/about.mdx index 321a4d43d..b6806b1a2 100644 --- a/docs/configure-plugins/switchyard/about.mdx +++ b/docs/configure-plugins/switchyard/about.mdx @@ -1,7 +1,7 @@ --- -title: "Switchyard (Experimental)" -sidebar-title: "Switchyard (Experimental)" -description: "Set up and validate the experimental Switchyard Decision API integration for NeMo Relay 0.6.0." +title: "Switchyard (Deprecated)" +sidebar-title: "Switchyard (Deprecated)" +description: "Set up and validate the deprecated Switchyard Decision API integration for NeMo Relay 0.6.0 and 0.7.0." position: 5 --- import { MermaidStyles } from "@/components/MermaidStyles"; @@ -9,13 +9,21 @@ import { MermaidStyles } from "@/components/MermaidStyles"; {/* SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 */} + +The experimental `nemo-relay-switchyard` plugin will be removed in NeMo Relay +0.8 and replaced by a Switchyard-owned native plugin. Continue using this page +for the existing integration. The NeMo Relay 0.8 documentation will include an +updated configuration guide and migration plan when the replacement is +available. + + > **Experimental:** The Switchyard integration is an early-access feature. It > is not enabled in default Relay builds, its configuration and contracts can > change, and the current deployment requires a separately running Switchyard > Decision API service. -> **NeMo Relay 0.6.0 architecture:** This release uses the external Switchyard -> Decision API from the +> **NeMo Relay 0.6.0 and 0.7.0 architecture:** These releases use the external +> Switchyard Decision API from the > [`topic/nemo-relay-integration`](https://github.com/NVIDIA-NeMo/Switchyard/tree/topic/nemo-relay-integration) > branch. @@ -27,7 +35,7 @@ and a complex prompt to a more capable model. ## Architecture -The following diagram shows the NeMo Relay 0.6.0 service boundary: +The following diagram shows the NeMo Relay 0.6.0 and 0.7.0 service boundary: @@ -65,7 +73,7 @@ Install the following prerequisites before you start: | Requirement | Version or Value | | --- | --- | -| NeMo Relay | Tag `0.6.0` | +| NeMo Relay | Tags `0.6.0` and `0.7.0` | | Rust | `1.96.1` | | Switchyard branch | `topic/nemo-relay-integration` | | Switchyard commit | `8f9db9a6a47f848cdff1d262276ba25a8ae9cbc8` | diff --git a/docs/configure-plugins/switchyard/configuration.mdx b/docs/configure-plugins/switchyard/configuration.mdx index 2b09b555e..0c406af58 100644 --- a/docs/configure-plugins/switchyard/configuration.mdx +++ b/docs/configure-plugins/switchyard/configuration.mdx @@ -1,17 +1,24 @@ --- title: "Switchyard Configuration" sidebar-title: "Configuration" -description: "Configure the experimental Relay-native Switchyard Decision API plugin." +description: "Configure the deprecated Relay-native Switchyard Decision API plugin." position: 2 --- {/* SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 */} -> **Experimental NeMo Relay 0.6.0 integration:** This release calls a separately -> running Switchyard Decision API. + +The experimental `nemo-relay-switchyard` plugin will be removed in NeMo Relay +0.8 and replaced by a Switchyard-owned native plugin. The NeMo Relay 0.8 +documentation will include an updated configuration guide and migration plan +when the replacement is available. + + +> **Experimental NeMo Relay 0.6.0 and 0.7.0 integration:** These releases call a +> separately running Switchyard Decision API. Start with the -[Switchyard 0.6.0 setup and validation guide](./about.mdx) +[Switchyard setup and validation guide](./about.mdx) before using this page as the complete configuration reference. The Switchyard component connects Relay's CLI gateway to a separately running diff --git a/docs/getting-started/installation.mdx b/docs/getting-started/installation.mdx index eb6830f3c..254ccc442 100644 --- a/docs/getting-started/installation.mdx +++ b/docs/getting-started/installation.mdx @@ -45,12 +45,6 @@ pip install "nemo-relay[cli]" ``` - -```bash -npm install --global nemo-relay-cli-bin -``` - - ```bash curl -fsSL https://raw.githubusercontent.com/NVIDIA/NeMo-Relay/main/install.sh | sh @@ -114,9 +108,9 @@ user `PATH`; the Unix installer does not edit shell configuration. Use the GNU target on glibc-based distributions such as Ubuntu, Debian, and RHEL. Use the musl target on Alpine Linux and other musl-based systems. The Unix shell installer intentionally selects the portable musl target on Linux. -For package versions that publish the corresponding artifacts, `pip`, `uv`, and -`npm` automatically select a target compatible with the host CPU and C library -on supported platforms. +For package versions that publish the corresponding artifacts, `pip` and `uv` +automatically select a target compatible with the host CPU and C library on +supported platforms. The installer reports the detected operating system and architecture when no matching asset exists. Intel macOS is not supported because no matching release @@ -186,7 +180,7 @@ new download before replacing the binary in the selected installation directory. ### Alternative Installation Methods -The PyPI, npm, and installer methods install the same CLI release with a binary +The PyPI and installer methods install the same CLI release with a binary compatible with the host platform. Use Cargo on unsupported platforms or when you prefer to build from source: diff --git a/docs/index.yml b/docs/index.yml index fca80db2d..ff4dc3a70 100644 --- a/docs/index.yml +++ b/docs/index.yml @@ -41,7 +41,7 @@ navigation: title: "PII Redaction" title-source: frontmatter - folder: ./configure-plugins/switchyard - title: "Switchyard (Experimental)" + title: "Switchyard (Deprecated)" title-source: frontmatter - page: "Model Pricing" path: ./configure-plugins/model-pricing.mdx diff --git a/docs/nemo-relay-cli/basic-usage.mdx b/docs/nemo-relay-cli/basic-usage.mdx index 13417c964..55d9b4e8a 100644 --- a/docs/nemo-relay-cli/basic-usage.mdx +++ b/docs/nemo-relay-cli/basic-usage.mdx @@ -516,9 +516,17 @@ stores the canonical absolute command and trusts only its exact event pairs; it does not enable global hook auto-acceptance. `hook-forward` reads the canonical hook payload from standard input, sends it -to the matching endpoint, and prints the endpoint response. It fails open by -default so observability outages do not block the coding agent. Add -`--fail-closed` only when policy requires hook delivery to block the agent. +to the matching endpoint, and prints the endpoint response. Generated +`PreToolUse`, `PermissionRequest`, and Hermes `pre_tool_call` hooks use +`--fail-closed`; generated lifecycle and after-the-fact hooks use +`--fail-open`. This blocks permission-bearing operations when Relay cannot +evaluate them without making observability-only hooks a runtime dependency. +Rerun `nemo-relay install --force` after upgrading to replace legacy +generated hooks. + +Manually authored commands fail open when neither policy flag is present. +`NEMO_RELAY_FAIL_CLOSED=1` changes that default for compatibility, while an +explicit flag takes precedence over the environment. These flags control delivery and metadata: @@ -530,9 +538,11 @@ These flags control delivery and metadata: - `--session-metadata` sets `x-nemo-relay-session-metadata`. - `--profile` sets `x-nemo-relay-config-profile`. - `--gateway-mode` sets `x-nemo-relay-gateway-mode`. +- `--fail-open` allows the agent to continue after a delivery failure, even + when `NEMO_RELAY_FAIL_CLOSED=1` is set. Structured guardrail rejections still + return a failure. - `--fail-closed` returns a failure when delivery fails or Relay rejects the - hook. Without it, forwarding fails open so an observability outage does not - block the coding agent. + hook. ## Agent Guides diff --git a/docs/reference/operational-logging.mdx b/docs/reference/operational-logging.mdx index 942a75149..c3912f29f 100644 --- a/docs/reference/operational-logging.mdx +++ b/docs/reference/operational-logging.mdx @@ -29,7 +29,7 @@ Use the source that matches how Relay is launched: | Use Case | Configuration Source | | --- | --- | | Run the Relay CLI with temporary settings | `--log-*` options | -| Configure a process without a Relay config file | `NEMO_RELAY_LOG*` environment variables | +| Configure a language binding or CLI process | `NEMO_RELAY_LOG*` environment variables | | Reuse logging settings across runs | `[logging]` in TOML | | Embed Relay in a Rust application | `LoggingConfig` and `LoggingRuntime` | @@ -40,8 +40,10 @@ For CLI processes, Relay selects one source in this order: 3. `[logging]` in the resolved Relay `config.toml` 4. Built-in defaults -Sources are selected rather than merged. Rust applications explicitly choose -which `LoggingRuntime` initialization method to use and do not apply the CLI +Sources are selected rather than merged. Python, Node.js, and Go install a +process-lifetime `LoggingRuntime` when the binding loads, using environment +configuration or built-in defaults. Rust applications explicitly choose which +`LoggingRuntime` initialization method to use and do not apply the CLI precedence rules. ## CLI Options @@ -63,8 +65,9 @@ Do not combine `--log-config-path` with `--log-level` or ## Environment Variables -Set these variables for the CLI or a Rust application that initializes logging -with `LoggingRuntime::configure_from_environment()`: +Set these variables for a Python, Node.js, or Go process, the CLI, or a Rust +application that initializes logging with +`LoggingRuntime::configure_from_environment()`: ```bash export NEMO_RELAY_LOG=debug @@ -85,6 +88,22 @@ export NEMO_RELAY_LOG_CONFIG_PATH=/absolute/path/to/logging.toml `NEMO_RELAY_LOG_CONFIG_PATH` cannot be combined with the other logging environment variables. +When none of these variables are set, Python, Node.js, and Go install Relay's +built-in default logger unless the host already owns Rust's process-global +`log` facade. They also preserve an existing Relay logger rather than replacing +it. Records emitted before any logger is installed are discarded, and a host +cannot install its own logger after Relay has claimed the facade. Set one of +these variables when Relay must configure its own logging. + +This binding behavior differs from +`LoggingRuntime::configure_from_environment()`, which always attempts to +install Relay's built-in defaults when no variables are set. + +Python and Node.js drain pending file-sink records during normal runtime +teardown. Go applications that configure file sinks must call +`nemo_relay.ShutdownLogging` before `main` returns; defer it near the start of +`main` so it runs after other Relay cleanup. + ## TOML Configuration Logging settings use a `[logging]` table: diff --git a/examples/switchyard/README.md b/examples/switchyard/README.md index 87a2cff68..b5fedb23e 100644 --- a/examples/switchyard/README.md +++ b/examples/switchyard/README.md @@ -5,9 +5,16 @@ SPDX-License-Identifier: Apache-2.0 # Switchyard Integration Examples -These examples exercise the experimental NeMo Relay 0.6.0 integration with a separately running -Switchyard Decision API service and the in-process Switchyard translation library. They are -manual, local validation workflows rather than production startup orchestration. +> **Deprecated:** These examples exercise the experimental +> `nemo-relay-switchyard` plugin, which will be removed in NeMo Relay 0.8 and +> replaced by a Switchyard-owned native plugin. The NeMo Relay 0.8 documentation +> will include updated examples and a migration plan when the replacement is +> available. + +These examples exercise the experimental NeMo Relay 0.6.0 and 0.7.0 integration with a +separately running Switchyard Decision API service and the in-process Switchyard translation +library. They are manual, local validation workflows rather than production startup +orchestration. For the canonical architecture, setup, configuration, validation, and troubleshooting workflow, refer to the @@ -15,7 +22,7 @@ refer to the ## Required Switchyard Revision -The NeMo Relay 0.6.0 scripts require the following public topic branch and commit: +The scripts for NeMo Relay 0.6.0 and 0.7.0 require the following public topic branch and commit: ```text https://github.com/NVIDIA-NeMo/Switchyard/tree/topic/nemo-relay-integration diff --git a/go/nemo_relay/README.md b/go/nemo_relay/README.md index 57bbca94c..5162c1bf9 100644 --- a/go/nemo_relay/README.md +++ b/go/nemo_relay/README.md @@ -110,6 +110,12 @@ import ( ) func main() { + defer func() { + if err := nemo.ShutdownLogging(); err != nil { + log.Printf("shut down NeMo Relay logging: %v", err) + } + }() + if err := nemo.RegisterSubscriber("printer", func(event nemo.Event) { fmt.Printf("%s %s\n", event.Kind(), event.Name()) fmt.Println(string(event.JSON())) diff --git a/go/nemo_relay/logging_test.go b/go/nemo_relay/logging_test.go new file mode 100644 index 000000000..09af8e799 --- /dev/null +++ b/go/nemo_relay/logging_test.go @@ -0,0 +1,111 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nemo_relay + +import ( + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" +) + +const loggingHelperEnvironment = "NEMO_RELAY_TEST_LOGGING_HELPER" + +var loggingEnvironmentNames = map[string]struct{}{ + "NEMO_RELAY_LOG": {}, + "NEMO_RELAY_LOG_STDERR_FORMAT": {}, + "NEMO_RELAY_LOG_CONFIG_PATH": {}, +} + +func loggingTestEnvironment(values ...string) []string { + environment := make([]string, 0, len(os.Environ())+len(values)) + for _, value := range os.Environ() { + name, _, _ := strings.Cut(value, "=") + if _, isLoggingEnvironment := loggingEnvironmentNames[name]; !isLoggingEnvironment { + environment = append(environment, value) + } + } + return append(environment, values...) +} + +func TestBindingLoggingEnvironment(t *testing.T) { + if helper := os.Getenv(loggingHelperEnvironment); helper != "" { + if helper == "shutdown" { + if err := ShutdownLogging(); err != nil { + t.Fatalf("logging shutdown failed: %v", err) + } + } + return + } + + t.Run("initializes from environment", func(t *testing.T) { + command := exec.Command(os.Args[0], "-test.run=TestBindingLoggingEnvironment") + command.Env = loggingTestEnvironment( + loggingHelperEnvironment+"=shutdown", + "NEMO_RELAY_LOG=info", + "NEMO_RELAY_LOG_STDERR_FORMAT=jsonl", + ) + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("binding import failed: %v\n%s", err, output) + } + if !strings.Contains(string(output), `"event":"logging_initialized"`) { + t.Fatalf("logging initialization event missing from output:\n%s", output) + } + }) + + t.Run("rejects invalid environment", func(t *testing.T) { + command := exec.Command(os.Args[0], "-test.run=TestBindingLoggingEnvironment") + command.Env = loggingTestEnvironment( + loggingHelperEnvironment+"=1", + "NEMO_RELAY_LOG=", + ) + output, err := command.CombinedOutput() + if err == nil { + t.Fatalf("binding initialization unexpectedly succeeded:\n%s", output) + } + if !strings.Contains(string(output), "NEMO_RELAY_LOG must not be empty") { + t.Fatalf("logging initialization error missing from output:\n%s", output) + } + }) + + t.Run("flushes file sink during shutdown", func(t *testing.T) { + directory := t.TempDir() + configPath := filepath.Join(directory, "logging.toml") + logPath := filepath.Join(directory, "operational.jsonl") + config := `[logging] +level = "info" +stderr_format = "human" +flush_interval_millis = 0 + +[[logging.sinks]] +path = ` + strconv.Quote(logPath) + ` +level = "info" +format = "jsonl" +queue_capacity = 16 +` + if err := os.WriteFile(configPath, []byte(config), 0o600); err != nil { + t.Fatalf("write logging config: %v", err) + } + + command := exec.Command(os.Args[0], "-test.run=TestBindingLoggingEnvironment") + command.Env = loggingTestEnvironment( + loggingHelperEnvironment+"=shutdown", + "NEMO_RELAY_LOG_CONFIG_PATH="+configPath, + ) + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("binding logging shutdown failed: %v\n%s", err, output) + } + contents, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read operational log: %v", err) + } + if !strings.Contains(string(contents), `"event":"logging_shutdown_started"`) { + t.Fatalf("logging shutdown event missing from file:\n%s", contents) + } + }) +} diff --git a/go/nemo_relay/nemo_relay.go b/go/nemo_relay/nemo_relay.go index 3046120f5..8646c122a 100644 --- a/go/nemo_relay/nemo_relay.go +++ b/go/nemo_relay/nemo_relay.go @@ -46,6 +46,8 @@ typedef struct NemoRelayLlmSanitizeResponseContext { uint32_t codec_kind; const typedef void (*NemoRelayFreeFn)(void* user_data); // Core API +extern int32_t nemo_relay_initialize_default_logging(void); +extern int32_t nemo_relay_shutdown_default_logging(void); extern int32_t nemo_relay_get_handle(FfiScopeHandle** out); extern int32_t nemo_relay_push_scope(const char* name, int32_t scope_type, const FfiScopeHandle* parent, uint32_t attributes, const char* data_json, const char* metadata_json, const char* input_json, const int64_t* timestamp_unix_micros, FfiScopeHandle** out); extern int32_t nemo_relay_pop_scope(const FfiScopeHandle* handle, const char* output_json, const char* metadata_json, const int64_t* timestamp_unix_micros); @@ -298,6 +300,18 @@ import ( const defaultServiceName = "nemo-relay" +func init() { + if err := checkStatus(C.nemo_relay_initialize_default_logging()); err != nil { + panic(fmt.Sprintf("failed to initialize NeMo Relay operational logging: %v", err)) + } +} + +// ShutdownLogging drains pending operational log records and releases the default logging runtime. +// Callers that configure file sinks should defer ShutdownLogging from main. +func ShutdownLogging() error { + return checkStatus(C.nemo_relay_shutdown_default_logging()) +} + func checkedValue[T any](status int32, value T) (T, error) { if err := checkStatus(C.int32_t(status)); err != nil { var zero T diff --git a/integrations/coding-agents/README.md b/integrations/coding-agents/README.md index 20ab6d57f..0ef4d79e9 100644 --- a/integrations/coding-agents/README.md +++ b/integrations/coding-agents/README.md @@ -262,14 +262,16 @@ URL. For Codex, the installed plugin file is the sole persistent Relay hook source; installation does not add Relay groups to `~/.codex/hooks.json`. -Since hook forwarding fails open by default, gateway or sidecar outages do not -block the coding agent. The hook command exits successfully after logging the -forwarding problem, so the host agent can continue even though that hook -payload can be missing from telemetry. For wrapper-generated `hook-forward` -commands, add `--fail-closed` when policy requires hook delivery to block the -agent. For generated persistent hooks, set `NEMO_RELAY_FAIL_CLOSED=1` in the hook -execution environment. In that mode, forwarding failures return a non-zero -hook command status to the host. +Generated hooks select an explicit failure policy by event. `PreToolUse`, +`PermissionRequest`, and Hermes `pre_tool_call` hooks use `--fail-closed`, so +Relay startup, authentication, delivery, and response failures block the +permission-bearing operation. Lifecycle and after-the-fact hooks use +`--fail-open`, so observability outages do not block unrelated agent work. + +After upgrading, rerun `nemo-relay install --force` to replace legacy +generated hooks that did not carry an explicit policy. Manually authored +`hook-forward` commands still fail open by default; set +`NEMO_RELAY_FAIL_CLOSED=1` or add `--fail-closed` when they enforce policy. These `hook-forward` options control delivery and metadata: @@ -284,6 +286,8 @@ These `hook-forward` options control delivery and metadata: - `--profile ` records a configuration profile in session metadata. - `--gateway-mode hook-only|passthrough|required` records the expected gateway behavior in session metadata. +- `--fail-open` allows the coding agent to continue after a delivery failure, + even when `NEMO_RELAY_FAIL_CLOSED=1` is set. - `--fail-closed` returns a failure when delivery fails or Relay rejects the hook instead of allowing the coding agent to continue. diff --git a/integrations/coding-agents/claude-code/hooks/hooks.json b/integrations/coding-agents/claude-code/hooks/hooks.json index 73cb06ef4..7524fa193 100644 --- a/integrations/coding-agents/claude-code/hooks/hooks.json +++ b/integrations/coding-agents/claude-code/hooks/hooks.json @@ -5,7 +5,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -16,7 +16,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -27,7 +27,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -39,7 +39,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -51,7 +51,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -63,7 +63,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -75,7 +75,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -86,7 +86,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -97,7 +97,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -108,7 +108,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -119,7 +119,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -130,7 +130,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -141,7 +141,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -152,7 +152,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward claude --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] diff --git a/integrations/coding-agents/codex/hooks/hooks.json b/integrations/coding-agents/codex/hooks/hooks.json index 550a462bc..a20626ad0 100644 --- a/integrations/coding-agents/codex/hooks/hooks.json +++ b/integrations/coding-agents/codex/hooks/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -17,7 +17,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -29,7 +29,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -41,7 +41,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -53,7 +53,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-closed", "timeout": 30 } ] @@ -64,7 +64,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -75,7 +75,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -86,7 +86,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -97,7 +97,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] @@ -108,7 +108,7 @@ "hooks": [ { "type": "command", - "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only", + "command": "nemo-relay hook-forward codex --gateway-url http://127.0.0.1:47632 --forward-only --fail-open", "timeout": 30 } ] diff --git a/justfile b/justfile index 5f058e28c..4d331e8c3 100644 --- a/justfile +++ b/justfile @@ -453,8 +453,17 @@ import sys version = sys.argv[1] if version.startswith("v"): raise SystemExit("Release tags must not start with 'v'; use raw SemVer such as 0.1.0") -if not re.fullmatch(r"\d+\.\d+\.\d+(?:-(?:alpha|beta|rc)\.\d+)?", version): - raise SystemExit(f"Unsupported release tag '{version}'; use 0.1.0 or prereleases like 0.1.0-rc.1") +numeric_identifier = r"(?:0|[1-9][0-9]*)" +if not re.fullmatch( + rf"{numeric_identifier}\.{numeric_identifier}\.{numeric_identifier}" + rf"(?:-(?:alpha|beta|rc)\.{numeric_identifier})?" + r"(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?", + version, +): + raise SystemExit( + f"Unsupported Cargo version '{version}'; use 0.1.0, prereleases like " + "0.1.0-rc.1, or build metadata like 0.1.0+deadbeef" + ) path = Path("Cargo.toml") text = path.read_text() @@ -564,7 +573,6 @@ set_node_package_versions() { local version="$1" set_npm_package_version crates/node/package.json package-lock.json "$version" crates/node set_npm_package_version integrations/openclaw/package.json package-lock.json "$version" integrations/openclaw - set_npm_package_version packages/cli-bin/package.json package-lock.json "$version" packages/cli-bin set_npm_package_dependency_version integrations/openclaw/package.json package-lock.json integrations/openclaw nemo-relay-node "$version" } @@ -1485,6 +1493,22 @@ set-version version="": cd "$NEMO_RELAY_REPO_ROOT" set_project_version "$version" +# Set only the Cargo workspace version for release artifact builds. +# [version] or --set ref_name= +set-cargo-version version="": + #!/usr/bin/env bash + {{ bash_helpers }} + version="{{ version }}" + if [[ -z "$version" ]]; then + version="{{ ref_name }}" + fi + if [[ -z "$version" ]]; then + echo "Error: version is required for set-cargo-version" >&2 + exit 1 + fi + cd "$NEMO_RELAY_REPO_ROOT" + set_cargo_workspace_version "$version" + # --set [output_dir=] [ref_name=] package-rust: #!/usr/bin/env bash @@ -1769,8 +1793,8 @@ package-python-plugin: exit 1 fi -# Package a prebuilt CLI binary for PyPI and npm. -package-cli-bin binary target version package_dir npm_launcher="false": +# Package a prebuilt CLI binary for PyPI. +package-cli-bin binary target version package_dir: #!/usr/bin/env bash set -euo pipefail cd "$NEMO_RELAY_REPO_ROOT" @@ -1780,7 +1804,4 @@ package-cli-bin binary target version package_dir npm_launcher="false": --version "{{ version }}" --output-dir "{{ package_dir }}" ) - if [[ "{{ npm_launcher }}" == "true" ]]; then - args+=(--npm-launcher) - fi uv run --no-project python scripts/package-cli-bin.py "${args[@]}" diff --git a/package-lock.json b/package-lock.json index ead50344b..475fe716e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,8 +7,7 @@ "name": "nemo-relay-workspace", "workspaces": [ "crates/node", - "integrations/openclaw", - "packages/cli-bin" + "integrations/openclaw" ], "devDependencies": { "fern-api": "5.57.0", @@ -929,10 +928,6 @@ "node": ">=16 || 14 >=14.17" } }, - "node_modules/nemo-relay-cli-bin": { - "resolved": "packages/cli-bin", - "link": true - }, "node_modules/nemo-relay-node": { "resolved": "crates/node", "link": true @@ -5134,11 +5129,6 @@ "funding": { "url": "https://github.com/sponsors/eemeli" } - }, - "packages/cli-bin": { - "name": "nemo-relay-cli-bin", - "version": "0.7.0", - "license": "Apache-2.0" } } } diff --git a/package.json b/package.json index 9c8d01d4e..cf8803a04 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,7 @@ }, "workspaces": [ "crates/node", - "integrations/openclaw", - "packages/cli-bin" + "integrations/openclaw" ], "devDependencies": { "fern-api": "5.57.0", diff --git a/packages/cli-bin/package.json b/packages/cli-bin/package.json deleted file mode 100644 index 43cc3afba..000000000 --- a/packages/cli-bin/package.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "name": "nemo-relay-cli-bin", - "version": "0.7.0", - "description": "Prebuilt NeMo Relay command-line interface.", - "private": true, - "license": "Apache-2.0" -} diff --git a/python/nemo_relay/__init__.py b/python/nemo_relay/__init__.py index 585fbe683..1fa187220 100644 --- a/python/nemo_relay/__init__.py +++ b/python/nemo_relay/__init__.py @@ -77,6 +77,7 @@ async def main(): from __future__ import annotations +import atexit import contextvars import typing from collections.abc import Callable as AbcCallable @@ -119,6 +120,7 @@ async def main(): ToolAttributes, ToolExecutionInterceptOutcome, ToolHandle, + _shutdown_default_logging, ) from nemo_relay._native import ( capture_propagation_context as _capture_propagation_context, @@ -136,6 +138,8 @@ async def main(): from nemo_relay._native import set_thread_scope_stack as _set_thread_scope_stack from nemo_relay._native import sync_thread_scope_stack as _sync_thread_scope_stack +atexit.register(_shutdown_default_logging) + #: Scalar JSON leaf values accepted in NeMo Relay payloads. This alias has no #: runtime behavior; it exists to document and type JSON-compatible public API #: arguments and return values. diff --git a/python/nemo_relay/_native.pyi b/python/nemo_relay/_native.pyi index 43f2fa5f1..cc025751c 100644 --- a/python/nemo_relay/_native.pyi +++ b/python/nemo_relay/_native.pyi @@ -32,6 +32,8 @@ _JsonObject: TypeAlias = dict[str, _JsonValue] _Json: TypeAlias = _JsonValue _MessageContent: TypeAlias = str | Sequence[Mapping[str, _JsonValue]] +def _shutdown_default_logging() -> None: ... + class _EventSanitizeFields(TypedDict): data: _Json | None category_profile: _JsonObject | None diff --git a/python/nemo_relay/plugin.py b/python/nemo_relay/plugin.py index dbd807083..478c30bce 100644 --- a/python/nemo_relay/plugin.py +++ b/python/nemo_relay/plugin.py @@ -11,8 +11,13 @@ from __future__ import annotations import asyncio +import json +import os +import tomllib +from collections.abc import Sequence from contextlib import asynccontextmanager from dataclasses import dataclass, field, fields, is_dataclass +from pathlib import Path from typing import TYPE_CHECKING, AsyncIterator, Callable, Literal, Protocol, Self, TypedDict, cast from nemo_relay import ( @@ -417,6 +422,114 @@ async def __aexit__( await self.close() +def load_dynamic_plugin_activation_specs( + plugin_config_path: str | os.PathLike[str], +) -> list[DynamicPluginActivationSpec]: + """Load dynamic activation specs from one standard ``plugins.toml``. + + Args: + plugin_config_path: Explicit path to the ``plugins.toml`` file. + + Returns: + Activation specs for every ``[[plugins.dynamic]]`` record, in file + order. Manifest paths are resolved relative to ``plugins.toml`` and + each plugin's identifier and execution lane come from its manifest. + + Behavior: + This 0.7 compatibility helper parses one explicit file only. It does + not perform standard discovery, inspect CLI lifecycle state, provision + worker environments, change enablement, or activate plugins. It is + planned for deprecation after a unified file-backed initializer is + available. Keep its use localized and pass the result to + :func:`initialize_with_dynamic_plugins`. + """ + source = Path(os.fspath(plugin_config_path)).resolve() + document = _load_plugin_toml(source, "plugin TOML") + version = document.get("version", 1) + if not isinstance(version, int) or isinstance(version, bool) or version != 1: + raise ValueError(f"plugin config version {version!r} in {source} is unsupported; expected 1") + plugins = document.get("plugins", {}) + if not isinstance(plugins, dict): + raise ValueError(f"invalid dynamic plugin config in {source}: 'plugins' must be a table") + plugins = cast(dict[str, object], plugins) + dynamic_plugins = plugins.get("dynamic", []) + if not isinstance(dynamic_plugins, list): + raise ValueError(f"invalid dynamic plugin config in {source}: 'plugins.dynamic' must be an array of tables") + + specs: list[DynamicPluginActivationSpec] = [] + seen_plugin_ids: set[str] = set() + for index, entry in enumerate(dynamic_plugins): + if not isinstance(entry, dict): + raise ValueError(f"invalid dynamic plugin config in {source}: plugins.dynamic[{index}] must be a table") + entry = cast(dict[str, object], entry) + unknown_fields = sorted(set(entry) - {"manifest", "config"}) + if unknown_fields: + raise ValueError( + f"invalid dynamic plugin config in {source}: plugins.dynamic[{index}] has unknown fields: " + + ", ".join(unknown_fields) + ) + manifest_ref = entry.get("manifest") + if not isinstance(manifest_ref, str) or not manifest_ref.strip(): + raise ValueError( + f"invalid dynamic plugin config in {source}: " + f"plugins.dynamic[{index}].manifest must be a non-empty string" + ) + manifest_path = Path(manifest_ref) + if not manifest_path.is_absolute(): + manifest_path = source.parent / manifest_path + manifest_path = manifest_path.resolve() + + manifest = _load_plugin_toml(manifest_path, "dynamic plugin manifest") + identity = manifest.get("plugin") + if not isinstance(identity, dict): + raise ValueError(f"invalid dynamic plugin manifest in {manifest_path}: 'plugin' must be a table") + identity = cast(dict[str, object], identity) + plugin_id = identity.get("id") + if not isinstance(plugin_id, str) or not plugin_id.strip(): + raise ValueError( + f"invalid dynamic plugin manifest in {manifest_path}: 'plugin.id' must be a non-empty string" + ) + plugin_id = plugin_id.strip() + kind = identity.get("kind") + if kind not in ("rust_dynamic", "worker"): + raise ValueError( + f"invalid dynamic plugin manifest in {manifest_path}: 'plugin.kind' must be 'rust_dynamic' or 'worker'" + ) + if plugin_id in seen_plugin_ids: + raise ValueError(f"duplicate dynamic plugin id {plugin_id!r} in {source}") + seen_plugin_ids.add(plugin_id) + + config = entry.get("config", {}) + if not isinstance(config, dict): + raise ValueError( + f"invalid dynamic plugin config in {source}: plugins.dynamic[{index}].config must be a table" + ) + try: + normalized_config = cast(JsonObject, json.loads(json.dumps(config, allow_nan=False))) + except (TypeError, ValueError) as error: + raise ValueError( + f"invalid dynamic plugin config in {source}: " + f"plugins.dynamic[{index}].config must contain JSON values: {error}" + ) from error + specs.append( + DynamicPluginActivationSpec( + plugin_id=plugin_id, + kind=cast(DynamicPluginKind, kind), + manifest_ref=str(manifest_path), + config=normalized_config, + ) + ) + return specs + + +def _load_plugin_toml(path: Path, description: str) -> dict[str, object]: + try: + with path.open("rb") as file: + return cast(dict[str, object], tomllib.load(file)) + except tomllib.TOMLDecodeError as error: + raise ValueError(f"invalid {description} in {path}: {error}") from error + + def validate(config: PluginConfig | JsonObject) -> ConfigReport: """Validate a plugin configuration without changing runtime state. @@ -452,7 +565,7 @@ async def initialize(config: PluginConfig | JsonObject) -> ConfigReport: async def initialize_with_dynamic_plugins( config: PluginConfig | JsonObject, - dynamic_plugins: list[DynamicPluginActivationSpec | JsonObject], + dynamic_plugins: Sequence[DynamicPluginActivationSpec | JsonObject], ) -> PluginHostActivation: """Initialize registered components with dynamic plugins as one owned host. @@ -607,6 +720,7 @@ def deregister(plugin_kind: str) -> bool: "PluginContext", "PluginHostActivation", "Plugin", + "load_dynamic_plugin_activation_specs", "initialize_with_dynamic_plugins", "clear", "clear_async", diff --git a/python/nemo_relay/plugin.pyi b/python/nemo_relay/plugin.pyi index 39bc464d5..ee8f165dd 100644 --- a/python/nemo_relay/plugin.pyi +++ b/python/nemo_relay/plugin.pyi @@ -1,7 +1,8 @@ # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -from collections.abc import Callable +import os +from collections.abc import Callable, Sequence from types import TracebackType from typing import AsyncContextManager, Literal, Protocol, Self, TypedDict @@ -160,11 +161,14 @@ class PluginHostActivation: traceback: TracebackType | None, ) -> None: ... +def load_dynamic_plugin_activation_specs( + plugin_config_path: str | os.PathLike[str], +) -> list[DynamicPluginActivationSpec]: ... def validate(config: PluginConfig | JsonObject) -> ConfigReport: ... async def initialize(config: PluginConfig | JsonObject) -> ConfigReport: ... async def initialize_with_dynamic_plugins( config: PluginConfig | JsonObject, - dynamic_plugins: list[DynamicPluginActivationSpec | JsonObject], + dynamic_plugins: Sequence[DynamicPluginActivationSpec | JsonObject], ) -> PluginHostActivation: ... def clear() -> None: ... async def clear_async() -> None: ... diff --git a/python/tests/test_dynamic_plugin_host.py b/python/tests/test_dynamic_plugin_host.py index 97340cec5..f06673032 100644 --- a/python/tests/test_dynamic_plugin_host.py +++ b/python/tests/test_dynamic_plugin_host.py @@ -197,6 +197,175 @@ def test_dynamic_plugin_activation_spec_preserves_nested_json_nulls(): } +def test_load_dynamic_plugin_activation_specs_resolves_standard_toml(tmp_path: Path): + manifests = tmp_path / "plugins" + manifests.mkdir() + native_manifest = manifests / "native.toml" + native_manifest.write_text( + textwrap.dedent( + """ + manifest_version = 1 + + [plugin] + id = "fixture.native" + kind = "rust_dynamic" + """ + ) + ) + worker_manifest = manifests / "worker.toml" + worker_manifest.write_text( + textwrap.dedent( + """ + manifest_version = 1 + + [plugin] + id = "fixture.worker" + kind = "worker" + """ + ) + ) + plugins_toml = tmp_path / "plugins.toml" + plugins_toml.write_text( + textwrap.dedent( + f""" + version = 1 + + [[plugins.dynamic]] + manifest = "plugins/native.toml" + + [plugins.dynamic.config] + mode = "strict" + nested = {{ enabled = true }} + + [[plugins.dynamic]] + manifest = {str(worker_manifest)!r} + """ + ) + ) + + specs = plugin.load_dynamic_plugin_activation_specs(plugins_toml) + + assert [spec.to_dict() for spec in specs] == [ + { + "plugin_id": "fixture.native", + "kind": "rust_dynamic", + "manifest_ref": str(native_manifest.resolve()), + "config": {"mode": "strict", "nested": {"enabled": True}}, + }, + { + "plugin_id": "fixture.worker", + "kind": "worker", + "manifest_ref": str(worker_manifest.resolve()), + "config": {}, + }, + ] + + +@pytest.mark.parametrize( + ("toml_version", "version"), + [("true", True), ("0", 0), ("2", 2), ('"1"', "1")], +) +def test_load_dynamic_plugin_activation_specs_rejects_unsupported_version( + tmp_path: Path, toml_version: str, version: object +): + plugins_toml = tmp_path / "plugins.toml" + plugins_toml.write_text(f"version = {toml_version}\n") + + with pytest.raises(ValueError) as error: + plugin.load_dynamic_plugin_activation_specs(plugins_toml) + assert str(error.value) == ( + f"plugin config version {version!r} in {plugins_toml.resolve()} is unsupported; expected 1" + ) + + +def test_load_dynamic_plugin_activation_specs_rejects_duplicate_ids(tmp_path: Path): + manifest = tmp_path / "relay-plugin.toml" + manifest.write_text("[plugin]\nid = 'duplicate'\nkind = 'rust_dynamic'\n") + plugins_toml = tmp_path / "plugins.toml" + plugins_toml.write_text( + "[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\n[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\n" + ) + + with pytest.raises(ValueError, match="duplicate dynamic plugin id 'duplicate'"): + plugin.load_dynamic_plugin_activation_specs(plugins_toml) + + +@pytest.mark.parametrize( + ("plugins_toml", "manifest", "message"), + [ + ("plugins = []\n", None, "'plugins' must be a table"), + ("[plugins]\ndynamic = 'invalid'\n", None, "'plugins.dynamic' must be an array of tables"), + ("[plugins]\ndynamic = ['invalid']\n", None, r"plugins.dynamic\[0\] must be a table"), + ( + "[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\nunsupported = true\n", + "[plugin]\nid = 'fixture'\nkind = 'rust_dynamic'\n", + "has unknown fields: unsupported", + ), + ("[[plugins.dynamic]]\nmanifest = ''\n", None, "manifest must be a non-empty string"), + ( + "[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\n", + "plugin = 'invalid'\n", + "'plugin' must be a table", + ), + ( + "[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\n", + "[plugin]\nkind = 'rust_dynamic'\n", + "'plugin.id' must be a non-empty string", + ), + ( + "[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\n", + "[plugin]\nid = 'fixture'\nkind = 'invalid'\n", + "'plugin.kind' must be 'rust_dynamic' or 'worker'", + ), + ( + "[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\nconfig = 'invalid'\n", + "[plugin]\nid = 'fixture'\nkind = 'rust_dynamic'\n", + "config must be a table", + ), + ( + "[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\n" + "[plugins.dynamic.config]\nwhen = 1979-05-27T07:32:00Z\n", + "[plugin]\nid = 'fixture'\nkind = 'rust_dynamic'\n", + "config must contain JSON values", + ), + ], +) +def test_load_dynamic_plugin_activation_specs_rejects_invalid_records( + tmp_path: Path, + plugins_toml: str, + manifest: str | None, + message: str, +): + config_path = tmp_path / "plugins.toml" + config_path.write_text(plugins_toml) + if manifest is not None: + (tmp_path / "relay-plugin.toml").write_text(manifest) + + with pytest.raises(ValueError, match=message): + plugin.load_dynamic_plugin_activation_specs(config_path) + + +@pytest.mark.parametrize("filename", ["plugins.toml", "relay-plugin.toml"]) +def test_load_dynamic_plugin_activation_specs_rejects_invalid_toml(tmp_path: Path, filename: str): + plugins_toml = tmp_path / "plugins.toml" + plugins_toml.write_text("[[plugins.dynamic]]\nmanifest = 'relay-plugin.toml'\n") + (tmp_path / "relay-plugin.toml").write_text("[plugin]\nid = 'fixture'\nkind = 'rust_dynamic'\n") + (tmp_path / filename).write_text("invalid = [\n") + + with pytest.raises(ValueError, match="invalid .* in"): + plugin.load_dynamic_plugin_activation_specs(plugins_toml) + + +def test_load_dynamic_plugin_activation_specs_reports_missing_manifest(tmp_path: Path): + plugins_toml = tmp_path / "plugins.toml" + plugins_toml.write_text("[[plugins.dynamic]]\nmanifest = 'missing/relay-plugin.toml'\n") + missing_manifest = (tmp_path / "missing" / "relay-plugin.toml").resolve() + + with pytest.raises(FileNotFoundError) as error: + plugin.load_dynamic_plugin_activation_specs(plugins_toml) + assert Path(error.value.filename) == missing_manifest + + def test_validate_omits_raw_plugin_config_nulls_but_preserves_component_config_nulls( monkeypatch: pytest.MonkeyPatch, ): @@ -332,6 +501,9 @@ def register(self, _plugin_config, context): [[components]] kind = {static_kind!r} enabled = true + + [[plugins.dynamic]] + manifest = {str(native_dynamic_plugin.manifest)!r} """ ) ) @@ -343,7 +515,8 @@ def register(self, _plugin_config, context): plugin.register(static_kind, cast(plugin.Plugin, FileStaticPlugin())) activation = None try: - activation = await plugin.initialize_with_dynamic_plugins(plugin.PluginConfig(), [native_dynamic_plugin.spec()]) + dynamic_plugins = plugin.load_dynamic_plugin_activation_specs(plugins_toml) + activation = await plugin.initialize_with_dynamic_plugins(plugin.PluginConfig(), tuple(dynamic_plugins)) assert activation.report == { "diagnostics": [ { diff --git a/python/tests/test_llm.py b/python/tests/test_llm.py index 0065ed786..ba441981b 100644 --- a/python/tests/test_llm.py +++ b/python/tests/test_llm.py @@ -1338,22 +1338,101 @@ async def test_stream_execution_intercept_propagates_direct___anext__error(self) finally: intercepts.deregister_llm_stream_execution("py_llm_stream_direct_error") + async def test_stream_execution_intercept_failure_emits_exception_type(self): + events = [] + subscribers.register("py_llm_stream_intercept_failure_sub", events.append) + + def failing_middleware(request, next): + raise ValueError("stream intercept boom") + + intercepts.register_llm_stream_execution( + "py_llm_stream_failure", + 1, + failing_middleware, + ) + try: + with pytest.raises(RuntimeError, match="stream intercept boom"): + await llm.stream_execute( + "stream_intercept_failure_llm", + make_request(), + lambda request: _single_chunk_stream(), + lambda chunk: None, + lambda: {}, + ) + finally: + intercepts.deregister_llm_stream_execution("py_llm_stream_failure") + await subscribers.flush_async() + subscribers.deregister("py_llm_stream_intercept_failure_sub") + + metadata = _llm_event(events, "stream_intercept_failure_llm", "end").metadata + assert isinstance(metadata, dict) + assert metadata["exception.type"] == "ValueError" + async def test_stream_execute_collector_failure_raises(self): + events = [] + subscribers.register("py_llm_stream_collector_failure_sub", events.append) + def stream_func(request): async def gen(): yield {"token": "hello"} return gen() - stream = await llm.stream_execute( - "stream_collector_fail_llm", - make_request(), - stream_func, - lambda chunk: raise_runtime_error("collector boom"), - lambda: {}, - ) - with pytest.raises(RuntimeError, match="collector boom"): - await anext(stream) + try: + stream = await llm.stream_execute( + "stream_collector_fail_llm", + make_request(), + stream_func, + lambda chunk: raise_runtime_error("collector boom"), + lambda: {}, + ) + with pytest.raises(RuntimeError, match="collector boom"): + await anext(stream) + await subscribers.flush_async() + finally: + subscribers.deregister("py_llm_stream_collector_failure_sub") + + metadata = _llm_event(events, "stream_collector_fail_llm", "end").metadata + assert isinstance(metadata, dict) + assert metadata["exception.type"] == "RuntimeError" + + async def test_stream_execute_callback_failure_emits_exception_type(self): + events = [] + subscribers.register("py_llm_stream_callback_failure_sub", events.append) + + def stream_func(request): + raise ValueError("stream callback boom") + + async def async_stream_func(request): + raise TypeError("async stream callback boom") + + try: + with pytest.raises(RuntimeError, match="stream callback boom"): + await llm.stream_execute( + "stream_callback_fail_llm", + make_request(), + stream_func, + lambda chunk: None, + lambda: {}, + ) + with pytest.raises(RuntimeError, match="async stream callback boom"): + await llm.stream_execute( + "async_stream_callback_fail_llm", + make_request(), + async_stream_func, + lambda chunk: None, + lambda: {}, + ) + await subscribers.flush_async() + finally: + subscribers.deregister("py_llm_stream_callback_failure_sub") + + metadata = _llm_event(events, "stream_callback_fail_llm", "end").metadata + assert isinstance(metadata, dict) + assert metadata["exception.type"] == "ValueError" + metadata = _llm_event(events, "async_stream_callback_fail_llm", "end").metadata + assert isinstance(metadata, dict) + assert metadata["exception.type"] == "TypeError" async def test_stream_execute_finalizer_failure_records_null_output(self): events = [] diff --git a/python/tests/test_logging.py b/python/tests/test_logging.py new file mode 100644 index 000000000..9312a7ac9 --- /dev/null +++ b/python/tests/test_logging.py @@ -0,0 +1,67 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import json +import os +import subprocess +import sys + +_LOG_ENVIRONMENT = ( + "NEMO_RELAY_LOG", + "NEMO_RELAY_LOG_STDERR_FORMAT", + "NEMO_RELAY_LOG_CONFIG_PATH", +) + + +def _import_nemo_relay(**logging_environment: str) -> subprocess.CompletedProcess[str]: + environment = os.environ.copy() + for name in _LOG_ENVIRONMENT: + environment.pop(name, None) + environment.update(logging_environment) + return subprocess.run( + [sys.executable, "-c", "import nemo_relay"], + check=False, + capture_output=True, + text=True, + env=environment, + ) + + +def test_binding_initializes_logging_from_environment(): + completed = _import_nemo_relay( + NEMO_RELAY_LOG="info", + NEMO_RELAY_LOG_STDERR_FORMAT="jsonl", + ) + + assert completed.returncode == 0, completed.stderr + assert '"event":"logging_initialized"' in completed.stderr + + +def test_binding_rejects_invalid_logging_environment(): + completed = _import_nemo_relay(NEMO_RELAY_LOG="") + + assert completed.returncode != 0 + assert "NEMO_RELAY_LOG must not be empty" in completed.stderr + + +def test_binding_flushes_file_sink_during_shutdown(tmp_path): + config_path = tmp_path / "logging.toml" + log_path = tmp_path / "operational.jsonl" + config_path.write_text( + f"""[logging] +level = "info" +stderr_format = "human" +flush_interval_millis = 0 + +[[logging.sinks]] +path = {json.dumps(str(log_path))} +level = "info" +format = "jsonl" +queue_capacity = 16 +""" + ) + + completed = _import_nemo_relay(NEMO_RELAY_LOG_CONFIG_PATH=str(config_path)) + + assert completed.returncode == 0, completed.stderr + assert '"event":"logging_shutdown_started"' in log_path.read_text() diff --git a/python/tests/test_observability_plugin.py b/python/tests/test_observability_plugin.py index 68b7dd3b6..b457f703a 100644 --- a/python/tests/test_observability_plugin.py +++ b/python/tests/test_observability_plugin.py @@ -14,7 +14,7 @@ import pytest -from nemo_relay import ScopeType, plugin, scope +from nemo_relay import ScopeType, plugin, scope, subscribers from nemo_relay.observability import ( OBSERVABILITY_PLUGIN_KIND, AtifConfig, @@ -439,6 +439,51 @@ async def test_atif_flushes_open_agent_on_clear(self, tmp_path): finally: scope.pop(handle) + async def test_atif_non_string_metadata_is_reported_and_failed_clear_is_drainable(self, tmp_path): + await plugin.initialize( + plugin.PluginConfig( + components=[ + ComponentSpec( + ObservabilityConfig( + atif=AtifConfig( + enabled=True, + output_directory=str(tmp_path), + filename_template="{metadata.atif_prefix:-unassigned}/trajectory-{session_id}.json", + ) + ) + ) + ] + ) + ) + + try: + with scope.scope("python-invalid-metadata-agent", ScopeType.Agent, metadata={"atif_prefix": 123}): + pass + await subscribers.flush_async() + + report = plugin.report() + assert report is not None + assert any( + diagnostic["code"] == "atif.destination_render_failed" and "non-string" in diagnostic["message"] + for diagnostic in report["runtime_diagnostics"] + ) + assert not (tmp_path / "unassigned").exists() + + with pytest.raises(RuntimeError, match=r"atif\.destination_render_failed") as teardown: + await plugin.clear_async() + assert "could not be removed" not in str(teardown.value) + retained = plugin.report() + assert retained is not None + assert any( + diagnostic["code"] == "atif.destination_render_failed" for diagnostic in retained["runtime_diagnostics"] + ) + finally: + try: + await plugin.clear_async() + except RuntimeError: + await plugin.clear_async() + assert plugin.report() is None + async def test_atif_splits_multiple_top_level_agent_scopes(self, tmp_path): await plugin.initialize( plugin.PluginConfig( diff --git a/python/tests/test_tools.py b/python/tests/test_tools.py index 51de3c645..96b72affb 100644 --- a/python/tests/test_tools.py +++ b/python/tests/test_tools.py @@ -208,7 +208,7 @@ async def test_execute_failure_emits_end_event(self): subscribers.register("py_tool_exec_failure_sub", lambda e: events.append(e)) def failing(args): - raise RuntimeError("boom") + raise ValueError("boom") with pytest.raises(RuntimeError, match="boom"): await tools.execute("failing_tool", {"x": 1}, failing) @@ -224,6 +224,8 @@ def failing(args): assert all(e.category == "tool" for e in events) assert events[0].uuid == events[1].uuid assert events[1].data is None + assert events[1].metadata["error.type"] == "internal_error" + assert events[1].metadata["exception.type"] == "ValueError" class TestToolGuardrails: diff --git a/scripts/package-cli-bin.py b/scripts/package-cli-bin.py index 15d5eefaa..690452950 100755 --- a/scripts/package-cli-bin.py +++ b/scripts/package-cli-bin.py @@ -2,18 +2,15 @@ # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""Package a prebuilt NeMo Relay CLI binary for PyPI and npm.""" +"""Package a prebuilt NeMo Relay CLI binary for PyPI.""" from __future__ import annotations import argparse import base64 import hashlib -import io -import json import os import stat -import tarfile import zipfile from dataclasses import dataclass from pathlib import Path @@ -30,17 +27,8 @@ class Platform: """Describe one supported CLI distribution platform.""" target: str - npm_suffix: str - npm_os: str - npm_cpu: str wheel_platforms: tuple[str, ...] executable: str - libc: str | None = None - - @property - def npm_package(self) -> str: - """Return the npm package name for this platform.""" - return f"{PACKAGE_NAME}-{self.npm_suffix}" PLATFORMS = { @@ -48,61 +36,36 @@ def npm_package(self) -> str: for platform in ( Platform( "x86_64-unknown-linux-gnu", - "linux-x64", - "linux", - "x64", ("manylinux_2_17_x86_64",), "nemo-relay", - "glibc", ), Platform( "aarch64-unknown-linux-gnu", - "linux-arm64", - "linux", - "arm64", ("manylinux_2_17_aarch64",), "nemo-relay", - "glibc", ), Platform( "x86_64-unknown-linux-musl", - "linux-x64-musl", - "linux", - "x64", ("musllinux_1_2_x86_64",), "nemo-relay", - "musl", ), Platform( "aarch64-unknown-linux-musl", - "linux-arm64-musl", - "linux", - "arm64", ("musllinux_1_2_aarch64",), "nemo-relay", - "musl", ), Platform( "aarch64-apple-darwin", - "darwin-arm64", - "darwin", - "arm64", ("macosx_11_0_arm64",), "nemo-relay", ), Platform( "x86_64-pc-windows-msvc", - "win32-x64", - "win32", - "x64", ("win_amd64",), "nemo-relay.exe", ), Platform( "aarch64-pc-windows-msvc", - "win32-arm64", - "win32", - "arm64", ("win_arm64",), "nemo-relay.exe", ), @@ -190,109 +153,6 @@ def build_wheel(binary: Path, platform: Platform, version: str, output: Path) -> return destination -def add_tar_bytes(archive: tarfile.TarFile, path: str, content: bytes, mode: int = 0o644) -> None: - """Add one regular file to an npm tarball.""" - info = tarfile.TarInfo(path) - info.size = len(content) - info.mode = mode - archive.addfile(info, io.BytesIO(content)) - - -def build_npm_platform(binary: Path, platform: Platform, version: str, output: Path) -> Path: - """Build an OS- and CPU-constrained npm package containing the CLI binary.""" - filename = f"nemo-relay-bin-npm-{platform.npm_suffix}-{version}.tgz" - destination = output / filename - manifest = { - "name": platform.npm_package, - "version": version, - "description": f"{SUMMARY} ({platform.npm_os} {platform.npm_cpu})", - "os": [platform.npm_os], - "cpu": [platform.npm_cpu], - "files": [f"bin/{platform.executable}"], - "license": LICENSE, - "repository": {"type": "git", "url": f"git+{REPOSITORY}.git"}, - } - if platform.libc is not None: - manifest["libc"] = [platform.libc] - with tarfile.open(destination, "w:gz") as archive: - add_tar_bytes(archive, "package/package.json", json.dumps(manifest, indent=2).encode() + b"\n") - add_tar_bytes( - archive, - f"package/bin/{platform.executable}", - binary.read_bytes(), - mode=0o755, - ) - add_tar_bytes(archive, "package/LICENSE", (ROOT / "LICENSE").read_bytes()) - return destination - - -def launcher_source() -> bytes: - """Return the Node.js launcher that selects the installed native package.""" - mapping: dict[str, dict[str, dict[str, str]]] = {} - for platform in PLATFORMS.values(): - key = f"{platform.npm_os}-{platform.npm_cpu}" - libc = platform.libc or "default" - mapping.setdefault(key, {})[libc] = { - "package": platform.npm_package, - "executable": platform.executable, - } - return ( - "#!/usr/bin/env node\n" - "// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.\n" - "// SPDX-License-Identifier: Apache-2.0\n\n" - "const { spawnSync } = require('node:child_process');\n" - "const path = require('node:path');\n\n" - f"const platforms = {json.dumps(mapping, indent=2)};\n" - "const key = `${process.platform}-${process.arch}`;\n" - "const libc = process.platform === 'linux'\n" - " ? (process.report?.getReport?.().header.glibcVersionRuntime ? 'glibc' : 'musl')\n" - " : 'default';\n" - "const selected = platforms[key]?.[libc] ?? platforms[key]?.default;\n" - "if (!selected) {\n" - " console.error(`nemo-relay-cli-bin does not support ${process.platform}/${process.arch}`);\n" - " process.exit(1);\n" - "}\n" - "let manifest;\n" - "try {\n" - " manifest = require.resolve(`${selected.package}/package.json`);\n" - "} catch (error) {\n" - " console.error(\n" - " `The native package ${selected.package} is missing. ` +\n" - " 'Reinstall nemo-relay-cli-bin without omitting optional dependencies.',\n" - " );\n" - " process.exit(1);\n" - "}\n" - "const executable = path.join(path.dirname(manifest), 'bin', selected.executable);\n" - "const result = spawnSync(executable, process.argv.slice(2), { stdio: 'inherit' });\n" - "if (result.error) {\n" - " console.error(`Failed to start ${executable}: ${result.error.message}`);\n" - " process.exit(1);\n" - "}\n" - "process.exit(result.status === null ? 1 : result.status);\n" - ).encode() - - -def build_npm_launcher(version: str, output: Path) -> Path: - """Build the portable npm launcher package.""" - destination = output / f"nemo-relay-bin-npm-{version}.tgz" - manifest = { - "name": PACKAGE_NAME, - "version": version, - "description": SUMMARY, - "bin": {"nemo-relay": "bin/nemo-relay.js"}, - "files": ["bin/nemo-relay.js"], - "engines": {"node": ">=24.0.0"}, - "optionalDependencies": {platform.npm_package: version for platform in PLATFORMS.values()}, - "license": LICENSE, - "repository": {"type": "git", "url": f"git+{REPOSITORY}.git"}, - } - with tarfile.open(destination, "w:gz") as archive: - add_tar_bytes(archive, "package/package.json", json.dumps(manifest, indent=2).encode() + b"\n") - add_tar_bytes(archive, "package/bin/nemo-relay.js", launcher_source(), mode=0o755) - add_tar_bytes(archive, "package/LICENSE", (ROOT / "LICENSE").read_bytes()) - return destination - - def parse_args() -> argparse.Namespace: """Parse CLI package assembly arguments.""" parser = argparse.ArgumentParser() @@ -300,25 +160,17 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--target", choices=sorted(PLATFORMS), required=True) parser.add_argument("--version", required=True) parser.add_argument("--output-dir", type=Path, required=True) - parser.add_argument("--npm-launcher", action="store_true") return parser.parse_args() def main() -> None: - """Build the wheel and npm artifacts requested on the command line.""" + """Build the wheel requested on the command line.""" args = parse_args() if not args.binary.is_file(): raise SystemExit(f"CLI binary does not exist: {args.binary}") args.output_dir.mkdir(parents=True, exist_ok=True) platform = PLATFORMS[args.target] - artifacts = [ - build_wheel(args.binary, platform, args.version, args.output_dir), - build_npm_platform(args.binary, platform, args.version, args.output_dir), - ] - if args.npm_launcher: - artifacts.append(build_npm_launcher(args.version, args.output_dir)) - for artifact in artifacts: - print(artifact) + print(build_wheel(args.binary, platform, args.version, args.output_dir)) if __name__ == "__main__": diff --git a/scripts/tests/test_package_cli_bin.py b/scripts/tests/test_package_cli_bin.py index 6d352cd46..90e1a78da 100644 --- a/scripts/tests/test_package_cli_bin.py +++ b/scripts/tests/test_package_cli_bin.py @@ -1,20 +1,16 @@ # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""Tests for CLI wheel and npm package assembly.""" +"""Tests for CLI wheel assembly.""" import importlib.util -import json import os import stat -import subprocess import sys -import tarfile import tempfile import unittest import zipfile from pathlib import Path -from typing import IO ROOT = Path(__file__).resolve().parents[2] SPEC = importlib.util.spec_from_file_location("package_cli_bin", ROOT / "scripts" / "package-cli-bin.py") @@ -24,15 +20,8 @@ SPEC.loader.exec_module(PACKAGE_CLI_BIN) -def required_member(archive: tarfile.TarFile, name: str) -> IO[bytes]: - member = archive.extractfile(name) - if member is None: - raise AssertionError(f"archive is missing {name}") - return member - - class PackageCliBinTests(unittest.TestCase): - def test_packages_linux_binaries_for_python_and_npm(self) -> None: + def test_packages_linux_binaries_for_python(self) -> None: with tempfile.TemporaryDirectory() as temporary: output = Path(temporary) binary = output / "nemo-relay" @@ -44,10 +33,7 @@ def test_packages_linux_binaries_for_python_and_npm(self) -> None: try: os.chdir(output) wheel = PACKAGE_CLI_BIN.build_wheel(binary, gnu_platform, "0.7.0-rc.1", output) - native = PACKAGE_CLI_BIN.build_npm_platform(binary, gnu_platform, "0.7.0-rc.1", output) musl_wheel = PACKAGE_CLI_BIN.build_wheel(binary, musl_platform, "0.7.0-rc.1", output) - musl_native = PACKAGE_CLI_BIN.build_npm_platform(binary, musl_platform, "0.7.0-rc.1", output) - launcher = PACKAGE_CLI_BIN.build_npm_launcher("0.7.0-rc.1", output) finally: os.chdir(previous_directory) @@ -63,48 +49,6 @@ def test_packages_linux_binaries_for_python_and_npm(self) -> None: self.assertIn("0.7.0rc1-py3-none-musllinux_1_2_x86_64", musl_wheel.name) - self.assertEqual(native.name, "nemo-relay-bin-npm-linux-x64-0.7.0-rc.1.tgz") - with tarfile.open(native) as archive: - manifest = json.load(required_member(archive, "package/package.json")) - self.assertEqual(manifest["os"], ["linux"]) - self.assertEqual(manifest["cpu"], ["x64"]) - self.assertEqual(manifest["libc"], ["glibc"]) - self.assertEqual( - required_member(archive, "package/bin/nemo-relay").read(), - b"test-binary", - ) - - self.assertEqual(musl_native.name, "nemo-relay-bin-npm-linux-x64-musl-0.7.0-rc.1.tgz") - with tarfile.open(musl_native) as archive: - manifest = json.load(required_member(archive, "package/package.json")) - self.assertEqual(manifest["libc"], ["musl"]) - - self.assertEqual(launcher.name, "nemo-relay-bin-npm-0.7.0-rc.1.tgz") - with tarfile.open(launcher) as archive: - manifest = json.load(required_member(archive, "package/package.json")) - self.assertEqual(manifest["bin"]["nemo-relay"], "bin/nemo-relay.js") - self.assertEqual( - manifest["optionalDependencies"]["nemo-relay-cli-bin-linux-x64"], - "0.7.0-rc.1", - ) - self.assertEqual( - manifest["optionalDependencies"]["nemo-relay-cli-bin-linux-x64-musl"], - "0.7.0-rc.1", - ) - launcher_path = output / "launcher/package/bin/nemo-relay.js" - launcher_path.parent.mkdir(parents=True) - launcher_path.write_bytes(required_member(archive, "package/bin/nemo-relay.js").read()) - - result = subprocess.run( - ["node", output / "launcher/package/bin/nemo-relay.js", "--version"], - check=False, - capture_output=True, - text=True, - ) - self.assertEqual(result.returncode, 1) - self.assertIn("The native package nemo-relay-cli-bin-", result.stderr) - self.assertIn("is missing", result.stderr) - def test_rejects_unsupported_version(self) -> None: with self.assertRaisesRegex(ValueError, "unsupported package version"): PACKAGE_CLI_BIN.wheel_version("dev-deadbeef")