fix: repair Hermes shutdown-only ATIF artifacts

[bbednarski9]

Overview

Fixes a Hermes CLI hook-forward issue where sparse pre_tool_call payloads could create a separate task-id session and export a one-step ATIF trajectory containing only gateway_shutdown.

• I confirm this contribution is my own work, or I have the right to submit it under this project's license.
• I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

Public Hermes releases can emit pre_tool_call hooks with a task id but without enough stable session/tool-call identity to pair the hook with the eventual post_tool_call. NeMo Relay previously treated the task id as a session id, opened a synthetic Hermes session, and later closed it during gateway shutdown.

This PR drops uncorrelatable Hermes pre_tool_call hooks unless they include both a real session identifier and a stable tool-call id. The post_tool_call hook still records the tool result and attaches it to the main trajectory.

Added regression coverage for the sparse pre_tool_call case and updated adapter coverage for correlatable Hermes tool payloads.

Where should the reviewer start?

Start with crates/cli/src/adapters/hermes.rs, especially the pretoolcall guard and hermes_pre_tool_call_is_correlatable helper. The key regression test is hermes_uncorrelatable_pre_tool_call_does_not_create_shutdown_trajectory in crates/cli/tests/coverage/session_tests.rs.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• Relates to [Bug]: duplicate LLM steps when CLI hook-forward and gateway instrumentation both observe the same request #176

Summary by CodeRabbit

• Improvements

  • Hermes adapter now filters pre-tool-call events that lack valid session and tool call identifiers, reducing noise in the event stream.

• Tests

  • Added test coverage validating proper filtering of uncorrelatable Hermes payloads and session trajectory behavior.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Walkthrough

The PR adds filtering to the Hermes adapter that suppresses pretoolcall events when they lack correlatable session or tool-call identifiers. Core helpers detect these identifiers from headers and payload fields; unit and integration tests validate the filtering at adapter and session levels.

Changes

Hermes pre-tool-call correlatability gating

Layer / File(s)	Summary
Correlatability detection helpers and early return gate crates/cli/src/adapters/hermes.rs	hermes_pre_tool_call_is_correlatable and sub-helpers detect explicit session and tool-call IDs from headers and payload fields. Early return in adapt suppresses uncorrelatable pretoolcall events, returning no events and {}.
Adapter unit test coverage crates/cli/tests/coverage/adapters_tests.rs	Test payload corrected to use valid session/task IDs. New test drops_uncorrelatable_hermes_pre_tool_call validates uncorrelatable payloads emit no events and empty response.
Session integration regression test crates/cli/tests/coverage/session_tests.rs	New test verifies uncorrelatable pre-tool-call does not populate ATIF trajectory entries through the full session pipeline; ATIF includes session but excludes uncorrelatable task and shutdown reason.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	Description includes all required template sections with clear details about the issue, solution, reviewer guidance, and related issue reference.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title 'fix: repair Hermes shutdown-only ATIF artifacts' follows Conventional Commits format with lowercase type, concise imperative summary, under 72 characters, and no trailing period. It accurately reflects the main change: fixing Hermes adapter behavior for uncorrelatable pre_tool_call payloads to prevent spurious shutdown-only ATIF artifacts.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/cli/src/adapters/hermes.rs`:
- Around line 249-257: The check in has_explicit_hermes_tool_call_id that calls
hermes_string_at(payload, "id") is too broad and can match unrelated top-level
or extra.id fields; remove that fallback (the hermes_string_at(payload, "id")
check) or replace it with a more specific path check (e.g., only accept
["tool","id"] / ["tool_input","id"] or a documented version-specific key) and
add a short comment in has_explicit_hermes_tool_call_id explaining why the
generic "id" check was removed or when a bare "id" is allowed so future readers
know the rationale.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: ed3eb128-887e-4097-8c1b-50605ed5a0f0

📥 Commits

Reviewing files that changed from the base of the PR and between c3bffba and 8d15d20.

📒 Files selected for processing (3)

• crates/cli/src/adapters/hermes.rs
• crates/cli/tests/coverage/adapters_tests.rs
• crates/cli/tests/coverage/session_tests.rs

📜 Review details

🧰 Additional context used

📓 Path-based instructions (6)

**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Keep SPDX headers on Rust source files. The project is Apache-2.0.
Use snake_case for Rust binding naming conventions.
Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Preserve async behavior on the existing tokio-based model i...

Files:

• crates/cli/src/adapters/hermes.rs
• crates/cli/tests/coverage/session_tests.rs
• crates/cli/tests/coverage/adapters_tests.rs

**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

• crates/cli/src/adapters/hermes.rs
• crates/cli/tests/coverage/session_tests.rs
• crates/cli/tests/coverage/adapters_tests.rs

**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

• crates/cli/src/adapters/hermes.rs
• crates/cli/tests/coverage/session_tests.rs
• crates/cli/tests/coverage/adapters_tests.rs

**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

• crates/cli/src/adapters/hermes.rs
• crates/cli/tests/coverage/session_tests.rs
• crates/cli/tests/coverage/adapters_tests.rs

{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}

📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)

Maintain documented and tested validation and report behavior for adaptive surfaces

Files:

• crates/cli/tests/coverage/session_tests.rs
• crates/cli/tests/coverage/adapters_tests.rs

{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}

⚙️ CodeRabbit configuration file

{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.

Files:

• crates/cli/tests/coverage/session_tests.rs
• crates/cli/tests/coverage/adapters_tests.rs

🔇 Additional comments (8)

crates/cli/src/adapters/hermes.rs (4)

51-56: LGTM!

---

230-236: LGTM!

---

238-247: LGTM!

---

259-264: LGTM!

crates/cli/tests/coverage/adapters_tests.rs (2)

282-309: LGTM!

---

311-326: LGTM!

crates/cli/tests/coverage/session_tests.rs (2)

1454-1499: LGTM!

---

1454-1499: Re-run required Rust validation for this change (just test-rust, cargo fmt --all, cargo clippy ... -D warnings)

• just test-rust
• cargo fmt --all --check
• cargo clippy --workspace --all-targets -- -D warnings

[willkill07]

/ok to test 8d15d20

[bbednarski9]

/merge