Add destructive confirmation and import guidance

2026-06-21 01:42:45 +00:00
parent ef2912855b
commit 5447e1bad0
29 changed files with 1099 additions and 24 deletions
@@ -2337,6 +2337,7 @@ mod tests {
                retry_policy: None,
                response_cache: None,
                idempotency: None,
+                safety: None,
                auth_profile_ref: None,
                headers: BTreeMap::new(),
            },
@@ -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<Value> {
            "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,
@@ -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<String> {
+    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(),
+        }
+    }
+}
@@ -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;
@@ -59,6 +59,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
    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());
@@ -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(),
        }
@@ -49,6 +49,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
    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(),
            },
@@ -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-конфигурация загружена.',
@@ -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,
@@ -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 включайте только те поля, которые хотите изменить.' } } },
@@ -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();