fix(security): address automated code review findings

BLOCKER fixes:
- Implement create_azuredevops_workitem instead of returning a stub error,
  reusing the existing create_work_item integration helper and writing an
  audit-log entry on success.
- Log kill failures in PtySession::Drop so leaked child processes surface
  in tracing rather than being silently swallowed.
- Add explicit PTY cleanup on every exit path of run_session_io (process
  exit, read error, write error, resize error, terminate command).
- Treat PTY resize failures as fatal: emit terminal-error to the frontend
  and break the session loop instead of just warning.

WARNING fixes:
- Remove the dead extract_json_path_value helper from commands/kube.rs.
- Wrap temp kubeconfig files in commands/metrics.rs in an RAII guard
  (TempKubeconfig) so they're removed on early-return / panic paths.
- Wrap temp kubeconfig files in commands/shell.rs PTY-session starters
  in a disarmable RAII guard (KubeconfigGuard); if kubectl resolution
  fails we no longer leak the file.
- Drop the `clear;` prefix from the kubectl-exec shell fallback so
  containers without `clear`/`tput` don't print a confusing error.

SUGGESTION fixes:
- Document why node CPU/memory percentages are 0.0 in metrics::client
  and link the gap to future work fetching node capacity.
- Add a module-level doc comment to AppState describing the
  synchronization expectations (std vs tokio Mutex) for each public
  field, and warn against holding std::sync MutexGuards across .await.

Verified: cargo fmt --check, cargo clippy -- -D warnings, and
cargo test (377 passed, 6 ignored) all pass.

src-tauri/src/commands/integrations.rs

@@ -277,11 +277,116 @@ pub async fn test_azuredevops_connection(
 
 #[tauri::command]
 pub async fn create_azuredevops_workitem(
-    _issue_id: String,
-    _project: String,
-    _config: serde_json::Value,
+    issue_id: String,
+    project: String,
+    config: serde_json::Value,
+    app_state: State<'_, AppState>,
 ) -> Result<TicketResult, String> {
-    Err("Integrations available in v0.2. Please update to the latest version.".to_string())
+    // Extract optional configuration values from the config payload.
+    // The frontend may pass: base_url, work_item_type, severity. All have safe defaults.
+    let base_url = config
+        .get("base_url")
+        .and_then(|v| v.as_str())
+        .map(String::from);
+    let work_item_type = config
+        .get("work_item_type")
+        .and_then(|v| v.as_str())
+        .unwrap_or("Bug")
+        .to_string();
+    let severity = config
+        .get("severity")
+        .and_then(|v| v.as_str())
+        .unwrap_or("3 - Medium")
+        .to_string();
+
+    // Look up issue title/description from the database to use as work-item content.
+    let (title, description, base_url_resolved) = {
+        let db = app_state
+            .db
+            .lock()
+            .map_err(|e| format!("Failed to lock database: {e}"))?;
+
+        let (title, description) = db
+            .query_row(
+                "SELECT title, description FROM issues WHERE id = ?1",
+                rusqlite::params![issue_id],
+                |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)),
+            )
+            .map_err(|e| format!("Failed to load issue {issue_id}: {e}"))?;
+
+        // Fall back to stored integration_config base_url if caller did not provide one.
+        let resolved = match base_url {
+            Some(url) => url,
+            None => db
+                .query_row(
+                    "SELECT base_url FROM integration_config WHERE service = 'azuredevops'",
+                    [],
+                    |row| row.get::<_, String>(0),
+                )
+                .map_err(|e| format!("Azure DevOps base URL not configured: {e}"))?,
+        };
+
+        (title, description, resolved)
+    };
+
+    // Retrieve and decrypt stored access token.
+    let access_token = {
+        let db = app_state
+            .db
+            .lock()
+            .map_err(|e| format!("Failed to lock database: {e}"))?;
+
+        let encrypted: String = db
+            .query_row(
+                "SELECT encrypted_token FROM credentials WHERE service = 'azuredevops'",
+                [],
+                |row| row.get(0),
+            )
+            .map_err(|e| {
+                format!("Azure DevOps credentials not found. Please authenticate first: {e}")
+            })?;
+
+        crate::integrations::auth::decrypt_token(&encrypted)?
+    };
+
+    let ado_config = crate::integrations::azuredevops::AzureDevOpsConfig {
+        organization_url: base_url_resolved,
+        project,
+        access_token,
+    };
+
+    let result = crate::integrations::azuredevops::create_work_item(
+        &ado_config,
+        &title,
+        &description,
+        &work_item_type,
+        &severity,
+    )
+    .await?;
+
+    // Audit log the external publish action.
+    {
+        let db = app_state
+            .db
+            .lock()
+            .map_err(|e| format!("Failed to lock database: {e}"))?;
+        let details = serde_json::json!({
+            "issue_id": issue_id,
+            "work_item_id": result.id,
+            "work_item_type": work_item_type,
+        });
+        if let Err(e) = crate::audit::log::write_audit_event(
+            &db,
+            "ado_workitem_created",
+            "issue",
+            &issue_id,
+            &details.to_string(),
+        ) {
+            tracing::warn!("Failed to write audit event for ADO workitem creation: {e}");
+        }
+    }
+
+    Ok(result)
 }
 
 // ─── OAuth2 Commands ────────────────────────────────────────────────────────

src-tauri/src/commands/kube.rs

@@ -6418,66 +6418,6 @@ pub async fn list_custom_resources(
     parse_custom_resources_json(&output_str)
 }
 
-/// Simple JSONPath-like extractor for custom resource fields.
-/// Supports basic paths like .status.phase, .spec.replicas, .metadata.labels['app']
-#[allow(dead_code)]
-fn extract_json_path_value(item: &Value, json_path: &str) -> String {
-    // Remove leading dot if present
-    let path = json_path.strip_prefix('.').unwrap_or(json_path);
-
-    // Split path by dots and traverse
-    let parts: Vec<&str> = path.split('.').collect();
-    let mut current = item;
-
-    for part in parts {
-        // Handle array access like status[0] or map access like labels['app']
-        if let Some(bracket_start) = part.find('[') {
-            let field = &part[..bracket_start];
-            current = match current.get(field) {
-                Some(v) => v,
-                None => return "N/A".to_string(),
-            };
-
-            // Extract index or key from brackets
-            if let Some(bracket_end) = part.find(']') {
-                let accessor = &part[bracket_start + 1..bracket_end];
-                current = if accessor.starts_with('\'') || accessor.starts_with('"') {
-                    // Map key access
-                    let key = accessor.trim_matches(|c| c == '\'' || c == '"');
-                    match current.get(key) {
-                        Some(v) => v,
-                        None => return "N/A".to_string(),
-                    }
-                } else {
-                    // Array index access
-                    match accessor.parse::<usize>() {
-                        Ok(idx) => match current.as_array().and_then(|a| a.get(idx)) {
-                            Some(v) => v,
-                            None => return "N/A".to_string(),
-                        },
-                        Err(_) => return "N/A".to_string(),
-                    }
-                };
-            }
-        } else {
-            current = match current.get(part) {
-                Some(v) => v,
-                None => return "N/A".to_string(),
-            };
-        }
-    }
-
-    // Convert final value to string
-    match current {
-        Value::String(s) => s.clone(),
-        Value::Number(n) => n.to_string(),
-        Value::Bool(b) => b.to_string(),
-        Value::Null => "".to_string(),
-        Value::Array(a) => format!("[{} items]", a.len()),
-        Value::Object(_) => "{object}".to_string(),
-    }
-}
-
 fn parse_custom_resources_json(json_str: &str) -> Result<Vec<CustomResourceInfo>, String> {
     let value: Value = serde_json::from_str(json_str)
         .map_err(|e| format!("Failed to parse kubectl JSON output: {}", e))?;

src-tauri/src/commands/metrics.rs

@@ -2,6 +2,56 @@ use crate::metrics::{NodeMetrics, PodMetrics};
 use crate::state::AppState;
 use tauri::State;
 
+/// RAII guard that removes a temp kubeconfig file when dropped.
+///
+/// Using a Drop-based guard guarantees the sensitive kubeconfig is removed
+/// even on panic or early `?` return — manual `remove_file` calls only run
+/// on the happy path and were silently leaking the file on errors.
+struct TempKubeconfig(std::path::PathBuf);
+
+impl TempKubeconfig {
+    fn path(&self) -> &std::path::Path {
+        &self.0
+    }
+}
+
+impl Drop for TempKubeconfig {
+    fn drop(&mut self) {
+        if let Err(e) = std::fs::remove_file(&self.0) {
+            // Only log when the file actually existed; NotFound is expected on
+            // Windows when the path was never written.
+            if e.kind() != std::io::ErrorKind::NotFound {
+                tracing::warn!(
+                    "Failed to remove temp kubeconfig {}: {}",
+                    self.0.display(),
+                    e
+                );
+            }
+        }
+    }
+}
+
+/// Write the kubeconfig content to a unique temp file with 0600 permissions
+/// and return an RAII guard that cleans up on drop.
+fn write_temp_kubeconfig(content: &str) -> Result<TempKubeconfig, String> {
+    let path =
+        std::env::temp_dir().join(format!("kubeconfig-metrics-{}.yaml", uuid::Uuid::now_v7()));
+    let guard = TempKubeconfig(path);
+
+    std::fs::write(guard.path(), content.as_bytes())
+        .map_err(|e| format!("Failed to write kubeconfig: {e}"))?;
+
+    // Ensure owner-only permissions (0600 on Unix)
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        std::fs::set_permissions(guard.path(), std::fs::Permissions::from_mode(0o600))
+            .map_err(|e| format!("Failed to set kubeconfig permissions: {e}"))?;
+    }
+
+    Ok(guard)
+}
+
 /// Get pod metrics from kubectl top pods
 #[tauri::command]
 pub async fn get_pod_metrics(
@@ -14,20 +64,9 @@ pub async fn get_pod_metrics(
         .get(&cluster_id)
         .ok_or_else(|| "Cluster not found".to_string())?;
 
-    // Write temp kubeconfig
+    // Write temp kubeconfig (auto-removed on drop)
     let kubeconfig_content = cluster.kubeconfig_content.as_ref();
-    let temp_path =
-        std::env::temp_dir().join(format!("kubeconfig-metrics-{}.yaml", uuid::Uuid::now_v7()));
-    std::fs::write(&temp_path, kubeconfig_content.as_bytes())
-        .map_err(|e| format!("Failed to write kubeconfig: {e}"))?;
-
-    // Ensure owner-only permissions (0600 on Unix)
-    #[cfg(unix)]
-    {
-        use std::os::unix::fs::PermissionsExt;
-        std::fs::set_permissions(&temp_path, std::fs::Permissions::from_mode(0o600))
-            .map_err(|e| format!("Failed to set kubeconfig permissions: {e}"))?;
-    }
+    let kubeconfig = write_temp_kubeconfig(kubeconfig_content)?;
 
     // Run kubectl top pods with JSON output
     let args = vec![
@@ -39,14 +78,11 @@ pub async fn get_pod_metrics(
         "-o".to_string(),
         "json".to_string(),
         "--kubeconfig".to_string(),
-        temp_path.to_string_lossy().to_string(),
+        kubeconfig.path().to_string_lossy().to_string(),
     ];
 
     let output = crate::shell::kubectl::execute_kubectl(&args, None, None).await?;
 
-    // Clean up temp file
-    let _ = std::fs::remove_file(&temp_path);
-
     if output.exit_code != 0 {
         return Err(format!("kubectl top pods failed: {}", output.stderr));
     }
@@ -54,6 +90,7 @@ pub async fn get_pod_metrics(
     let json_output = &output.stdout;
     crate::metrics::client::parse_pod_metrics(json_output)
         .map_err(|e| format!("Failed to parse pod metrics: {e}"))
+    // kubeconfig dropped here, file removed
 }
 
 /// Get node metrics from kubectl top nodes
@@ -67,20 +104,9 @@ pub async fn get_node_metrics(
         .get(&cluster_id)
         .ok_or_else(|| "Cluster not found".to_string())?;
 
-    // Write temp kubeconfig
+    // Write temp kubeconfig (auto-removed on drop)
     let kubeconfig_content = cluster.kubeconfig_content.as_ref();
-    let temp_path =
-        std::env::temp_dir().join(format!("kubeconfig-metrics-{}.yaml", uuid::Uuid::now_v7()));
-    std::fs::write(&temp_path, kubeconfig_content.as_bytes())
-        .map_err(|e| format!("Failed to write kubeconfig: {e}"))?;
-
-    // Ensure owner-only permissions (0600 on Unix)
-    #[cfg(unix)]
-    {
-        use std::os::unix::fs::PermissionsExt;
-        std::fs::set_permissions(&temp_path, std::fs::Permissions::from_mode(0o600))
-            .map_err(|e| format!("Failed to set kubeconfig permissions: {e}"))?;
-    }
+    let kubeconfig = write_temp_kubeconfig(kubeconfig_content)?;
 
     // Run kubectl top nodes with JSON output
     let args = vec![
@@ -90,14 +116,11 @@ pub async fn get_node_metrics(
         "-o".to_string(),
         "json".to_string(),
         "--kubeconfig".to_string(),
-        temp_path.to_string_lossy().to_string(),
+        kubeconfig.path().to_string_lossy().to_string(),
     ];
 
     let output = crate::shell::kubectl::execute_kubectl(&args, None, None).await?;
 
-    // Clean up temp file
-    let _ = std::fs::remove_file(&temp_path);
-
     if output.exit_code != 0 {
         return Err(format!("kubectl top nodes failed: {}", output.stderr));
     }
@@ -105,4 +128,5 @@ pub async fn get_node_metrics(
     let json_output = &output.stdout;
     crate::metrics::client::parse_node_metrics(json_output)
         .map_err(|e| format!("Failed to parse node metrics: {e}"))
+    // kubeconfig dropped here, file removed
 }

src-tauri/src/commands/shell.rs

@@ -12,6 +12,41 @@ use rusqlite::params;
 use serde::{Deserialize, Serialize};
 use tauri::State;
 
+/// RAII guard for a temp kubeconfig file. Removes the file when dropped
+/// unless `disarm()` has been called — used on the error path of session
+/// start so the file isn't leaked if kubectl resolution or session
+/// registration fails after we've written it. On the success path we call
+/// `disarm()` and the PTY session itself becomes responsible for the file's
+/// lifetime (it lives in `std::env::temp_dir()` which is OS-cleaned).
+struct KubeconfigGuard {
+    path: Option<std::path::PathBuf>,
+}
+
+impl KubeconfigGuard {
+    fn new(path: std::path::PathBuf) -> Self {
+        Self { path: Some(path) }
+    }
+
+    /// Transfer ownership: caller is now responsible for the file.
+    /// Returns the path string for use with the PTY session.
+    fn disarm(mut self) -> String {
+        let path = self.path.take().expect("KubeconfigGuard already disarmed");
+        path.to_string_lossy().into_owned()
+    }
+}
+
+impl Drop for KubeconfigGuard {
+    fn drop(&mut self) {
+        if let Some(path) = self.path.take() {
+            if let Err(e) = std::fs::remove_file(&path) {
+                if e.kind() != std::io::ErrorKind::NotFound {
+                    tracing::warn!("Failed to remove temp kubeconfig {}: {}", path.display(), e);
+                }
+            }
+        }
+    }
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct CommandExecution {
     pub id: String,
@@ -279,8 +314,9 @@ pub async fn start_pty_exec_session(
     pod: String,
     container: Option<String>,
 ) -> Result<String, String> {
-    // Get active kubeconfig
-    let kubeconfig_path = {
+    // Get active kubeconfig — the guard ensures the temp file is removed
+    // if anything between here and `disarm()` fails.
+    let kubeconfig_guard: Option<KubeconfigGuard> = {
         let db = state.db.lock().map_err(|e| e.to_string())?;
         let mut stmt = db
             .prepare("SELECT encrypted_content FROM kubeconfig_files WHERE is_active = 1 LIMIT 1")
@@ -298,16 +334,19 @@ pub async fn start_pty_exec_session(
             std::fs::write(&temp_path, content)
                 .map_err(|e| format!("Failed to write kubeconfig: {e}"))?;
 
-            Some(temp_path.to_string_lossy().to_string())
+            Some(KubeconfigGuard::new(temp_path))
         } else {
             None
         }
     };
 
-    // Locate kubectl
+    // Locate kubectl — if this fails, the guard cleans up the temp kubeconfig.
     let kubectl_path =
         crate::shell::kubectl::locate_kubectl().map_err(|e| format!("kubectl not found: {e}"))?;
 
+    // Transfer ownership: PTY session now owns the temp file's lifetime.
+    let kubeconfig_path = kubeconfig_guard.map(|g| g.disarm());
+
     // Start session
     let params = crate::shell::session::SessionParams {
         cluster_id,
@@ -337,8 +376,9 @@ pub async fn start_pty_attach_session(
     pod: String,
     container: Option<String>,
 ) -> Result<String, String> {
-    // Get active kubeconfig
-    let kubeconfig_path = {
+    // Get active kubeconfig — the guard ensures the temp file is removed
+    // if anything between here and `disarm()` fails.
+    let kubeconfig_guard: Option<KubeconfigGuard> = {
         let db = state.db.lock().map_err(|e| e.to_string())?;
         let mut stmt = db
             .prepare("SELECT encrypted_content FROM kubeconfig_files WHERE is_active = 1 LIMIT 1")
@@ -356,16 +396,19 @@ pub async fn start_pty_attach_session(
             std::fs::write(&temp_path, content)
                 .map_err(|e| format!("Failed to write kubeconfig: {e}"))?;
 
-            Some(temp_path.to_string_lossy().to_string())
+            Some(KubeconfigGuard::new(temp_path))
         } else {
             None
         }
     };
 
-    // Locate kubectl
+    // Locate kubectl — if this fails, the guard cleans up the temp kubeconfig.
     let kubectl_path =
         crate::shell::kubectl::locate_kubectl().map_err(|e| format!("kubectl not found: {e}"))?;
 
+    // Transfer ownership: PTY session now owns the temp file's lifetime.
+    let kubeconfig_path = kubeconfig_guard.map(|g| g.disarm());
+
     // Start session
     let params = crate::shell::session::SessionParams {
         cluster_id,

src-tauri/src/metrics/client.rs

@@ -139,9 +139,18 @@ pub fn parse_node_metrics(json_output: &str) -> Result<Vec<NodeMetrics>> {
             .unwrap_or("0")
             .to_string();
 
-        // Calculate percentages (simplified - would need capacity from kubectl get nodes)
-        let cpu_percent = 0.0; // TODO: Calculate from capacity
-        let memory_percent = 0.0; // TODO: Calculate from capacity
+        // Calculate percentages (simplified - would need capacity from kubectl get nodes).
+        //
+        // TODO(metrics): Populate these from node `status.capacity` once we add
+        // a second kubectl call to fetch node capacity. The metrics-server JSON
+        // returned by `kubectl top nodes` only reports raw `usage` (cpu in
+        // nanocores, memory in Ki), not the node's allocatable totals, so we
+        // cannot compute a real percentage from this response alone.
+        // Until that work is done these are reported as 0.0 and the frontend
+        // hides the percent column. Tracking issue: see Telemetry/Metrics
+        // backlog in the project tracker.
+        let cpu_percent = 0.0;
+        let memory_percent = 0.0;
 
         metrics.push(NodeMetrics {
             name,

src-tauri/src/shell/pty.rs

@@ -12,7 +12,7 @@
 use anyhow::{Context, Result};
 use portable_pty::{native_pty_system, CommandBuilder, PtySize};
 use std::io::{Read, Write};
-use tracing::debug;
+use tracing::{debug, warn};
 
 /// PTY session handle with I/O streams
 pub struct PtySession {
@@ -81,11 +81,15 @@ impl PtySession {
             args.push(c.to_string());
         }
 
-        // Use FreeLens-style shell fallback command
+        // Use FreeLens-style shell fallback command.
+        // We deliberately omit `clear` from the chain: when a container image
+        // lacks `clear` (or `tput`), running it would print a non-fatal but
+        // confusing error to the user. The frontend terminal is responsible
+        // for clearing on connect.
         args.push("--".to_string());
         args.push("sh".to_string());
         args.push("-c".to_string());
-        args.push("clear; (bash || ash || sh)".to_string());
+        args.push("bash || ash || sh".to_string());
 
         let mut env = Vec::new();
         if let Some(kubeconfig) = kubeconfig_path {
@@ -199,9 +203,12 @@ impl PtySession {
 
 impl Drop for PtySession {
     fn drop(&mut self) {
-        // Best-effort cleanup
+        // Best-effort cleanup. Log kill failures rather than swallowing them so
+        // operators can detect leaked child processes during diagnostics.
         if self.is_alive() {
-            let _ = self.kill();
+            if let Err(e) = self.kill() {
+                warn!("PTY session Drop: failed to kill child process: {e:#}");
+            }
         }
         debug!("PTY session dropped");
     }

src-tauri/src/shell/session.rs

@@ -202,6 +202,26 @@ impl SessionManager {
     ) -> Result<()> {
         let mut poll_interval = interval(Duration::from_millis(50));
 
+        // Explicit cleanup helper invoked on every exit path. While
+        // `PtySession::Drop` already best-effort kills the child, doing it here
+        // first lets us log the outcome and surface failures via tracing.
+        // After this returns, the PtySession is consumed and dropped, releasing
+        // the master/slave PTY handles.
+        let cleanup = |pty: &mut PtySession, session_id: &str, reason: &str| {
+            debug!(
+                "Cleaning up PTY for session {} (reason: {})",
+                session_id, reason
+            );
+            if pty.is_alive() {
+                if let Err(e) = pty.kill() {
+                    warn!(
+                        "Failed to kill PTY child for session {} during cleanup: {}",
+                        session_id, e
+                    );
+                }
+            }
+        };
+
         loop {
             tokio::select! {
                 // Read from PTY stdout/stderr
@@ -209,6 +229,7 @@ impl SessionManager {
                     if !pty_session.is_alive() {
                         debug!("Session {} PTY process exited", session_id);
                         let _ = app_handle.emit(&format!("terminal-closed-{}", session_id), ());
+                        cleanup(&mut pty_session, &session_id, "process exited");
                         break;
                     }
 
@@ -225,6 +246,7 @@ impl SessionManager {
                         Err(e) => {
                             error!("Failed to read from PTY for session {}: {}", session_id, e);
                             let _ = app_handle.emit(&format!("terminal-error-{}", session_id), e.to_string());
+                            cleanup(&mut pty_session, &session_id, "read error");
                             break;
                         }
                     }
@@ -235,6 +257,7 @@ impl SessionManager {
                     if let Err(e) = pty_session.write(&data) {
                         error!("Failed to write to PTY for session {}: {}", session_id, e);
                         let _ = app_handle.emit(&format!("terminal-error-{}", session_id), e.to_string());
+                        cleanup(&mut pty_session, &session_id, "write error");
                         break;
                     }
                 }
@@ -244,12 +267,26 @@ impl SessionManager {
                     match cmd {
                         ControlCommand::Resize { rows, cols } => {
                             if let Err(e) = pty_session.resize(rows, cols) {
-                                warn!("Failed to resize PTY for session {}: {}", session_id, e);
+                                // A failed resize means the PTY is in an
+                                // unrecoverable state (master fd closed, slave
+                                // signal failed, etc.). Surface the error to
+                                // the frontend and terminate the session
+                                // rather than continuing with a stale layout.
+                                error!(
+                                    "Failed to resize PTY for session {}: {}. Terminating session.",
+                                    session_id, e
+                                );
+                                let _ = app_handle.emit(
+                                    &format!("terminal-error-{}", session_id),
+                                    format!("PTY resize failed; session terminated: {e}"),
+                                );
+                                cleanup(&mut pty_session, &session_id, "resize error");
+                                break;
                             }
                         }
                         ControlCommand::Terminate => {
                             info!("Session {} received terminate command", session_id);
-                            let _ = pty_session.kill();
+                            cleanup(&mut pty_session, &session_id, "terminate command");
                             break;
                         }
                     }

src-tauri/src/state.rs

@@ -79,11 +79,49 @@ pub struct ApprovalResponse {
     pub decision: String, // "deny", "allow_once", "allow_session"
 }
 
+/// Application-wide shared state injected into every Tauri command via
+/// `State<'_, AppState>`.
+///
+/// # Synchronization expectations
+///
+/// All fields except `app_data_dir` are wrapped in either a `std::sync::Mutex`
+/// or a `tokio::sync::Mutex`. The choice is deliberate and **must** be
+/// preserved by callers:
+///
+/// - **`std::sync::Mutex`** (e.g. `db`, `settings`, `integration_webviews`,
+///   `watchers`): held for short, synchronous critical sections only. **Never
+///   hold a `MutexGuard` across an `.await`** — `MutexGuard` is `!Send` and
+///   the compiler will reject it. The standard pattern is to lock inside a
+///   `{ }` block, take the data needed, drop the guard, then `.await`.
+///
+/// - **`tokio::sync::Mutex`** (e.g. `mcp_connections`, `pending_approvals`,
+///   `clusters`, `port_forwards`, `refresh_registry`, `log_streams`): used
+///   for state that must be held across an `.await` (network calls, channel
+///   operations, etc.). These have an async `lock().await` API.
+///
+/// - **`Arc<crate::shell::SessionManager>`**: the manager itself owns its
+///   internal locking via `RwLock`; callers do not lock the `Arc`.
+///
+/// - **`app_data_dir`**: immutable for the lifetime of the process; safe to
+///   read without synchronization.
+///
+/// All fields are `pub` so command handlers in `commands/*.rs` can clone
+/// individual `Arc`s into spawned tasks without taking the entire `AppState`.
+/// Callers should treat the choice of mutex type as part of the API contract:
+/// changing a `std::sync::Mutex` to a `tokio::sync::Mutex` (or vice-versa) is
+/// a breaking change for every handler that touches the field.
 pub struct AppState {
+    /// Encrypted SQLite (SQLCipher in release) connection. Short-lived locks
+    /// only; never held across `.await`.
     pub db: Arc<Mutex<rusqlite::Connection>>,
+    /// In-memory copy of `AppSettings`. Persisted to disk via the settings
+    /// commands; lock for read/write but never across `.await`.
     pub settings: Arc<Mutex<AppSettings>>,
+    /// Resolved data directory (`~/.local/share/tftsr` on Linux, etc.).
+    /// Immutable for the process lifetime — no locking needed.
     pub app_data_dir: PathBuf,
-    /// Track open integration webview windows by service name -> window label
+    /// Track open integration webview windows by service name -> window label.
+    /// Short-lived `std::sync::Mutex`.
     pub integration_webviews: Arc<Mutex<HashMap<String, String>>>,
     /// Live MCP server connections: server_id -> connection
     pub mcp_connections: