From 8ce00ede31f748f33288ebf21b3396b11b9dc88b Mon Sep 17 00:00:00 2001 From: github-ops Date: Wed, 24 Jun 2026 13:23:04 +0000 Subject: [PATCH] Make approval decisions idempotent --- .../tests/integration/catalog_access.rs | 42 +++++++++++++++++-- crates/crank-community-mcp/src/app.rs | 33 ++++++++++++++- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/apps/mcp-server/tests/integration/catalog_access.rs b/apps/mcp-server/tests/integration/catalog_access.rs index 9c3be59..3f98f08 100644 --- a/apps/mcp-server/tests/integration/catalog_access.rs +++ b/apps/mcp-server/tests/integration/catalog_access.rs @@ -18,10 +18,11 @@ use axum::{ use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use crank_core::{ Agent, AgentId, AgentOperationBinding, AgentStatus, AgentVersion, ApprovalRequest, - ApprovalRequestId, ApprovalRequestStatus, ExecutionConfig, HttpMethod, Operation, - OperationApprovalPayloadPreviewMode, OperationApprovalPolicy, OperationApprovalRiskLevel, - OperationId, OperationStatus, PlatformApiKey, PlatformApiKeyId, PlatformApiKeyScope, - PlatformApiKeyStatus, Protocol, RestTarget, Target, ToolDescription, WorkspaceId, + ApprovalRequestId, ApprovalRequestStatus, ExecutionConfig, HttpMethod, InvocationSource, + Operation, OperationApprovalPayloadPreviewMode, OperationApprovalPolicy, + OperationApprovalRiskLevel, OperationId, OperationStatus, PlatformApiKey, PlatformApiKeyId, + PlatformApiKeyScope, PlatformApiKeyStatus, Protocol, RestTarget, Target, ToolDescription, + WorkspaceId, }; use crank_mapping::{MappingRule, MappingSet}; use crank_registry::{ @@ -639,6 +640,24 @@ async fn approval_key_lists_and_decides_pending_requests() { json!({ "id": "lead_123" }) ); + let repeated_approve = client + .post(&approve_url) + .header(header::AUTHORIZATION, format!("Bearer {approval_key}")) + .json(&json!({ "approve": "yes", "note": "duplicate confirmation" })) + .send() + .await + .unwrap(); + assert_eq!(repeated_approve.status(), reqwest::StatusCode::OK); + let repeated_body = repeated_approve.json::().await.unwrap(); + assert_eq!( + repeated_body["approval"]["status"], + Value::String("completed".to_owned()) + ); + assert_eq!( + repeated_body["approval"]["response_payload"], + json!({ "id": "lead_123" }) + ); + let pending_after = client .get(&approvals_url) .header(header::AUTHORIZATION, format!("Bearer {approval_key}")) @@ -649,6 +668,21 @@ async fn approval_key_lists_and_decides_pending_requests() { .await .unwrap(); assert!(pending_after["items"].as_array().unwrap().is_empty()); + + let logs = registry + .list_invocation_logs(ListInvocationLogsQuery { + workspace_id: &test_workspace_id(), + level: None, + search_text: None, + source: Some(InvocationSource::AgentToolCall), + operation_id: Some(&operation.id), + agent_id: Some(&test_agent_id("sales-human-approval")), + created_after: None, + limit: 10, + }) + .await + .unwrap(); + assert_eq!(logs.len(), 1); } #[tokio::test] diff --git a/crates/crank-community-mcp/src/app.rs b/crates/crank-community-mcp/src/app.rs index 2321989..bea2d39 100644 --- a/crates/crank-community-mcp/src/app.rs +++ b/crates/crank-community-mcp/src/app.rs @@ -326,7 +326,38 @@ async fn decide_approval_request( } } Ok(Some(record)) => Json(json!(record)).into_response(), - Ok(None) => StatusCode::CONFLICT.into_response(), + Ok(None) => { + existing_decision_response(&state, &key.api_key.workspace_id, agent_id, &approval_id) + .await + } + Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), + } +} + +async fn existing_decision_response( + state: &Arc, + workspace_id: &crank_core::WorkspaceId, + agent_id: &crank_core::AgentId, + approval_id: &ApprovalRequestId, +) -> Response { + match state + .registry + .get_approval_request_for_agent(workspace_id, agent_id, approval_id) + .await + { + Ok(Some(record)) + if matches!( + record.approval.status, + ApprovalRequestStatus::Completed + | ApprovalRequestStatus::Failed + | ApprovalRequestStatus::Denied + | ApprovalRequestStatus::Expired + ) => + { + Json(json!(record)).into_response() + } + Ok(Some(_)) => StatusCode::CONFLICT.into_response(), + Ok(None) => StatusCode::NOT_FOUND.into_response(), Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), } }