From 5447e1bad0439255aedff5082c0df7bb55fe6117 Mon Sep 17 00:00:00 2001 From: github-ops Date: Sun, 21 Jun 2026 01:42:45 +0000 Subject: [PATCH] Add destructive confirmation and import guidance --- Cargo.lock | 1 + README.md | 2 + apps/admin-api/src/app.rs | 1 + apps/admin-api/src/error.rs | 17 ++ apps/admin-api/src/import_guidance.rs | 210 +++++++++++++++++ apps/admin-api/src/lib.rs | 1 + apps/admin-api/src/main.rs | 1 + apps/admin-api/src/service.rs | 15 +- apps/mcp-server/src/main.rs | 2 + apps/ui/js/i18n.js | 2 + apps/ui/js/wizard-live.js | 22 +- apps/ui/js/wizard.js | 4 +- apps/ui/tests/e2e/wizard.spec.js | 5 +- crates/crank-community-mcp/src/app.rs | 38 ++- crates/crank-community-mcp/src/manifest.rs | 59 ++++- crates/crank-community-mcp/src/tool_error.rs | 26 ++ .../tests/unit/manifest.rs | 23 ++ crates/crank-core/src/lib.rs | 7 +- crates/crank-core/src/operation.rs | 35 +++ crates/crank-runtime/Cargo.toml | 1 + crates/crank-runtime/src/error.rs | 15 +- crates/crank-runtime/src/executor.rs | 213 ++++++++++++++++- crates/crank-runtime/src/executor_builder.rs | 12 +- crates/crank-runtime/src/request_context.rs | 20 ++ crates/crank-runtime/tests/integration.rs | 1 + .../tests/integration/confirmation.rs | 222 ++++++++++++++++++ .../tests/integration/idempotency.rs | 1 + .../tests/integration/no_input_get.rs | 1 + docs/tool-design.md | 166 +++++++++++++ 29 files changed, 1099 insertions(+), 24 deletions(-) create mode 100644 apps/admin-api/src/import_guidance.rs create mode 100644 crates/crank-runtime/tests/integration/confirmation.rs create mode 100644 docs/tool-design.md diff --git a/Cargo.lock b/Cargo.lock index 6b28c36..db5322d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -503,6 +503,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "uuid", ] [[package]] diff --git a/README.md b/README.md index 3f0a3b2..527b1a3 100644 --- a/README.md +++ b/README.md @@ -151,6 +151,8 @@ docker compose up -d --build Каждый агент имеет свой каталог инструментов. MCP-клиент видит только те REST-инструменты, которые явно привязаны к этому агенту. +Рекомендации по названиям, описаниям, схемам, ошибкам и опасным операциям описаны в документе [Проектирование MCP-инструментов](./docs/tool-design.md). + ## Что входит в эту версию Этот репозиторий содержит открытую версию Crank: diff --git a/apps/admin-api/src/app.rs b/apps/admin-api/src/app.rs index b8b30a2..5a122e0 100644 --- a/apps/admin-api/src/app.rs +++ b/apps/admin-api/src/app.rs @@ -2337,6 +2337,7 @@ mod tests { retry_policy: None, response_cache: None, idempotency: None, + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), }, diff --git a/apps/admin-api/src/error.rs b/apps/admin-api/src/error.rs index 46c8eb5..b94289f 100644 --- a/apps/admin-api/src/error.rs +++ b/apps/admin-api/src/error.rs @@ -400,6 +400,9 @@ fn runtime_test_failure_code(error: &RuntimeError) -> &'static str { RuntimeError::UnsupportedProtocol { .. } => "runtime_protocol_error", RuntimeError::ConcurrencyLimitExceeded { .. } => "runtime_overloaded", RuntimeError::InvalidPreparedRequest { .. } => "runtime_request_error", + RuntimeError::ConfirmationRequired { .. } => "runtime_confirmation_required", + RuntimeError::InvalidConfirmationToken { .. } => "runtime_confirmation_error", + RuntimeError::ConfirmationStoreUnavailable { .. } => "runtime_confirmation_unavailable", RuntimeError::UnsupportedExecutionMode { .. } => "runtime_streaming_mode_error", RuntimeError::MissingAuthProfile { .. } => "runtime_auth_profile_error", RuntimeError::MissingSecret { .. } | RuntimeError::MissingSecretVersion { .. } => { @@ -416,6 +419,20 @@ pub fn runtime_error_context(error: &RuntimeError) -> Option { "field": field, "reason": reason, })), + RuntimeError::ConfirmationRequired { + confirmation_token, + expires_in_ms, + safety_class, + .. + } => Some(json!({ + "confirmation_token": confirmation_token, + "expires_in_ms": expires_in_ms, + "safety_class": safety_class, + })), + RuntimeError::InvalidConfirmationToken { operation_id } + | RuntimeError::ConfirmationStoreUnavailable { operation_id } => Some(json!({ + "operation_id": operation_id, + })), RuntimeError::InvalidAuthSecretValue { secret_id, reason } => Some(json!({ "secret_id": secret_id, "reason": reason, diff --git a/apps/admin-api/src/import_guidance.rs b/apps/admin-api/src/import_guidance.rs new file mode 100644 index 0000000..b191455 --- /dev/null +++ b/apps/admin-api/src/import_guidance.rs @@ -0,0 +1,210 @@ +use crank_core::{HttpMethod, Target}; +use crank_mapping::MappingSet; +use crank_registry::RegistryOperation; +use crank_schema::{Schema, SchemaKind}; + +pub fn import_guidance_warnings(operation: &RegistryOperation) -> Vec { + let mut warnings = Vec::new(); + + if is_generic_tool_name(&operation.name) { + warnings.push( + "Имя инструмента слишком общее. Используйте конкретное действие и объект, например get_customer_by_email.".to_owned(), + ); + } + + if operation + .tool_description + .description + .trim() + .chars() + .count() + < 40 + { + warnings.push( + "Описание инструмента короткое. Добавьте, когда его вызывать, какие входные данные нужны и что означает успешный ответ.".to_owned(), + ); + } + + if schema_has_multi_action_field(&operation.input_schema) { + warnings.push( + "Входная схема похожа на endpoint с несколькими действиями. Если параметр action/mode/type выбирает разные сценарии, лучше разделить это на несколько MCP-инструментов.".to_owned(), + ); + } + + if output_mapping_returns_broad_response(&operation.output_mapping) { + warnings.push( + "Настройка результата может отдавать агенту слишком широкий ответ. Выберите только поля, которые нужны модели для следующего шага.".to_owned(), + ); + } + + match &operation.target { + Target::Rest(target) => { + if operation.category == "general" { + warnings.push( + "Операция импортирована в общей категории. Перед привязкой к агенту сгруппируйте похожие endpoint-ы по конкретной задаче.".to_owned(), + ); + } + + if target.method != HttpMethod::Get && operation.execution_config.idempotency.is_none() + { + warnings.push( + "Операция меняет состояние и не задает ключ идемпотентности. Для POST, PUT и PATCH добавьте стабильный ключ, если повторный вызов может создать дубль.".to_owned(), + ); + } + + if target.method == HttpMethod::Delete { + warnings.push( + "DELETE будет выполняться через двухшаговое подтверждение: первый вызов выдаст токен, второй выполнит запрос.".to_owned(), + ); + } + } + } + + warnings +} + +fn is_generic_tool_name(name: &str) -> bool { + matches!( + name.trim().to_ascii_lowercase().as_str(), + "call_api" | "execute" | "execute_request" | "manage" | "manage_resource" | "request" + ) +} + +fn schema_has_multi_action_field(schema: &Schema) -> bool { + if !matches!(schema.kind, SchemaKind::Object) { + return false; + } + + ["action", "mode", "operation", "type"] + .iter() + .any(|field_name| schema.fields.contains_key(*field_name)) +} + +fn output_mapping_returns_broad_response(mapping: &MappingSet) -> bool { + mapping.rules.iter().any(|rule| { + let source = rule.source.trim(); + let target = rule.target.trim(); + matches!(source, "$.response" | "$.response.body" | "$.response.data") + || matches!(target, "$.output" | "$") + }) +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use crank_core::{ + ExecutionConfig, HttpMethod, Operation, OperationId, OperationSecurityLevel, + OperationStatus, Protocol, RestTarget, Target, ToolDescription, + }; + use crank_mapping::{MappingRule, MappingSet}; + use crank_registry::RegistryOperation; + use crank_schema::{Schema, SchemaKind}; + use time::OffsetDateTime; + + use super::import_guidance_warnings; + + #[test] + fn flags_generic_multi_action_delete() { + let operation = imported_delete_operation(); + + let warnings = import_guidance_warnings(&operation); + let joined = warnings.join("\n"); + + assert!(joined.contains("Имя инструмента слишком общее")); + assert!(joined.contains("несколькими действиями")); + assert!(joined.contains("Настройка результата")); + assert!(joined.contains("ключ идемпотентности")); + assert!(joined.contains("DELETE будет выполняться")); + assert!(joined.contains("сгруппируйте похожие endpoint-ы")); + } + + fn imported_delete_operation() -> RegistryOperation { + Operation { + id: OperationId::new("op_imported_delete"), + name: "call_api".to_owned(), + display_name: "Call API".to_owned(), + category: "general".to_owned(), + protocol: Protocol::Rest, + security_level: OperationSecurityLevel::Standard, + status: OperationStatus::Draft, + version: 1, + target: Target::Rest(RestTarget { + base_url: "http://example.invalid".to_owned(), + method: HttpMethod::Delete, + path_template: "/items/{id}".to_owned(), + static_headers: BTreeMap::new(), + }), + input_schema: Schema { + kind: SchemaKind::Object, + description: None, + required: true, + nullable: false, + default_value: None, + fields: BTreeMap::from([("action".to_owned(), string_schema())]), + items: None, + enum_values: Vec::new(), + variants: Vec::new(), + }, + output_schema: Schema { + kind: SchemaKind::Object, + description: None, + required: false, + nullable: false, + default_value: None, + fields: BTreeMap::new(), + items: None, + enum_values: Vec::new(), + variants: Vec::new(), + }, + input_mapping: MappingSet { rules: Vec::new() }, + output_mapping: MappingSet { + rules: vec![MappingRule { + source: "$.response.body".to_owned(), + target: "$.output".to_owned(), + required: false, + default_value: None, + transform: None, + condition: None, + notes: None, + }], + }, + execution_config: ExecutionConfig { + timeout_ms: 1_000, + retry_policy: None, + response_cache: None, + idempotency: None, + safety: None, + auth_profile_ref: None, + headers: BTreeMap::new(), + }, + tool_description: ToolDescription { + title: "Call API".to_owned(), + description: "Calls API".to_owned(), + tags: Vec::new(), + examples: Vec::new(), + }, + samples: None, + generated_draft: None, + config_export: None, + wizard_state: None, + created_at: OffsetDateTime::UNIX_EPOCH, + updated_at: OffsetDateTime::UNIX_EPOCH, + published_at: None, + } + } + + fn string_schema() -> Schema { + Schema { + kind: SchemaKind::String, + description: None, + required: true, + nullable: false, + default_value: None, + fields: BTreeMap::new(), + items: None, + enum_values: Vec::new(), + variants: Vec::new(), + } + } +} diff --git a/apps/admin-api/src/lib.rs b/apps/admin-api/src/lib.rs index 51e3520..7cf82c5 100644 --- a/apps/admin-api/src/lib.rs +++ b/apps/admin-api/src/lib.rs @@ -1,6 +1,7 @@ pub mod app; pub mod auth; pub mod error; +pub mod import_guidance; pub mod rate_limit; pub mod request_context; pub mod routes; diff --git a/apps/admin-api/src/main.rs b/apps/admin-api/src/main.rs index 79ed754..8906671 100644 --- a/apps/admin-api/src/main.rs +++ b/apps/admin-api/src/main.rs @@ -59,6 +59,7 @@ async fn main() -> Result<(), Box> { let runtime = community_default() .with_limits(runtime_limits) .with_response_cache(cache_stores.response.clone()) + .with_coordination_store(cache_stores.coordination.clone()) .build(); let identity_provider = PasswordIdentityProvider::new(registry.clone(), auth_settings.password_pepper.clone()); diff --git a/apps/admin-api/src/service.rs b/apps/admin-api/src/service.rs index f0fb5c1..5727eff 100644 --- a/apps/admin-api/src/service.rs +++ b/apps/admin-api/src/service.rs @@ -47,6 +47,7 @@ use crate::{ hash_session_secret, verify_password, }, error::ApiError, + import_guidance::import_guidance_warnings, storage::LocalArtifactStorage, }; @@ -2685,6 +2686,7 @@ impl AdminService { tool_description: document.operation.tool_description.clone(), wizard_state: document.operation.wizard_state.clone(), }; + let warnings = import_guidance_warnings(&document.operation); match query.mode { ImportMode::Create => { @@ -2694,7 +2696,7 @@ impl AdminService { workspace_id: created.workspace_id, version: created.version, import_mode: ImportMode::Create, - warnings: Vec::new(), + warnings, }) } ImportMode::Upsert => { @@ -2718,7 +2720,7 @@ impl AdminService { workspace_id: created.workspace_id, version: created.version, import_mode: ImportMode::Upsert, - warnings: Vec::new(), + warnings, }; info!( operation_id = %response.operation_id, @@ -2733,7 +2735,7 @@ impl AdminService { workspace_id: created.workspace_id, version: created.version, import_mode: ImportMode::Upsert, - warnings: Vec::new(), + warnings, }; info!( operation_id = %response.operation_id, @@ -3446,6 +3448,7 @@ fn demo_rest_operation_payload() -> OperationPayload { retry_policy: None, response_cache: None, idempotency: None, + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), }, @@ -3513,6 +3516,7 @@ fn demo_archived_operation_payload() -> OperationPayload { retry_policy: None, response_cache: None, idempotency: None, + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), }, @@ -3602,6 +3606,9 @@ fn runtime_error_code(error: &RuntimeError) -> &'static str { RuntimeError::Schema(_) => "schema_error", RuntimeError::Mapping(_) => "mapping_error", RuntimeError::InvalidPreparedRequest { .. } => "invalid_request", + RuntimeError::ConfirmationRequired { .. } => "confirmation_required", + RuntimeError::InvalidConfirmationToken { .. } => "invalid_confirmation_token", + RuntimeError::ConfirmationStoreUnavailable { .. } => "confirmation_unavailable", RuntimeError::RestAdapter(_) => "rest_error", RuntimeError::ProtocolAdapter(_) => "adapter_error", RuntimeError::UnsupportedProtocol { .. } => "unsupported_protocol", @@ -3926,6 +3933,7 @@ mod tests { retry_policy: None, response_cache: Some(ResponseCachePolicy { ttl_ms: 5_000 }), idempotency: None, + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), } @@ -3942,6 +3950,7 @@ mod tests { input_field: Some("request_id".to_owned()), header_name: Some("Idempotency-Key".to_owned()), }), + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), } diff --git a/apps/mcp-server/src/main.rs b/apps/mcp-server/src/main.rs index 74cd6ad..056afa3 100644 --- a/apps/mcp-server/src/main.rs +++ b/apps/mcp-server/src/main.rs @@ -49,6 +49,7 @@ async fn main() -> Result<(), Box> { let runtime = community_default() .with_limits(runtime_limits) .with_response_cache(cache_stores.response.clone()) + .with_coordination_store(cache_stores.coordination.clone()) .build(); let app = build_app( registry, @@ -1853,6 +1854,7 @@ mod tests { retry_policy: None, response_cache: None, idempotency: None, + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), }, diff --git a/apps/ui/js/i18n.js b/apps/ui/js/i18n.js index 964c3ff..8747dea 100644 --- a/apps/ui/js/i18n.js +++ b/apps/ui/js/i18n.js @@ -685,6 +685,7 @@ var TRANSLATIONS = { 'wizard.yaml.none': 'No YAML provided', 'wizard.yaml.none_body': 'Paste a YAML document or load it from a file before importing.', 'wizard.yaml.imported': 'YAML imported', + 'wizard.yaml.imported_with_warnings': 'YAML imported with recommendations', 'wizard.yaml.imported_body': 'The imported operation was saved. The wizard will reopen it now.', 'wizard.yaml.loaded': 'YAML loaded', 'wizard.yaml.loaded_body': 'The YAML configuration was loaded.', @@ -1500,6 +1501,7 @@ var TRANSLATIONS = { 'wizard.yaml.none': 'YAML не предоставлен', 'wizard.yaml.none_body': 'Вставьте YAML-документ или загрузите файл перед импортом.', 'wizard.yaml.imported': 'YAML импортирован', + 'wizard.yaml.imported_with_warnings': 'YAML импортирован с рекомендациями', 'wizard.yaml.imported_body': 'Импортированная операция сохранена. Мастер сейчас откроет ее заново.', 'wizard.yaml.loaded': 'YAML загружен', 'wizard.yaml.loaded_body': 'YAML-конфигурация загружена.', diff --git a/apps/ui/js/wizard-live.js b/apps/ui/js/wizard-live.js index f768b37..bc56f4b 100644 --- a/apps/ui/js/wizard-live.js +++ b/apps/ui/js/wizard-live.js @@ -185,6 +185,22 @@ function showWizardLiveStatus(title, text, isError) { root.classList.toggle('is-success', !isError); } +function showPendingImportGuidance() { + var raw = sessionStorage.getItem('crank_import_guidance'); + if (!raw) return; + sessionStorage.removeItem('crank_import_guidance'); + try { + var warnings = JSON.parse(raw); + if (!Array.isArray(warnings) || warnings.length === 0) return; + showWizardLiveStatus( + tKey('wizard.yaml.imported_with_warnings'), + warnings.map(function(warning) { return '• ' + warning; }).join('\n') + ); + } catch (_) { + // Ignore invalid stale session data. + } +} + function currentDraftVersion() { if (wizardCurrentOperation && wizardCurrentOperation.draft_version_ref) { return wizardCurrentOperation.draft_version_ref.version; @@ -487,6 +503,9 @@ async function importWizardYaml() { return; } var imported = await window.CrankApi.importOperation(wizardWorkspaceId, yamlDocument, 'upsert'); + if (Array.isArray(imported.warnings) && imported.warnings.length > 0) { + sessionStorage.setItem('crank_import_guidance', JSON.stringify(imported.warnings)); + } showWizardLiveStatus(tKey('wizard.yaml.imported'), tKey('wizard.yaml.imported_body')); window.location.href = window.location.pathname + '?mode=edit&operationId=' + encodeURIComponent(imported.operation_id); } @@ -587,9 +606,10 @@ function copyTestResponseToOutputSample() { showWizardLiveStatus(tKey('wizard.test.response_copied'), tKey('wizard.test.response_copied_body')); } -window.CrankWizardLive = { + window.CrankWizardLive = { saveOperation: saveOperation, loadOperationForEdit: loadOperationForEdit, + showPendingImportGuidance: showPendingImportGuidance, bindWizardLiveActions: bindWizardLiveActions, updateWizardProtocolVisibility: updateWizardProtocolVisibility, renderAgentFacingPreview: renderAgentFacingPreview, diff --git a/apps/ui/js/wizard.js b/apps/ui/js/wizard.js index 262e719..b668e7e 100644 --- a/apps/ui/js/wizard.js +++ b/apps/ui/js/wizard.js @@ -17,6 +17,7 @@ var applyEditionProtocolVisibility = window.CrankWizardShell.applyEditionProtoco var step3PanelId = window.CrankWizardShell.step3PanelId; var saveOperation = window.CrankWizardLive.saveOperation; var loadOperationForEdit = window.CrankWizardLive.loadOperationForEdit; +var showPendingImportGuidance = window.CrankWizardLive.showPendingImportGuidance; var bindWizardLiveActions = window.CrankWizardLive.bindWizardLiveActions; var updateWizardProtocolVisibility = window.CrankWizardLive.updateWizardProtocolVisibility; var currentStep = window.currentStep; @@ -138,6 +139,7 @@ async function initWizardPage() { window.wizardEditId = wizardEditId; document.title = 'Crank — ' + tKey('wizard.progress.edit'); await loadOperationForEdit(); + showPendingImportGuidance(); } updateWizardProtocolVisibility(); @@ -172,7 +174,7 @@ if (window.CrankDiagnostics && typeof window.CrankDiagnostics.bootstrap === 'fun var METHOD_CALLOUTS = { GET: null, - DELETE: null, + DELETE: { icon: 'info', text: { en: { title: 'DELETE selected — confirmation is required.', body: 'Crank will expose this MCP tool as a two-step action: the first call returns a confirmation token, the second call with the same arguments and token executes the API request.' }, ru: { title: 'Выбран DELETE — требуется подтверждение.', body: 'Crank откроет этот MCP инструмент как двухшаговое действие: первый вызов вернет токен подтверждения, второй вызов с теми же аргументами и токеном выполнит запрос к API.' } } }, POST: { icon: 'info', text: { en: { title: 'POST selected — request body is required.', body: 'In step 4 you will define the JSON schema for the request body. Crank will convert MCP tool parameters into the selected format.' }, ru: { title: 'Выбран POST — требуется тело запроса.', body: 'На шаге 4 вы зададите JSON-схему тела запроса. Crank преобразует параметры MCP инструмента в выбранный формат.' } } }, PUT: { icon: 'info', text: { en: { title: 'PUT selected — full-resource replacement.', body: 'The request body must contain a complete resource representation. Define the request schema in step 4.' }, ru: { title: 'Выбран PUT — полная замена ресурса.', body: 'Тело запроса должно содержать полное представление ресурса. Задайте схему запроса на шаге 4.' } } }, PATCH: { icon: 'info', text: { en: { title: 'PATCH selected — partial update.', body: 'Only include the fields you want to modify in the request schema defined in step 4.' }, ru: { title: 'Выбран PATCH — частичное обновление.', body: 'В схему запроса на шаге 4 включайте только те поля, которые хотите изменить.' } } }, diff --git a/apps/ui/tests/e2e/wizard.spec.js b/apps/ui/tests/e2e/wizard.spec.js index 1cc05b8..1160eb2 100644 --- a/apps/ui/tests/e2e/wizard.spec.js +++ b/apps/ui/tests/e2e/wizard.spec.js @@ -85,8 +85,9 @@ test('REST method callout clears when switching to methods without request body' await expect(callout).toContainText(/PATCH|Выбран PATCH/); await page.locator('.method-card[data-method="DELETE"]').click(); - await expect(callout).toBeHidden(); - await expect(callout).toHaveText(''); + await expect(callout).toBeVisible(); + await expect(callout).toContainText(/DELETE|Выбран DELETE/); + await expect(callout).toContainText(/confirmation|подтверждение/i); await page.locator('.method-card[data-method="GET"]').click(); await expect(callout).toBeHidden(); diff --git a/crates/crank-community-mcp/src/app.rs b/crates/crank-community-mcp/src/app.rs index 9e7e413..bc3562a 100644 --- a/crates/crank-community-mcp/src/app.rs +++ b/crates/crank-community-mcp/src/app.rs @@ -98,6 +98,12 @@ struct ResolvedToolCall { tool: PublishedAgentTool, } +struct ToolCallExecution { + tool: PublishedAgentTool, + arguments: Value, + confirmation_token: Option, +} + #[derive(Clone, Debug, Deserialize)] struct AgentRoutePath { workspace_slug: String, @@ -381,11 +387,12 @@ async fn mcp_post( } }; - let arguments = if tool_call_params.arguments.is_null() { + let mut arguments = if tool_call_params.arguments.is_null() { json!({}) } else { tool_call_params.arguments }; + let confirmation_token = take_confirmation_token(&mut arguments); match state .catalog @@ -402,6 +409,7 @@ async fn mcp_post( &credential, resolved, arguments, + confirmation_token, &transport_request_id, ) .await @@ -457,6 +465,7 @@ async fn handle_tool_call( credential: &VerifiedMachineCredential, resolved: ResolvedToolCall, arguments: Value, + confirmation_token: Option, transport_request_id: &str, ) -> Response { if !credential_allows_security_level(credential, resolved.tool.operation.security_level) { @@ -483,8 +492,11 @@ async fn handle_tool_call( session, message, response_mode, - resolved.tool, - arguments, + ToolCallExecution { + tool: resolved.tool, + arguments, + confirmation_token, + }, transport_request_id, ) .await @@ -583,12 +595,13 @@ async fn handle_base_tool_call( session: &SessionState, message: &Value, response_mode: ResponseMode, - tool: PublishedAgentTool, - arguments: Value, + execution: ToolCallExecution, transport_request_id: &str, ) -> Response { + let tool = execution.tool; + let arguments = execution.arguments; let operation = runtime_operation(&tool); - let runtime_request_context = RuntimeRequestContext::from_request_id(transport_request_id) + let mut runtime_request_context = RuntimeRequestContext::from_request_id(transport_request_id) .with_response_cache_scope( tool.workspace_id.as_str().to_owned(), tool.agent_id.as_str().to_owned(), @@ -598,6 +611,9 @@ async fn handle_base_tool_call( Some(tool.agent_id.clone()), InvocationSource::AgentToolCall, ); + if let Some(token) = execution.confirmation_token { + runtime_request_context = runtime_request_context.with_confirmation_token(token); + } let request_preview = build_request_preview(&state.runtime, &operation, &arguments); let started_at = Instant::now(); let resolved_auth = @@ -1151,6 +1167,16 @@ fn internal_jsonrpc_error(message: &Value, error: impl std::fmt::Display) -> Res ) } +fn take_confirmation_token(arguments: &mut Value) -> Option { + let Value::Object(object) = arguments else { + return None; + }; + object + .remove("_crank_confirmation_token") + .and_then(|value| value.as_str().map(str::to_owned)) + .filter(|value| !value.trim().is_empty()) +} + fn build_request_preview( runtime: &RuntimeExecutor, operation: &RuntimeOperation, diff --git a/crates/crank-community-mcp/src/manifest.rs b/crates/crank-community-mcp/src/manifest.rs index 68c5638..e9fd51a 100644 --- a/crates/crank-community-mcp/src/manifest.rs +++ b/crates/crank-community-mcp/src/manifest.rs @@ -1,12 +1,29 @@ +use crank_core::{HttpMethod, OperationSafetyClass, OperationSafetyPolicy, Target}; use crank_registry::PublishedAgentTool; use serde_json::{Value, json}; pub fn tool_definitions(tool: &PublishedAgentTool) -> Vec { + let safety = effective_safety_policy(tool); + let requires_confirmation = safety.class.requires_confirmation(); + let input_schema = if requires_confirmation { + add_confirmation_token(schema_to_json_schema(&tool.operation.input_schema)) + } else { + schema_to_json_schema(&tool.operation.input_schema) + }; + let description = if requires_confirmation { + format!( + "{}\n\nЭта операция выполняется в два шага: первый вызов возвращает токен подтверждения без обращения к внешнему API, второй вызов должен повторить те же аргументы и добавить _crank_confirmation_token.", + tool.tool_description + ) + } else { + tool.tool_description.clone() + }; + vec![tool_definition( &tool.tool_name, &tool.tool_title, - &tool.tool_description, - schema_to_json_schema(&tool.operation.input_schema), + &description, + input_schema, )] } @@ -19,6 +36,44 @@ pub fn tool_definition(name: &str, title: &str, description: &str, input_schema: }) } +fn effective_safety_policy(tool: &PublishedAgentTool) -> OperationSafetyPolicy { + tool.operation + .execution_config + .safety + .clone() + .unwrap_or_else(|| OperationSafetyPolicy { + class: infer_safety_class(tool), + confirmation: None, + }) +} + +fn infer_safety_class(tool: &PublishedAgentTool) -> OperationSafetyClass { + match &tool.operation.target { + Target::Rest(target) => match target.method { + HttpMethod::Get => OperationSafetyClass::Read, + HttpMethod::Delete => OperationSafetyClass::Destructive, + HttpMethod::Post | HttpMethod::Put | HttpMethod::Patch => OperationSafetyClass::Write, + }, + } +} + +fn add_confirmation_token(mut schema: Value) -> Value { + let Some(object) = schema.as_object_mut() else { + return schema; + }; + let properties = object.entry("properties").or_insert_with(|| json!({})); + if let Some(properties) = properties.as_object_mut() { + properties.insert( + "_crank_confirmation_token".to_owned(), + json!({ + "type": "string", + "description": "Одноразовый токен подтверждения, который Crank возвращает после первого вызова опасной операции." + }), + ); + } + schema +} + pub fn schema_to_json_schema(schema: &crank_schema::Schema) -> Value { match schema.kind { crank_schema::SchemaKind::Object => { diff --git a/crates/crank-community-mcp/src/tool_error.rs b/crates/crank-community-mcp/src/tool_error.rs index 099f482..7db2d24 100644 --- a/crates/crank-community-mcp/src/tool_error.rs +++ b/crates/crank-community-mcp/src/tool_error.rs @@ -83,6 +83,9 @@ pub fn runtime_error_code(error: &RuntimeError) -> &'static str { RuntimeError::ConcurrencyLimitExceeded { .. } => "runtime_overloaded", RuntimeError::UnsupportedExecutionMode { .. } => "streaming_mode_error", RuntimeError::InvalidPreparedRequest { .. } => "runtime_error", + RuntimeError::ConfirmationRequired { .. } => "confirmation_required", + RuntimeError::InvalidConfirmationToken { .. } => "invalid_confirmation_token", + RuntimeError::ConfirmationStoreUnavailable { .. } => "confirmation_unavailable", RuntimeError::MissingAuthProfile { .. } => "auth_profile_not_found", RuntimeError::MissingSecret { .. } | RuntimeError::MissingSecretVersion { .. } => { "secret_not_found" @@ -128,6 +131,21 @@ fn safe_runtime_error_message(error: &RuntimeError) -> String { RuntimeError::InvalidPreparedRequest { .. } => { "Не удалось подготовить корректный API-запрос.".to_owned() } + RuntimeError::ConfirmationRequired { + confirmation_token, + expires_in_ms, + .. + } => format!( + "Операция требует подтверждения. Повторите вызов с _crank_confirmation_token=\"{}\" в течение {} секунд.", + confirmation_token, + expires_in_ms / 1000 + ), + RuntimeError::InvalidConfirmationToken { .. } => { + "Токен подтверждения недействителен, истек или уже был использован.".to_owned() + } + RuntimeError::ConfirmationStoreUnavailable { .. } => { + "Хранилище подтверждений временно недоступно.".to_owned() + } RuntimeError::MissingAuthProfile { .. } => "Профиль авторизации не найден.".to_owned(), RuntimeError::MissingSecret { .. } | RuntimeError::MissingSecretVersion { .. } => { "Секрет авторизации не найден.".to_owned() @@ -150,6 +168,7 @@ fn is_recoverable_runtime_error(error: &RuntimeError) -> bool { }) | RuntimeError::RestAdapter(RestAdapterError::Transport(_)) | RuntimeError::ConcurrencyLimitExceeded { .. } | RuntimeError::SecretCrypto { .. } + | RuntimeError::ConfirmationRequired { .. } ) } @@ -172,6 +191,13 @@ fn suggested_action(error: &RuntimeError) -> Option<&'static str> { | RuntimeError::InvalidPreparedRequest { .. } => { Some("Проверьте параметры вызова инструмента.") } + RuntimeError::ConfirmationRequired { .. } => { + Some("Повторите вызов с указанным токеном подтверждения.") + } + RuntimeError::InvalidConfirmationToken { .. } => { + Some("Запросите новый токен подтверждения.") + } + RuntimeError::ConfirmationStoreUnavailable { .. } => Some("Повторите запрос позже."), RuntimeError::MissingAuthProfile { .. } | RuntimeError::MissingSecret { .. } | RuntimeError::MissingSecretVersion { .. } diff --git a/crates/crank-community-mcp/tests/unit/manifest.rs b/crates/crank-community-mcp/tests/unit/manifest.rs index 2943548..d4957d4 100644 --- a/crates/crank-community-mcp/tests/unit/manifest.rs +++ b/crates/crank-community-mcp/tests/unit/manifest.rs @@ -35,6 +35,28 @@ fn builds_tools_list_manifest_from_published_agent_tool() { ); } +#[test] +fn marks_destructive_tools_as_two_step_confirmation_calls() { + let mut tool = published_tool(); + let Target::Rest(target) = &mut tool.operation.target; + target.method = HttpMethod::Delete; + + let definitions = tool_definitions(&tool); + let definition = &definitions[0]; + + assert!( + definition["description"] + .as_str() + .unwrap() + .contains("_crank_confirmation_token") + ); + assert_eq!( + definition["inputSchema"]["properties"]["_crank_confirmation_token"]["type"], + "string" + ); + assert_eq!(definition["inputSchema"]["required"], json!(["base"])); +} + fn published_tool() -> PublishedAgentTool { PublishedAgentTool { workspace_id: WorkspaceId::new("ws_01"), @@ -83,6 +105,7 @@ fn operation() -> RegistryOperation { retry_policy: None, response_cache: None, idempotency: None, + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), }, diff --git a/crates/crank-core/src/lib.rs b/crates/crank-core/src/lib.rs index 3eaa86c..facf59a 100644 --- a/crates/crank-core/src/lib.rs +++ b/crates/crank-core/src/lib.rs @@ -56,9 +56,10 @@ pub use observability::{ InvocationLevel, InvocationLog, InvocationSource, InvocationStatus, UsagePeriod, UsageRollup, }; pub use operation::{ - ConfigExport, ExecutionConfig, GeneratedDraft, GeneratedDraftStatus, IdempotencyMode, - IdempotencyPolicy, Operation, OperationStatus, ResponseCachePolicy, RestTarget, RetryPolicy, - Samples, Target, ToolDescription, ToolExample, WizardState, + ConfigExport, ConfirmationPolicy, ExecutionConfig, GeneratedDraft, GeneratedDraftStatus, + IdempotencyMode, IdempotencyPolicy, Operation, OperationSafetyClass, OperationSafetyPolicy, + OperationStatus, ResponseCachePolicy, RestTarget, RetryPolicy, Samples, Target, + ToolDescription, ToolExample, WizardState, }; pub use protocol::{AuthKind, ExportMode, HttpMethod, Protocol}; pub use secret::{Secret, SecretKind, SecretStatus, SecretVersion}; diff --git a/crates/crank-core/src/operation.rs b/crates/crank-core/src/operation.rs index bfa23ee..cc152f7 100644 --- a/crates/crank-core/src/operation.rs +++ b/crates/crank-core/src/operation.rs @@ -70,6 +70,38 @@ pub struct IdempotencyPolicy { pub header_name: Option, } +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum OperationSafetyClass { + #[default] + Read, + Write, + Destructive, + ExternalMessage, + FinancialOrIrreversible, +} + +impl OperationSafetyClass { + pub fn requires_confirmation(self) -> bool { + matches!( + self, + Self::Destructive | Self::ExternalMessage | Self::FinancialOrIrreversible + ) + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ConfirmationPolicy { + pub ttl_ms: u64, +} + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct OperationSafetyPolicy { + pub class: OperationSafetyClass, + #[serde(skip_serializing_if = "Option::is_none")] + pub confirmation: Option, +} + #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct ExecutionConfig { pub timeout_ms: u64, @@ -80,6 +112,8 @@ pub struct ExecutionConfig { #[serde(skip_serializing_if = "Option::is_none")] pub idempotency: Option, #[serde(skip_serializing_if = "Option::is_none")] + pub safety: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub auth_profile_ref: Option, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub headers: BTreeMap, @@ -373,6 +407,7 @@ updated_at: 2026-03-25T08:10:00Z retry_policy: None, response_cache: None, idempotency: None, + safety: None, auth_profile_ref: Some(AuthProfileId::new("auth_01")), headers: BTreeMap::new(), }, diff --git a/crates/crank-runtime/Cargo.toml b/crates/crank-runtime/Cargo.toml index aec9862..de54471 100644 --- a/crates/crank-runtime/Cargo.toml +++ b/crates/crank-runtime/Cargo.toml @@ -28,6 +28,7 @@ thiserror.workspace = true time.workspace = true tokio = { workspace = true, features = ["sync"] } tracing.workspace = true +uuid.workspace = true [dev-dependencies] axum.workspace = true diff --git a/crates/crank-runtime/src/error.rs b/crates/crank-runtime/src/error.rs index 978e8ba..2042438 100644 --- a/crates/crank-runtime/src/error.rs +++ b/crates/crank-runtime/src/error.rs @@ -1,5 +1,5 @@ use crank_adapter_rest::RestAdapterError; -use crank_core::{ExecutionMode, Protocol}; +use crank_core::{ExecutionMode, OperationSafetyClass, Protocol}; use crank_mapping::MappingError; use crank_schema::SchemaError; use thiserror::Error; @@ -25,6 +25,19 @@ pub enum RuntimeError { ConcurrencyLimitExceeded { kind: &'static str, limit: usize }, #[error("invalid prepared request at {field}: {reason}")] InvalidPreparedRequest { field: String, reason: String }, + #[error( + "operation {operation_id} requires confirmation for {safety_class:?} action; retry with confirmation token {confirmation_token}" + )] + ConfirmationRequired { + operation_id: String, + safety_class: OperationSafetyClass, + confirmation_token: String, + expires_in_ms: u64, + }, + #[error("confirmation token for operation {operation_id} is invalid, expired, or already used")] + InvalidConfirmationToken { operation_id: String }, + #[error("confirmation store is unavailable for operation {operation_id}")] + ConfirmationStoreUnavailable { operation_id: String }, #[error("auth profile {auth_profile_id} was not found")] MissingAuthProfile { auth_profile_id: String }, #[error("secret {secret_id} was not found")] diff --git a/crates/crank-runtime/src/executor.rs b/crates/crank-runtime/src/executor.rs index eca9af4..9c5333e 100644 --- a/crates/crank-runtime/src/executor.rs +++ b/crates/crank-runtime/src/executor.rs @@ -4,15 +4,17 @@ use std::time::{Duration, Instant}; use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use crank_core::{ - AdapterRegistry, CachedHeader, CachedResponse, ExecutionMode, HttpMethod, IdempotencyMode, - IdempotencyPolicy, InvocationStatus, MeteringEvent, ResponseCacheStore, SharedMeteringSink, - SharedProtocolAdapter, Target, + AdapterRegistry, CacheScope, CachedHeader, CachedResponse, CoordinationStateStore, + CoordinationStateValue, ExecutionMode, HttpMethod, IdempotencyMode, IdempotencyPolicy, + InvocationStatus, MeteringEvent, OperationSafetyClass, OperationSafetyPolicy, + ResponseCacheStore, SharedMeteringSink, SharedProtocolAdapter, Target, }; use serde_json::{Map, Value, json}; use sha2::{Digest, Sha256}; use time::OffsetDateTime; use tokio::sync::{OwnedSemaphorePermit, Semaphore}; use tracing::debug; +use uuid::Uuid; use crate::{ AdapterResponse, PreparedRequest, ResolvedAuth, RuntimeError, RuntimeLimits, RuntimeOperation, @@ -25,6 +27,7 @@ pub struct RuntimeExecutor { limits: RuntimeLimits, unary_limiter: Arc, response_cache: Option>, + coordination_store: Option>, metering_sink: SharedMeteringSink, } @@ -49,6 +52,7 @@ impl RuntimeExecutor { limits: RuntimeLimits, adapters: AdapterRegistry, response_cache: Option>, + coordination_store: Option>, metering_sink: SharedMeteringSink, ) -> Self { Self { @@ -56,6 +60,7 @@ impl RuntimeExecutor { unary_limiter: Arc::new(Semaphore::new(limits.max_concurrent_unary)), limits, response_cache, + coordination_store, metering_sink, } } @@ -68,6 +73,14 @@ impl RuntimeExecutor { self } + pub fn with_coordination_store( + mut self, + coordination_store: Arc, + ) -> Self { + self.coordination_store = Some(coordination_store); + self + } + pub async fn execute( &self, operation: &RuntimeOperation, @@ -141,6 +154,8 @@ impl RuntimeExecutor { ) -> Result { let mut prepared_request = prepared_request; let idempotency_key = self.prepare_idempotency(operation, input, &mut prepared_request)?; + self.confirm_destructive_operation(operation, input, request_context) + .await?; if let Some(response) = self .load_idempotent_adapter_response( operation, @@ -222,6 +237,43 @@ impl RuntimeExecutor { Ok(Some(key)) } + async fn confirm_destructive_operation( + &self, + operation: &RuntimeOperation, + input: &Value, + request_context: Option<&RuntimeRequestContext>, + ) -> Result<(), RuntimeError> { + let safety = effective_safety_policy(operation); + if !safety.class.requires_confirmation() { + return Ok(()); + } + + let Some(store) = self.coordination_store.as_ref() else { + return Err(RuntimeError::ConfirmationStoreUnavailable { + operation_id: operation.operation_id.as_str().to_owned(), + }); + }; + + let scope = confirmation_scope(operation, request_context)?; + let input_hash = hash_json(input)?; + let provided_token = request_context + .and_then(RuntimeRequestContext::confirmation_token) + .or_else(|| confirmation_token_from_input(input)); + + let Some(provided_token) = provided_token else { + let token = + issue_confirmation_token(store.as_ref(), &scope, &input_hash, &safety).await?; + return Err(RuntimeError::ConfirmationRequired { + operation_id: operation.operation_id.as_str().to_owned(), + safety_class: safety.class, + confirmation_token: token, + expires_in_ms: confirmation_ttl_ms(&safety), + }); + }; + + consume_confirmation_token(store.as_ref(), &scope, provided_token, &input_hash).await + } + async fn record_metering( &self, operation: &RuntimeOperation, @@ -632,6 +684,161 @@ fn idempotency_policy(operation: &RuntimeOperation) -> Option<&IdempotencyPolicy Some(policy) } +fn effective_safety_policy(operation: &RuntimeOperation) -> OperationSafetyPolicy { + operation + .execution_config + .safety + .clone() + .unwrap_or_else(|| OperationSafetyPolicy { + class: infer_safety_class(operation), + confirmation: None, + }) +} + +fn infer_safety_class(operation: &RuntimeOperation) -> OperationSafetyClass { + match &operation.target { + Target::Rest(target) => match target.method { + HttpMethod::Get => OperationSafetyClass::Read, + HttpMethod::Delete => OperationSafetyClass::Destructive, + HttpMethod::Post | HttpMethod::Put | HttpMethod::Patch => OperationSafetyClass::Write, + }, + } +} + +fn confirmation_ttl_ms(policy: &OperationSafetyPolicy) -> u64 { + policy + .confirmation + .as_ref() + .map(|confirmation| confirmation.ttl_ms) + .filter(|ttl_ms| *ttl_ms > 0) + .unwrap_or(300_000) +} + +fn confirmation_scope( + operation: &RuntimeOperation, + request_context: Option<&RuntimeRequestContext>, +) -> Result { + let Some(scope) = request_context.and_then(RuntimeRequestContext::response_cache_scope) else { + return Err(RuntimeError::InvalidPreparedRequest { + field: "runtime_context.response_cache_scope".to_owned(), + reason: "confirmation requires workspace and agent scope".to_owned(), + }); + }; + + Ok(format!( + "workspace:{}:agent:{}:operation:{}:version:{}", + scope.workspace_key, + scope.agent_key, + operation.operation_id.as_str(), + operation.operation_version + )) +} + +async fn issue_confirmation_token( + store: &dyn CoordinationStateStore, + scope: &str, + input_hash: &str, + safety: &OperationSafetyPolicy, +) -> Result { + let token = format!("ct_{}", Uuid::now_v7().simple()); + let key = confirmation_cache_key(scope, &token); + let ttl_ms = confirmation_ttl_ms(safety); + store + .put_value( + CacheScope::Coordination, + &key, + CoordinationStateValue { + payload: json!({ + "scope": scope, + "input_hash": input_hash, + }), + }, + Duration::from_millis(ttl_ms), + ) + .await + .map_err(|error| RuntimeError::InvalidPreparedRequest { + field: "confirmation_token".to_owned(), + reason: error.to_string(), + })?; + Ok(token) +} + +async fn consume_confirmation_token( + store: &dyn CoordinationStateStore, + operation_scope: &str, + token: &str, + input_hash: &str, +) -> Result<(), RuntimeError> { + let key = confirmation_cache_key(operation_scope, token); + let stored = store + .get_value(CacheScope::Coordination, &key) + .await + .map_err(|error| RuntimeError::InvalidPreparedRequest { + field: "confirmation_token".to_owned(), + reason: error.to_string(), + })?; + let _ = store.delete_value(CacheScope::Coordination, &key).await; + + let Some(stored) = stored else { + return Err(RuntimeError::InvalidConfirmationToken { + operation_id: operation_id_from_scope(operation_scope), + }); + }; + + let matches_scope = + stored.payload.get("scope").and_then(Value::as_str) == Some(operation_scope); + let matches_input = + stored.payload.get("input_hash").and_then(Value::as_str) == Some(input_hash); + + if matches_scope && matches_input { + Ok(()) + } else { + Err(RuntimeError::InvalidConfirmationToken { + operation_id: operation_id_from_scope(operation_scope), + }) + } +} + +fn confirmation_cache_key(scope: &str, token: &str) -> String { + let token_hash = URL_SAFE_NO_PAD.encode(Sha256::digest(token.as_bytes())); + format!("crank:confirmation:{scope}:token:{token_hash}") +} + +fn confirmation_token_from_input(input: &Value) -> Option<&str> { + input + .get("_crank_confirmation_token") + .and_then(Value::as_str) + .filter(|value| !value.trim().is_empty()) +} + +fn hash_json(input: &Value) -> Result { + let sanitized = input_without_confirmation_token(input); + let encoded = + serde_json::to_vec(&sanitized).map_err(|error| RuntimeError::InvalidPreparedRequest { + field: "confirmation_input".to_owned(), + reason: error.to_string(), + })?; + Ok(URL_SAFE_NO_PAD.encode(Sha256::digest(encoded))) +} + +fn input_without_confirmation_token(input: &Value) -> Value { + let Value::Object(object) = input else { + return input.clone(); + }; + let mut sanitized = object.clone(); + sanitized.remove("_crank_confirmation_token"); + Value::Object(sanitized) +} + +fn operation_id_from_scope(scope: &str) -> String { + scope + .split(":operation:") + .nth(1) + .and_then(|tail| tail.split(":version:").next()) + .unwrap_or("") + .to_owned() +} + fn idempotency_key_from_policy( policy: &IdempotencyPolicy, input: &Value, diff --git a/crates/crank-runtime/src/executor_builder.rs b/crates/crank-runtime/src/executor_builder.rs index 0fd1089..3e8a75f 100644 --- a/crates/crank-runtime/src/executor_builder.rs +++ b/crates/crank-runtime/src/executor_builder.rs @@ -2,8 +2,8 @@ use std::sync::Arc; use crank_adapter_rest::RestAdapter; use crank_core::{ - AdapterRegistry, MeteringSink, NoopMeteringSink, ResponseCacheStore, SharedMeteringSink, - SharedProtocolAdapter, + AdapterRegistry, CoordinationStateStore, MeteringSink, NoopMeteringSink, ResponseCacheStore, + SharedMeteringSink, SharedProtocolAdapter, }; use crate::{RuntimeExecutor, RuntimeLimits}; @@ -12,6 +12,7 @@ pub struct RuntimeExecutorBuilder { limits: RuntimeLimits, adapters: AdapterRegistry, response_cache: Option>, + coordination_store: Option>, metering_sink: SharedMeteringSink, } @@ -21,6 +22,7 @@ impl RuntimeExecutorBuilder { limits: RuntimeLimits::default(), adapters: AdapterRegistry::empty(), response_cache: None, + coordination_store: None, metering_sink: Arc::new(NoopMeteringSink) as Arc, } } @@ -40,6 +42,11 @@ impl RuntimeExecutorBuilder { self } + pub fn with_coordination_store(mut self, store: Arc) -> Self { + self.coordination_store = Some(store); + self + } + pub fn with_metering_sink(mut self, sink: SharedMeteringSink) -> Self { self.metering_sink = sink; self @@ -50,6 +57,7 @@ impl RuntimeExecutorBuilder { self.limits, self.adapters, self.response_cache, + self.coordination_store, self.metering_sink, ) } diff --git a/crates/crank-runtime/src/request_context.rs b/crates/crank-runtime/src/request_context.rs index 5344c1a..e5b15ad 100644 --- a/crates/crank-runtime/src/request_context.rs +++ b/crates/crank-runtime/src/request_context.rs @@ -14,6 +14,7 @@ pub struct RuntimeRequestContext { pub correlation_id: String, pub response_cache_scope: Option, pub metering_context: Option, + pub confirmation_token: Option, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -30,6 +31,7 @@ impl RuntimeRequestContext { correlation_id: correlation_id.into(), response_cache_scope: None, metering_context: None, + confirmation_token: None, } } @@ -78,6 +80,15 @@ impl RuntimeRequestContext { pub fn metering_context(&self) -> Option<&MeteringContext> { self.metering_context.as_ref() } + + pub fn with_confirmation_token(mut self, token: impl Into) -> Self { + self.confirmation_token = Some(token.into()); + self + } + + pub fn confirmation_token(&self) -> Option<&str> { + self.confirmation_token.as_deref() + } } impl From for crank_core::ResponseCacheScope { @@ -142,6 +153,7 @@ mod tests { assert_eq!(context.request_id, "req_123"); assert_eq!(context.correlation_id, "req_123"); assert!(context.response_cache_scope.is_none()); + assert!(context.confirmation_token.is_none()); } #[test] @@ -167,4 +179,12 @@ mod tests { assert_eq!(metering.agent_id, None); assert_eq!(metering.source, InvocationSource::AdminTestRun); } + + #[test] + fn attaches_confirmation_token() { + let context = + RuntimeRequestContext::from_request_id("req_123").with_confirmation_token("ct_123"); + + assert_eq!(context.confirmation_token(), Some("ct_123")); + } } diff --git a/crates/crank-runtime/tests/integration.rs b/crates/crank-runtime/tests/integration.rs index ac5b022..ba989f4 100644 --- a/crates/crank-runtime/tests/integration.rs +++ b/crates/crank-runtime/tests/integration.rs @@ -1,4 +1,5 @@ mod integration { + mod confirmation; mod idempotency; mod no_input_get; } diff --git a/crates/crank-runtime/tests/integration/confirmation.rs b/crates/crank-runtime/tests/integration/confirmation.rs new file mode 100644 index 0000000..746cc39 --- /dev/null +++ b/crates/crank-runtime/tests/integration/confirmation.rs @@ -0,0 +1,222 @@ +use std::collections::BTreeMap; +use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, +}; + +use async_trait::async_trait; +use crank_core::{ + AdapterResponse, ConfirmationPolicy, ExecutionConfig, ExecutionMode, HttpMethod, Operation, + OperationId, OperationSafetyClass, OperationSafetyPolicy, OperationSecurityLevel, + OperationStatus, Protocol, ProtocolAdapter, ProtocolAdapterError, RestTarget, Target, + ToolDescription, +}; +use crank_mapping::{MappingRule, MappingSet}; +use crank_runtime::{ + InMemoryCoordinationStateStore, RuntimeError, RuntimeExecutorBuilder, RuntimeRequestContext, +}; +use crank_schema::{Schema, SchemaKind}; +use serde_json::json; +use time::OffsetDateTime; + +#[tokio::test] +async fn destructive_operation_requires_one_time_confirmation_token() { + let call_count = Arc::new(AtomicUsize::new(0)); + let executor = RuntimeExecutorBuilder::new() + .register_adapter(Arc::new(CountingAdapter { + call_count: Arc::clone(&call_count), + })) + .with_coordination_store(Arc::new(InMemoryCoordinationStateStore::default())) + .build(); + let operation = destructive_delete_operation().into(); + let context = RuntimeRequestContext::from_request_id("req_confirm") + .with_response_cache_scope("workspace_1", "agent_1"); + + let first = executor + .execute_with_context( + &operation, + &json!({ "order_id": "ord_123" }), + Some(&context), + ) + .await + .expect_err("first destructive call must only stage confirmation"); + let confirmation_token = match first { + RuntimeError::ConfirmationRequired { + confirmation_token, .. + } => confirmation_token, + error => panic!("expected confirmation required, got {error:?}"), + }; + assert_eq!(call_count.load(Ordering::SeqCst), 0); + + let confirmed_context = context + .clone() + .with_confirmation_token(confirmation_token.clone()); + let confirmed = executor + .execute_with_context( + &operation, + &json!({ "order_id": "ord_123" }), + Some(&confirmed_context), + ) + .await + .unwrap(); + assert_eq!(confirmed, json!({ "deleted": true })); + assert_eq!(call_count.load(Ordering::SeqCst), 1); + + let replay = executor + .execute_with_context( + &operation, + &json!({ "order_id": "ord_123" }), + Some(&confirmed_context), + ) + .await + .expect_err("confirmation token must be one-time"); + assert!(matches!( + replay, + RuntimeError::InvalidConfirmationToken { .. } + )); + assert_eq!(call_count.load(Ordering::SeqCst), 1); +} + +struct CountingAdapter { + call_count: Arc, +} + +#[async_trait] +impl ProtocolAdapter for CountingAdapter { + fn protocol(&self) -> Protocol { + Protocol::Rest + } + + fn supports_mode(&self, mode: ExecutionMode) -> bool { + mode == ExecutionMode::Unary + } + + async fn invoke_unary( + &self, + _target: &Target, + prepared: &crank_core::PreparedRequest, + _context: &crank_core::RuntimeRequestContext, + ) -> Result { + assert_eq!( + prepared.path_params.get("order_id"), + Some(&"ord_123".to_owned()) + ); + self.call_count.fetch_add(1, Ordering::SeqCst); + Ok(AdapterResponse { + status_code: 200, + headers: BTreeMap::new(), + body: json!({ "deleted": true }), + data: json!({ "deleted": true }), + }) + } +} + +fn destructive_delete_operation() -> Operation { + Operation { + id: OperationId::new("op_delete_order"), + name: "delete_order".to_owned(), + display_name: "Delete order".to_owned(), + category: "orders".to_owned(), + protocol: Protocol::Rest, + security_level: OperationSecurityLevel::Standard, + status: OperationStatus::Published, + version: 1, + target: Target::Rest(RestTarget { + base_url: "https://api.example.invalid".to_owned(), + method: HttpMethod::Delete, + path_template: "/orders/{order_id}".to_owned(), + static_headers: BTreeMap::new(), + }), + input_schema: object_schema(BTreeMap::from([("order_id".to_owned(), string_schema())])), + output_schema: object_schema(BTreeMap::from([("deleted".to_owned(), bool_schema())])), + input_mapping: MappingSet { + rules: vec![MappingRule { + source: "$.mcp.order_id".to_owned(), + target: "$.request.path.order_id".to_owned(), + required: true, + default_value: None, + transform: None, + condition: None, + notes: None, + }], + }, + output_mapping: MappingSet { + rules: vec![MappingRule { + source: "$.response.body.deleted".to_owned(), + target: "$.output.deleted".to_owned(), + required: true, + default_value: None, + transform: None, + condition: None, + notes: None, + }], + }, + execution_config: ExecutionConfig { + timeout_ms: 1_000, + retry_policy: None, + response_cache: None, + idempotency: None, + safety: Some(OperationSafetyPolicy { + class: OperationSafetyClass::Destructive, + confirmation: Some(ConfirmationPolicy { ttl_ms: 60_000 }), + }), + auth_profile_ref: None, + headers: BTreeMap::new(), + }, + tool_description: ToolDescription { + title: "Delete order".to_owned(), + description: "Deletes an order after explicit confirmation.".to_owned(), + tags: Vec::new(), + examples: Vec::new(), + }, + samples: None, + generated_draft: None, + config_export: None, + wizard_state: None, + created_at: OffsetDateTime::UNIX_EPOCH, + updated_at: OffsetDateTime::UNIX_EPOCH, + published_at: None, + } +} + +fn object_schema(fields: BTreeMap) -> Schema { + Schema { + kind: SchemaKind::Object, + description: None, + required: true, + nullable: false, + default_value: None, + fields, + items: None, + enum_values: Vec::new(), + variants: Vec::new(), + } +} + +fn string_schema() -> Schema { + Schema { + kind: SchemaKind::String, + description: None, + required: true, + nullable: false, + default_value: None, + fields: BTreeMap::new(), + items: None, + enum_values: Vec::new(), + variants: Vec::new(), + } +} + +fn bool_schema() -> Schema { + Schema { + kind: SchemaKind::Boolean, + description: None, + required: true, + nullable: false, + default_value: None, + fields: BTreeMap::new(), + items: None, + enum_values: Vec::new(), + variants: Vec::new(), + } +} diff --git a/crates/crank-runtime/tests/integration/idempotency.rs b/crates/crank-runtime/tests/integration/idempotency.rs index 1c88137..1cc175f 100644 --- a/crates/crank-runtime/tests/integration/idempotency.rs +++ b/crates/crank-runtime/tests/integration/idempotency.rs @@ -145,6 +145,7 @@ fn idempotent_post_operation() -> Operation { input_field: Some("request_id".to_owned()), header_name: Some("Idempotency-Key".to_owned()), }), + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), }, diff --git a/crates/crank-runtime/tests/integration/no_input_get.rs b/crates/crank-runtime/tests/integration/no_input_get.rs index 5bc8b4c..5396356 100644 --- a/crates/crank-runtime/tests/integration/no_input_get.rs +++ b/crates/crank-runtime/tests/integration/no_input_get.rs @@ -70,6 +70,7 @@ fn no_input_get_operation() -> Operation { retry_policy: None, response_cache: None, idempotency: None, + safety: None, auth_profile_ref: None, headers: BTreeMap::new(), }, diff --git a/docs/tool-design.md b/docs/tool-design.md new file mode 100644 index 0000000..0ed0a4b --- /dev/null +++ b/docs/tool-design.md @@ -0,0 +1,166 @@ +# Проектирование MCP-инструментов + +Этот документ описывает, как делать REST-инструменты, с которыми LLM-клиент работает предсказуемо. + +## Главная идея + +Хороший инструмент решает одну понятную задачу. Модель должна легко понять: + +- когда вызывать инструмент; +- какие параметры нужны; +- что считается успешным результатом; +- что делать при ошибке. + +Плохой инструмент обычно слишком общий: `call_api`, `manage_user`, `execute_request`. Такие названия и описания заставляют модель угадывать. + +## Название и описание + +Имя инструмента должно быть техническим и стабильным: + +```text +get_customer_by_email +create_support_ticket +delete_draft_invoice +``` + +Описание должно начинаться с действия и объяснять сценарий: + +```text +Получает карточку клиента по email. Используйте, когда нужно найти клиента перед созданием обращения или проверкой статуса заказа. Возвращает идентификатор клиента, имя, email и текущий статус. +``` + +Не используйте расплывчатые формулировки: + +```text +Работает с клиентами. +Обрабатывает данные. +Выполняет запрос к API. +``` + +## Входные параметры + +Каждый параметр должен иметь понятный смысл. Если значение выбирается из небольшого набора, лучше задать `enum`. + +Плохо: + +```json +{ + "action": "string", + "data": "object" +} +``` + +Хорошо: + +```json +{ + "email": "user@example.com", + "include_orders": true +} +``` + +Если endpoint делает несколько разных действий через параметр `action`, чаще всего лучше разделить его на несколько инструментов. + +## Ответ инструмента + +Не отдавайте модели весь ответ внешнего API без необходимости. Лучше выбрать только поля, которые нужны для следующего шага рассуждения. + +Плохо: + +```json +{ + "response": "{ весь JSON от внешнего API }" +} +``` + +Хорошо: + +```json +{ + "customer_id": "cus_123", + "status": "active", + "open_orders_count": 2 +} +``` + +Для массивов ограничивайте размер ответа и возвращайте только важные поля. + +## Ошибки + +Crank возвращает структурированные ошибки. MCP-клиент получает код ошибки, сообщение, признак повторяемости и рекомендацию. + +Пример: + +```json +{ + "error_code": "upstream_rate_limited", + "message": "Внешний API вернул HTTP 429.", + "recoverable": true, + "suggested_action": "Повторите запрос позже.", + "request_id": "req_123" +} +``` + +Это лучше, чем отдавать сырой ответ внешнего API или stack trace. + +## Повторное выполнение POST/PATCH + +Для операций, которые меняют состояние, используйте idempotency key. Это защищает от повторного создания сущности, если MCP-клиент повторит вызов после таймаута. + +Пример входа: + +```json +{ + "request_id": "order-2026-06-21-001", + "customer_id": "cus_123", + "amount": 1200 +} +``` + +В конфигурации операции можно указать, что `request_id` используется как ключ идемпотентности и передается во внешний API через заголовок `Idempotency-Key`. + +## Опасные операции + +Для `DELETE` Crank использует двухшаговое подтверждение. + +Первый вызов не отправляет запрос во внешний API. Он возвращает confirmation token: + +```json +{ + "error_code": "confirmation_required", + "message": "Операция требует подтверждения. Повторите вызов с _crank_confirmation_token=\"ct_...\"." +} +``` + +Второй вызов должен повторить те же аргументы и добавить токен: + +```json +{ + "order_id": "ord_123", + "_crank_confirmation_token": "ct_..." +} +``` + +Токен одноразовый, имеет короткий срок жизни и привязан к агенту, операции, версии и исходным аргументам. + +## Каталог агента + +Не привязывайте к одному агенту весь API. Лучше создать несколько агентов под конкретные задачи: + +- агент для курсов валют; +- агент для поддержки клиентов; +- агент для заявок и инцидентов; +- агент для отчетов. + +Если у агента слишком много похожих инструментов, модель чаще ошибается при выборе. + +## Проверочный список + +- Имя инструмента конкретное и не похоже на `call_api`. +- Описание объясняет, когда вызывать инструмент. +- У каждого важного параметра есть понятное назначение. +- Multi-action endpoint разделен на отдельные инструменты. +- Ответ не содержит лишний большой JSON. +- Для POST/PATCH задан idempotency key, если повторный вызов может создать дубль. +- Для DELETE пользователь видит двухшаговое подтверждение. +- Агенту привязаны только инструменты, нужные для его задачи.