fix: address valid PR review findings
Some checks failed
Test / rust-fmt-check (pull_request) Successful in 2m19s
Test / rust-clippy (pull_request) Failing after 4m15s
Test / frontend-typecheck (pull_request) Successful in 2m35s
Test / frontend-tests (pull_request) Failing after 1m47s
Test / rust-tests (pull_request) Successful in 6m17s
PR Review Automation / review (pull_request) Successful in 8m17s

Fix two valid issues identified in automated code review:

1. Fix OAuth callback AppState to preserve pending_approvals
   - Clone existing pending_approvals instead of creating empty HashMap
   - Prevents loss of shell approval requests during OAuth flow

2. Add validation to activate_kubeconfig
   - Check that kubeconfig ID exists before activation
   - Return error if ID not found to prevent silent failure

Invalid findings clarified:
- Ollama retry logic is correct (anyhow::bail exits immediately)
- systemctl classification already handles subcommands correctly
  (lines 230-239: status/is-active/is-enabled are Tier 1)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Shaun Arman 2026-06-05 09:04:28 -05:00
parent b0961e7a60
commit 276fdae104
2 changed files with 12 additions and 8 deletions

View File

@ -325,6 +325,7 @@ pub async fn initiate_oauth(
let app_data_dir = app_state.app_data_dir.clone();
let integration_webviews = app_state.integration_webviews.clone();
let mcp_connections = app_state.mcp_connections.clone();
let pending_approvals = app_state.pending_approvals.clone();
tokio::spawn(async move {
let app_state_for_callback = AppState {
@ -333,9 +334,7 @@ pub async fn initiate_oauth(
app_data_dir,
integration_webviews,
mcp_connections,
pending_approvals: Arc::new(tokio::sync::Mutex::new(
std::collections::HashMap::new(),
)),
pending_approvals,
};
while let Some(callback) = callback_rx.recv().await {
tracing::info!("Received OAuth callback for state: {}", callback.state);

View File

@ -97,11 +97,16 @@ pub fn activate_kubeconfig(id: String, state: State<'_, AppState>) -> Result<(),
.map_err(|e| format!("Failed to deactivate configs: {e}"))?;
// Activate the specified config
db.execute(
"UPDATE kubeconfig_files SET is_active = 1 WHERE id = ?1",
params![&id],
)
.map_err(|e| format!("Failed to activate config: {e}"))?;
let rows_updated = db
.execute(
"UPDATE kubeconfig_files SET is_active = 1 WHERE id = ?1",
params![&id],
)
.map_err(|e| format!("Failed to activate config: {e}"))?;
if rows_updated == 0 {
return Err(format!("Kubeconfig with id '{}' not found", id));
}
Ok(())
}