From 745ad8a4411f8a677642b1f8bb2e18625f484d60 Mon Sep 17 00:00:00 2001 From: Sharang Parnerkar Date: Wed, 25 Mar 2026 16:26:09 +0000 Subject: [PATCH] fix: check Gitea API response status and fallback for PR reviews (#47) ## Summary - Add HTTP response status checking to all Gitea tracker methods that were silently swallowing errors - Add fallback in create_pr_review: if inline comments fail, retry as plain PR comment ## Test plan - [ ] Deploy and trigger a PR review, check logs for actual error details - [ ] Verify fallback posts summary comment when inline comments fail Co-authored-by: Sharang Parnerkar Co-authored-by: Sharang Parnerkar <30073382+mighty840@users.noreply.github.com> Reviewed-on: https://gitea.meghsakha.com/sharang/compliance-scanner-agent/pulls/47 --- Cargo.lock | 6 +- compliance-agent/src/trackers/gitea.rs | 67 ++++++++++++++++++- .../src/infrastructure/mcp.rs | 66 ++++++++++++++++++ compliance-dashboard/src/pages/mcp_servers.rs | 13 +++- compliance-mcp/src/main.rs | 4 +- 5 files changed, 148 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3fbe410..378dada 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4699,9 +4699,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.9" +version = "0.103.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7df23109aa6c1567d1c575b9952556388da57401e4ace1d15f79eedad0d8f53" +checksum = "df33b2b81ac578cabaf06b89b0631153a3f416b0a886e8a7a1707fb51abbd1ef" dependencies = [ "ring", "rustls-pki-types", @@ -5171,7 +5171,7 @@ version = "0.8.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c1c97747dbf44bb1ca44a561ece23508e99cb592e862f22222dcf42f51d1e451" dependencies = [ - "heck 0.5.0", + "heck 0.4.1", "proc-macro2", "quote", "syn", diff --git a/compliance-agent/src/trackers/gitea.rs b/compliance-agent/src/trackers/gitea.rs index 8d3debb..9f1667f 100644 --- a/compliance-agent/src/trackers/gitea.rs +++ b/compliance-agent/src/trackers/gitea.rs @@ -98,7 +98,8 @@ impl IssueTracker for GiteaTracker { _ => "open", }; - self.http + let resp = self + .http .patch(&url) .header( "Authorization", @@ -109,6 +110,14 @@ impl IssueTracker for GiteaTracker { .await .map_err(|e| CoreError::IssueTracker(format!("Gitea update issue failed: {e}")))?; + if !resp.status().is_success() { + let status = resp.status(); + let text = resp.text().await.unwrap_or_default(); + return Err(CoreError::IssueTracker(format!( + "Gitea update issue returned {status}: {text}" + ))); + } + Ok(()) } @@ -123,7 +132,8 @@ impl IssueTracker for GiteaTracker { "/repos/{owner}/{repo}/issues/{external_id}/comments" )); - self.http + let resp = self + .http .post(&url) .header( "Authorization", @@ -134,6 +144,14 @@ impl IssueTracker for GiteaTracker { .await .map_err(|e| CoreError::IssueTracker(format!("Gitea add comment failed: {e}")))?; + if !resp.status().is_success() { + let status = resp.status(); + let text = resp.text().await.unwrap_or_default(); + return Err(CoreError::IssueTracker(format!( + "Gitea add comment returned {status}: {text}" + ))); + } + Ok(()) } @@ -158,7 +176,8 @@ impl IssueTracker for GiteaTracker { }) .collect(); - self.http + let resp = self + .http .post(&url) .header( "Authorization", @@ -173,6 +192,48 @@ impl IssueTracker for GiteaTracker { .await .map_err(|e| CoreError::IssueTracker(format!("Gitea PR review failed: {e}")))?; + if !resp.status().is_success() { + let status = resp.status(); + let text = resp.text().await.unwrap_or_default(); + + // If inline comments caused the failure, retry with just the summary body + if !comments.is_empty() { + tracing::warn!( + "Gitea PR review with inline comments failed ({status}): {text}, retrying as plain comment" + ); + let fallback_url = self.api_url(&format!( + "/repos/{owner}/{repo}/issues/{pr_number}/comments" + )); + let fallback_resp = self + .http + .post(&fallback_url) + .header( + "Authorization", + format!("token {}", self.token.expose_secret()), + ) + .json(&serde_json::json!({ "body": body })) + .send() + .await + .map_err(|e| { + CoreError::IssueTracker(format!("Gitea PR comment fallback failed: {e}")) + })?; + + if !fallback_resp.status().is_success() { + let fb_status = fallback_resp.status(); + let fb_text = fallback_resp.text().await.unwrap_or_default(); + return Err(CoreError::IssueTracker(format!( + "Gitea PR comment fallback returned {fb_status}: {fb_text}" + ))); + } + + return Ok(()); + } + + return Err(CoreError::IssueTracker(format!( + "Gitea PR review returned {status}: {text}" + ))); + } + Ok(()) } diff --git a/compliance-dashboard/src/infrastructure/mcp.rs b/compliance-dashboard/src/infrastructure/mcp.rs index 3844682..3c9a3a3 100644 --- a/compliance-dashboard/src/infrastructure/mcp.rs +++ b/compliance-dashboard/src/infrastructure/mcp.rs @@ -113,6 +113,72 @@ pub async fn add_mcp_server( Ok(()) } +/// Probe each MCP server's health endpoint and update status in MongoDB. +#[server] +pub async fn refresh_mcp_status() -> Result<(), ServerFnError> { + use chrono::Utc; + use compliance_core::models::McpServerStatus; + use mongodb::bson::doc; + + let state: super::server_state::ServerState = + dioxus_fullstack::FullstackContext::extract().await?; + + let mut cursor = state + .db + .mcp_servers() + .find(doc! {}) + .await + .map_err(|e| ServerFnError::new(e.to_string()))?; + + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(5)) + .build() + .map_err(|e| ServerFnError::new(e.to_string()))?; + + while cursor + .advance() + .await + .map_err(|e| ServerFnError::new(e.to_string()))? + { + let server: compliance_core::models::McpServerConfig = cursor + .deserialize_current() + .map_err(|e| ServerFnError::new(e.to_string()))?; + + let Some(oid) = server.id else { continue }; + + // Derive health URL from the endpoint (replace trailing /mcp with /health) + let health_url = if server.endpoint_url.ends_with("/mcp") { + format!( + "{}health", + &server.endpoint_url[..server.endpoint_url.len() - 3] + ) + } else { + format!("{}/health", server.endpoint_url.trim_end_matches('/')) + }; + + let new_status = match client.get(&health_url).send().await { + Ok(resp) if resp.status().is_success() => McpServerStatus::Running, + _ => McpServerStatus::Stopped, + }; + + let status_bson = match bson::to_bson(&new_status) { + Ok(b) => b, + Err(_) => continue, + }; + + let _ = state + .db + .mcp_servers() + .update_one( + doc! { "_id": oid }, + doc! { "$set": { "status": status_bson, "updated_at": Utc::now().to_rfc3339() } }, + ) + .await; + } + + Ok(()) +} + #[server] pub async fn delete_mcp_server(server_id: String) -> Result<(), ServerFnError> { use mongodb::bson::doc; diff --git a/compliance-dashboard/src/pages/mcp_servers.rs b/compliance-dashboard/src/pages/mcp_servers.rs index 583dab1..2a47f37 100644 --- a/compliance-dashboard/src/pages/mcp_servers.rs +++ b/compliance-dashboard/src/pages/mcp_servers.rs @@ -5,7 +5,7 @@ use dioxus_free_icons::Icon; use crate::components::page_header::PageHeader; use crate::components::toast::{ToastType, Toasts}; use crate::infrastructure::mcp::{ - add_mcp_server, delete_mcp_server, fetch_mcp_servers, regenerate_mcp_token, + add_mcp_server, delete_mcp_server, fetch_mcp_servers, refresh_mcp_status, regenerate_mcp_token, }; #[component] @@ -22,6 +22,17 @@ pub fn McpServersPage() -> Element { let mut new_mongo_uri = use_signal(String::new); let mut new_mongo_db = use_signal(String::new); + // Probe health of all MCP servers on page load, then refresh the list + let mut refreshing = use_signal(|| true); + use_effect(move || { + spawn(async move { + refreshing.set(true); + let _ = refresh_mcp_status().await; + servers.restart(); + refreshing.set(false); + }); + }); + // Track which server's token is visible let mut visible_token: Signal> = use_signal(|| None); // Track which server is pending delete confirmation diff --git a/compliance-mcp/src/main.rs b/compliance-mcp/src/main.rs index df152c7..7bcfd96 100644 --- a/compliance-mcp/src/main.rs +++ b/compliance-mcp/src/main.rs @@ -41,7 +41,9 @@ async fn main() -> Result<(), Box> { StreamableHttpServerConfig::default(), ); - let router = axum::Router::new().nest_service("/mcp", service); + let router = axum::Router::new() + .route("/health", axum::routing::get(|| async { "ok" })) + .nest_service("/mcp", service); let listener = tokio::net::TcpListener::bind(("0.0.0.0", port)).await?; tracing::info!("MCP HTTP server listening on 0.0.0.0:{port}"); axum::serve(listener, router).await?;