Make approval decisions idempotent
CI / Rust Checks (push) Has been cancelled
CI / UI Checks (push) Has been cancelled
CI / Frontend E2E (push) Has been cancelled
CI / Deployment Manifests (push) Has been cancelled
CI / Deploy (push) Has been cancelled

This commit is contained in:
github-ops
2026-06-24 13:23:04 +00:00
parent 267061e226
commit 8ce00ede31
2 changed files with 70 additions and 5 deletions
@@ -18,10 +18,11 @@ use axum::{
use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD};
use crank_core::{ use crank_core::{
Agent, AgentId, AgentOperationBinding, AgentStatus, AgentVersion, ApprovalRequest, Agent, AgentId, AgentOperationBinding, AgentStatus, AgentVersion, ApprovalRequest,
ApprovalRequestId, ApprovalRequestStatus, ExecutionConfig, HttpMethod, Operation, ApprovalRequestId, ApprovalRequestStatus, ExecutionConfig, HttpMethod, InvocationSource,
OperationApprovalPayloadPreviewMode, OperationApprovalPolicy, OperationApprovalRiskLevel, Operation, OperationApprovalPayloadPreviewMode, OperationApprovalPolicy,
OperationId, OperationStatus, PlatformApiKey, PlatformApiKeyId, PlatformApiKeyScope, OperationApprovalRiskLevel, OperationId, OperationStatus, PlatformApiKey, PlatformApiKeyId,
PlatformApiKeyStatus, Protocol, RestTarget, Target, ToolDescription, WorkspaceId, PlatformApiKeyScope, PlatformApiKeyStatus, Protocol, RestTarget, Target, ToolDescription,
WorkspaceId,
}; };
use crank_mapping::{MappingRule, MappingSet}; use crank_mapping::{MappingRule, MappingSet};
use crank_registry::{ use crank_registry::{
@@ -639,6 +640,24 @@ async fn approval_key_lists_and_decides_pending_requests() {
json!({ "id": "lead_123" }) 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::<Value>().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 let pending_after = client
.get(&approvals_url) .get(&approvals_url)
.header(header::AUTHORIZATION, format!("Bearer {approval_key}")) .header(header::AUTHORIZATION, format!("Bearer {approval_key}"))
@@ -649,6 +668,21 @@ async fn approval_key_lists_and_decides_pending_requests() {
.await .await
.unwrap(); .unwrap();
assert!(pending_after["items"].as_array().unwrap().is_empty()); 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] #[tokio::test]
+32 -1
View File
@@ -326,7 +326,38 @@ async fn decide_approval_request(
} }
} }
Ok(Some(record)) => Json(json!(record)).into_response(), 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<AppState>,
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(), Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(),
} }
} }