From d9223e3f5218ececd76c4f7762b4f10f7f3f25b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Wed, 4 Mar 2026 18:18:54 +0100 Subject: [PATCH 1/2] fix(quality): reduce cognitive complexity and fix shell script issues --- .../web-quality-audit/scripts/analyze.sh | 13 +- clients/agent-runtime/src/agent/agent.rs | 244 ++++++++++-------- clients/agent-runtime/src/gateway/mod.rs | 185 +++++++------ clients/agent-runtime/src/main.rs | 62 ++--- 4 files changed, 287 insertions(+), 217 deletions(-) diff --git a/.agents/skills/web-quality-audit/scripts/analyze.sh b/.agents/skills/web-quality-audit/scripts/analyze.sh index 37bedf7bf..a91bac0b7 100644 --- a/.agents/skills/web-quality-audit/scripts/analyze.sh +++ b/.agents/skills/web-quality-audit/scripts/analyze.sh @@ -8,9 +8,10 @@ usage() { echo "Usage: $0 " >&2 echo "Analyzes HTML files for web quality issues." >&2 exit 1 + return 1 } -if [ -z "$1" ]; then +if [[ -z "$1" ]]; then usage fi @@ -56,14 +57,16 @@ analyze_html() { if grep -qE 'http://' "$file"; then WARNINGS+=("$file: Contains non-HTTPS URLs") fi + + return 0 } # Process files -if [ -d "$TARGET" ]; then +if [[ -d "$TARGET" ]]; then find "$TARGET" -name "*.html" -o -name "*.htm" | while read -r file; do analyze_html "$file" done -elif [ -f "$TARGET" ]; then +elif [[ -f "$TARGET" ]]; then analyze_html "$TARGET" else echo "Error: $TARGET is not a valid file or directory" >&2 @@ -74,14 +77,14 @@ fi echo '{' echo ' "issues": [' for i in "${!ISSUES[@]}"; do - if [ $i -gt 0 ]; then echo ','; fi + if [[ $i -gt 0 ]]; then echo ','; fi echo -n " \"${ISSUES[$i]}\"" done echo '' echo ' ],' echo ' "warnings": [' for i in "${!WARNINGS[@]}"; do - if [ $i -gt 0 ]; then echo ','; fi + if [[ $i -gt 0 ]]; then echo ','; fi echo -n " \"${WARNINGS[$i]}\"" done echo '' diff --git a/clients/agent-runtime/src/agent/agent.rs b/clients/agent-runtime/src/agent/agent.rs index db66e4c5a..d5f959457 100755 --- a/clients/agent-runtime/src/agent/agent.rs +++ b/clients/agent-runtime/src/agent/agent.rs @@ -4,8 +4,8 @@ use crate::agent::dispatcher::{ }; use crate::agent::memory_loader::{DefaultMemoryLoader, MemoryLoader}; use crate::agent::mission::{ - MissionCoordinator, MissionOutcome, MissionPlan, MissionResumeMetadata, MissionState, - MissionTerminationReason, + MissionCheckpoint, MissionCoordinator, MissionOutcome, MissionPlan, MissionResumeMetadata, + MissionState, MissionTerminationReason, }; use crate::agent::prompt::{PromptContext, SystemPromptBuilder}; use crate::config::Config; @@ -829,6 +829,7 @@ impl Agent { .checkpoints .get(checkpoint_position) .map(|checkpoint| checkpoint.index); + if let Err(reason) = coordinator.enforce_pre_checkpoint() { return self.terminated_mission_outcome( coordinator, @@ -841,137 +842,176 @@ impl Agent { } let checkpoint = plan.checkpoints[checkpoint_position].clone(); - let checkpoint_start = Instant::now(); - self.observer - .record_event(&ObserverEvent::MissionCheckpointProgress { - mission_id: mission_id.to_string(), - checkpoint_index: checkpoint.index, - status: "started".to_string(), - duration: std::time::Duration::ZERO, - }); - match self.execute_mission_checkpoint(&checkpoint).await { - Ok(_) => { - let checkpoint_elapsed_ms = - u64::try_from(checkpoint_start.elapsed().as_millis()).unwrap_or(u64::MAX); - if let Err(reason) = - coordinator.record_checkpoint_accounting(checkpoint_elapsed_ms, 0) - { - return self.terminated_mission_outcome( + let (next_plan, outcome, increment) = self + .process_mission_checkpoint( + coordinator, + mission_id, + mission_start, + plan, + &checkpoint, + ) + .await?; + + plan = next_plan; + if let Some(outcome) = outcome { + return Ok(outcome); + } + + if increment { + checkpoint_position = checkpoint_position.saturating_add(1); + } + } + + coordinator + .transition(MissionState::Completed) + .map_err(Self::mission_error)?; + let resume_metadata = coordinator.resume_metadata().map_err(Self::mission_error)?; + let checkpoints_completed = resume_metadata + .last_successful_checkpoint + .map_or(0, |index| index + 1); + self.observer + .record_event(&ObserverEvent::MissionCompleted { + mission_id: mission_id.to_string(), + checkpoints_completed, + duration: mission_start.elapsed(), + }); + + Ok(MissionOutcome { + mission_id: mission_id.to_string(), + state: MissionState::Completed, + termination: None, + checkpoints_completed, + resume_metadata, + }) + } + + #[allow(clippy::too_many_arguments)] + async fn process_mission_checkpoint( + &mut self, + coordinator: &MissionCoordinator, + mission_id: &str, + mission_start: Instant, + mut plan: MissionPlan, + checkpoint: &MissionCheckpoint, + ) -> Result<(MissionPlan, Option, bool)> { + let checkpoint_start = Instant::now(); + self.observer + .record_event(&ObserverEvent::MissionCheckpointProgress { + mission_id: mission_id.to_string(), + checkpoint_index: checkpoint.index, + status: "started".to_string(), + duration: std::time::Duration::ZERO, + }); + + match self.execute_mission_checkpoint(checkpoint).await { + Ok(_) => { + let checkpoint_elapsed_ms = + u64::try_from(checkpoint_start.elapsed().as_millis()).unwrap_or(u64::MAX); + if let Err(reason) = + coordinator.record_checkpoint_accounting(checkpoint_elapsed_ms, 0) + { + return Ok(( + plan, + Some(self.terminated_mission_outcome( coordinator, mission_id, reason, mission_start, Some(checkpoint.index), false, - ); - } - - coordinator - .record_checkpoint_success(checkpoint.index) - .map_err(Self::mission_error)?; - self.observer - .record_event(&ObserverEvent::MissionCheckpointProgress { - mission_id: mission_id.to_string(), - checkpoint_index: checkpoint.index, - status: "completed".to_string(), - duration: checkpoint_start.elapsed(), - }); - checkpoint_position = checkpoint_position.saturating_add(1); + )?), + false, + )); } - Err(error) => { - let checkpoint_elapsed_ms = - u64::try_from(checkpoint_start.elapsed().as_millis()).unwrap_or(u64::MAX); - if let Err(reason) = - coordinator.record_checkpoint_accounting(checkpoint_elapsed_ms, 0) - { - return self.terminated_mission_outcome( + + coordinator + .record_checkpoint_success(checkpoint.index) + .map_err(Self::mission_error)?; + self.observer + .record_event(&ObserverEvent::MissionCheckpointProgress { + mission_id: mission_id.to_string(), + checkpoint_index: checkpoint.index, + status: "completed".to_string(), + duration: checkpoint_start.elapsed(), + }); + Ok((plan, None, true)) + } + Err(error) => { + let checkpoint_elapsed_ms = + u64::try_from(checkpoint_start.elapsed().as_millis()).unwrap_or(u64::MAX); + if let Err(reason) = + coordinator.record_checkpoint_accounting(checkpoint_elapsed_ms, 0) + { + return Ok(( + plan, + Some(self.terminated_mission_outcome( coordinator, mission_id, reason, mission_start, Some(checkpoint.index), false, - ); - } + )?), + false, + )); + } - let reason_text = error.to_string(); - self.observer.record_event(&ObserverEvent::Error { - component: "mission".to_string(), - message: Self::sanitize_observer_error( - &reason_text, - &[("X-Mission-Context".to_string(), "checkpoint".to_string())], - &reason_text, - ), + let reason_text = error.to_string(); + self.observer.record_event(&ObserverEvent::Error { + component: "mission".to_string(), + message: Self::sanitize_observer_error( + &reason_text, + &[("X-Mission-Context".to_string(), "checkpoint".to_string())], + &reason_text, + ), + }); + let recoverable = coordinator.should_replan(&reason_text); + self.observer + .record_event(&ObserverEvent::MissionCheckpointProgress { + mission_id: mission_id.to_string(), + checkpoint_index: checkpoint.index, + status: "failed".to_string(), + duration: checkpoint_start.elapsed(), }); - let recoverable = coordinator.should_replan(&reason_text); + coordinator + .record_checkpoint_failure(checkpoint.index, reason_text.clone(), recoverable) + .map_err(Self::mission_error)?; + + if recoverable { + coordinator + .transition(MissionState::Replanning) + .map_err(Self::mission_error)?; self.observer .record_event(&ObserverEvent::MissionCheckpointProgress { mission_id: mission_id.to_string(), checkpoint_index: checkpoint.index, - status: "failed".to_string(), - duration: checkpoint_start.elapsed(), + status: "replanning".to_string(), + duration: std::time::Duration::ZERO, }); + plan = self.replan_after_failure(&plan, checkpoint.index, &reason_text); coordinator - .record_checkpoint_failure( - checkpoint.index, - reason_text.clone(), - recoverable, - ) + .transition(MissionState::Planned) .map_err(Self::mission_error)?; + coordinator + .transition(MissionState::Active) + .map_err(Self::mission_error)?; + return Ok((plan, None, false)); + } - if recoverable { - coordinator - .transition(MissionState::Replanning) - .map_err(Self::mission_error)?; - self.observer - .record_event(&ObserverEvent::MissionCheckpointProgress { - mission_id: mission_id.to_string(), - checkpoint_index: checkpoint.index, - status: "replanning".to_string(), - duration: std::time::Duration::ZERO, - }); - plan = self.replan_after_failure(&plan, checkpoint.index, &reason_text); - coordinator - .transition(MissionState::Planned) - .map_err(Self::mission_error)?; - coordinator - .transition(MissionState::Active) - .map_err(Self::mission_error)?; - continue; - } - return self.terminated_mission_outcome( + Ok(( + plan, + Some(self.terminated_mission_outcome( coordinator, mission_id, Self::mission_termination_reason_from_error(&reason_text), mission_start, Some(checkpoint.index), false, - ); - } + )?), + false, + )) } } - - coordinator - .transition(MissionState::Completed) - .map_err(Self::mission_error)?; - let resume_metadata = coordinator.resume_metadata().map_err(Self::mission_error)?; - let checkpoints_completed = resume_metadata - .last_successful_checkpoint - .map_or(0, |index| index + 1); - self.observer - .record_event(&ObserverEvent::MissionCompleted { - mission_id: mission_id.to_string(), - checkpoints_completed, - duration: mission_start.elapsed(), - }); - - Ok(MissionOutcome { - mission_id: mission_id.to_string(), - state: MissionState::Completed, - termination: None, - checkpoints_completed, - resume_metadata, - }) } pub async fn run_mission( diff --git a/clients/agent-runtime/src/gateway/mod.rs b/clients/agent-runtime/src/gateway/mod.rs index 4b5aa81ea..49ee48cdf 100755 --- a/clients/agent-runtime/src/gateway/mod.rs +++ b/clients/agent-runtime/src/gateway/mod.rs @@ -448,65 +448,80 @@ fn compare_gateway_fields( fields: &mut Vec<&'static str>, ) { if let Some(gw) = gateway { - compare_primitive(gw.port, cfg.gateway.port, "gateway.port", fields); - compare_trimmed_string( - gw.host.as_ref(), - Some(&cfg.gateway.host), - "gateway.host", - fields, - ); - compare_primitive( - gw.require_pairing, - cfg.gateway.require_pairing, - "gateway.require_pairing", - fields, - ); - compare_primitive( - gw.allow_public_bind, - cfg.gateway.allow_public_bind, - "gateway.allow_public_bind", - fields, - ); - compare_primitive( - gw.pair_rate_limit_per_minute, - cfg.gateway.pair_rate_limit_per_minute, - "gateway.pair_rate_limit_per_minute", - fields, - ); - compare_primitive( - gw.webhook_rate_limit_per_minute, - cfg.gateway.webhook_rate_limit_per_minute, - "gateway.webhook_rate_limit_per_minute", - fields, - ); - compare_primitive( - gw.trust_forwarded_headers, - cfg.gateway.trust_forwarded_headers, - "gateway.trust_forwarded_headers", - fields, - ); + compare_gateway_basic_fields(gw, cfg, fields); + compare_gateway_limits_fields(gw, cfg, fields); + } +} - if let Some(max_keys) = gw.rate_limit_max_keys { - let normalized = normalize_max_keys(max_keys, cfg.gateway.rate_limit_max_keys); - if normalized != cfg.gateway.rate_limit_max_keys { - fields.push("gateway.rate_limit_max_keys"); - } +fn compare_gateway_basic_fields( + gw: &AdminGatewayPatch, + cfg: &Config, + fields: &mut Vec<&'static str>, +) { + compare_primitive(gw.port, cfg.gateway.port, "gateway.port", fields); + compare_trimmed_string( + gw.host.as_ref(), + Some(&cfg.gateway.host), + "gateway.host", + fields, + ); + compare_primitive( + gw.require_pairing, + cfg.gateway.require_pairing, + "gateway.require_pairing", + fields, + ); + compare_primitive( + gw.allow_public_bind, + cfg.gateway.allow_public_bind, + "gateway.allow_public_bind", + fields, + ); + compare_primitive( + gw.pair_rate_limit_per_minute, + cfg.gateway.pair_rate_limit_per_minute, + "gateway.pair_rate_limit_per_minute", + fields, + ); + compare_primitive( + gw.webhook_rate_limit_per_minute, + cfg.gateway.webhook_rate_limit_per_minute, + "gateway.webhook_rate_limit_per_minute", + fields, + ); + compare_primitive( + gw.trust_forwarded_headers, + cfg.gateway.trust_forwarded_headers, + "gateway.trust_forwarded_headers", + fields, + ); +} + +fn compare_gateway_limits_fields( + gw: &AdminGatewayPatch, + cfg: &Config, + fields: &mut Vec<&'static str>, +) { + if let Some(max_keys) = gw.rate_limit_max_keys { + let normalized = normalize_max_keys(max_keys, cfg.gateway.rate_limit_max_keys); + if normalized != cfg.gateway.rate_limit_max_keys { + fields.push("gateway.rate_limit_max_keys"); } - if let Some(ttl) = gw.idempotency_ttl_secs { - let normalized_ttl = if ttl == 0 { - cfg.gateway.idempotency_ttl_secs - } else { - ttl - }; - if normalized_ttl != cfg.gateway.idempotency_ttl_secs { - fields.push("gateway.idempotency_ttl_secs"); - } + } + if let Some(ttl) = gw.idempotency_ttl_secs { + let normalized_ttl = if ttl == 0 { + cfg.gateway.idempotency_ttl_secs + } else { + ttl + }; + if normalized_ttl != cfg.gateway.idempotency_ttl_secs { + fields.push("gateway.idempotency_ttl_secs"); } - if let Some(max_keys) = gw.idempotency_max_keys { - let normalized = normalize_max_keys(max_keys, cfg.gateway.idempotency_max_keys); - if normalized != cfg.gateway.idempotency_max_keys { - fields.push("gateway.idempotency_max_keys"); - } + } + if let Some(max_keys) = gw.idempotency_max_keys { + let normalized = normalize_max_keys(max_keys, cfg.gateway.idempotency_max_keys); + if normalized != cfg.gateway.idempotency_max_keys { + fields.push("gateway.idempotency_max_keys"); } } } @@ -553,32 +568,40 @@ fn compare_webhook_fields( } } - if let Some(secret) = wh.secret.as_ref() { - match secret { - AdminSecretUpdate::Unchanged => {} - AdminSecretUpdate::Clear => { - if cfg - .channels_config - .webhook - .as_ref() - .and_then(|w| w.secret.as_ref()) - .map(|value| !value.trim().is_empty()) - .unwrap_or(false) - { - fields.push("webhook.secret"); - } + compare_webhook_secret_field(wh, cfg, fields); + } +} + +fn compare_webhook_secret_field( + wh: &AdminWebhookPatch, + cfg: &Config, + fields: &mut Vec<&'static str>, +) { + if let Some(secret) = wh.secret.as_ref() { + match secret { + AdminSecretUpdate::Unchanged => {} + AdminSecretUpdate::Clear => { + if cfg + .channels_config + .webhook + .as_ref() + .and_then(|w| w.secret.as_ref()) + .map(|value| !value.trim().is_empty()) + .unwrap_or(false) + { + fields.push("webhook.secret"); } - AdminSecretUpdate::Replace { value } => { - let next = value.trim(); - let current = cfg - .channels_config - .webhook - .as_ref() - .and_then(|w| w.secret.as_deref()) - .unwrap_or(""); - if next != current { - fields.push("webhook.secret"); - } + } + AdminSecretUpdate::Replace { value } => { + let next = value.trim(); + let current = cfg + .channels_config + .webhook + .as_ref() + .and_then(|w| w.secret.as_deref()) + .unwrap_or(""); + if next != current { + fields.push("webhook.secret"); } } } diff --git a/clients/agent-runtime/src/main.rs b/clients/agent-runtime/src/main.rs index 5a7d74837..917e88cda 100644 --- a/clients/agent-runtime/src/main.rs +++ b/clients/agent-runtime/src/main.rs @@ -667,35 +667,7 @@ async fn handle_cli_command(command: Commands, config: Config) -> Result<()> { }, Commands::Providers => { - let providers = providers::list_providers(); - let current = config - .default_provider - .as_deref() - .unwrap_or("openrouter") - .trim() - .to_ascii_lowercase(); - println!("Supported providers ({} total):\n", providers.len()); - println!(" ID (use in config) DESCRIPTION"); - println!(" ─────────────────── ───────────"); - for p in &providers { - let is_active = p.name.eq_ignore_ascii_case(¤t) - || p.aliases - .iter() - .any(|alias| alias.eq_ignore_ascii_case(¤t)); - let marker = if is_active { " (active)" } else { "" }; - let local_tag = if p.local { " [local]" } else { "" }; - let aliases = if p.aliases.is_empty() { - String::new() - } else { - format!(" (aliases: {})", p.aliases.join(", ")) - }; - println!( - " {:<19} {}{}{}{}", - p.name, p.display_name, local_tag, marker, aliases - ); - } - println!("\n custom: Any OpenAI-compatible endpoint"); - println!(" anthropic-custom: Any Anthropic-compatible endpoint"); + handle_providers_command(config); Ok(()) } @@ -733,6 +705,38 @@ async fn handle_cli_command(command: Commands, config: Config) -> Result<()> { } } +fn handle_providers_command(config: Config) { + let providers = providers::list_providers(); + let current = config + .default_provider + .as_deref() + .unwrap_or("openrouter") + .trim() + .to_ascii_lowercase(); + println!("Supported providers ({} total):\n", providers.len()); + println!(" ID (use in config) DESCRIPTION"); + println!(" ─────────────────── ───────────"); + for p in &providers { + let is_active = p.name.eq_ignore_ascii_case(¤t) + || p.aliases + .iter() + .any(|alias| alias.eq_ignore_ascii_case(¤t)); + let marker = if is_active { " (active)" } else { "" }; + let local_tag = if p.local { " [local]" } else { "" }; + let aliases = if p.aliases.is_empty() { + String::new() + } else { + format!(" (aliases: {})", p.aliases.join(", ")) + }; + println!( + " {:<19} {}{}{}{}", + p.name, p.display_name, local_tag, marker, aliases + ); + } + println!("\n custom: Any OpenAI-compatible endpoint"); + println!(" anthropic-custom: Any Anthropic-compatible endpoint"); +} + async fn handle_agent_command( config: Config, message: Option, From 21caeac49caa9fe9ffb2957a22b6ed029e30bb90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Wed, 4 Mar 2026 18:34:17 +0100 Subject: [PATCH 2/2] fix(quality): improve shell script robustness and avoid unreachable code --- .agents/skills/web-quality-audit/scripts/analyze.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.agents/skills/web-quality-audit/scripts/analyze.sh b/.agents/skills/web-quality-audit/scripts/analyze.sh index a91bac0b7..045a72233 100644 --- a/.agents/skills/web-quality-audit/scripts/analyze.sh +++ b/.agents/skills/web-quality-audit/scripts/analyze.sh @@ -8,7 +8,6 @@ usage() { echo "Usage: $0 " >&2 echo "Analyzes HTML files for web quality issues." >&2 exit 1 - return 1 } if [[ -z "$1" ]]; then @@ -63,9 +62,9 @@ analyze_html() { # Process files if [[ -d "$TARGET" ]]; then - find "$TARGET" -name "*.html" -o -name "*.htm" | while read -r file; do + while IFS= read -r -d '' file; do analyze_html "$file" - done + done < <(find "$TARGET" \( -name "*.html" -o -name "*.htm" \) -print0) elif [[ -f "$TARGET" ]]; then analyze_html "$TARGET" else