Skip to content

refactor: resolve CLI and script Sonar findings #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rapids-bot merged 2 commits into from
Jul 15, 2026
Merged

refactor: resolve CLI and script Sonar findings #428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5147cec
refactor: resolve CLI Sonar findings
willkill07 Jul 15, 2026
4043f99
refactor: model marketplace install transaction state
willkill07 Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 56 additions & 43 deletions crates/cli/src/commands/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,51 +31,64 @@ pub(crate) struct AgentsCommand {

pub(super) async fn execute(command: DoctorCommand) -> Result<ExitCode, CliError> {
if let Some(plugin) = command.plugin {
let candidates = plugin.agents();
let agents = if plugin.is_all() {
crate::agents::installed_integrations(&candidates, command.install_dir.as_deref())
} else {
candidates
};
if agents.is_empty() {
return Err(CliError::Install(
"no installed Claude Code, Codex, or Hermes integration state was found".into(),
));
}
let options = crate::installation::marketplace::plugin_doctor_options(command.install_dir);
if command.json {
let reports = agents
.iter()
.copied()
.map(|agent| crate::agents::doctor_integration_report(agent, &options))
.collect::<Result<Vec<_>, _>>()?;
let ready = reports
.iter()
.all(|report| report.get("ok").and_then(Value::as_bool) == Some(true));
let output = if reports.len() > 1 {
json!({ "schema_version": 1, "plugins": reports })
} else {
with_schema(reports.into_iter().next().expect("reports is not empty"))
};
println!(
"{}",
serde_json::to_string_pretty(&output)
.map_err(|error| CliError::Install(error.to_string()))?
);
Ok(if ready {
ExitCode::SUCCESS
} else {
ExitCode::FAILURE
})
} else {
for agent in agents {
crate::agents::doctor_integration(agent, &options)?;
}
Ok(ExitCode::SUCCESS)
}
return execute_plugin_doctor(plugin, command.install_dir, command.json);
}
crate::diagnostics::run_doctor(command.agent.map(Into::into), command.json).await
}

fn execute_plugin_doctor(
plugin: InstallTarget,
install_dir: Option<PathBuf>,
json: bool,
) -> Result<ExitCode, CliError> {
let candidates = plugin.agents();
let agents = if plugin.is_all() {
crate::agents::installed_integrations(&candidates, install_dir.as_deref())
} else {
crate::diagnostics::run_doctor(command.agent.map(Into::into), command.json).await
candidates
};
if agents.is_empty() {
return Err(CliError::Install(
"no installed Claude Code, Codex, or Hermes integration state was found".into(),
));
}
let options = crate::installation::marketplace::plugin_doctor_options(install_dir);
if !json {
for agent in agents {
crate::agents::doctor_integration(agent, &options)?;
}
return Ok(ExitCode::SUCCESS);
}
print_plugin_doctor_json(&agents, &options)
}

fn print_plugin_doctor_json(
agents: &[crate::agents::CodingAgent],
options: &crate::installation::marketplace::state::PluginInstallOptions,
) -> Result<ExitCode, CliError> {
let reports = agents
.iter()
.copied()
.map(|agent| crate::agents::doctor_integration_report(agent, options))
.collect::<Result<Vec<_>, _>>()?;
let ready = reports
.iter()
.all(|report| report.get("ok").and_then(Value::as_bool) == Some(true));
let output = if reports.len() > 1 {
json!({ "schema_version": 1, "plugins": reports })
} else {
with_schema(reports.into_iter().next().expect("reports is not empty"))
};
println!(
"{}",
serde_json::to_string_pretty(&output)
.map_err(|error| CliError::Install(error.to_string()))?
);
Ok(if ready {
ExitCode::SUCCESS
} else {
ExitCode::FAILURE
})
}

fn with_schema(mut value: Value) -> Value {
Expand Down
170 changes: 98 additions & 72 deletions crates/cli/src/diagnostics/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,82 +299,108 @@ async fn collect_agents(
if target_agent.is_some_and(|target| target != agent) {
continue;
}
let configured = agent_configured(agent, &resolved.agents);
let target_requested = target_agent == Some(agent);
let command = agent_command(agent, &resolved.agents);
let argv = crate::process::command_argv(&command);
let exec = argv.first().map(String::as_str).unwrap_or_default();
let path = crate::process::resolve_executable(exec);
let version = match &path {
Some(_) => {
let probe = crate::process::version_probe_argv(agent, &argv);
probe_version(&probe).await
}
None => None,
};
let mut status = agent_command_status(path.as_deref(), configured, target_requested);
let (hook_status, hook_details) = hook_status(agent, &resolved.agents);
status = combine_status(status, hook_status, configured || target_requested);
let mut details = Vec::new();
details.push(if configured {
"configured".to_string()
} else if target_requested {
"not configured; first run will launch setup".to_string()
} else {
"not configured".to_string()
});
if path.is_none() {
details.push(format!("command `{exec}` not found"));
}
if !hook_details.is_empty() {
details.push(hook_details);
}
let version_required = configured || target_requested;
match version.as_deref() {
Some(version) => {
if let Err(error) = agent.validate_version_output(version) {
status = combine_status(
status,
if version_required {
Status::Fail
} else {
Status::Warn
},
true,
);
details.push(error);
}
}
None if path.is_some() => {
status = combine_status(
status,
if version_required {
Status::Fail
} else {
Status::Warn
},
true,
);
details.push(format!(
"could not determine version; NeMo Relay requires {}",
agent.version_requirement()
));
}
None => {}
}
out.push(AgentInfo {
name: agent.as_arg(),
status,
configured,
command,
path,
version,
annotation: details.join("; "),
});
out.push(collect_agent(agent, target_agent == Some(agent), resolved).await);
}
out
}

async fn collect_agent(
agent: CodingAgent,
target_requested: bool,
resolved: &ResolvedConfig,
) -> AgentInfo {
let configured = agent_configured(agent, &resolved.agents);
let command = agent_command(agent, &resolved.agents);
let argv = crate::process::command_argv(&command);
let exec = argv.first().map(String::as_str).unwrap_or_default();
let path = crate::process::resolve_executable(exec);
let version = if path.is_some() {
probe_version(&crate::process::version_probe_argv(agent, &argv)).await
} else {
None
};
let mut status = agent_command_status(path.as_deref(), configured, target_requested);
let (hook_status, hook_details) = hook_status(agent, &resolved.agents);
status = combine_status(status, hook_status, configured || target_requested);
let mut details = agent_details(
configured,
target_requested,
path.as_deref(),
exec,
hook_details,
);
apply_agent_version_status(
agent,
version.as_deref(),
path.is_some(),
configured || target_requested,
&mut status,
&mut details,
);
AgentInfo {
name: agent.as_arg(),
status,
configured,
command,
path,
version,
annotation: details.join("; "),
}
}

fn agent_details(
configured: bool,
target_requested: bool,
path: Option<&Path>,
exec: &str,
hook_details: String,
) -> Vec<String> {
let mut details = vec![if configured {
"configured".to_string()
} else if target_requested {
"not configured; first run will launch setup".to_string()
} else {
"not configured".to_string()
}];
if path.is_none() {
details.push(format!("command `{exec}` not found"));
}
if !hook_details.is_empty() {
details.push(hook_details);
}
details
}

fn apply_agent_version_status(
agent: CodingAgent,
version: Option<&str>,
executable_found: bool,
version_required: bool,
status: &mut Status,
details: &mut Vec<String>,
) {
let problem = match version {
Some(version) => agent.validate_version_output(version).err(),
None if executable_found => Some(format!(
"could not determine version; NeMo Relay requires {}",
agent.version_requirement()
)),
None => None,
};
if let Some(problem) = problem {
*status = combine_status(
*status,
if version_required {
Status::Fail
} else {
Status::Warn
},
true,
);
details.push(problem);
}
}

fn agent_command(agent: CodingAgent, agents: &AgentConfigs) -> String {
configured_agent_command(agent, agents)
.cloned()
Expand Down
71 changes: 38 additions & 33 deletions crates/cli/src/hooks/delivery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use reqwest::header::{CONTENT_TYPE, HeaderMap, HeaderName, HeaderValue};
use serde_json::Value;

use crate::error::CliError;
use crate::installation::generation::InstallGeneration;
use crate::installation::generation::{ActiveGenerationGuard, InstallGeneration};

use super::destination::{
HookGatewayLifecycle, hook_destination, recovery_plan, transparent_gateway_spec,
Expand All @@ -32,10 +32,7 @@ pub(crate) async fn hook_forward(command: HookForwardRequest) -> Result<(), CliE
let fail_closed =
command.fail_closed || std::env::var("NEMO_RELAY_FAIL_CLOSED").ok().as_deref() == Some("1");
let destination = hook_destination(&command);
let persistent = match (destination.lifecycle != HookGatewayLifecycle::Transparent)
.then(|| recovery_plan(&destination.gateway_url))
.transpose()
{
let persistent = match persistent_gateway(&destination) {
Ok(persistent) => persistent,
Err(error) => return handle_hook_error(error, fail_closed),
};
Expand All @@ -47,34 +44,9 @@ pub(crate) async fn hook_forward(command: HookForwardRequest) -> Result<(), CliE
Ok(gateway) => gateway,
Err(error) => return handle_hook_error(error, fail_closed),
};
let _generation_guard = if destination.lifecycle == HookGatewayLifecycle::Existing
&& !command.forward_only
{
let install_host = command.agent.install_arg();
let Some(generation_file) = command.generation_file.clone() else {
return handle_hook_error(
CliError::Launch(format!(
"persistent {} hook is missing its install-generation fence; run `nemo-relay install {install_host} --force`",
command.agent.label()
)),
fail_closed,
);
};
let Some(generation_token) = command.generation_token.as_deref() else {
return handle_hook_error(
CliError::Launch(format!(
"persistent {} hook is missing its expected install-generation identity; run `nemo-relay install {install_host} --force`",
command.agent.label()
)),
fail_closed,
);
};
match InstallGeneration::capture_guarded_expected(generation_file, generation_token) {
Ok((_generation, guard)) => Some(guard),
Err(error) => return handle_hook_error(CliError::Launch(error), fail_closed),
}
} else {
None
let _generation_guard = match capture_generation_guard(&command, destination.lifecycle) {
Ok(guard) => guard,
Err(error) => return handle_hook_error(error, fail_closed),
};
let input = match read_hook_payload(persistent.as_ref().map_or(
crate::configuration::DEFAULT_MAX_HOOK_PAYLOAD_BYTES,
Expand Down Expand Up @@ -126,6 +98,39 @@ pub(crate) async fn hook_forward(command: HookForwardRequest) -> Result<(), CliE
handle_hook_forward_response(response, fail_closed).await
}

fn persistent_gateway(
destination: &super::destination::HookDestination,
) -> Result<Option<crate::bootstrap::PluginGatewaySpec>, CliError> {
(destination.lifecycle != HookGatewayLifecycle::Transparent)
.then(|| recovery_plan(&destination.gateway_url))
.transpose()
}

fn capture_generation_guard(
command: &HookForwardRequest,
lifecycle: HookGatewayLifecycle,
) -> Result<Option<ActiveGenerationGuard>, CliError> {
if lifecycle != HookGatewayLifecycle::Existing || command.forward_only {
return Ok(None);
}
let install_host = command.agent.install_arg();
let generation_file = command.generation_file.clone().ok_or_else(|| {
CliError::Launch(format!(
"persistent {} hook is missing its install-generation fence; run `nemo-relay install {install_host} --force`",
command.agent.label()
))
})?;
let generation_token = command.generation_token.as_deref().ok_or_else(|| {
CliError::Launch(format!(
"persistent {} hook is missing its expected install-generation identity; run `nemo-relay install {install_host} --force`",
command.agent.label()
))
})?;
InstallGeneration::capture_guarded_expected(generation_file, generation_token)
.map(|(_generation, guard)| Some(guard))
.map_err(CliError::Launch)
}

fn handle_hook_error(error: CliError, fail_closed: bool) -> Result<(), CliError> {
eprintln!("nemo-relay hook forward failed: {error}");
if fail_closed { Err(error) } else { Ok(()) }
Expand Down
Loading
Loading