runtime: stop fabricating tool-call success on backend-less runners (honest UnavailableToolExecutor)

Author: symbibot

crates/repl-core/src/dsl/reasoning_builtins.rs

@@ -64,7 +64,7 @@ pub async fn builtin_reason(args: &[DslValue], ctx: &ReasoningBuiltinContext) ->
     use symbi_runtime::reasoning::circuit_breaker::CircuitBreakerRegistry;
     use symbi_runtime::reasoning::context_manager::DefaultContextManager;
     use symbi_runtime::reasoning::conversation::{Conversation, ConversationMessage};
-    use symbi_runtime::reasoning::executor::DefaultActionExecutor;
+    use symbi_runtime::reasoning::executor::UnavailableToolExecutor;
     use symbi_runtime::reasoning::loop_types::{BufferedJournal, LoopConfig};
     use symbi_runtime::reasoning::policy_bridge::DefaultPolicyGate;
     use symbi_runtime::reasoning::reasoning_loop::ReasoningLoopRunner;
@@ -83,7 +83,7 @@ pub async fn builtin_reason(args: &[DslValue], ctx: &ReasoningBuiltinContext) ->
     let runner = ReasoningLoopRunner {
         provider: Arc::clone(provider),
         policy_gate,
-        executor: Arc::new(DefaultActionExecutor::default()),
+        executor: Arc::new(UnavailableToolExecutor),
         context_manager: Arc::new(DefaultContextManager::default()),
         circuit_breakers: Arc::new(CircuitBreakerRegistry::default()),
         journal: Arc::new(BufferedJournal::new(1000)),
@@ -221,14 +221,23 @@ pub async fn builtin_tool_call(
         }
     };
 
-    // In a full setup, this would go through ToolInvocationEnforcer.
-    // For now, return a structured result indicating the tool call was made.
+    // There is no tool backend wired into the DSL runtime yet (MCP-backed
+    // execution via ToolInvocationEnforcer is a planned feature). Return an
+    // honest `not_executed` result rather than fabricating a success — callers
+    // must not assume the tool actually ran.
     let mut result = HashMap::new();
     result.insert("tool".to_string(), DslValue::String(name));
     result.insert("arguments".to_string(), DslValue::String(arguments));
     result.insert(
         "status".to_string(),
-        DslValue::String("executed".to_string()),
+        DslValue::String("not_executed".to_string()),
+    );
+    result.insert(
+        "reason".to_string(),
+        DslValue::String(
+            "no tool backend configured (MCP-backed tool execution is not yet available)"
+                .to_string(),
+        ),
     );
 
     Ok(DslValue::Map(result))

crates/runtime/src/reasoning/executor.rs

@@ -174,6 +174,54 @@ async fn execute_tool_call(name: &str, arguments: &str) -> Result<String, String
     ))
 }
 
+/// An [`ActionExecutor`] for runners that have **no tool backend wired**.
+///
+/// It advertises no tools (so the model is not offered any) and, if a tool
+/// call is nonetheless proposed, returns a clear `is_error` observation
+/// instead of fabricating a success. This is the honest default for entry
+/// points like `symbi run` and the DSL `reason()` builtin, which do not yet
+/// construct an MCP client / [`EnforcedActionExecutor`]. It exists so those
+/// paths never tell the model a tool "executed successfully" when nothing ran.
+///
+/// Contrast: [`DefaultActionExecutor`] is a parallel-dispatch executor whose
+/// per-call result is an echo placeholder (useful as a test double, not for
+/// production tool execution); [`EnforcedActionExecutor`] performs real,
+/// enforcer-gated execution when a tool backend is available.
+#[derive(Default)]
+pub struct UnavailableToolExecutor;
+
+#[async_trait]
+impl ActionExecutor for UnavailableToolExecutor {
+    async fn execute_actions(
+        &self,
+        actions: &[ProposedAction],
+        _config: &LoopConfig,
+        _circuit_breakers: &CircuitBreakerRegistry,
+    ) -> Vec<Observation> {
+        actions
+            .iter()
+            .filter_map(|action| match action {
+                ProposedAction::ToolCall { call_id, name, .. } => Some(Observation {
+                    source: name.clone(),
+                    content: format!(
+                        "Tool '{}' was not executed: this runner has no tool backend \
+                         configured (MCP-backed tool execution is not yet available). \
+                         Reason about the task and respond without tool results.",
+                        name
+                    ),
+                    is_error: true,
+                    call_id: Some(call_id.clone()),
+                    metadata: Default::default(),
+                }),
+                _ => None,
+            })
+            .collect()
+    }
+
+    // tool_definitions() falls back to the trait default (empty) — advertise
+    // no tools, so the model isn't offered capabilities the runner can't run.
+}
+
 /// An executor that delegates to a real ToolInvocationEnforcer.
 pub struct EnforcedActionExecutor {
     enforcer: std::sync::Arc<dyn crate::integrations::tool_invocation::ToolInvocationEnforcer>,
@@ -351,6 +399,42 @@ mod tests {
         assert_eq!(obs[0].call_id.as_deref(), Some("c1"));
     }
 
+    #[tokio::test]
+    async fn test_unavailable_tool_executor_reports_error_not_fake_success() {
+        let executor = UnavailableToolExecutor;
+        let config = LoopConfig::default();
+        let circuit_breakers = CircuitBreakerRegistry::default();
+
+        // Advertises no tools.
+        assert!(executor.tool_definitions().is_empty());
+
+        let actions = vec![ProposedAction::ToolCall {
+            call_id: "c1".into(),
+            name: "search".into(),
+            arguments: r#"{"q": "test"}"#.into(),
+        }];
+        let obs = executor
+            .execute_actions(&actions, &config, &circuit_breakers)
+            .await;
+
+        assert_eq!(obs.len(), 1);
+        // The key property: a call is surfaced as an error, never as a
+        // fabricated success.
+        assert!(obs[0].is_error);
+        assert_eq!(obs[0].source, "search");
+        assert_eq!(obs[0].call_id.as_deref(), Some("c1"));
+        assert!(obs[0].content.contains("not executed"));
+
+        // Non-tool actions produce no observations.
+        let non_tool = vec![ProposedAction::Respond {
+            content: "hi".into(),
+        }];
+        assert!(executor
+            .execute_actions(&non_tool, &config, &circuit_breakers)
+            .await
+            .is_empty());
+    }
+
     #[tokio::test]
     async fn test_default_executor_parallel_dispatch() {
         let executor = DefaultActionExecutor::default();

src/commands/run.rs

@@ -88,7 +88,7 @@ pub async fn run(matches: &ArgMatches) {
     use symbi_runtime::reasoning::circuit_breaker::CircuitBreakerRegistry;
     use symbi_runtime::reasoning::context_manager::DefaultContextManager;
     use symbi_runtime::reasoning::conversation::{Conversation, ConversationMessage};
-    use symbi_runtime::reasoning::executor::DefaultActionExecutor;
+    use symbi_runtime::reasoning::executor::UnavailableToolExecutor;
     use symbi_runtime::reasoning::loop_types::{BufferedJournal, LoopConfig};
     use symbi_runtime::reasoning::policy_bridge::DefaultPolicyGate;
     use symbi_runtime::reasoning::reasoning_loop::ReasoningLoopRunner;
@@ -122,7 +122,10 @@ pub async fn run(matches: &ArgMatches) {
     let runner = ReasoningLoopRunner {
         provider,
         policy_gate,
-        executor: Arc::new(DefaultActionExecutor::default()),
+        // `symbi run` has no tool backend wired, so use the honest executor
+        // that surfaces tool calls as errors rather than fabricating success.
+        // (MCP-backed execution is a separate, planned feature.)
+        executor: Arc::new(UnavailableToolExecutor),
         context_manager: Arc::new(DefaultContextManager::default()),
         circuit_breakers: Arc::new(CircuitBreakerRegistry::default()),
         journal: Arc::new(BufferedJournal::new(1000)),