feat(cli)!: add MCP-managed shared gateway for coding agents#395
feat(cli)!: add MCP-managed shared gateway for coding agents#395willkill07 wants to merge 92 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR restructures the CLI into an ChangesCodex plugin_shim trust flow and MCP packaging
Persistent agent integration platform
Test coverage for restructured CLI
Documentation and packaging assets
Estimated code review effort: 5 (Critical) | ~180 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XL
willkill07
force-pushed
the
wkk_relay-447-codex-bootstrap
branch
from
37d6608 to
d8740a1
Compare
willkill07
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/plugin_shim/shared.rs`:
- Around line 41-60: The sidecar lock acquisition loop in shared.rs can time out
before stale-lock recovery becomes eligible, so adjust the timing constants used
by ensure_sidecar and related lock handling: either increase
SIDECAR_LOCK_TIMEOUT to be at least as long as STALE_LOCK_AFTER, or reduce the
stale threshold so repair_stale_lock can reclaim an orphaned lock within the
same call. Keep the behavior around fs::create_dir, healthz, repair_stale_lock,
and sidecar_start_error consistent so retryable hook calls do not fail early
with “sidecar lock timed out”.
In `@README.md`:
- Around line 123-126: The README warning uses trust-state wording too broadly
and should match CodexHookTrustReport terminology. Update the text around the
install and doctor guidance so it distinguishes modified hooks from untrusted
hooks instead of collapsing them into one state, and make sure the wording
matches what nemo-relay doctor actually reports for generated, modified, and
manual/source-marketplace hooks.
🪄 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: f2e7bcd6-9e1a-48bf-93b6-e41075a2cd5d
📒 Files selected for processing (11)
README.mdcrates/cli/src/plugin_shim/codex.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/tests/coverage/plugin_shim_tests.rscrates/cli/tests/coverage/server_tests.rsdocs/nemo-relay-cli/plugin-installation.mdxintegrations/coding-agents/codex/README.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (25)
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
README.mdintegrations/coding-agents/codex/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
README.mdintegrations/coding-agents/codex/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
README.mdintegrations/coding-agents/codex/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
README.mdintegrations/coding-agents/codex/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
README.mdintegrations/coding-agents/codex/README.md
{README.md,docs/**/*.{md,rst,txt},fern/**/*}
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Search and update documentation source for references to the old version in
README.md,docs, andferndirectories, updating current-version install commands, package examples, and configuration examples to<next-version>
Files:
README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
README.mddocs/nemo-relay-cli/plugin-installation.mdxintegrations/coding-agents/codex/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
README.mddocs/nemo-relay-cli/plugin-installation.mdxintegrations/coding-agents/codex/README.md
{docs/**/*.md,README.md}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update reference docs, language-binding docs, READMEs, and example documentation when the public surface or expected usage changes.
Files:
README.md
{README.md,docs/index.md}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update entry-point docs when examples or reading paths change
Files:
README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
README.mdcrates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rsdocs/nemo-relay-cli/plugin-installation.mdxintegrations/coding-agents/codex/README.mdcrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_tests.rs
README.md
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands in
README.mdchange, run the targeted docs checks appropriate to the change.
Files:
README.md
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings,
documentation, integrations, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile skills/ # Published Codex/agent skills for NeMo Relay usage patternsPrerequisites
Insta...
Files:
README.mdcrates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rsdocs/nemo-relay-cli/plugin-installation.mdxintegrations/coding-agents/codex/README.mdcrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_tests.rs
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
README.mddocs/nemo-relay-cli/plugin-installation.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/tests/coverage/server_tests.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rscrates/cli/tests/coverage/plugin_shim_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/server_tests.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/tests/coverage/plugin_shim_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/cli/src/server.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugin_shim/codex_app_server.rscrates/cli/src/plugin_shim/codex.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/nemo-relay-cli/plugin-installation.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/nemo-relay-cli/plugin-installation.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/nemo-relay-cli/plugin-installation.mdx
🔇 Additional comments (18)
crates/cli/src/server.rs (2)
300-314: LGTM!
316-335: 🩺 Stability & Availability
idle_shutdown_readyis reachable fromserver_tests.rs#[cfg(test)] #[path = "../tests/coverage/server_tests.rs"] mod tests;makes that file a child module, so the private helper is visible there.> Likely an incorrect or invalid review comment.crates/cli/tests/coverage/server_tests.rs (1)
540-568: LGTM!docs/nemo-relay-cli/plugin-installation.mdx (1)
81-99: LGTM!Also applies to: 127-132, 165-167
integrations/coding-agents/codex/README.md (1)
46-63: LGTM!Also applies to: 192-194, 292-304
crates/cli/src/plugin_shim/codex.rs (4)
117-122: Same let-chain / MSRV concern flagged incodex_app_server.rsLines 158-160 applies here (&& let Some(client) = ...); resolve once by confirming the pinned toolchain is>= 1.88.
236-240: 🎯 Functional Correctness | ⚡ Quick winLexical path equality may reject legitimately-matching hooks.
Path::new(&hook.source_path) == hooks_pathcompares components lexically. If the app-server reports a canonicalized/symlink-resolved path (common on macOS, e.g./var→/private/var) whilehome_dir().join(".codex/hooks.json")is not resolved, this filter yields no hooks, andauto_trust_codex_hooksthen fails with "missing required hooks". Consider comparing canonicalized paths (or file identity) when both exist.
22-59: LGTM!Also applies to: 61-116, 123-135, 144-234, 245-302
42-43: 🗄️ Data Integrity & IntegrationNo rollback gap here
prepare_codex_configonly parses and validatesconfig.toml; it doesn’t mutate files, so taking snapshots afterward doesn’t miss any install-time changes.> Likely an incorrect or invalid review comment.crates/cli/src/plugin_shim/codex_app_server.rs (3)
45-94: LGTM!Also applies to: 96-122, 124-139, 141-147, 167-182, 184-197, 199-209, 211-225
158-160: 🎯 Functional CorrectnessLet chains are supported by the pinned toolchain
rust-toolchain.tomlpins Rust1.93.0, soif let Some(errors) = ... && !errors.is_empty()is valid. The MSRV concern does not apply here. The same applies tocrates/cli/src/plugin_shim/codex.rs.> Likely an incorrect or invalid review comment.
18-30: 🗄️ Data Integrity & IntegrationNo change needed The
CodexHookMetadatafield names are already mapped to the app-server’s camelCase schema by#[serde(rename_all = "camelCase")]; the snake_case Rust fields are correct here.> Likely an incorrect or invalid review comment.crates/cli/src/plugin_shim/mod.rs (2)
101-110: LGTM!
202-223: LGTM!Also applies to: 233-245
crates/cli/src/plugin_shim/shared.rs (1)
382-476: LGTM!crates/cli/tests/coverage/plugin_shim_tests.rs (2)
17-140: LGTM!
311-403: LGTM!Also applies to: 468-645, 985-1058
crates/cli/tests/coverage/plugin_install_tests.rs (1)
385-408: LGTM!
willkill07
changed the title
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/plugin_install/host.rs`:
- Around line 293-296: The relay executable path handling in resolve_executable
should keep canonicalization for matching, but avoid serializing a Windows \\?\
path into the MCP config command. Update the host.rs flow around RELAY_COMMAND
so the value written to .mcp.json is normalized to a Windows-friendly path while
preserving the canonicalized form only for comparison/lookup. Use the existing
executable resolution and config-writing logic in this area to ensure the
serialized MCP command remains executable on Windows.
In `@crates/cli/src/plugin_install/mod.rs`:
- Around line 594-605: The Generated MCP server check builds the MCP config path
inline with a literal ".mcp.json", which is inconsistent with the other manifest
path helpers and may drift from PluginLayout::new. Add a dedicated
mcp_config_path helper near the existing path helpers and use it in the
readiness.plugin branch instead of calling plugin.join directly, keeping the
path construction centralized alongside marketplace_manifest_path and
plugin_manifest_path.
🪄 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: 088df63e-c8eb-465a-9b69-608258e79bb2
📒 Files selected for processing (21)
crates/cli/Cargo.tomlcrates/cli/src/config.rscrates/cli/src/main.rscrates/cli/src/mcp.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/plugin_install/state.rscrates/cli/src/plugin_shim/codex.rscrates/cli/src/server.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/tests/coverage/main_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/tests/coverage/plugin_install_tests.rsintegrations/coding-agents/README.mdintegrations/coding-agents/codex/.codex-plugin/plugin.jsonintegrations/coding-agents/codex/.mcp.jsonintegrations/coding-agents/codex/README.mdintegrations/coding-agents/codex/hooks/hooks.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (19)
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
integrations/coding-agents/codex/hooks/hooks.jsoncrates/cli/Cargo.tomlcrates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rsintegrations/coding-agents/README.mdcrates/cli/src/plugin_install/marketplace.rsintegrations/coding-agents/codex/README.mdcrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings,
documentation, integrations, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile skills/ # Published Codex/agent skills for NeMo Relay usage patternsPrerequisites
Insta...
Files:
integrations/coding-agents/codex/hooks/hooks.jsoncrates/cli/Cargo.tomlcrates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rsintegrations/coding-agents/README.mdcrates/cli/src/plugin_install/marketplace.rsintegrations/coding-agents/codex/README.mdcrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.rs
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.Keep Rust package names and workspace metadata in
Cargo.tomlinternally consistent across the project.
Files:
crates/cli/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all TOML files using the
#comment form.
Files:
crates/cli/Cargo.toml
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/tests/coverage/main_tests.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/src/plugin_shim/codex.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/main_tests.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/mcp_tests.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/tests/coverage/plugin_install_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/cli/src/plugin_install/setup.rscrates/cli/src/main.rscrates/cli/src/config.rscrates/cli/src/plugin_install/host.rscrates/cli/src/plugin_install/state.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/mcp.rscrates/cli/src/server.rscrates/cli/src/plugin_shim/codex.rs
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
integrations/coding-agents/README.mdintegrations/coding-agents/codex/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
integrations/coding-agents/README.mdintegrations/coding-agents/codex/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
integrations/coding-agents/README.mdintegrations/coding-agents/codex/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
integrations/coding-agents/README.mdintegrations/coding-agents/codex/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
integrations/coding-agents/README.mdintegrations/coding-agents/codex/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
integrations/coding-agents/README.mdintegrations/coding-agents/codex/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
integrations/coding-agents/README.mdintegrations/coding-agents/codex/README.md
🔇 Additional comments (26)
crates/cli/src/server.rs (2)
53-63: LGTM!Also applies to: 65-85, 87-91, 132-146, 178-188
148-164: 🩺 Stability & AvailabilityReadiness signal is correctly ordered. It fires after plugin/router setup and before
axum::serve, and the MCP launcher only waits on it before starting stdio handling.crates/cli/Cargo.toml (1)
53-53: LGTM!crates/cli/src/config.rs (1)
72-83: LGTM!crates/cli/src/main.rs (1)
17-17: LGTM!Also applies to: 84-84
crates/cli/src/mcp.rs (2)
1-104: LGTM!Also applies to: 131-187
105-129: 🗄️ Data Integrity & IntegrationMCP stdio uses newline-delimited JSON, so
read_lineis the right framing here.> Likely an incorrect or invalid review comment.integrations/coding-agents/codex/.mcp.json (1)
1-15: LGTM!integrations/coding-agents/codex/README.md (1)
22-23: LGTM!Also applies to: 48-68, 196-200, 227-227, 299-317, 338-342
integrations/coding-agents/codex/hooks/hooks.json (1)
2-151: LGTM!crates/cli/tests/cli_tests.rs (1)
164-217: LGTM!Also applies to: 1329-1329
crates/cli/tests/coverage/main_tests.rs (1)
86-96: LGTM!crates/cli/tests/coverage/mcp_tests.rs (1)
1-201: LGTM!Good coverage of readiness gating, EOF shutdown, and the plugin-activation error path — satisfies the path instruction to prefer lifecycle/error-path assertions over shallow smoke tests.
crates/cli/src/plugin_shim/codex.rs (3)
146-314: LGTM!Trust classification, canonicalized path matching, missing-required computation, and the snapshot/rollback aggregation helpers all look correct and consistent with the PR's trust-hardening goals.
117-122: 🩺 Stability & AvailabilityNo issue — the workspace targets Rust 1.93.0 and edition 2024, so this
letchain is supported.
36-53: 🗄️ Data Integrity & IntegrationSnapshot order is fine
prepare_codex_configonly parsesconfig.tomland returns an error on invalid TOML; it does not write to disk, so taking the snapshot afterward still captures the pre-install state.> Likely an incorrect or invalid review comment.integrations/coding-agents/README.md (1)
25-36: LGTM!Also applies to: 69-85
integrations/coding-agents/codex/.codex-plugin/plugin.json (1)
4-22: LGTM!crates/cli/src/plugin_install/state.rs (1)
50-50: LGTM!Also applies to: 74-83
crates/cli/src/plugin_install/host.rs (1)
319-338: LGTM! Mirrors the existingvalidate_relay_plugin_shimpattern and matches the mocked-failure test expectations.crates/cli/src/plugin_install/marketplace.rs (2)
143-160: 🎯 Functional CorrectnessConfirm the hardcoded MCP bind address doesn't drift from a single source of truth, and how port conflicts are handled.
NEMO_RELAY_GATEWAY_BINDis hardcoded here to a fixed port (127.0.0.1:47632), while docs describe the transparent-wrapper flow using a dynamic port. Since this config is marked"required": true, ifconfig.rs/mcp.rs(default bind handling, not in this file set) ever changes the default bind or if the port is already in use (e.g., two installs, a leftover process), Codex startup could fail hard. Worth confirming this literal is sourced from (or kept in sync with) the same constant used by themcpsubcommand's default bind, and whether a bind failure is retried/surfaced gracefully.
20-28: LGTM! Consistent with updated tests (installer_tests.rs, plugin_install_tests.rs) confirming the generated.mcp.jsoncontent, manifest description/interface text, and the new Codex hooks-template behavior.Also applies to: 49-51, 100-131, 176-176
crates/cli/src/plugin_install/mod.rs (1)
361-369: LGTM! Correctly gates Codex plugin generation behindvalidate_relay_mcp, matchingrelay_without_native_mcp_fails_codex_install_before_generating_plugin.crates/cli/src/plugin_install/setup.rs (1)
58-58: LGTM!crates/cli/tests/coverage/plugin_install_tests.rs (1)
287-293: LGTM! Good coverage of the new MCP validation gate, generated config content, and readiness failure modes (missing config, mismatched binary), consistent with the source changes.Also applies to: 346-362, 474-474, 529-534, 847-870, 1045-1069, 1444-1487
crates/cli/tests/coverage/installer_tests.rs (1)
171-179: LGTM!Also applies to: 228-245, 283-283
willkill07
force-pushed
the
wkk_relay-447-codex-bootstrap
branch
from
027d8ed to
f7779fc
Compare
willkill07
changed the title
github-actions
Bot
added
size:XXL
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Signed-off-by: Will Killian <wkillian@nvidia.com>
✅ Action performedReview finished.
|
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/agents/hermes/integration.rs`:
- Around line 491-494: Update the config lookup in the Hermes persistence check
to construct the /mcp_servers path using MCP_SERVER_NAME instead of the
hardcoded "nemo-relay" string, matching the other lookups in the file while
preserving the existing exact-value comparison.
In `@crates/cli/src/bootstrap/state.rs`:
- Around line 285-296: Update remove_if_matches and the OwnerGuard::drop cleanup
path to use the same endpoint lock that protects owner-record publication,
covering the read-and-delete sequence atomically so a handoff cannot remove a
newer owner record. Acquire the lock only after gateway shutdown has completed,
and avoid holding it while waiting for shutdown.
In `@crates/cli/src/commands/diagnostics.rs`:
- Around line 55-59: Update the JSON output selection in the diagnostics command
to use plugin.is_all() rather than reports.len() when choosing between the
aggregate plugins shape and with_schema for a single report. Preserve the
existing output structures while ensuring --plugin all --json always returns the
aggregate shape, regardless of report count.
In `@crates/cli/src/installation/marketplace/state.rs`:
- Around line 312-333: The remove_path function currently discards a
non-NotFound error from remove_dir_all when the remove_file fallback also fails.
Preserve the original remove_dir_all error through the fallback and report it
when applicable, while retaining successful removal and NotFound handling for
both operations.
In `@crates/cli/src/process/mod.rs`:
- Around line 101-107: Update command_argv to preserve quoted executable paths
and arguments containing spaces instead of using raw split_whitespace. Use a
quote-aware argument parser, or change the configuration representation to
accept argv arrays, while preserving the existing command parsing and
diagnostics behavior.
- Around line 179-189: Update resolve_candidate to require executable permission
for PATH candidates on Unix, rather than accepting every regular file via
is_file. Preserve the existing extension iteration and candidate construction,
while allowing regular-file selection on non-Unix platforms as appropriate.
In `@crates/cli/src/sessions/idle.rs`:
- Around line 43-59: Update the idle-session sweep around the sessions removal
and subsequent close/extend flow so a concurrently created session with the same
ID cannot be overwritten. Serialize per-session mutation or reserve each removed
session ID while sweeping, and only restore retained sessions when the map entry
is still the removed session rather than replacing a newer active session; apply
the same protection to the corresponding logic at the additional affected
location.
In `@crates/cli/src/sessions/routing.rs`:
- Around line 40-43: Update promote_pending_subagent and its callers so removing
the pending entry and child session occurs only after all fallible
Session::apply operations and the alias commit succeed. Preserve the pending
child state, or restore it on failure, allowing promotion to be retried without
losing lifecycle state; keep successful promotion behavior unchanged.
In `@crates/cli/tests/architecture_tests.rs`:
- Around line 86-98: Update tests_are_not_embedded_in_the_source_tree to detect
any test-only module declaration, not just the exact #[cfg(test)] mod tests
pattern. Account for module names other than tests, same-line formatting, and
intervening attributes while preserving the existing failure message and
source-tree scan.
In `@crates/cli/tests/coverage/shared/bootstrap_state_tests.rs`:
- Around line 9-51: Consolidate the duplicated EnvScope RAII environment-lock
helper into the shared parent module, preserving its ENV_TEST_LOCK acquisition,
variable snapshotting, and Drop restoration behavior. Remove the local EnvScope
definitions from crates/cli/tests/coverage/shared/bootstrap_state_tests.rs:9-51,
bootstrap_tests.rs:10-76, completions_install_tests.rs:107-147, and
doctor_tests.rs:47-87; ensure the shared definition is reachable through the
existing use super::* import and uses the canonical field naming.
🪄 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: 10663960-dfd4-4677-9c2f-752bfbc35a56
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (188)
.claude-plugin/marketplace.json.config/nextest.tomlATTRIBUTIONS-Rust.mdcodecov.ymlcrates/cli/Cargo.tomlcrates/cli/src/agents/claude/adapter.rscrates/cli/src/agents/claude/alignment.rscrates/cli/src/agents/claude/assets.rscrates/cli/src/agents/claude/doctor.rscrates/cli/src/agents/claude/host.rscrates/cli/src/agents/claude/install.rscrates/cli/src/agents/claude/launch.rscrates/cli/src/agents/claude/mod.rscrates/cli/src/agents/codex/adapter.rscrates/cli/src/agents/codex/alignment.rscrates/cli/src/agents/codex/app_server.rscrates/cli/src/agents/codex/assets.rscrates/cli/src/agents/codex/doctor.rscrates/cli/src/agents/codex/host.rscrates/cli/src/agents/codex/install.rscrates/cli/src/agents/codex/launch.rscrates/cli/src/agents/codex/mod.rscrates/cli/src/agents/hermes/adapter.rscrates/cli/src/agents/hermes/alignment.rscrates/cli/src/agents/hermes/config.rscrates/cli/src/agents/hermes/doctor.rscrates/cli/src/agents/hermes/files.rscrates/cli/src/agents/hermes/install.rscrates/cli/src/agents/hermes/integration.rscrates/cli/src/agents/hermes/launch.rscrates/cli/src/agents/hermes/mod.rscrates/cli/src/agents/hermes/trust.rscrates/cli/src/agents/mod.rscrates/cli/src/agents/shared/adapters.rscrates/cli/src/agents/shared/alignment.rscrates/cli/src/agents/shared/host.rscrates/cli/src/agents/shared/mod.rscrates/cli/src/banner.rscrates/cli/src/bootstrap/mod.rscrates/cli/src/bootstrap/state.rscrates/cli/src/commands/completions/install.rscrates/cli/src/commands/completions/mod.rscrates/cli/src/commands/configure/mod.rscrates/cli/src/commands/configure/model.rscrates/cli/src/commands/configure/wizard.rscrates/cli/src/commands/diagnostics.rscrates/cli/src/commands/hook_forward.rscrates/cli/src/commands/install.rscrates/cli/src/commands/mcp.rscrates/cli/src/commands/mod.rscrates/cli/src/commands/model_pricing/mod.rscrates/cli/src/commands/plugins/mod.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/root.rscrates/cli/src/commands/run.rscrates/cli/src/commands/serve.rscrates/cli/src/config.rscrates/cli/src/configuration/mod.rscrates/cli/src/configuration/types.rscrates/cli/src/diagnostics/environment.rscrates/cli/src/diagnostics/mod.rscrates/cli/src/diagnostics/model.rscrates/cli/src/diagnostics/probes.rscrates/cli/src/diagnostics/render.rscrates/cli/src/events/json_path.rscrates/cli/src/events/mod.rscrates/cli/src/filesystem/atomic.rscrates/cli/src/filesystem/bounded.rscrates/cli/src/filesystem/locks.rscrates/cli/src/filesystem/mod.rscrates/cli/src/filesystem/snapshots.rscrates/cli/src/filesystem/temp.rscrates/cli/src/gateway/client.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/src/gateway/response.rscrates/cli/src/gateway/routes.rscrates/cli/src/hooks/delivery.rscrates/cli/src/hooks/destination.rscrates/cli/src/hooks/encoding.rscrates/cli/src/hooks/merging.rscrates/cli/src/hooks/mod.rscrates/cli/src/hooks/response.rscrates/cli/src/hooks/types.rscrates/cli/src/installation/generation.rscrates/cli/src/installation/marketplace/assets.rscrates/cli/src/installation/marketplace/host.rscrates/cli/src/installation/marketplace/mod.rscrates/cli/src/installation/marketplace/setup.rscrates/cli/src/installation/marketplace/spec.rscrates/cli/src/installation/marketplace/state.rscrates/cli/src/installation/mod.rscrates/cli/src/installation/operation_lock.rscrates/cli/src/installer.rscrates/cli/src/lib.rscrates/cli/src/main.rscrates/cli/src/mcp/gateway.rscrates/cli/src/mcp/mod.rscrates/cli/src/mcp/protocol.rscrates/cli/src/mcp/session.rscrates/cli/src/mcp/transport.rscrates/cli/src/mcp_environment.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/plugin_shim/claude.rscrates/cli/src/plugin_shim/codex.rscrates/cli/src/plugin_shim/command.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugins/config_io.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/lifecycle.rscrates/cli/src/plugins/lifecycle/environment.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/plugins/lifecycle/render.rscrates/cli/src/plugins/lifecycle/responses.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/target.rscrates/cli/src/plugins/lifecycle/trust.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/pricing.rscrates/cli/src/plugins/schema/mod.rscrates/cli/src/plugins/schema/secrets.rscrates/cli/src/plugins/types.rscrates/cli/src/process/detached.rscrates/cli/src/process/launcher.rscrates/cli/src/process/mod.rscrates/cli/src/process/prepared.rscrates/cli/src/process/supervision.rscrates/cli/src/process/supervision/fallback.rscrates/cli/src/process/supervision/unix.rscrates/cli/src/process/supervision/windows.rscrates/cli/src/process/types.rscrates/cli/src/server/mod.rscrates/cli/src/server/types.rscrates/cli/src/sessions/correlation.rscrates/cli/src/sessions/idle.rscrates/cli/src/sessions/mod.rscrates/cli/src/sessions/routing.rscrates/cli/src/sessions/types.rscrates/cli/tests/architecture_tests.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/agents/adapters_tests.rscrates/cli/tests/coverage/agents/alignment_claude_code_tests.rscrates/cli/tests/coverage/agents/alignment_codex_tests.rscrates/cli/tests/coverage/agents/alignment_tests.rscrates/cli/tests/coverage/agents/coding_agent_tests.rscrates/cli/tests/coverage/agents/hermes_tests.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/tests/coverage/agents/plugin_install_setup_tests.rscrates/cli/tests/coverage/agents/plugin_install_tests.rscrates/cli/tests/coverage/commands/main_tests.rscrates/cli/tests/coverage/commands/model_pricing_tests.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/tests/coverage/plugin_shim_tests.rscrates/cli/tests/coverage/shared/agent_process_tests.rscrates/cli/tests/coverage/shared/banner_tests.rscrates/cli/tests/coverage/shared/bootstrap_state_tests.rscrates/cli/tests/coverage/shared/bootstrap_tests.rscrates/cli/tests/coverage/shared/completions_install_tests.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/tests/coverage/shared/file_io_tests.rscrates/cli/tests/coverage/shared/gateway_client_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/hook_assertions.rscrates/cli/tests/coverage/shared/install_generation_tests.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/shared/mcp_gateway_tests.rscrates/cli/tests/coverage/shared/mcp_tests.rscrates/cli/tests/coverage/shared/plugins_lifecycle_target_tests.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/tests/fixtures/windows_hook_relay.rsintegrations/coding-agents/claude-code/.claude-plugin/plugin.jsonintegrations/coding-agents/claude-code/.mcp.jsonintegrations/coding-agents/claude-code/hooks/hooks.jsonintegrations/coding-agents/codex/.codex-plugin/plugin.jsonintegrations/coding-agents/codex/.mcp.jsonintegrations/coding-agents/codex/hooks/hooks.jsonjustfile
💤 Files with no reviewable changes (1)
- crates/cli/src/config.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (10)
crates/cli/src/agents/hermes/integration.rs (1)
491-494: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm the value of the shared MCP server name constant. rg -nP 'SERVER_NAME\s*(:|=)' crates/cli/src/mcp/Repository: NVIDIA/NeMo-Relay
Length of output: 2049
Use
MCP_SERVER_NAMEin the config lookup
"/mcp_servers/nemo-relay"should come fromMCP_SERVER_NAMEhere, matching the other lookups in this file. If the shared server name changes, this check will fail even though the expected config was built from the new name.🤖 Prompt for 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. In `@crates/cli/src/agents/hermes/integration.rs` around lines 491 - 494, Update the config lookup in the Hermes persistence check to construct the /mcp_servers path using MCP_SERVER_NAME instead of the hardcoded "nemo-relay" string, matching the other lookups in the file while preserving the existing exact-value comparison.crates/cli/src/bootstrap/state.rs (1)
285-296: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Serialize owner comparison and deletion.
remove_if_matchescan read the expected record, race with a new owner publishing its record, and then delete the new record.OwnerGuard::dropcalls this without the endpoint lock, so a gateway handoff can lose its ownership metadata.Protect publication and cleanup with the same synchronization mechanism, without holding that lock while waiting for gateway shutdown.
🤖 Prompt for 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. In `@crates/cli/src/bootstrap/state.rs` around lines 285 - 296, Update remove_if_matches and the OwnerGuard::drop cleanup path to use the same endpoint lock that protects owner-record publication, covering the read-and-delete sequence atomically so a handoff cannot remove a newer owner record. Acquire the lock only after gateway shutdown has completed, and avoid holding it while waiting for shutdown.crates/cli/src/commands/diagnostics.rs (1)
55-59: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep
--plugin all --jsonOutput Shape StableSelecting the schema by report count makes the same command return either a single object or a
pluginscollection depending on machine state. Select the aggregate shape fromplugin.is_all()instead.Proposed fix
if let Some(plugin) = command.plugin { + let all_plugins = plugin.is_all(); let candidates = plugin.agents(); - let agents = if plugin.is_all() { + let agents = if all_plugins { crate::agents::installed_integrations(&candidates, command.install_dir.as_deref()) } else { candidates }; ... - let output = if reports.len() > 1 { + let output = if all_plugins { json!({ "schema_version": 1, "plugins": reports }) } else { with_schema(reports.into_iter().next().expect("reports is not empty")) };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let output = if all_plugins { json!({ "schema_version": 1, "plugins": reports }) } else { with_schema(reports.into_iter().next().expect("reports is not empty")) };🤖 Prompt for 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. In `@crates/cli/src/commands/diagnostics.rs` around lines 55 - 59, Update the JSON output selection in the diagnostics command to use plugin.is_all() rather than reports.len() when choosing between the aggregate plugins shape and with_schema for a single report. Preserve the existing output structures while ensuring --plugin all --json always returns the aggregate shape, regardless of report count.crates/cli/src/installation/marketplace/state.rs (1)
312-333: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
remove_pathmasks the original error on fallback failure.If
remove_dir_allfails for a reason other thanNotFound(e.g., permission denied), the code retries withremove_file, and on that second failure it reports only theremove_fileerror, discarding the more informative originalremove_dir_allerror.🩹 Proposed fix: preserve the original error
pub(super) fn remove_path(path: &Path, options: &PluginInstallOptions) -> Result<(), String> { if options.dry_run { println!("remove {}", path.display()); return Ok(()); } - fs::remove_dir_all(path) - .or_else(|error| { - if error.kind() == std::io::ErrorKind::NotFound { - Ok(()) - } else { - Err(error) - } - }) - .or_else(|_| fs::remove_file(path)) - .or_else(|error| { - if error.kind() == std::io::ErrorKind::NotFound { - Ok(()) - } else { - Err(format!("failed to remove {}: {error}", path.display())) - } - }) + let dir_error = match fs::remove_dir_all(path) { + Ok(()) => return Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(error) => error, + }; + match fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(_) => Err(format!("failed to remove {}: {dir_error}", path.display())), + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub(super) fn remove_path(path: &Path, options: &PluginInstallOptions) -> Result<(), String> { if options.dry_run { println!("remove {}", path.display()); return Ok(()); } let dir_error = match fs::remove_dir_all(path) { Ok(()) => return Ok(()), Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(()), Err(error) => error, }; match fs::remove_file(path) { Ok(()) => Ok(()), Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), Err(_) => Err(format!("failed to remove {}: {dir_error}", path.display())), } }🤖 Prompt for 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. In `@crates/cli/src/installation/marketplace/state.rs` around lines 312 - 333, The remove_path function currently discards a non-NotFound error from remove_dir_all when the remove_file fallback also fails. Preserve the original remove_dir_all error through the fallback and report it when applicable, while retaining successful removal and NotFound handling for both operations.crates/cli/src/process/mod.rs (2)
101-107: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve quoted arguments in configured commands.
split_whitespaceturns values such as"C:\Program Files\agent.exe" --profile "Team A"into invalid argv elements. Represent configured commands as argv arrays or use a quote-aware parser so executable paths and arguments can contain spaces.🤖 Prompt for 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. In `@crates/cli/src/process/mod.rs` around lines 101 - 107, Update command_argv to preserve quoted executable paths and arguments containing spaces instead of using raw split_whitespace. Use a quote-aware argument parser, or change the configuration representation to accept argv arrays, while preserving the existing command parsing and diagnostics behavior.
179-189: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Skip non-executable PATH candidates on Unix.
is_fileselects the first matching regular file even when it lacks execute permission. This overrides normal PATH lookup and prevents a valid executable in a later directory from launching.Proposed fix
fn resolve_candidate(base: &Path, extensions: &[OsString]) -> Option<PathBuf> { extensions.iter().find_map(|extension| { let candidate = if extension.is_empty() { base.to_path_buf() } else { let mut value = base.as_os_str().to_os_string(); value.push(extension); PathBuf::from(value) }; - candidate.is_file().then_some(candidate) + is_executable_candidate(&candidate).then_some(candidate) }) } + +#[cfg(unix)] +fn is_executable_candidate(candidate: &Path) -> bool { + use std::os::unix::fs::PermissionsExt; + + candidate + .metadata() + .is_ok_and(|metadata| metadata.is_file() && metadata.permissions().mode() & 0o111 != 0) +} + +#[cfg(not(unix))] +fn is_executable_candidate(candidate: &Path) -> bool { + candidate.is_file() +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn resolve_candidate(base: &Path, extensions: &[OsString]) -> Option<PathBuf> { extensions.iter().find_map(|extension| { let candidate = if extension.is_empty() { base.to_path_buf() } else { let mut value = base.as_os_str().to_os_string(); value.push(extension); PathBuf::from(value) }; is_executable_candidate(&candidate).then_some(candidate) }) } #[cfg(unix)] fn is_executable_candidate(candidate: &Path) -> bool { use std::os::unix::fs::PermissionsExt; candidate .metadata() .is_ok_and(|metadata| metadata.is_file() && metadata.permissions().mode() & 0o111 != 0) } #[cfg(not(unix))] fn is_executable_candidate(candidate: &Path) -> bool { candidate.is_file() }🤖 Prompt for 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. In `@crates/cli/src/process/mod.rs` around lines 179 - 189, Update resolve_candidate to require executable permission for PATH candidates on Unix, rather than accepting every regular file via is_file. Preserve the existing extension iteration and candidate construction, while allowing regular-file selection on non-Unix platforms as appropriate.crates/cli/src/sessions/idle.rs (1)
43-59: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Prevent idle sweeping from overwriting active sessions.
After removing an idle session, the mutex is released across
close_turn_for_reason. Concurrent traffic can create a new session with the same ID, whichsessions.extend(retained_sessions)then silently replaces, losing its events.Serialize per-session mutation or reserve the session ID while sweeping; do not overwrite a concurrently created entry.
Also applies to: 82-89
🤖 Prompt for 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. In `@crates/cli/src/sessions/idle.rs` around lines 43 - 59, Update the idle-session sweep around the sessions removal and subsequent close/extend flow so a concurrently created session with the same ID cannot be overwritten. Serialize per-session mutation or reserve each removed session ID while sweeping, and only restore retained sessions when the map entry is still the removed session rather than replacing a newer active session; apply the same protection to the corresponding logic at the additional affected location.crates/cli/src/sessions/routing.rs (1)
40-43: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make subagent promotion transactional.
The pending entry and child session are removed before the fallible
Session::applycalls complete. If either call fails, promotion state is lost and retrying cannot recover the child lifecycle.Retain or restore the pending child state until both events and the alias commit successfully.
Also applies to: 104-129
🤖 Prompt for 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. In `@crates/cli/src/sessions/routing.rs` around lines 40 - 43, Update promote_pending_subagent and its callers so removing the pending entry and child session occurs only after all fallible Session::apply operations and the alias commit succeed. Preserve the pending child state, or restore it on failure, allowing promotion to be retried without losing lifecycle state; keep successful promotion behavior unchanged.crates/cli/tests/architecture_tests.rs (1)
86-98: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Inline-test guard only catches modules literally named
tests.The match requires
#[cfg(test)]immediately followed bymod tests {. A#[cfg(test)]\nmod inline {, an attribute on the same line (#[cfg(test)] mod tests {), or intervening attributes all slip past, so the guideline "do not add tests undersrc" isn't fully enforced. Consider matching any#[cfg(test)]that precedes amod ... {rather than the exacttestsname.🤖 Prompt for 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. In `@crates/cli/tests/architecture_tests.rs` around lines 86 - 98, Update tests_are_not_embedded_in_the_source_tree to detect any test-only module declaration, not just the exact #[cfg(test)] mod tests pattern. Account for module names other than tests, same-line formatting, and intervening attributes while preserving the existing failure message and source-tree scan.Source: Coding guidelines
crates/cli/tests/coverage/shared/bootstrap_state_tests.rs (1)
9-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consolidate the duplicated
EnvScopeenv-lock helper into the shared parent module. The same RAII helper (lockENV_TEST_LOCK, snapshot vars, restore onDrop) is reimplemented in four test modules, and drift has already started (field namedpreviousin some,valuesin others). Hoisting one copy into thesharedparent (reachable via the existinguse super::*;) removes the divergence risk.
crates/cli/tests/coverage/shared/bootstrap_state_tests.rs#L9-L51: remove the localEnvScopestruct/impls and rely on the shared definition.crates/cli/tests/coverage/shared/bootstrap_tests.rs#L10-L76: remove the localEnvScopestruct/impls and rely on the shared definition.crates/cli/tests/coverage/shared/completions_install_tests.rs#L107-L147: remove the localEnvScope(rename itsvaluesfield to match the canonical one) and use the shared definition.crates/cli/tests/coverage/shared/doctor_tests.rs#L47-L87: remove the localEnvScopeand use the shared definition.📍 Affects 4 files
crates/cli/tests/coverage/shared/bootstrap_state_tests.rs#L9-L51(this comment)crates/cli/tests/coverage/shared/bootstrap_tests.rs#L10-L76crates/cli/tests/coverage/shared/completions_install_tests.rs#L107-L147crates/cli/tests/coverage/shared/doctor_tests.rs#L47-L87🤖 Prompt for 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. In `@crates/cli/tests/coverage/shared/bootstrap_state_tests.rs` around lines 9 - 51, Consolidate the duplicated EnvScope RAII environment-lock helper into the shared parent module, preserving its ENV_TEST_LOCK acquisition, variable snapshotting, and Drop restoration behavior. Remove the local EnvScope definitions from crates/cli/tests/coverage/shared/bootstrap_state_tests.rs:9-51, bootstrap_tests.rs:10-76, completions_install_tests.rs:107-147, and doctor_tests.rs:47-87; ensure the shared definition is reachable through the existing use super::* import and uses the canonical field naming.
There was a problem hiding this comment.
Actionable comments posted: 47
🤖 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/agents/claude/host.rs`:
- Around line 164-170: Update the backup and restore flow around
restore_claude_provider so an originally absent settings.json is represented
explicitly rather than by writing an empty JSON object. Record the absence in
the backup state, and during restore remove the settings file when that marker
indicates Relay created it; preserve existing file contents and restoration
behavior otherwise.
In `@crates/cli/src/agents/codex/host.rs`:
- Around line 727-728: Update codex_provider_client_token and its callers,
including the installation/ownership detection path around has_managed_proof, to
read client tokens from both inline and regular TOML table forms, matching
codex_provider_header behavior. Preserve existing token validation while
ensuring regular-table http_headers still detects the valid token and handles
Relay-state backup correctly.
In `@crates/cli/src/agents/hermes/config.rs`:
- Around line 192-220: Update crates/cli/src/agents/hermes/config.rs lines
192-220 so modern ownership is recognized from the Relay MCP command, expected
arguments, and generation fence without requiring a complete hook set; keep hook
completeness as a separate validation result. In
crates/cli/src/agents/hermes/integration.rs lines 146-177, add constrained
recognition for orphaned Relay approvals or persist an explicit ownership marker
so damaged or orphaned state can be repaired or uninstalled.
In `@crates/cli/src/agents/hermes/integration.rs`:
- Around line 232-245: Update the validation around forwarded_environment_names
to verify values as well as keys: for each required environment name, compare
mcp_env’s entry with the exact placeholder generated by expected_mcp_server, and
include entries that are missing or mismatched in the existing error path.
Preserve the current success behavior only when every required key has the
expected placeholder value.
In `@crates/cli/src/bootstrap/state.rs`:
- Around line 55-60: Update State::valid_for to remove the exact
CARGO_PKG_VERSION comparison and rely on bootstrap_protocol compatibility, while
retaining the version field as informational metadata. Preserve the service,
URL, and non-empty shutdown_token checks so authenticated owners from compatible
CLI versions are accepted during refresh.
In `@crates/cli/src/commands/configure/mod.rs`:
- Around line 20-29: Update the reset flow in execute and model::reset to
support the global scope as well as the project scope, using an explicit scope
option or clearing both applicable configuration files when requested. Preserve
agent filtering and existing project-reset behavior, and add tests covering
project, global, and both-scope resets.
In `@crates/cli/src/commands/install.rs`:
- Around line 99-101: Preserve non-success exit statuses in both agent loops:
update install_integration at crates/cli/src/commands/install.rs:99-101 and
uninstall_integration at crates/cli/src/commands/install.rs:119-121 to capture
and return or aggregate each ExitCode instead of discarding it, so install all
and uninstall all report agent-specific failures.
In `@crates/cli/src/commands/run.rs`:
- Around line 76-86: Update the setup decision in the run command to invoke
configure::run only when no explicit config path was supplied and no default
config exists; when explicit_config points to a missing path, skip default setup
and preserve that path for the subsequent RunOverrides construction.
In `@crates/cli/src/configuration/mod.rs`:
- Around line 151-159: Update the managed-bootstrap handling around
args.ready_file and BOOTSTRAP_FINGERPRINT_ENV so a supplied ready_file requires
a non-empty bootstrap fingerprint; return an error instead of Ok(None) when the
fingerprint is missing or empty, while preserving Ok(None) for unmanaged
children without a ready_file.
In `@crates/cli/src/diagnostics/environment.rs`:
- Around line 8-27: Update collect_environment and os_version so the OS string
omits the separator when no version is available instead of producing “windows
”. Select the shell variable by platform, using COMSPEC on Windows and SHELL
elsewhere, while preserving basename extraction and the existing Unix version
lookup.
In `@crates/cli/src/diagnostics/probes.rs`:
- Around line 29-43: Update check_dir_writable to verify actual write access by
creating a uniquely named temporary file in the validated directory with
create_new(true), then remove that file before returning success. Preserve the
existing directory validation and propagate creation or cleanup I/O errors
appropriately; do not rely on metadata.permissions().readonly().
In `@crates/cli/src/diagnostics/render.rs`:
- Around line 214-218: Update format_human_checks to include each Check’s status
marker in the rendered line, using the existing status representation and
preserving the current name/details alignment. Ensure both human-check output
paths covered by this function expose whether each check passed, warned, or
failed.
In `@crates/cli/src/filesystem/bounded.rs`:
- Around line 22-89: Update the generic bounded-file reader’s error messages to
remove the hardcoded “for persistent gateway identity” suffix and construct them
solely from the provided description. Apply this consistently to the metadata,
open, and read error paths while preserving the existing path and error details.
In `@crates/cli/src/filesystem/snapshots.rs`:
- Around line 13-26: Update backup to create a temporary file beside the
destination, copy the source into it, preserve the source permissions and
platform-specific DACLs, and sync it before publication. Atomically rename the
completed temporary file to the path returned by backup_path, ensuring existing
destination checks never accept an interrupted partial copy and cleaning up
temporary files on failure.
In `@crates/cli/src/filesystem/temp.rs`:
- Around line 8-25: Update private_temp_dir to call protect_private_windows_path
after builder.create(&path) succeeds, applying it only on Windows as
appropriate. If hardening fails, remove the newly created directory before
returning the error; otherwise preserve the existing successful path return
behavior.
In `@crates/cli/src/gateway/client.rs`:
- Around line 77-90: Update the address resolution and connection logic in
parse_loopback_url and the corresponding callers around the shutdown-token and
fingerprint connections: filter resolved addresses with
address.ip().is_loopback(), then attempt TcpStream::connect_timeout for each
remaining address until one succeeds. Do not use only the first resolver result;
return the existing connection error when no loopback address connects, while
preserving the current rejection behavior when resolution yields no valid
loopback addresses.
- Around line 120-180: Update the gateway connection flow surrounding the
bootstrap challenge and verified POST so it establishes an authenticated
encrypted channel before sending any hook payload. Use TLS with the appropriate
pinned per-user Relay identity, perform certificate/peer validation before
transmitting the request, and preserve the existing bootstrap health
verification as an additional check rather than relying on it to authenticate
the TCP peer.
In `@crates/cli/src/gateway/response.rs`:
- Around line 56-67: Make header filtering map-aware so every header named by
the Connection header’s tokens is excluded, in addition to the existing
hop-by-hop and sensitive-header rules. Update should_forward_request_header and
the response filtering logic in crates/cli/src/gateway/response.rs lines 56-67
and 29-35 to receive the complete header map, then update both forwarding call
sites in crates/cli/src/gateway/mod.rs lines 649-659 and 857-867 to use the new
request filter, preserving the existing exclusions including Accept-Encoding.
In `@crates/cli/src/hooks/delivery.rs`:
- Around line 102-115: Update both forwarding branches in the hook delivery flow
to handle transport setup or task errors through handle_hook_error instead of
propagating them with ?. Preserve the existing fail_closed behavior by routing
errors from send_verified_hook_forward_request and send_hook_forward_request
through handle_hook_error, while leaving successful response handling unchanged.
In `@crates/cli/src/installation/marketplace/assets.rs`:
- Around line 100-107: Update absolute_or_self to return Result<PathBuf, String>
instead of falling back to the relative path when env::current_dir fails.
Propagate the current-directory error to callers generating MCP and hook
configurations, and adjust those call sites to handle or return the failure.
In `@crates/cli/src/installation/marketplace/host.rs`:
- Around line 261-272: Update require_relay to prefer the PATH-resolved
nemo-relay executable so generated integrations retain a stable command path,
using current_executable only when PATH resolution fails. Preserve the existing
dry-run behavior and portable_executable_path processing, while leaving
subsequent capability validation unchanged.
In `@crates/cli/src/mcp/gateway.rs`:
- Around line 175-178: Update GatewayLease cleanup around Drop::drop and the
recovery flow so dropping the lease cooperatively signals shutdown, prevents
further gateway adoption, and waits for any in-flight recover_gateway
work—including spawn_blocking operations—to finish before teardown completes.
Ensure the monitor and blocking recovery share the shutdown state and preserve
normal recovery behavior while the lease is active.
In `@crates/cli/src/mcp/protocol.rs`:
- Around line 27-38: Update response_for to validate the method and request ID
before applying notification suppression: reject missing or non-string method
values and IDs that are neither strings nor integers with a -32600 Invalid
Request response, while preserving null IDs for malformed requests as
appropriate. Only return None for valid JSON-RPC notifications with a valid
method and no ID.
In `@crates/cli/src/plugins/lifecycle/environment.rs`:
- Around line 96-106: Update the entrypoint validation around the
module/callable parsing to reject callable values containing additional ':'
separators, while preserving valid module:function entries and the existing
module-segment checks. Ensure inputs such as pkg.module:factory:extra fail
provisioning validation.
- Around line 478-480: Update the tree-digest construction around the digest
update calls to use unambiguous framing like the snapshot digest: encode the
entry type, raw path bytes, and payload length before the payload, preserving
non-UTF-8 path bytes without lossy conversion. Ensure distinct file trees cannot
produce the same digest before HMAC authentication.
In `@crates/cli/src/plugins/lifecycle/mod.rs`:
- Around line 271-300: Call validate_python_entrypoint_artifact for the loaded
manifest in both plugins validate branches, including the path branch shown and
the plugin-ID branch near the corresponding success handling. Perform the check
after loading the manifest and before reporting success or recording valid
status, propagating any validation error consistently with the existing flow.
In `@crates/cli/src/plugins/mod.rs`:
- Around line 1185-1194: The clear_value_field function must refuse to remove
required fields. After retrieving the selected schema field, check
field.optional and return false without calling remove_value_field when it is
not optional; preserve the existing removal and true return for optional fields.
In `@crates/cli/src/plugins/pricing.rs`:
- Around line 143-156: Update read_pricing_catalog to replace
std::fs::read_to_string with the repository’s bounded regular-file reader,
applying the configured size limit before allocation and preserving the existing
read-error context. Convert the bounded reader’s bytes to UTF-8 explicitly,
mapping invalid UTF-8 to CliError::Config, then pass the resulting string to
PricingCatalog::from_json_str with the existing parse-error context.
In `@crates/cli/src/plugins/schema/mod.rs`:
- Around line 1231-1242: Update resolve_schema_chain and its resolve_fragment
call to resolve fragments against the currently active schema resource, honoring
nested $id boundaries rather than searching the entire root document. Ensure
duplicate anchors are scoped to that resource while preserving cycle and
missing-reference errors, and apply the same resource-aware resolution in the
related reference path around the downstream resolve_fragment usage.
In `@crates/cli/src/process/detached.rs`:
- Around line 216-224: Update the Windows cleanup branch in the detached-process
termination flow to remove the child.kill() fallback after taskkill fails. Use
the existing Windows Job Object termination mechanism if available, or
propagate/report the cleanup failure instead of attempting direct-child
termination, preserving complete process-tree cleanup.
- Around line 80-106: Update spawn_detached to use an explicit Windows handle
allowlist through PROC_THREAD_ATTRIBUTE_HANDLE_LIST with spawn_with_attributes
or equivalent CreateProcessW setup. Pass only the intended standard handles to
the child, replacing the current HandleInheritanceGuard::suppress approach while
preserving the existing spawn, restoration, and cleanup error handling.
In `@crates/cli/src/process/launcher.rs`:
- Around line 635-654: Redact credentials before formatting exporter
destinations in the destination-building logic around the OpenTelemetry and
OpenInference branches. Reuse or add a URL-sanitization helper that removes
userinfo and sensitive query values from ATIF HTTP and OTLP endpoints, then use
the sanitized value in both the automatic TTY banner and --print output while
preserving the existing fallback text.
In `@crates/cli/src/server/mod.rs`:
- Around line 98-107: Update the AddrInUse error message in the server launch
error path to remove the broad `pkill -f nemo-relay` command. Replace it with
guidance to use managed shutdown or identify and terminate only the owning
daemon PID, while preserving the existing Windows and free-port alternatives.
- Around line 239-241: Update serve_listener_with_dynamic_inner() to read and
remove NEMO_RELAY_BOOTSTRAP_SHUTDOWN_TOKEN from the environment before
PluginActivation::initialize() can spawn worker plugins, then pass the captured
token exclusively into bootstrap_shutdown_channel() or its bootstrap shutdown
handling path. Preserve the existing plugin activation flow while ensuring
workers cannot inherit the token.
In `@crates/cli/src/sessions/correlation.rs`:
- Around line 320-352: The single_active_session_id fallback must not equate one
retained session with one active session. Update single_active_session_id to
select the sole session only when it is currently active or recent according to
the existing Session state/timestamps; otherwise return None so
gateway_session_for_call isolates headerless requests through its unique-root
path. Preserve explicit session_id handling and the empty-session behavior.
- Around line 201-210: Remove "subagent_start" from the statuses accepted by
owner_status_teaches_request_affinity. Preserve affinity teaching for explicit,
hint-based, active-subagent, and request-affinity statuses so the weak guess
from Session::set_last_subagent_start_owner is not persisted.
In `@crates/cli/src/sessions/idle.rs`:
- Around line 43-95: Prevent idle-session cleanup from overwriting or clearing a
newer session created with the same ID while close_turn_for_reason runs. Update
the idle-session removal and reinsertion flow around the sessions map and
retained_sessions so each removed session remains exclusively owned, or use a
tombstone/generation check before reinserting and applying
clear_for_ended_subagent. Preserve cleanup for the original session while
skipping stale retention and alignment cleanup when a newer live session exists.
In `@crates/cli/tests/architecture_tests.rs`:
- Around line 47-124: The architecture tests currently rely on raw source-text
matching, allowing grouped imports through and producing false positives from
comments. Update shared-services, clap-syntax, and agent-dependency checks to
parse or tokenize Rust imports and attributes, correctly detecting grouped forms
such as crate and agent imports while ignoring comments. Extend the tests to
cover these legal grouped-import cases and the promised boundary behavior.
In `@crates/cli/tests/cli_tests.rs`:
- Around line 3199-3207: Update the startup-timeout cleanup in the PID-file wait
loop to terminate Relay through its registered signal handler instead of calling
relay.kill(), which sends SIGKILL. Send the graceful termination signal to
Relay, then wait for it to exit so its agent and sleep descendants are cleaned
up before the test panics.
In `@crates/cli/tests/coverage/agents/hermes_tests.rs`:
- Around line 603-642: Extend the reinstallation assertions around first_command
to verify that the old hook command occurs zero times in the updated
hooks["on_session_start"] array. Keep the existing assertion for exactly one
second_command, ensuring the lifecycle test validates removal of the previous
generation command.
In `@crates/cli/tests/coverage/agents/launcher_tests.rs`:
- Around line 850-883: Update
concurrent_hermes_runs_use_independent_overlays_without_mutating_user_config to
construct both PreparedAgentLaunch instances concurrently, using separate
threads synchronized by a barrier so both calls begin together. Join the
threads, collect their results, then assert the generated overlays are distinct
and the original user configuration remains unchanged.
- Around line 1512-1524: Update dry_run_does_not_spawn_agent so the nonexistent
path is used as the inferred executable rather than being appended to the
default Codex command. Configure the command inputs accordingly while preserving
dry_run: true, ensuring the test fails deterministically without launching a
real Codex installation.
In `@crates/cli/tests/coverage/agents/plugin_install_tests.rs`:
- Around line 3663-3697: Correct failed staging rollback so the preexisting
generation-lock symlink remains dangling and its external target is neither
followed nor created. Update the cleanup logic used by
stage_plugin_marketplace_at, then adjust
failed_staging_preserves_a_preexisting_dangling_generation_lock_symlink to
assert the symlink remains while symlink_target stays absent, preserving the
existing staging-error and temporary-directory cleanup assertions.
In `@crates/cli/tests/coverage/shared/bootstrap_state_tests.rs`:
- Around line 225-266: Bound all mock network-server operations to prevent
indefinite test hangs: in
crates/cli/tests/coverage/shared/bootstrap_state_tests.rs:225-266, add deadlines
to both expected listener accepts and request reads; in
crates/cli/tests/coverage/shared/bootstrap_tests.rs:118-181, replace assumed
connection counts with bounded accepts; in
crates/cli/tests/coverage/shared/doctor_tests.rs:13-45, add accept and read
timeouts to the shared capture server; and in
crates/cli/tests/coverage/shared/doctor_tests.rs:1294-1454, apply the same
bounded-server helper to the standalone probe servers.
In `@crates/cli/tests/coverage/shared/config_tests.rs`:
- Around line 480-522: Replace the permission-mode-based unreadable-file setup
in crates/cli/tests/coverage/shared/config_tests.rs:480-522 with a deterministic
configuration-read failure, or explicitly skip the permission-only assertions
when running as root. Apply the same bounded-reader failure injection or root
skip to the permission assertion at
crates/cli/tests/coverage/shared/config_tests.rs:1765-1787, preserving checks
that errors include the relevant source path.
In `@crates/cli/tests/coverage/shared/install_generation_tests.rs`:
- Around line 393-423: Add crate::test_support::CwdTestScope::locked() at the
start of relative_legacy_marker_reencodes_an_absolute_lock_for_rollback and
retain the guard for the entire test, covering all relative-path operations and
InstallGeneration::capture.
In `@crates/cli/tests/coverage/shared/mcp_gateway_tests.rs`:
- Around line 27-55: Ensure maintain_gateway_with is polled once before
advancing simulated time in
production_heartbeat_recovers_after_one_thirty_second_interval. Add a startup
synchronization barrier or await tokio::task::yield_now() immediately after
spawning the monitor and before advance(Duration::from_secs(30)), preserving the
test’s verification that recovery occurs after the first interval.
🪄 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: 10663960-dfd4-4677-9c2f-752bfbc35a56
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (188)
.claude-plugin/marketplace.json.config/nextest.tomlATTRIBUTIONS-Rust.mdcodecov.ymlcrates/cli/Cargo.tomlcrates/cli/src/agents/claude/adapter.rscrates/cli/src/agents/claude/alignment.rscrates/cli/src/agents/claude/assets.rscrates/cli/src/agents/claude/doctor.rscrates/cli/src/agents/claude/host.rscrates/cli/src/agents/claude/install.rscrates/cli/src/agents/claude/launch.rscrates/cli/src/agents/claude/mod.rscrates/cli/src/agents/codex/adapter.rscrates/cli/src/agents/codex/alignment.rscrates/cli/src/agents/codex/app_server.rscrates/cli/src/agents/codex/assets.rscrates/cli/src/agents/codex/doctor.rscrates/cli/src/agents/codex/host.rscrates/cli/src/agents/codex/install.rscrates/cli/src/agents/codex/launch.rscrates/cli/src/agents/codex/mod.rscrates/cli/src/agents/hermes/adapter.rscrates/cli/src/agents/hermes/alignment.rscrates/cli/src/agents/hermes/config.rscrates/cli/src/agents/hermes/doctor.rscrates/cli/src/agents/hermes/files.rscrates/cli/src/agents/hermes/install.rscrates/cli/src/agents/hermes/integration.rscrates/cli/src/agents/hermes/launch.rscrates/cli/src/agents/hermes/mod.rscrates/cli/src/agents/hermes/trust.rscrates/cli/src/agents/mod.rscrates/cli/src/agents/shared/adapters.rscrates/cli/src/agents/shared/alignment.rscrates/cli/src/agents/shared/host.rscrates/cli/src/agents/shared/mod.rscrates/cli/src/banner.rscrates/cli/src/bootstrap/mod.rscrates/cli/src/bootstrap/state.rscrates/cli/src/commands/completions/install.rscrates/cli/src/commands/completions/mod.rscrates/cli/src/commands/configure/mod.rscrates/cli/src/commands/configure/model.rscrates/cli/src/commands/configure/wizard.rscrates/cli/src/commands/diagnostics.rscrates/cli/src/commands/hook_forward.rscrates/cli/src/commands/install.rscrates/cli/src/commands/mcp.rscrates/cli/src/commands/mod.rscrates/cli/src/commands/model_pricing/mod.rscrates/cli/src/commands/plugins/mod.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/root.rscrates/cli/src/commands/run.rscrates/cli/src/commands/serve.rscrates/cli/src/config.rscrates/cli/src/configuration/mod.rscrates/cli/src/configuration/types.rscrates/cli/src/diagnostics/environment.rscrates/cli/src/diagnostics/mod.rscrates/cli/src/diagnostics/model.rscrates/cli/src/diagnostics/probes.rscrates/cli/src/diagnostics/render.rscrates/cli/src/events/json_path.rscrates/cli/src/events/mod.rscrates/cli/src/filesystem/atomic.rscrates/cli/src/filesystem/bounded.rscrates/cli/src/filesystem/locks.rscrates/cli/src/filesystem/mod.rscrates/cli/src/filesystem/snapshots.rscrates/cli/src/filesystem/temp.rscrates/cli/src/gateway/client.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/src/gateway/response.rscrates/cli/src/gateway/routes.rscrates/cli/src/hooks/delivery.rscrates/cli/src/hooks/destination.rscrates/cli/src/hooks/encoding.rscrates/cli/src/hooks/merging.rscrates/cli/src/hooks/mod.rscrates/cli/src/hooks/response.rscrates/cli/src/hooks/types.rscrates/cli/src/installation/generation.rscrates/cli/src/installation/marketplace/assets.rscrates/cli/src/installation/marketplace/host.rscrates/cli/src/installation/marketplace/mod.rscrates/cli/src/installation/marketplace/setup.rscrates/cli/src/installation/marketplace/spec.rscrates/cli/src/installation/marketplace/state.rscrates/cli/src/installation/mod.rscrates/cli/src/installation/operation_lock.rscrates/cli/src/installer.rscrates/cli/src/lib.rscrates/cli/src/main.rscrates/cli/src/mcp/gateway.rscrates/cli/src/mcp/mod.rscrates/cli/src/mcp/protocol.rscrates/cli/src/mcp/session.rscrates/cli/src/mcp/transport.rscrates/cli/src/mcp_environment.rscrates/cli/src/plugin_install/marketplace.rscrates/cli/src/plugin_install/mod.rscrates/cli/src/plugin_install/setup.rscrates/cli/src/plugin_shim/claude.rscrates/cli/src/plugin_shim/codex.rscrates/cli/src/plugin_shim/command.rscrates/cli/src/plugin_shim/mod.rscrates/cli/src/plugin_shim/shared.rscrates/cli/src/plugins/config_io.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/lifecycle.rscrates/cli/src/plugins/lifecycle/environment.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/plugins/lifecycle/render.rscrates/cli/src/plugins/lifecycle/responses.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/target.rscrates/cli/src/plugins/lifecycle/trust.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/pricing.rscrates/cli/src/plugins/schema/mod.rscrates/cli/src/plugins/schema/secrets.rscrates/cli/src/plugins/types.rscrates/cli/src/process/detached.rscrates/cli/src/process/launcher.rscrates/cli/src/process/mod.rscrates/cli/src/process/prepared.rscrates/cli/src/process/supervision.rscrates/cli/src/process/supervision/fallback.rscrates/cli/src/process/supervision/unix.rscrates/cli/src/process/supervision/windows.rscrates/cli/src/process/types.rscrates/cli/src/server/mod.rscrates/cli/src/server/types.rscrates/cli/src/sessions/correlation.rscrates/cli/src/sessions/idle.rscrates/cli/src/sessions/mod.rscrates/cli/src/sessions/routing.rscrates/cli/src/sessions/types.rscrates/cli/tests/architecture_tests.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/agents/adapters_tests.rscrates/cli/tests/coverage/agents/alignment_claude_code_tests.rscrates/cli/tests/coverage/agents/alignment_codex_tests.rscrates/cli/tests/coverage/agents/alignment_tests.rscrates/cli/tests/coverage/agents/coding_agent_tests.rscrates/cli/tests/coverage/agents/hermes_tests.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/tests/coverage/agents/plugin_install_setup_tests.rscrates/cli/tests/coverage/agents/plugin_install_tests.rscrates/cli/tests/coverage/commands/main_tests.rscrates/cli/tests/coverage/commands/model_pricing_tests.rscrates/cli/tests/coverage/installer_tests.rscrates/cli/tests/coverage/plugin_install_tests.rscrates/cli/tests/coverage/plugin_shim_tests.rscrates/cli/tests/coverage/shared/agent_process_tests.rscrates/cli/tests/coverage/shared/banner_tests.rscrates/cli/tests/coverage/shared/bootstrap_state_tests.rscrates/cli/tests/coverage/shared/bootstrap_tests.rscrates/cli/tests/coverage/shared/completions_install_tests.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/tests/coverage/shared/file_io_tests.rscrates/cli/tests/coverage/shared/gateway_client_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/hook_assertions.rscrates/cli/tests/coverage/shared/install_generation_tests.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/shared/mcp_gateway_tests.rscrates/cli/tests/coverage/shared/mcp_tests.rscrates/cli/tests/coverage/shared/plugins_lifecycle_target_tests.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/tests/fixtures/windows_hook_relay.rsintegrations/coding-agents/claude-code/.claude-plugin/plugin.jsonintegrations/coding-agents/claude-code/.mcp.jsonintegrations/coding-agents/claude-code/hooks/hooks.jsonintegrations/coding-agents/codex/.codex-plugin/plugin.jsonintegrations/coding-agents/codex/.mcp.jsonintegrations/coding-agents/codex/hooks/hooks.jsonjustfile
💤 Files with no reviewable changes (1)
- crates/cli/src/config.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (47)
crates/cli/src/agents/claude/host.rs (1)
164-170: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Preserve an originally absent settings file.
Writing
{}as the backup loses the fact thatsettings.jsondid not exist.restore_claude_providerthen leaves a new empty file after uninstall. Store absence explicitly and remove the settings file during restore when Relay created it.🤖 Prompt for 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. In `@crates/cli/src/agents/claude/host.rs` around lines 164 - 170, Update the backup and restore flow around restore_claude_provider so an originally absent settings.json is represented explicitly rather than by writing an empty JSON object. Record the absence in the backup state, and during restore remove the settings file when that marker indicates Relay created it; preserve existing file contents and restoration behavior otherwise.crates/cli/src/agents/codex/host.rs (1)
727-728: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Read the client token from both valid TOML table forms.
codex_provider_headersupports inline and regular tables, butcodex_provider_client_tokenonly accepts inline tables. Reformattinghttp_headersas a regular table makes installation and ownership detection lose a valid token, potentially preserving generated Relay state in the uninstall backup.Proposed fix
pub(crate) fn codex_provider_client_token(doc: &DocumentMut) -> Option<&str> { - doc.get("model_providers") - .and_then(Item::as_table) - .and_then(|providers| providers.get("nemo-relay-openai")) - .and_then(Item::as_table) - .and_then(|provider| provider.get("http_headers")) - .and_then(Item::as_inline_table) - .and_then(|headers| headers.get(BOOTSTRAP_CLIENT_TOKEN_HEADER)) + codex_provider_header(doc, BOOTSTRAP_CLIENT_TOKEN_HEADER) .and_then(TomlValue::as_str) }Also applies to: 1528-1554
🤖 Prompt for 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. In `@crates/cli/src/agents/codex/host.rs` around lines 727 - 728, Update codex_provider_client_token and its callers, including the installation/ownership detection path around has_managed_proof, to read client tokens from both inline and regular TOML table forms, matching codex_provider_header behavior. Preserve existing token validation while ensuring regular-table http_headers still detects the valid token and handles Relay-state backup correctly.crates/cli/src/agents/hermes/config.rs (1)
192-220: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use durable ownership evidence for partial Hermes state.
Ownership currently depends on complete configuration or a previously known exact command, so damaged or orphaned Relay state can survive repair and uninstall.
crates/cli/src/agents/hermes/config.rs#L192-L220: Recognize modern ownership from the Relay MCP command, arguments, and generation fence; validate hook completeness separately.crates/cli/src/agents/hermes/integration.rs#L146-L177: Add constrained recognition for orphaned Relay approvals or persist an explicit ownership marker.📍 Affects 2 files
crates/cli/src/agents/hermes/config.rs#L192-L220(this comment)crates/cli/src/agents/hermes/integration.rs#L146-L177🤖 Prompt for 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. In `@crates/cli/src/agents/hermes/config.rs` around lines 192 - 220, Update crates/cli/src/agents/hermes/config.rs lines 192-220 so modern ownership is recognized from the Relay MCP command, expected arguments, and generation fence without requiring a complete hook set; keep hook completeness as a separate validation result. In crates/cli/src/agents/hermes/integration.rs lines 146-177, add constrained recognition for orphaned Relay approvals or persist an explicit ownership marker so damaged or orphaned state can be repaired or uninstalled.crates/cli/src/agents/hermes/integration.rs (1)
232-245: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate forwarded environment values, not only names.
Line 239 accepts any value for a required environment key. Doctor therefore reports success when
${NAME}has been replaced by a stale or fixed value, even though MCP forwarding is broken. Compare each entry with the exact placeholder generated byexpected_mcp_server.Proposed Fix
- let missing = forwarded_environment_names(&environment, plugin_config.as_ref()) + let invalid = forwarded_environment_names(&environment, plugin_config.as_ref()) .into_iter() - .filter(|name| !mcp_env.contains_key(name)) + .filter(|name| { + let expected = format!("${{{name}}}"); + mcp_env.get(name.as_str()).and_then(Value::as_str) != Some(expected.as_str()) + }) .collect::<Vec<_>>(); - if !missing.is_empty() { + if !invalid.is_empty() { return Err(format!( - "Hermes Relay MCP is missing environment names {}; run `nemo-relay install hermes --force`", - missing.join(", ") + "Hermes Relay MCP has missing or invalid environment entries {}; run `nemo-relay install hermes --force`", + invalid.join(", ") ));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let plugin_config = crate::configuration::user_plugin_runtime_config().map_err(|e| e.to_string())?; let environment = env::vars_os() .filter_map(|(name, _)| name.into_string().ok()) .collect::<Vec<_>>(); let invalid = forwarded_environment_names(&environment, plugin_config.as_ref()) .into_iter() .filter(|name| { let expected = format!("${{{name}}}"); mcp_env.get(name.as_str()).and_then(Value::as_str) != Some(expected.as_str()) }) .collect::<Vec<_>>(); if !invalid.is_empty() { return Err(format!( "Hermes Relay MCP has missing or invalid environment entries {}; run `nemo-relay install hermes --force`", invalid.join(", ") ));🤖 Prompt for 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. In `@crates/cli/src/agents/hermes/integration.rs` around lines 232 - 245, Update the validation around forwarded_environment_names to verify values as well as keys: for each required environment name, compare mcp_env’s entry with the exact placeholder generated by expected_mcp_server, and include entries that are missing or mismatched in the existing error path. Preserve the current success behavior only when every required key has the expected placeholder value.crates/cli/src/bootstrap/state.rs (1)
55-60: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Accept authenticated owners from compatible CLI versions.
The exact package-version check makes gateway refresh fail during upgrades before the authenticated probe and shutdown run. Use
bootstrap_protocolfor compatibility; retainversionas informational metadata.Proposed fix
fn valid_for(&self, url: &str) -> bool { self.service == "nemo-relay" - && self.version == env!("CARGO_PKG_VERSION") && self.bootstrap_protocol == BOOTSTRAP_PROTOCOL_VERSION && self.url == url && !self.shutdown_token.is_empty() }The marketplace
refresh_gatewaypath callsstop_owned_and_reset, making this reachable during installation and upgrades.Also applies to: 214-229
🤖 Prompt for 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. In `@crates/cli/src/bootstrap/state.rs` around lines 55 - 60, Update State::valid_for to remove the exact CARGO_PKG_VERSION comparison and rely on bootstrap_protocol compatibility, while retaining the version field as informational metadata. Preserve the service, URL, and non-empty shutdown_token checks so authenticated owners from compatible CLI versions are accepted during refresh.crates/cli/src/commands/configure/mod.rs (1)
20-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Support resetting global configuration.
The wizard can save global or both scopes, but
model::resetonly examines./.nemo-relay/config.toml. A globally configured user therefore gets “No project config” and remains configured. Add an explicit scope option or clear the applicable project and global files, with tests for each scope.🤖 Prompt for 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. In `@crates/cli/src/commands/configure/mod.rs` around lines 20 - 29, Update the reset flow in execute and model::reset to support the global scope as well as the project scope, using an explicit scope option or clearing both applicable configuration files when requested. Preserve agent filtering and existing project-reset behavior, and add tests covering project, global, and both-scope resets.crates/cli/src/commands/install.rs (1)
99-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Propagate Agent-Specific Exit Codes
Both loops discard a successful non-zero
ExitCode, causingnemo-relay install alloruninstall allto report success after an agent-specific operation reports failure. Preserve or aggregate non-success statuses.
crates/cli/src/commands/install.rs#L99-L101: capture and return or aggregate the result frominstall_integration.crates/cli/src/commands/install.rs#L119-L121: capture and return or aggregate the result fromuninstall_integration.📍 Affects 1 file
crates/cli/src/commands/install.rs#L99-L101(this comment)crates/cli/src/commands/install.rs#L119-L121🤖 Prompt for 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. In `@crates/cli/src/commands/install.rs` around lines 99 - 101, Preserve non-success exit statuses in both agent loops: update install_integration at crates/cli/src/commands/install.rs:99-101 and uninstall_integration at crates/cli/src/commands/install.rs:119-121 to capture and return or aggregate each ExitCode instead of discarding it, so install all and uninstall all report agent-specific failures.crates/cli/src/commands/run.rs (1)
76-86: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not run default setup for a missing explicit config path.
configure::runcannot receive or createexplicit_config, yet the launcher still receives that missing path afterward. A typo in--configtherefore launches an unrelated setup flow and then fails anyway.Proposed fix
- let needs_setup = explicit_config.map_or_else( - || !crate::configuration::any_config_file_exists(), - |path| !path.exists(), - ); + let needs_setup = + explicit_config.is_none() && !crate::configuration::any_config_file_exists();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let explicit_config = inherited.config.as_deref(); let needs_setup = explicit_config.is_none() && !crate::configuration::any_config_file_exists(); if needs_setup { super::configure::run(Some(agent)).await?; } let runtime = crate::process::RunOverrides { agent: Some(agent), config: explicit_config.map(PathBuf::from),🤖 Prompt for 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. In `@crates/cli/src/commands/run.rs` around lines 76 - 86, Update the setup decision in the run command to invoke configure::run only when no explicit config path was supplied and no default config exists; when explicit_config points to a missing path, skip default setup and preserve that path for the subsequent RunOverrides construction.crates/cli/src/configuration/mod.rs (1)
151-159: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require the bootstrap fingerprint for every managed child.
A supplied
ready_filemarks this as managed bootstrap, but a missing fingerprint silently returnsNoneand bypasses identity verification. Fail closed instead.Proposed fix
- let Some(expected) = env::var(BOOTSTRAP_FINGERPRINT_ENV) + let expected = env::var(BOOTSTRAP_FINGERPRINT_ENV) .ok() .filter(|fingerprint| !fingerprint.is_empty()) - else { - return Ok(None); - }; + .ok_or_else(|| { + CliError::Config( + "managed bootstrap requires a non-empty bootstrap fingerprint".into(), + ) + })?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if args.ready_file.is_none() { return Ok(None); } let expected = env::var(BOOTSTRAP_FINGERPRINT_ENV) .ok() .filter(|fingerprint| !fingerprint.is_empty()) .ok_or_else(|| { CliError::Config( "managed bootstrap requires a non-empty bootstrap fingerprint".into(), ) })?;🤖 Prompt for 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. In `@crates/cli/src/configuration/mod.rs` around lines 151 - 159, Update the managed-bootstrap handling around args.ready_file and BOOTSTRAP_FINGERPRINT_ENV so a supplied ready_file requires a non-empty bootstrap fingerprint; return an error instead of Ok(None) when the fingerprint is missing or empty, while preserving Ok(None) for unmanaged children without a ready_file.crates/cli/src/diagnostics/environment.rs (1)
8-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle Windows diagnostics without Unix-only assumptions.
Line 10 renders
windowsbecauseos_versionis always empty there, while Lines 12-16 only inspectSHELL. Omit the separator when no version is available and useCOMSPECon Windows so doctor reports remain accurate.🤖 Prompt for 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. In `@crates/cli/src/diagnostics/environment.rs` around lines 8 - 27, Update collect_environment and os_version so the OS string omits the separator when no version is available instead of producing “windows ”. Select the shell variable by platform, using COMSPEC on Windows and SHELL elsewhere, while preserving basename extraction and the existing Unix version lookup.crates/cli/src/diagnostics/probes.rs (1)
29-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -nP -C5 '\bcheck_dir_writable\s*\(|\bcheck_directory\s*\(' \ crates/cli/src crates/cli/testsRepository: NVIDIA/NeMo-Relay
Length of output: 5133
🏁 Script executed:
sed -n '1,120p' crates/cli/src/diagnostics/probes.rs printf '\n--- tests ---\n' sed -n '400,430p' crates/cli/tests/coverage/shared/doctor_tests.rs printf '\n--- more tests ---\n' sed -n '960,980p' crates/cli/tests/coverage/shared/doctor_tests.rsRepository: NVIDIA/NeMo-Relay
Length of output: 4290
Probe actual directory writability.
permissions().readonly()only inspects metadata, so it can pass even when this process cannot create entries there (ACLs, ownership, mount options, platform attributes). Use a temporarycreate_new(true)file and clean it up afterward.🤖 Prompt for 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. In `@crates/cli/src/diagnostics/probes.rs` around lines 29 - 43, Update check_dir_writable to verify actual write access by creating a uniquely named temporary file in the validated directory with create_new(true), then remove that file before returning success. Preserve the existing directory validation and propagate creation or cleanup I/O errors appropriately; do not rely on metadata.permissions().readonly().crates/cli/src/diagnostics/render.rs (1)
214-218: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render each check's status marker.
These checks affect the exit code and conclusion, but the human output discards
check.status, leaving users unable to identify failures or warnings.Proposed fix
- out.push_str(&format!(" {:<22} {}\n", check.name, check.details)); + out.push_str(&format!( + " {} {:<22} {}\n", + format_status(check.status), + check.name, + check.details + )); ... - out.push_str(&format!(" {}\n", check.details)); + out.push_str(&format!( + " {} {}\n", + format_status(check.status), + check.details + ));Also applies to: 222-226
🤖 Prompt for 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. In `@crates/cli/src/diagnostics/render.rs` around lines 214 - 218, Update format_human_checks to include each Check’s status marker in the rendered line, using the existing status representation and preserving the current name/details alignment. Ensure both human-check output paths covered by this function expose whether each check passed, warned, or failed.crates/cli/src/filesystem/bounded.rs (1)
22-89: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the gateway-specific suffix from this generic reader.
Every error says
for persistent gateway identity, including plugin manifest, artifact, signature, and runtime-closure reads. Build the message solely fromdescriptionso trust failures report the actual operation.🤖 Prompt for 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. In `@crates/cli/src/filesystem/bounded.rs` around lines 22 - 89, Update the generic bounded-file reader’s error messages to remove the hardcoded “for persistent gateway identity” suffix and construct them solely from the provided description. Apply this consistently to the metadata, open, and read error paths while preserving the existing path and error details.crates/cli/src/filesystem/temp.rs (1)
8-25: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files crates/cli/src/filesystem printf '\n--- outline temp.rs ---\n' ast-grep outline crates/cli/src/filesystem/temp.rs --view expanded printf '\n--- search protect_private_windows_path ---\n' rg -n "protect_private_windows_path|private_system_temp_dir|private_temp_dir|DirBuilderExt|mode\\(" crates/cli/src -S printf '\n--- maybe related filesystem helpers ---\n' rg -n "windows.*private|private.*windows|DACL|ACL" crates/cli/src -SRepository: NVIDIA/NeMo-Relay
Length of output: 6503
🏁 Script executed:
sed -n '1,140p' crates/cli/src/agents/hermes/launch.rs printf '\n--- atomic.rs relevant section ---\n' sed -n '173,250p' crates/cli/src/filesystem/atomic.rs printf '\n--- configuration usage ---\n' sed -n '660,725p' crates/cli/src/configuration/mod.rs printf '\n--- temp.rs ---\n' sed -n '1,120p' crates/cli/src/filesystem/temp.rsRepository: NVIDIA/NeMo-Relay
Length of output: 10937
Harden Windows temp directories before returning them.
private_temp_dir()only sets0o700on Unix; on Windows it returns a newly created directory with the inherited ACL. This can expose Hermes overlay contents if the parent ACL is broad. Applyprotect_private_windows_path()aftercreate(), and remove the directory if hardening fails.🤖 Prompt for 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. In `@crates/cli/src/filesystem/temp.rs` around lines 8 - 25, Update private_temp_dir to call protect_private_windows_path after builder.create(&path) succeeds, applying it only on Windows as appropriate. If hardening fails, remove the newly created directory before returning the error; otherwise preserve the existing successful path return behavior.crates/cli/src/gateway/client.rs (2)
77-90: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate resolved addresses instead of trusting the
localhosthostname.
parse_loopback_urlacceptslocalhost, but both callers use the first resolver result without verifyingaddress.ip().is_loopback(). A modified resolver entry can send the fingerprint—and the shutdown token—to a remote service. It also causes avoidable dual-stack failures.Filter all results to loopback addresses and attempt each until one connects.
Also applies to: 291-309, 472-497
🤖 Prompt for 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. In `@crates/cli/src/gateway/client.rs` around lines 77 - 90, Update the address resolution and connection logic in parse_loopback_url and the corresponding callers around the shutdown-token and fingerprint connections: filter resolved addresses with address.ip().is_loopback(), then attempt TcpStream::connect_timeout for each remaining address until one succeeds. Do not use only the first resolver result; return the existing connection error when no loopback address connects, while preserving the current rejection behavior when resolution yields no valid loopback addresses.
120-180: 🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
The bootstrap proof does not authenticate this TCP peer.
The proof covers only the fingerprint and nonce. A foreign listener can forward that challenge to another genuine Relay using the same per-user identity, replay its proof, and then receive the plaintext POST. Reusing the connection does not prevent challenge relaying.
Establish an authenticated encrypted channel, such as TLS with a pinned per-user identity, before transmitting hook payloads.
🤖 Prompt for 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. In `@crates/cli/src/gateway/client.rs` around lines 120 - 180, Update the gateway connection flow surrounding the bootstrap challenge and verified POST so it establishes an authenticated encrypted channel before sending any hook payload. Use TLS with the appropriate pinned per-user Relay identity, perform certificate/peer validation before transmitting the request, and preserve the existing bootstrap health verification as an additional check rather than relying on it to authenticate the TCP peer.crates/cli/src/gateway/response.rs (1)
56-67: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Strip every header nominated by
Connection.The current fixed-name filter removes
Connectionitself but forwards extension headers it names, allowing hop-specific or sensitive metadata to cross the proxy boundary.
crates/cli/src/gateway/response.rs#L56-L67: derive request exclusions from the complete header map and itsConnectiontokens.crates/cli/src/gateway/response.rs#L29-L35: apply the same map-aware filtering to upstream responses.crates/cli/src/gateway/mod.rs#L649-L659: use the map-aware request filter for general forwarding.crates/cli/src/gateway/mod.rs#L857-L867: use it for/modelsforwarding as well.📍 Affects 2 files
crates/cli/src/gateway/response.rs#L56-L67(this comment)crates/cli/src/gateway/response.rs#L29-L35crates/cli/src/gateway/mod.rs#L649-L659crates/cli/src/gateway/mod.rs#L857-L867🤖 Prompt for 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. In `@crates/cli/src/gateway/response.rs` around lines 56 - 67, Make header filtering map-aware so every header named by the Connection header’s tokens is excluded, in addition to the existing hop-by-hop and sensitive-header rules. Update should_forward_request_header and the response filtering logic in crates/cli/src/gateway/response.rs lines 56-67 and 29-35 to receive the complete header map, then update both forwarding call sites in crates/cli/src/gateway/mod.rs lines 649-659 and 857-867 to use the new request filter, preserving the existing exclusions including Accept-Encoding.crates/cli/src/hooks/delivery.rs (1)
102-115: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve fail-open behavior for transport setup failures.
Lines 102 and 114 use
?, so header construction, client construction, or task failures abort the hook even whenfail_closedis false. Route these outer errors throughhandle_hook_error.Proposed fix
- let response = - send_verified_hook_forward_request(&command, gateway, &destination.gateway_url, input) - .await?; + let response = match send_verified_hook_forward_request( + &command, + gateway, + &destination.gateway_url, + input, + ) + .await + { + Ok(response) => response, + Err(error) => return handle_hook_error(error, fail_closed), + }; return handle_verified_hook_forward_response(response, fail_closed); @@ - let response = send_hook_forward_request(&command, &url, input).await?; + let response = match send_hook_forward_request(&command, &url, input).await { + Ok(response) => response, + Err(error) => return handle_hook_error(error, fail_closed), + };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if let Some(gateway) = verified_gateway { let response = match send_verified_hook_forward_request( &command, gateway, &destination.gateway_url, input, ) .await { Ok(response) => response, Err(error) => return handle_hook_error(error, fail_closed), }; return handle_verified_hook_forward_response(response, fail_closed); } let url = format!( "{}{}", destination.gateway_url.trim_end_matches('/'), command.agent.hook_path() ); let response = match send_hook_forward_request(&command, &url, input).await { Ok(response) => response, Err(error) => return handle_hook_error(error, fail_closed), }; handle_hook_forward_response(response, fail_closed).await🤖 Prompt for 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. In `@crates/cli/src/hooks/delivery.rs` around lines 102 - 115, Update both forwarding branches in the hook delivery flow to handle transport setup or task errors through handle_hook_error instead of propagating them with ?. Preserve the existing fail_closed behavior by routing errors from send_verified_hook_forward_request and send_hook_forward_request through handle_hook_error, while leaving successful response handling unchanged.crates/cli/src/installation/marketplace/assets.rs (1)
100-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Propagate failure instead of emitting a relative lifecycle path.
If
current_dirfails, the generated MCP and hook configurations retain a relative generation-fence path. The host can later launch them from another directory, making a successful installation unusable. ReturnResult<PathBuf, String>and propagate the error.🤖 Prompt for 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. In `@crates/cli/src/installation/marketplace/assets.rs` around lines 100 - 107, Update absolute_or_self to return Result<PathBuf, String> instead of falling back to the relative path when env::current_dir fails. Propagate the current-directory error to callers generating MCP and hook configurations, and adjust those call sites to handle or return the failure.crates/cli/src/installation/marketplace/host.rs (1)
261-272: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve a stable Relay executable path for generated integrations.
current_exefollowed bycanonicalizepins hooks and MCP configuration to the physical binary. Versioned package-manager targets can disappear after an upgrade, breaking existing integrations. Prefer the PATH-resolvednemo-relaypath, withcurrent_executableas fallback; the subsequent capability checks still reject an incompatible binary.Proposed fix
if options.dry_run { return Ok(PathBuf::from(RELAY_COMMAND)); } + if let Some(path) = runner.resolve_executable(RELAY_COMMAND)? { + return Ok(crate::process::portable_executable_path(path)); + } runner .current_executable() .map(|path| path.canonicalize().unwrap_or(path)) .map(crate::process::portable_executable_path)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub(crate) fn require_relay( options: &PluginInstallOptions, runner: &dyn CommandRunner, ) -> Result<PathBuf, String> { if options.dry_run { return Ok(PathBuf::from(RELAY_COMMAND)); } if let Some(path) = runner.resolve_executable(RELAY_COMMAND)? { return Ok(crate::process::portable_executable_path(path)); } runner .current_executable() .map(|path| path.canonicalize().unwrap_or(path)) .map(crate::process::portable_executable_path) }🤖 Prompt for 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. In `@crates/cli/src/installation/marketplace/host.rs` around lines 261 - 272, Update require_relay to prefer the PATH-resolved nemo-relay executable so generated integrations retain a stable command path, using current_executable only when PATH resolution fails. Preserve the existing dry-run behavior and portable_executable_path processing, while leaving subsequent capability validation unchanged.crates/cli/src/mcp/gateway.rs (1)
175-178: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Do Tokio 1.x docs state that an already-running spawn_blocking task cannot be aborted?💡 Result:
Yes, the official Tokio 1.x documentation explicitly states that tasks spawned using spawn_blocking cannot be aborted once they have started running [1][2][3]. Because spawn_blocking tasks are not asynchronous, they cannot be cancelled via the standard abort mechanism [1][4][3]. If you call abort on a spawn_blocking task that is already running, the call will have no effect, and the task will continue to execute normally [1][5][6]. The only exception is if the task has not yet started running; in that case, calling abort may prevent it from starting, though this is not guaranteed [1][7][6].
Citations:
- 1: https://docs.rs/tokio/1/tokio/task/fn.spawn_blocking.html
- 2: https://docs.rs/tokio/latest/tokio/task/fn.spawn_blocking.html
- 3: https://docs.rs/tokio/latest/tokio/task/fn.spawn%5Fblocking.html
- 4: https://docs.rs/tokio/latest/tokio/task/struct.AbortHandle.html
- 5: https://docs.rs/tokio/latest/tokio/task/struct.JoinHandle.html
- 6: https://docs.rs/tokio/latest/src/tokio/runtime/task/join.rs.html
- 7: https://docs.rs/tokio/1.48.0/tokio/task/fn.spawn_blocking.html
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant file and surrounding lines. FILE="crates/cli/src/mcp/gateway.rs" wc -l "$FILE" sed -n '1,260p' "$FILE"Repository: NVIDIA/NeMo-Relay
Length of output: 9041
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the maintenance loop and recovery call site. FILE="crates/cli/src/mcp/gateway.rs" sed -n '160,260p' "$FILE"Repository: NVIDIA/NeMo-Relay
Length of output: 3082
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the maintenance loop implementation and how the restart callback is awaited. FILE="crates/cli/src/mcp/gateway.rs" sed -n '260,393p' "$FILE"Repository: NVIDIA/NeMo-Relay
Length of output: 4550
Cancel blocking recovery on lease drop
crates/cli/src/mcp/gateway.rs:175-205—abort()only stops the outer monitor task; ifrecover_gateway()has already enteredspawn_blocking, that blocking work keeps running and can still recover/adopt the gateway after MCP stdio is gone. Add cooperative shutdown around recovery so teardown waits for in-flight maintenance to finish.🤖 Prompt for 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. In `@crates/cli/src/mcp/gateway.rs` around lines 175 - 178, Update GatewayLease cleanup around Drop::drop and the recovery flow so dropping the lease cooperatively signals shutdown, prevents further gateway adoption, and waits for any in-flight recover_gateway work—including spawn_blocking operations—to finish before teardown completes. Ensure the monitor and blocking recovery share the shutdown state and preserve normal recovery behavior while the lease is active.crates/cli/src/mcp/protocol.rs (1)
27-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What ID types and notification fields does MCP protocol revision 2025-06-18 require?💡 Result:
The Model Context Protocol (MCP) revision 2025-06-18 defines specific requirements for notification structures and data types [1][2]. Notification Fields In the MCP protocol, notifications are one-way messages and, according to the JSON-RPC standard used by the protocol, MUST NOT include an ID field [3][4]. All notifications share a common structure that allows for optional metadata via the
_metafield [5][6]. This_metaobject is reserved by MCP for additional metadata [5][6]. While not explicitly mandated for every notification type, the protocol supports the use of_metafor cross-cutting concerns [5]. For notifications delivered via specific streams (such assubscriptions/listen), the server is required to includeio.modelcontextprotocol/subscriptionIdwithin the_metafield, allowing the client to correlate the notification with the original request [3][7][4]. ID Types The protocol utilizes JSON-RPC 2.0 for messaging [8][9]. Within this framework, a request ID—when used for standard requests (not notifications)—is defined as either astringor anumber[10][7][11]. While notifications specifically must not include an ID, any request associated with a process that later triggers a notification (such as a subscription request) uses these string or number types for its identifier [9][7]. Summary of Key Requirements - Notifications: MUST NOT include an ID field [3][4]. - Notification Structure: May include an optional_metafield for metadata [5][6]. - Subscription Notifications: MUST includeio.modelcontextprotocol/subscriptionIdin_metaif delivered via a subscription stream [3][4]. - ID Data Type: Where IDs are required (e.g., in standard requests), the type is astringornumber[10][7][11].Citations:
- 1: https://modelcontextprotocol.io/specification/2025-06-18/changelog
- 2: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/ff960c9e/docs/specification/2025-06-18/index.mdx
- 3: https://modelcontextprotocol.io/specification/draft/basic
- 4: https://mcp.mintlify.app/specification/draft/basic
- 5: https://github.com/modelcontextprotocol/specification/blob/main/schema/2025-06-18/schema.ts
- 6: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-06-18/schema.ts
- 7: https://raw.githubusercontent.com/modelcontextprotocol/modelcontextprotocol/main/schema/draft/schema.ts
- 8: https://mcp-staging.mintlify.app/specification/2025-06-18/basic/lifecycle
- 9: https://modelcontextprotocol.io/specification/2025-06-18/basic/utilities/cancellation
- 10: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/draft/schema.ts
- 11: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-03-26/schema.ts
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '== protocol.rs ==\n' sed -n '1,220p' crates/cli/src/mcp/protocol.rs printf '\n== search for response_for usages/tests ==\n' rg -n "response_for\\(|jsonrpc_error\\(|Invalid Request|method" crates/cli/src -g '!target' printf '\n== candidate tests in cli/mcp ==\n' fd -a . crates/cli/src/mcpRepository: NVIDIA/NeMo-Relay
Length of output: 6642
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '== mcp/session.rs ==\n' sed -n '1,240p' crates/cli/src/mcp/session.rs printf '\n== mcp/transport.rs ==\n' sed -n '1,220p' crates/cli/src/mcp/transport.rs printf '\n== mcp/mod.rs ==\n' sed -n '1,220p' crates/cli/src/mcp/mod.rsRepository: NVIDIA/NeMo-Relay
Length of output: 8913
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '== shared/mcp_tests.rs (relevant parts) ==\n' sed -n '1,260p' crates/cli/tests/coverage/shared/mcp_tests.rs printf '\n== search for MCP notification/request shape docs in repo ==\n' rg -n "notification|jsonrpc|protocolVersion|id.*method|method.*id" crates/cli docs README.md -g '!target'Repository: NVIDIA/NeMo-Relay
Length of output: 15731
🌐 Web query:
MCP 2025-06-18 request id must be string or integer and notifications must not include id💡 Result:
According to the Model Context Protocol (MCP) specification version 2025-06-18, the requirements for request IDs and notifications are as follows: Requests Requests must include a unique ID to initiate an operation [1][2]. This ID must be either a string or an integer [1][2][3]. Unlike the base JSON-RPC 2.0 specification, the ID must not be null [1][2]. Furthermore, the request ID must not have been previously used by the requestor within the same session [1][2]. Notifications Notifications are defined as one-way messages that do not expect a response [1][4]. Under the MCP specification, notifications must not include an ID [5][1][4]. It is important to note that while notifications themselves must not contain an ID field in their JSON-RPC structure [1], certain specific notification types, such as the cancellation notification (notifications/cancelled), include a request ID within their params object to reference a specific pending request [6][7].
Citations:
- 1: https://mcp-staging.mintlify.app/specification/2025-06-18/basic
- 2: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/ff960c9e/docs/specification/2025-06-18/basic/index.mdx
- 3: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-06-18/schema.ts
- 4: https://mcp.mintlify.app/specification/2025-11-25/basic
- 5: https://modelcontextprotocol.io/specification/2025-06-18/basic
- 6: https://mcp.mintlify.app/specification/2025-06-18/basic/utilities/cancellation
- 7: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-03-26/schema.ts
Check
methodbefore treating an object as a notification. ID-less frames like{"jsonrpc":"2.0"}or{"jsonrpc":"2.0","method":123}are currently dropped instead of returning-32600, andidis echoed without enforcing MCP’s string/integer request-ID shape. Only suppress responses for valid notifications.🤖 Prompt for 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. In `@crates/cli/src/mcp/protocol.rs` around lines 27 - 38, Update response_for to validate the method and request ID before applying notification suppression: reject missing or non-string method values and IDs that are neither strings nor integers with a -32600 Invalid Request response, while preserving null IDs for malformed requests as appropriate. Only return None for valid JSON-RPC notifications with a valid method and no ID.crates/cli/src/plugins/mod.rs (1)
1185-1194: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent clear from removing required fields.
This path ignores
field.optional, despite the UI promising that clear applies only to optional fields. Required nested fields can consequently be removed with Backspace/Delete.Proposed fix
let Some(field) = schema.fields.get(selected) else { return false; }; + if !field.optional { + return false; + } remove_value_field(value, field.name);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn clear_value_field( value: &mut Value, schema: &nemo_relay::config_editor::EditorSchema, selected: usize, ) -> bool { let Some(field) = schema.fields.get(selected) else { return false; }; if !field.optional { return false; } remove_value_field(value, field.name); true🤖 Prompt for 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. In `@crates/cli/src/plugins/mod.rs` around lines 1185 - 1194, The clear_value_field function must refuse to remove required fields. After retrieving the selected schema field, check field.optional and return false without calling remove_value_field when it is not optional; preserve the existing removal and true return for optional fields.crates/cli/src/plugins/schema/mod.rs (1)
1231-1242: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect coverage for resource-local references and duplicate anchors. fd -a -t f 'plugins_schema_tests.rs' . \ --exec rg -n -C4 '\$id|\$anchor|resource-local|nested resource' {}Repository: NVIDIA/NeMo-Relay
Length of output: 435
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the resolver implementation and related tests. for f in crates/cli/src/plugins/schema/mod.rs crates/cli/tests/plugins_schema_tests.rs; do echo "=== $f ===" wc -l "$f" done echo "=== resolver symbols ===" rg -n 'resolve_fragment|resolve_schema_chain|decode_reference_fragment|\$id|\$anchor' crates/cli/src/plugins/schema/mod.rs crates/cli/tests/plugins_schema_tests.rs echo "=== source slice mod.rs 1180-1325 ===" sed -n '1180,1325p' crates/cli/src/plugins/schema/mod.rs echo "=== tests slice plugins_schema_tests.rs 1-260 ===" sed -n '1,260p' crates/cli/tests/plugins_schema_tests.rsRepository: NVIDIA/NeMo-Relay
Length of output: 363
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== schema-related test files ===" fd -a -t f '.*schema.*test.*\.rs$' crates/cli crates -i echo "=== resolver symbols in mod.rs ===" rg -n 'resolve_fragment|resolve_schema_chain|decode_reference_fragment|\$id|\$anchor|writeOnly|secret' crates/cli/src/plugins/schema/mod.rs echo "=== mod.rs slice 1180-1314 ===" sed -n '1180,1314p' crates/cli/src/plugins/schema/mod.rs echo "=== likely test file candidates with $id/$anchor ===" fd -a -t f '.+\.rs$' crates/cli -i --exec rg -n -C3 '\$id|\$anchor|writeOnly|secret' {}Repository: NVIDIA/NeMo-Relay
Length of output: 9787
🏁 Script executed:
#!/bin/bash set -euo pipefail test_file=crates/cli/tests/coverage/shared/plugins_schema_tests.rs echo "=== anchor/writeOnly references ===" rg -n -C4 '\$anchor|\$id|writeOnly|secret|redact|unwrap' "$test_file" echo "=== focused slices ===" sed -n '120,240p' "$test_file" sed -n '240,420p' "$test_file"Repository: NVIDIA/NeMo-Relay
Length of output: 28370
Resolve fragments within the active schema resource.
resolve_fragmentwalks the whole document, so nested$idboundaries are ignored and duplicate anchors can bind to the wrong schema. That can desync validation from secret discovery and leave awriteOnlyfield unredacted. (crates/cli/src/plugins/schema/mod.rs:1231-1242, 1287-1305)🤖 Prompt for 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. In `@crates/cli/src/plugins/schema/mod.rs` around lines 1231 - 1242, Update resolve_schema_chain and its resolve_fragment call to resolve fragments against the currently active schema resource, honoring nested $id boundaries rather than searching the entire root document. Ensure duplicate anchors are scoped to that resource while preserving cycle and missing-reference errors, and apply the same resource-aware resolution in the related reference path around the downstream resolve_fragment usage.crates/cli/src/process/launcher.rs (1)
635-654: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact credentials from displayed exporter endpoints.
ATIF HTTP and OTLP endpoints are rendered verbatim in both the automatic TTY banner and
Also applies to: 662-665
🤖 Prompt for 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. In `@crates/cli/src/process/launcher.rs` around lines 635 - 654, Redact credentials before formatting exporter destinations in the destination-building logic around the OpenTelemetry and OpenInference branches. Reuse or add a URL-sanitization helper that removes userinfo and sensitive query values from ATIF HTTP and OTLP endpoints, then use the sanitized value in both the automatic TTY banner and --print output while preserving the existing fallback text.crates/cli/src/server/mod.rs (2)
98-107: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove the broad
pkill -f nemo-relayrecovery command.This pattern kills every process whose command line contains
nemo-relay, including active agents and unrelated test runs. Direct users to managed shutdown or owner-PID identification instead.🤖 Prompt for 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. In `@crates/cli/src/server/mod.rs` around lines 98 - 107, Update the AddrInUse error message in the server launch error path to remove the broad `pkill -f nemo-relay` command. Replace it with guidance to use managed shutdown or identify and terminate only the owning daemon PID, while preserving the existing Windows and free-port alternatives.
239-241: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -nP -C5 \ '\b(?:pub(?:\([^)]*\))?\s+)?fn\s+load_worker_plugins\b|\bCommand::new\s*\(|\.clear_env\s*\(|\.env_remove\s*\(|NEMO_RELAY_BOOTSTRAP_SHUTDOWN_TOKEN' \ cratesRepository: NVIDIA/NeMo-Relay
Length of output: 50374
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the server and the cited surrounding code. sed -n '220,260p' crates/cli/src/server/mod.rs printf '\n---\n' sed -n '430,470p' crates/cli/src/server/mod.rs printf '\n---\n' sed -n '700,740p' crates/cli/src/server/mod.rs printf '\n=== plugin worker / plugin activation surfaces ===\n' rg -n -C 3 'PluginActivation::initialize|bootstrap_shutdown_channel|NEMO_RELAY_BOOTSTRAP_SHUTDOWN_TOKEN|env_remove|clear_env|Command::new\(' crates/cli/src crates/adaptive crates/core crates/node crates/python crates/ffiRepository: NVIDIA/NeMo-Relay
Length of output: 18572
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the activation and worker-launch code paths. ast-grep outline crates/core/src/plugin/dynamic/worker.rs --view expanded printf '\n---\n' ast-grep outline crates/cli/src/server/mod.rs --view expanded printf '\n---\n' rg -n -C 4 'struct PluginActivation|impl PluginActivation|async fn initialize|fn initialize\(' crates/cli/src crates/core/src crates/adaptive/srcRepository: NVIDIA/NeMo-Relay
Length of output: 17280
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '146,240p' crates/core/src/plugin/dynamic/worker.rs printf '\n---\n' sed -n '677,790p' crates/core/src/plugin/dynamic/worker.rs printf '\n---\n' sed -n '439,470p' crates/cli/src/server/mod.rs printf '\n---\n' sed -n '630,760p' crates/cli/src/server/mod.rsRepository: NVIDIA/NeMo-Relay
Length of output: 13858
Move the bootstrap shutdown token out of the ambient environment before plugin activation.
serve_listener_with_dynamic_inner()initializes worker plugins beforebootstrap_shutdown_channel()reads the token, and worker processes inherit the parent environment by default. Read and removeNEMO_RELAY_BOOTSTRAP_SHUTDOWN_TOKENbefore any plugin workers spawn, then pass it only to the bootstrap shutdown path.🤖 Prompt for 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. In `@crates/cli/src/server/mod.rs` around lines 239 - 241, Update serve_listener_with_dynamic_inner() to read and remove NEMO_RELAY_BOOTSTRAP_SHUTDOWN_TOKEN from the environment before PluginActivation::initialize() can spawn worker plugins, then pass the captured token exclusively into bootstrap_shutdown_channel() or its bootstrap shutdown handling path. Preserve the existing plugin activation flow while ensuring workers cannot inherit the token.crates/cli/src/sessions/correlation.rs (2)
201-210: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not persist the weak subagent-start ownership guess.
subagent_startis explicitly a weak newest-worker heuristic inSession::set_last_subagent_start_owner, but this list lets it teach request affinity. The first unhinted call can therefore permanently route matching request payloads to the guessed worker for the rest of the turn.Proposed fix
| "matched_hint" | "active_subagent" - | "subagent_start" | "request_affinity"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub(super) fn owner_status_teaches_request_affinity(status: &str) -> bool { matches!( status, "explicit" | "single_hint" | "matched_hint" | "active_subagent" | "request_affinity" )🤖 Prompt for 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. In `@crates/cli/src/sessions/correlation.rs` around lines 201 - 210, Remove "subagent_start" from the statuses accepted by owner_status_teaches_request_affinity. Preserve affinity teaching for explicit, hint-based, active-subagent, and request-affinity statuses so the weak guess from Session::set_last_subagent_start_owner is not persisted.
320-352: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not treat one retained session as one active session.
single_active_session_idchecks onlyHashMapcardinality. Idle closure deliberately retains dormant agent roots, so a later headerless request can join a stale session and inherit its agent identity, metadata, and configuration. Select an actually active or recent session, or isolate the request when the sole entry is dormant.🤖 Prompt for 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. In `@crates/cli/src/sessions/correlation.rs` around lines 320 - 352, The single_active_session_id fallback must not equate one retained session with one active session. Update single_active_session_id to select the sole session only when it is currently active or recent according to the existing Session state/timestamps; otherwise return None so gateway_session_for_call isolates headerless requests through its unique-root path. Preserve explicit session_id handling and the empty-session behavior.crates/cli/src/sessions/idle.rs (1)
43-95: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Reserve session IDs while idle closure runs.
After Line 55 removes an idle session, another hook or gateway request can recreate the same ID. Line 88 then overwrites that new live session with the old retained state, losing handles and lifecycle events; the subsequent alignment cleanup can also clear newly created ownership state. Use a tombstone or generation check, or keep the session exclusively owned until closure and reinsertion complete.
🤖 Prompt for 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. In `@crates/cli/src/sessions/idle.rs` around lines 43 - 95, Prevent idle-session cleanup from overwriting or clearing a newer session created with the same ID while close_turn_for_reason runs. Update the idle-session removal and reinsertion flow around the sessions map and retained_sessions so each removed session remains exclusively owned, or use a tombstone/generation check before reinserting and applying clear_for_ended_subagent. Preserve cleanup for the original session while skipping stale retention and alignment cleanup when a newer live session exists.crates/cli/tests/architecture_tests.rs (1)
47-124: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Inspect Rust syntax instead of matching raw source text.
Legal forms such as
use crate::{commands::run};,use clap::{Parser};, and grouped agent imports bypass these checks, while comments can trigger false failures. Parse or tokenize imports and attributes so these tests actually enforce the stated boundaries.As per path instructions, tests should cover the behavior promised by the changed surface.
Also applies to: 141-202
🤖 Prompt for 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. In `@crates/cli/tests/architecture_tests.rs` around lines 47 - 124, The architecture tests currently rely on raw source-text matching, allowing grouped imports through and producing false positives from comments. Update shared-services, clap-syntax, and agent-dependency checks to parse or tokenize Rust imports and attributes, correctly detecting grouped forms such as crate and agent imports while ignoring comments. Extend the tests to cover these legal grouped-import cases and the promised boundary behavior.Source: Path instructions
crates/cli/tests/cli_tests.rs (1)
3199-3207: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Terminate Relay through its signal handler on startup timeout.
Line 3202 sends SIGKILL only to Relay. If the agent spawned between lines 3152-3157 but has not published the PID file, the infinite-loop agent and its
sleepdescendant survive and can pollute subsequent tests.Proposed fix
- let _ = relay.kill(); - let output = relay.wait_with_output().unwrap(); + // SAFETY: Relay is owned by this test; SIGTERM permits supervised cleanup. + let _ = unsafe { libc::kill(relay.id() as i32, libc::SIGTERM) }; + let output = wait_child_with_output(relay);As per path instructions, review automation changes for reproducibility.
🤖 Prompt for 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. In `@crates/cli/tests/cli_tests.rs` around lines 3199 - 3207, Update the startup-timeout cleanup in the PID-file wait loop to terminate Relay through its registered signal handler instead of calling relay.kill(), which sends SIGKILL. Send the graceful termination signal to Relay, then wait for it to exit so its agent and sleep descendants are cleaned up before the test panics.Source: Path instructions
crates/cli/tests/coverage/agents/hermes_tests.rs (1)
603-642: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that reinstallation removes the previous hook command.
The test confirms one new command exists but would still pass if every event retained the old generation command. Assert that
first_commandoccurs zero times after reinstall to fully validate idempotency.As per path instructions, tests should cover lifecycle behavior promised by the changed API surface.
🤖 Prompt for 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. In `@crates/cli/tests/coverage/agents/hermes_tests.rs` around lines 603 - 642, Extend the reinstallation assertions around first_command to verify that the old hook command occurs zero times in the updated hooks["on_session_start"] array. Keep the existing assertion for exactly one second_command, ensuring the lifecycle test validates removal of the previous generation command.Source: Path instructions
crates/cli/tests/coverage/agents/launcher_tests.rs (2)
850-883: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Create both Hermes launches concurrently.
Both overlays are currently constructed serially, so this cannot catch races in temporary-path allocation or overlay population. Start the calls in separate threads behind a barrier before asserting isolation.
As per path instructions, tests should cover cross-request isolation where relevant.
🤖 Prompt for 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. In `@crates/cli/tests/coverage/agents/launcher_tests.rs` around lines 850 - 883, Update concurrent_hermes_runs_use_independent_overlays_without_mutating_user_config to construct both PreparedAgentLaunch instances concurrently, using separate threads synchronized by a barrier so both calls begin together. Join the threads, collect their results, then assert the generated overlays are distinct and the original user configuration remains unchanged.Source: Path instructions
1512-1524: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the nonexistent path the inferred executable.
With
agent: Some(CodingAgent::Codex), this path is appended to the defaultcodexcommand. A dry-run regression could therefore launch a real Codex installation instead of failing deterministically.Proposed fix
- agent: Some(CodingAgent::Codex), + agent: None, ... - command: vec!["/path/that/does/not/exist".into()], + command: vec!["/path/that/does/not/exist/codex".into()],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.#[tokio::test] async fn dry_run_does_not_spawn_agent() { let command = RunOverrides { agent: None, config: None, openai_base_url: None, anthropic_base_url: None, session_metadata: None, plugin_config_path: None, dry_run: true, print: false, command: vec!["/path/that/does/not/exist/codex".into()], };🤖 Prompt for 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. In `@crates/cli/tests/coverage/agents/launcher_tests.rs` around lines 1512 - 1524, Update dry_run_does_not_spawn_agent so the nonexistent path is used as the inferred executable rather than being appended to the default Codex command. Configure the command inputs accordingly while preserving dry_run: true, ensuring the test fails deterministically without launching a real Codex installation.crates/cli/tests/coverage/agents/plugin_install_tests.rs (1)
3663-3697: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the preexisting symlink dangling after rollback.
The target is absent before staging, but Line 3696 requires it to exist afterward. That permits staging to leave external filesystem residue instead of preserving the original state.
Proposed fix
- assert!(symlink_target.exists()); + assert!(!symlink_target.exists());If this fails, fix staging cleanup so it does not follow or create the symlink target.
As per path instructions, tests should cover error paths and the promised rollback behavior.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.#[cfg(unix)] #[test] fn failed_staging_preserves_a_preexisting_dangling_generation_lock_symlink() { use std::os::unix::fs::symlink; let dir = tempdir().unwrap(); let target = PluginLayout::new(CodingAgent::Codex, dir.path()); let symlink_target = dir.path().join("generation-lock-target"); symlink(&symlink_target, &target.generation_lock).unwrap(); let stage_parent = dir.path().join("deterministic-symlink-stage"); let staged = PluginLayout::new(CodingAgent::Codex, &stage_parent); crate::filesystem::fail_next_atomic_write(&staged.mcp_config); let error = match stage_plugin_marketplace_at( CodingAgent::Codex, Path::new("/bin/nemo-relay"), &target, true, &options(dir.path()), stage_parent.clone(), ) { Ok(_) => panic!("staging unexpectedly succeeded"), Err(error) => error, }; assert!(error.contains("injected test failure"), "{error}"); assert!(!stage_parent.exists()); assert!( std::fs::symlink_metadata(&target.generation_lock) .unwrap() .file_type() .is_symlink() ); assert!(!symlink_target.exists()); }🤖 Prompt for 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. In `@crates/cli/tests/coverage/agents/plugin_install_tests.rs` around lines 3663 - 3697, Correct failed staging rollback so the preexisting generation-lock symlink remains dangling and its external target is neither followed nor created. Update the cleanup logic used by stage_plugin_marketplace_at, then adjust failed_staging_preserves_a_preexisting_dangling_generation_lock_symlink to assert the symlink remains while symlink_target stays absent, preserving the existing staging-error and temporary-directory cleanup assertions.Source: Path instructions
crates/cli/tests/coverage/shared/bootstrap_state_tests.rs (1)
225-266: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound mock network-server operations across the test suite.
Blocking listeners and reads convert client regressions into indefinite CI hangs rather than test failures.
crates/cli/tests/coverage/shared/bootstrap_state_tests.rs#L225-L266: Add deadlines for both expected connections and request reads.crates/cli/tests/coverage/shared/bootstrap_tests.rs#L118-L181: Replace assumed connection counts with bounded accepts.crates/cli/tests/coverage/shared/doctor_tests.rs#L13-L45: Add accept/read timeouts to the shared capture server.crates/cli/tests/coverage/shared/doctor_tests.rs#L1294-L1454: Apply the same bounded helper to standalone probe servers.📍 Affects 3 files
crates/cli/tests/coverage/shared/bootstrap_state_tests.rs#L225-L266(this comment)crates/cli/tests/coverage/shared/bootstrap_tests.rs#L118-L181crates/cli/tests/coverage/shared/doctor_tests.rs#L13-L45crates/cli/tests/coverage/shared/doctor_tests.rs#L1294-L1454🤖 Prompt for 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. In `@crates/cli/tests/coverage/shared/bootstrap_state_tests.rs` around lines 225 - 266, Bound all mock network-server operations to prevent indefinite test hangs: in crates/cli/tests/coverage/shared/bootstrap_state_tests.rs:225-266, add deadlines to both expected listener accepts and request reads; in crates/cli/tests/coverage/shared/bootstrap_tests.rs:118-181, replace assumed connection counts with bounded accepts; in crates/cli/tests/coverage/shared/doctor_tests.rs:13-45, add accept and read timeouts to the shared capture server; and in crates/cli/tests/coverage/shared/doctor_tests.rs:1294-1454, apply the same bounded-server helper to the standalone probe servers.crates/cli/tests/coverage/shared/config_tests.rs (1)
480-522: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove root-dependent unreadability fixtures.
Unix mode
000does not prevent root from reading a file, so these tests fail in privileged containers.
crates/cli/tests/coverage/shared/config_tests.rs#L480-L522: Use deterministic configuration-read failures or handle privileged execution.crates/cli/tests/coverage/shared/config_tests.rs#L1765-L1787: Inject the bounded-reader failure or skip the permission-only assertion for root.📍 Affects 1 file
crates/cli/tests/coverage/shared/config_tests.rs#L480-L522(this comment)crates/cli/tests/coverage/shared/config_tests.rs#L1765-L1787🤖 Prompt for 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. In `@crates/cli/tests/coverage/shared/config_tests.rs` around lines 480 - 522, Replace the permission-mode-based unreadable-file setup in crates/cli/tests/coverage/shared/config_tests.rs:480-522 with a deterministic configuration-read failure, or explicitly skip the permission-only assertions when running as root. Apply the same bounded-reader failure injection or root skip to the permission assertion at crates/cli/tests/coverage/shared/config_tests.rs:1765-1787, preserving checks that errors include the relevant source path.crates/cli/tests/coverage/shared/install_generation_tests.rs (1)
393-423: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,260p' crates/cli/tests/coverage/shared/install_generation_tests.rsRepository: NVIDIA/NeMo-Relay
Length of output: 9753
🏁 Script executed:
sed -n '1,260p' crates/cli/src/commands/mod.rsRepository: NVIDIA/NeMo-Relay
Length of output: 7578
🏁 Script executed:
sed -n '340,460p' crates/cli/tests/coverage/shared/install_generation_tests.rsRepository: NVIDIA/NeMo-Relay
Length of output: 5042
🏁 Script executed:
rg -n "CwdTestScope|CWD_TEST_LOCK|lock_cwd|current_dir|set_current_dir" crates/cli/tests/coverage/shared/install_generation_tests.rs crates/cli/src/commands/mod.rsRepository: NVIDIA/NeMo-Relay
Length of output: 1841
🏁 Script executed:
rg -n "CwdTestScope::(locked|enter)\(" crates/cli tests crates/cli/testsRepository: NVIDIA/NeMo-Relay
Length of output: 1415
🏁 Script executed:
rg -n "pub\(crate\) mod test_support|use crate::commands::test_support|use super::test_support" crates/cli/src crates/cli/testsRepository: NVIDIA/NeMo-Relay
Length of output: 220
Serialize
relative_legacy_marker_reencodes_an_absolute_lock_for_rollbackwith the shared CWD guard
crates/cli/tests/coverage/shared/install_generation_tests.rs:393-423reads and later resolves multiple relative paths against the process CWD, but it doesn’t acquirecrate::test_support::CwdTestScope::locked()like the other CWD-dependent CLI tests. A parallel test that callsset_current_dircan redirecttempdir_in,rename, orInstallGeneration::capturemid-flight. Add the shared guard for the full test.🤖 Prompt for 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. In `@crates/cli/tests/coverage/shared/install_generation_tests.rs` around lines 393 - 423, Add crate::test_support::CwdTestScope::locked() at the start of relative_legacy_marker_reencodes_an_absolute_lock_for_rollback and retain the guard for the entire test, covering all relative-path operations and InstallGeneration::capture.crates/cli/tests/coverage/shared/mcp_gateway_tests.rs (1)
27-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail git ls-files crates/cli/tests/coverage/shared/mcp_gateway_tests.rs crates/cli/src | sed -n '1,120p' echo '--- mcp_gateway_tests.rs ---' cat -n crates/cli/tests/coverage/shared/mcp_gateway_tests.rs | sed -n '1,220p' echo '--- search maintain_gateway_with ---' rg -n "maintain_gateway_with|production_heartbeat_recovers_after_one_thirty_second_interval|advance\\(Duration::from_secs\\(30\\)\\)" crates/cli -S echo '--- relevant implementation ---' fd -a "mcp_gateway" crates/cli crates -t f 2>/dev/null | sed -n '1,120p'Repository: NVIDIA/NeMo-Relay
Length of output: 16482
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- maintain_gateway_with implementation ---' sed -n '235,360p' crates/cli/src/mcp/gateway.rs echo '--- similar paused-time tests in mcp_tests.rs ---' sed -n '300,700p' crates/cli/tests/coverage/shared/mcp_tests.rsRepository: NVIDIA/NeMo-Relay
Length of output: 19435
Yield once before advancing the clock.
tokio::spawnmay not pollmaintain_gateway_withbeforeadvance(30s), so this can auto-advance into the next timer and pass without proving the restart happened after the first interval. Add a startup barrier ortokio::task::yield_now().awaitbefore advancing.🤖 Prompt for 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. In `@crates/cli/tests/coverage/shared/mcp_gateway_tests.rs` around lines 27 - 55, Ensure maintain_gateway_with is polled once before advancing simulated time in production_heartbeat_recovers_after_one_thirty_second_interval. Add a startup synchronization barrier or await tokio::task::yield_now() immediately after spawning the monitor and before advance(Duration::from_secs(30)), preserving the test’s verification that recovery occurs after the first interval.Source: Path instructions
crates/cli/src/filesystem/snapshots.rs (1)
13-26: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Publish backup files atomically before accepting them.
Line 19 copies directly to the final path, while Line 15 treats any existing path as complete. An interrupted copy leaves a partial backup that future calls accept and rollback can restore over Claude or Codex configuration. Write and sync a temporary file, preserve permissions/DACLs, then atomically rename it.
🤖 Prompt for 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. In `@crates/cli/src/filesystem/snapshots.rs` around lines 13 - 26, Update backup to create a temporary file beside the destination, copy the source into it, preserve the source permissions and platform-specific DACLs, and sync it before publication. Atomically rename the completed temporary file to the path returned by backup_path, ensuring existing destination checks never accept an interrupted partial copy and cleaning up temporary files on failure.crates/cli/src/plugins/lifecycle/environment.rs (2)
96-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject entrypoints containing additional separators.
Line 96 accepts
pkg.module:factory:extrabecausesplit_onceleaves the second colon incallable. This passes provisioning validation but is not the promisedmodule:functionform.Proposed fix
if callable.is_empty() + || callable.contains(':') || module.is_empty()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let (module, callable) = entrypoint.split_once(':').ok_or_else(|| { format!( "Python worker load.entrypoint '{entrypoint}' must use the unambiguous module:function form" ) })?; if callable.is_empty() || callable.contains(':') || module.is_empty() || module .split('.') .any(|segment| segment.is_empty() || segment.contains(['/', '\\', ':'])) {🤖 Prompt for 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. In `@crates/cli/src/plugins/lifecycle/environment.rs` around lines 96 - 106, Update the entrypoint validation around the module/callable parsing to reject callable values containing additional ':' separators, while preserving valid module:function entries and the existing module-segment checks. Ensure inputs such as pkg.module:factory:extra fail provisioning validation.
478-480: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use an unambiguous tree-digest encoding.
Concatenating
lossy_path + NUL + contentspermits different file trees to produce the same digest, and non-UTF-8 paths can collapse underto_string_lossy. The HMAC authenticates that ambiguous digest, so it does not prevent this attestation bypass. Frame entry type, raw path, and payload lengths as the snapshot digest already does.Proposed fix
- digest.update(relative.to_string_lossy().as_bytes()); - digest.update([0]); + let relative = relative.as_os_str().as_encoded_bytes(); + digest.update([0]); // Regular file. + digest.update((relative.len() as u64).to_le_bytes()); + digest.update(relative); + digest.update((bytes.len() as u64).to_le_bytes()); digest.update(&bytes);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.digest.update([0]); // Regular file. let relative = relative.as_os_str().as_encoded_bytes(); digest.update((relative.len() as u64).to_le_bytes()); digest.update(relative); digest.update((bytes.len() as u64).to_le_bytes()); digest.update(&bytes);🤖 Prompt for 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. In `@crates/cli/src/plugins/lifecycle/environment.rs` around lines 478 - 480, Update the tree-digest construction around the digest update calls to use unambiguous framing like the snapshot digest: encode the entry type, raw path bytes, and payload length before the payload, preserving non-UTF-8 path bytes without lossy conversion. Ensure distinct file trees cannot produce the same digest before HMAC authentication.crates/cli/src/plugins/lifecycle/mod.rs (1)
271-300: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply Python entrypoint validation in both
plugins validatebranches.These paths report success—and the ID path records valid status—without calling
validate_python_entrypoint_artifact. A manifest rejected byplugins addor activation can therefore be reported as valid here.Proposed fix
let (manifest, manifest_ref) = load_manifest_for_action("validate", &path)?; + validate_python_entrypoint_artifact(&manifest, &manifest_ref).map_err(|message| { + plugin_failed_with_code( + "plugins validate", + Some(command.target.clone()), + "manifest_invalid", + message, + ) + })?; load_config_schema_for_manifest(&manifest, &manifest_ref)?;Apply the equivalent check after loading the manifest in the plugin-ID branch.
Also applies to: 310-327
🤖 Prompt for 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. In `@crates/cli/src/plugins/lifecycle/mod.rs` around lines 271 - 300, Call validate_python_entrypoint_artifact for the loaded manifest in both plugins validate branches, including the path branch shown and the plugin-ID branch near the corresponding success handling. Perform the check after loading the manifest and before reporting success or recording valid status, propagating any validation error consistently with the existing flow.crates/cli/src/plugins/pricing.rs (1)
143-156: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound pricing catalog reads before allocation.
read_to_stringaccepts an arbitrarily large configured file and can exhaust process memory before parsing. Use the repository’s bounded regular-file reader, then validate UTF-8.Proposed fix
- let raw = std::fs::read_to_string(path).map_err(|source| { - CliError::Config(format!( - "could not read model pricing catalog '{}': {source}", - path.display() - )) - })?; + let bytes = crate::filesystem::bounded::read_bounded_regular_file( + path, + "model pricing catalog", + ) + .map_err(CliError::Config)?; + let raw = std::str::from_utf8(&bytes).map_err(|source| { + CliError::Config(format!( + "model pricing catalog '{}' is not valid UTF-8: {source}", + path.display() + )) + })?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn read_pricing_catalog(path: &Path) -> Result<PricingCatalog, CliError> { let bytes = crate::filesystem::bounded::read_bounded_regular_file( path, "model pricing catalog", ) .map_err(CliError::Config)?; let raw = std::str::from_utf8(&bytes).map_err(|source| { CliError::Config(format!( "model pricing catalog '{}' is not valid UTF-8: {source}", path.display() )) })?; PricingCatalog::from_json_str(raw).map_err(|error| { CliError::Config(format!( "invalid model pricing catalog '{}': {error}", path.display() )) }) }🤖 Prompt for 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. In `@crates/cli/src/plugins/pricing.rs` around lines 143 - 156, Update read_pricing_catalog to replace std::fs::read_to_string with the repository’s bounded regular-file reader, applying the configured size limit before allocation and preserving the existing read-error context. Convert the bounded reader’s bytes to UTF-8 explicitly, mapping invalid UTF-8 to CliError::Config, then pass the resulting string to PricingCatalog::from_json_str with the existing parse-error context.crates/cli/src/process/detached.rs (2)
80-106: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Declared Rust toolchain:" fd -a '^rust-toolchain(\.toml)?$' . \ --exec sh -c 'echo "--- $1"; sed -n "1,120p" "$1"' sh {} echo "Process-spawn sites requiring inheritance coordination:" rg -n -C4 '\bspawn_detached\s*\(|\bCommand::new\s*\(|\.spawn\s*\(' crates/cli/srcRepository: NVIDIA/NeMo-Relay
Length of output: 14711
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== crates/cli/src/process/detached.rs ===" wc -l crates/cli/src/process/detached.rs sed -n '1,260p' crates/cli/src/process/detached.rs echo echo "=== HandleInheritanceGuard definition/usages ===" rg -n -C4 'struct HandleInheritanceGuard|impl HandleInheritanceGuard|suppress\(|restore\(' crates/cli/src echo echo "=== terminate_tree / kill fallback sites ===" rg -n -C5 'terminate_tree|child\.kill\(\)|taskkill|kill_on_drop|ProcessTree' crates/cli/src/process crates/cli/src/bootstrap crates/cli/src/gatewayRepository: NVIDIA/NeMo-Relay
Length of output: 36220
🏁 Script executed:
#!/bin/bash set -euo pipefail sysroot="$(rustc --print sysroot)" echo "SYSROOT=$sysroot" echo echo "=== Windows process implementation in std ===" fd -a 'process.rs$' "$sysroot/lib/rustlib/src/rust/library/std/src/os/windows" || true if [ -f "$sysroot/lib/rustlib/src/rust/library/std/src/os/windows/process.rs" ]; then sed -n '1,260p' "$sysroot/lib/rustlib/src/rust/library/std/src/os/windows/process.rs" fi echo echo "=== Search for handle list / inheritance in std sources ===" rg -n -C3 'PROC_THREAD_ATTRIBUTE_HANDLE_LIST|HANDLE_LIST|bInheritHandles|SetHandleInformation|handle_list|inherit' \ "$sysroot/lib/rustlib/src/rust/library/std/src" | head -n 200Repository: NVIDIA/NeMo-Relay
Length of output: 198
🌐 Web query:
Rust std::process::Command Windows PROC_THREAD_ATTRIBUTE_HANDLE_LIST handle inheritance source💡 Result:
In Rust, you can control handle inheritance when spawning a child process on Windows using the
std::os::windows::process::CommandExttrait [1][2]. To usePROC_THREAD_ATTRIBUTE_HANDLE_LISTfor explicit handle inheritance, you must interact with theProcThreadAttributeListAPI provided by the standard library [3][4]. Key Mechanisms: 1. CommandExt::spawn_with_attributes: This method allows you to pass aProcThreadAttributeListto theCreateProcesscall [1][3]. You can use this to include thePROC_THREAD_ATTRIBUTE_HANDLE_LISTattribute, which contains a specific whitelist of handles that the child process is permitted to inherit [5][4]. 2. Requirements for the Attribute: When using this attribute, you must set thebInheritHandlesparameter of the underlyingCreateProcessfunction toTRUE[4]. This is done via theCommandExt::inherit_handles(true)method [1][2]. 3. Handle Setup: The handles added to this list must have been created as inheritable handles (e.g., usingSECURITY_ATTRIBUTESwithbInheritHandleset to true) and cannot include pseudo-handles like those fromGetCurrentProcessorGetCurrentThread[5][4]. For more granular control, theCommandExttrait also providesinherit_handles(false), which disables the default behavior where all inheritable handles in the calling process are passed to the new process [1][2]. If you require a strict whitelist, the recommended approach is to setinherit_handles(true)and then supply aProcThreadAttributeListcontaining exactly the handles you wish to expose to the child [6][4][7]. Note that some methods related to these extensions (such asinherit_handlesor specificProcThreadAttributeListbuilders) may be behind unstable or nightly feature flags in certain Rust versions [1][2], so you should check the official documentation for the version of the compiler you are using [1].Citations:
- 1: https://doc.rust-lang.org/stable/std/os/windows/process/trait.CommandExt.html
- 2: https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html
- 3: https://doc.rust-lang.org/std/os/windows/process/struct.ProcThreadAttributeListBuilder.html
- 4: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute
- 5: https://learn.microsoft.com/en-us/windows/win32/procthread/inheritance
- 6: rust-lang/rust#115501
- 7: https://docs.rs/running-process-core/latest/src/running_process_core/spawn.rs.html
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Search the repo for explicit handle-list or Windows process spawn notes:" rg -n -C3 'PROC_THREAD_ATTRIBUTE_HANDLE_LIST|handle list|inherit handles|bInheritHandles|STARTUPINFOEX|SetHandleInformation|HandleInheritanceGuard' .Repository: NVIDIA/NeMo-Relay
Length of output: 4089
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,230p' crates/cli/src/process/detached.rs | rg -n -C2 'test|#[[:space:]]*cfg\(test\)|handle|inherit|taskkill|terminate_tree|spawn_detached'Repository: NVIDIA/NeMo-Relay
Length of output: 4269
🌐 Web query:
site:doc.rust-lang.org std::process::Command Windows handle_list bInheritHandles PROC_THREAD_ATTRIBUTE_HANDLE_LIST💡 Result:
In Rust's standard library, support for managing the inheritance of handles and specific process attributes on Windows is provided through the std::os::windows::process::CommandExt trait [1][2]. The specific Windows attribute PROC_THREAD_ATTRIBUTE_HANDLE_LIST is used with the UpdateProcThreadAttribute function to restrict handle inheritance to a specific subset of handles, rather than inheriting all inheritable handles [1][2]. While Rust provides a mechanism to use arbitrary attributes via spawn_with_attributes, handling handle lists requires manual construction of the attribute list [1][3]. 1. InheritHandles: The CommandExt trait includes an inherit_handles(bool) method (currently nightly-only, behind feature flag windows_process_extensions_inherit_handles) [4][2]. When set to true (the default), all inheritable handles in the calling process are inherited by the child [1][2]. Setting this to false prevents the default inheritance of all handles [1][4]. 2. PROC_THREAD_ATTRIBUTE_HANDLE_LIST: To achieve more granular control—such as passing only a specific list of handles—you can use the spawn_with_attributes method (nightly-only, behind feature flag windows_process_extensions_raw_attribute) [1][3][5]. You would construct a ProcThreadAttributeList and populate it using the raw_attribute method, providing the handle list as the value [3][6]. Note that the attribute value for PROC_THREAD_ATTRIBUTE_HANDLE_LIST is a pointer to an array of HANDLE values, and its size must be correctly specified [3]. Since both spawn_with_attributes and inherit_handles are currently experimental/nightly-only features, they require the use of #![feature(...)] attributes in your Rust code and are subject to change [4][2][7]. Accessing these functions requires importing the CommandExt trait [1][2].
Citations:
- 1: https://doc.rust-lang.org/stable/std/os/windows/process/trait.CommandExt.html
- 2: https://doc.rust-lang.org/beta/std/os/windows/process/trait.CommandExt.html
- 3: https://doc.rust-lang.org/std/os/windows/process/struct.ProcThreadAttributeListBuilder.html
- 4: https://doc.rust-lang.org/stable/std/process/struct.Command.html
- 5: https://doc.rust-lang.org/std/process/struct.Command.html?mut=
- 6: https://doc.rust-lang.org/stable/src/std/os/windows/process.rs.html
- 7: https://doc.rust-lang.org/nightly/std/process/struct.Command.html
Use an explicit Windows handle allowlist here. Clearing inherit bits on only stdin/stdout/stderr still leaves every other inheritable handle eligible for the detached gateway, and the temporary flag flip is only serialized for this helper. Switch to
PROC_THREAD_ATTRIBUTE_HANDLE_LIST/spawn_with_attributes(or equivalentCreateProcessWsetup) so only the intended handles can cross into the child.🤖 Prompt for 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. In `@crates/cli/src/process/detached.rs` around lines 80 - 106, Update spawn_detached to use an explicit Windows handle allowlist through PROC_THREAD_ATTRIBUTE_HANDLE_LIST with spawn_with_attributes or equivalent CreateProcessW setup. Pass only the intended standard handles to the child, replacing the current HandleInheritanceGuard::suppress approach while preserving the existing spawn, restoration, and cleanup error handling.
216-224: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -C5 '\bterminate_tree\s*\(|taskkill|child\.kill\s*\(' \ crates/cli/src crates/cli/testsRepository: NVIDIA/NeMo-Relay
Length of output: 9473
🏁 Script executed:
#!/bin/bash set -euo pipefail ast-grep outline crates/cli/src/process/detached.rs --view expanded sed -n '1,260p' crates/cli/src/process/detached.rs printf '\n--- windows spawn usage ---\n' rg -n -C3 'configure_detached\(|terminate_tree\(|creation_flags|DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP|job object|taskkill|kill\(\)' crates/cli/srcRepository: NVIDIA/NeMo-Relay
Length of output: 21421
Avoid the
child.kill()fallback on Windows.
Iftaskkill /T /Ffails,child.kill()only stops the direct process and can leave descendants running, which breaks tree cleanup during reaper shutdown. Switch this path to a Windows Job Object kill or surface the cleanup failure instead.🤖 Prompt for 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. In `@crates/cli/src/process/detached.rs` around lines 216 - 224, Update the Windows cleanup branch in the detached-process termination flow to remove the child.kill() fallback after taskkill fails. Use the existing Windows Job Object termination mechanism if available, or propagate/report the cleanup failure instead of attempting direct-child termination, preserving complete process-tree cleanup.
2 participants
Warning
BREAKING CHANGE: [Plugin configuration and installation] Existing
Codex, Claude Code, and Hermes Relay integrations must be reinstalled. Generated
MCP entries now invoke
nemo-relay mcp, persistent hooks are agent-owned andgeneration-fenced, and the shared gateway uses user-scoped configuration.
Run
nemo-relay install <agent> --forcefor each installed integration, ornemo-relay install all --force. Previously generated integration state isnot compatible with the new lifecycle and may fail readiness or doctor checks.
Overview
Add an MCP-managed shared Relay gateway for Codex, Claude Code, and Hermes. The new lifecycle guarantees that Relay is ready before installed coding agents emit hooks or routed provider traffic, fixing RELAY-447.
nemo-relay mcpeagerly starts or adopts the existing Rust gateway before reading MCP input, heartbeats it while stdio is open, and coordinates one recovery. Codex, Claude Code, and Hermes use the same lifecycle model.Details
Problems addressed
New behavior
nemo-relay mcpacquires the gateway before MCP initialization and advertises no tools.alwaysLoad; Hermes uses the same MCP lifecycle and canonical hooks.Component breakdown
crates/cli/src/commands/crates/cli/src/agents/crates/cli/src/mcp/gateway.rscrates/cli/src/server.rscrates/cli/src/bootstrap/crates/cli/src/gateway/client.rscrates/cli/src/process/detached.rssrc/crates/cli/tests/The CLI source now has two explicit ownership axes.
commands/owns command execution;agents/owns agent vertical slices and shared installation transactions. Gateway, MCP transport, sessions, bootstrap lifecycle, and runtime configuration remain agent-neutral. Architecture tests prevent retired top-level agent trees from returning and prohibit shared-service-to-command and cross-agent dependencies.Compatibility and limitations
/modelsbefore launching required MCP servers; captured turns and routed provider requests still wait for readiness.Validation
cargo fmt --allcargo clippy --workspace --all-targets -- -D warningsjust test-rust(992 CLI unit tests and 80 external CLI process tests, plus the complete Rust workspace)uv run pre-commit run --all-filescargo test -p nemo-relay-cli --test architecture_testsWhere should the reviewer start?
Review in four bounded passes:
crates/cli/src/commands/mod.rsand its thin executor modules to verify unchanged dispatch and exit behavior.crates/cli/src/agents/mod.rs, then one host directory at a time. Verify agent-specific adapters, alignment, hooks, trust, and setup are colocated.crates/cli/src/mcp/gateway.rs,bootstrap/mod.rs,gateway/client.rs,process/detached.rs, andserver/mod.rsfor eager acquisition, authenticated adoption, recovery, and idle shutdown.crates/cli/tests/architecture_tests.rsand the grouped coverage directories as the acceptance specification.The key runtime boundary remains: hosts own MCP stdio, MCP coordinates liveness, and the existing Relay gateway owns server lifecycle.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Bugs addressed
Gateway bootstrap and lifecycle
nemo-relay mcpstarts, before waiting for MCP initialization.Hooks and event delivery
nemo-relay runsessions from also invoking persistent hooks and duplicating events.Codex correlation
client_metadata.session_idfor Codex root-session correlation.prompt_cache_keyas a compatibility fallback.client_metadata.thread_idas subagent ownership forcollab_spawn.Installation, trust, and rollback
alwaysLoadconfiguration and validates it during diagnosis.Filesystem and ownership safety
marketplaceRootorpluginRootvalues from granting recursive deletion authority over arbitrary directories.Process management
Environment propagation
NEMO_RELAY_,OTEL_, andAWS_.header_env, AWS secret-key, and session-token variables.New features
Adds the Rust-native
nemo-relay mcpcommand.Makes
nemo-relay mcpa lifecycle coordinator rather than a tool-serving MCP implementation.Advertises no MCP tools while keeping the MCP stdio session alive.
Supports the architecture:
Starts or adopts a detached Rust
nemo-relay --bind 127.0.0.1:47632gateway.Allows multiple Codex, Claude Code, and Hermes MCP processes to share one gateway.
Starts gateway acquisition immediately when MCP stdio opens.
Heartbeats the gateway while MCP remains connected.
Performs one coordinated gateway recovery after failure.
Adds authenticated gateway health, shutdown, hook-delivery, and ownership operations.
Extends
/healthzwith Relay identity, version, protocol, and compatibility information.Adds readiness-file reporting for automatically assigned ports.
Uses system and user Relay configuration for persistent plugin mode.
Keeps project-specific configuration available through transparent
nemo-relay run.Adds a consistent persistent MCP lifecycle for Codex, Claude Code, and Hermes.
Adds generated MCP environment-forwarding declarations for all three agents.
Adds plugin-owned Codex hooks with automatic, verified trust management.
Adds Claude Code plugin activation through
alwaysLoad.Adds Hermes MCP, hook, trust, generation, install, uninstall, and doctor integration.
Adds shared gateway idle shutdown after the final MCP client disconnects.
Adds generation-fenced hook and MCP identities to prevent stale installations from controlling a replacement gateway.
Adds opt-in cold-start and concurrency E2E suites for Codex, Claude Code, and Hermes.
Improvements
Architecture and readability
Reorganizes the CLI around two explicit ownership axes:
commands/owns CLI syntax, validation, dispatch, rendering, and exit codes.agents/owns Codex, Claude Code, and Hermes behavior.Adds an internal library root with a minimal process entrypoint.
Reduces
main.rsto the application entrypoint.Moves all Clap-derived types under
commands/.Makes
alla command-only install target instead of a runtime agent identity.Replaces command-shaped configuration DTOs with subsystem-owned runtime inputs.
Gives Codex, Claude, and Hermes complete vertical slices for launch, hooks, assets, trust, setup, installation, and diagnosis.
Removes obsolete horizontal adapter, alignment, host, installer, and plugin-shim facades.
Removes the production Node bootstrap path; gateway bootstrap is entirely Rust-native.
Separates launch preparation from shared process supervision.
Separates gateway startup coordination from gateway transport and server execution.
Separates detached process handling from bootstrap ownership.
Splits gateway routing, request preparation, response handling, and authenticated client transport.
Splits session routing, correlation, ownership, and idle shutdown.
Splits configuration, diagnostics, filesystem operations, hooks, installation transactions, and plugin services by responsibility.
Keeps agent-neutral installation primitives free of host selection.
Centralizes concrete agent dispatch in
agents/mod.rs.Makes relative persistent output paths deterministic by starting the gateway from the user configuration directory.
Security and transactional integrity
Diagnostics and operator experience
Expands doctor checks for:
alwaysLoad;Provides actionable reinstall remediation when generated state is stale.
Distinguishes foreign listener conflicts from compatible Relay reuse.
Preserves existing CLI output, JSON schemas, aliases, paths, and exit behavior.
Documents operational
hook-forwardflags directly in CLI help without brittle prose-pinning tests.Testing and quality
Keeps all tests outside the
src/tree.Increases the CLI suite to 995 unit tests plus 80 process-level tests.
Adds architecture tests that enforce command, agent, and shared-service dependency boundaries.
Adds coverage for:
Adds real-agent opt-in E2Es without making Codex, Claude Code, or Hermes mandatory Rust CI dependencies.
Adds ten-cold-run and concurrent-session coverage for Codex and Claude Code.
Adds a Hermes MCP E2E target for environments where Hermes is installed.
Enforces CLI coverage through Codecov while avoiding artificial help-text or environment-name tests.
Adds test serialization for process-global environment and working-directory mutations.
Restores cross-platform compilation and coverage-module visibility after the CLI reorganization.
Summary by CodeRabbit