From 68b8febf3af4f6c77e8c5372749e59a5c10f8657 Mon Sep 17 00:00:00 2001
From: "Andrei G." <k05h31@gmail.com>
Date: Mon, 13 Jul 2026 18:12:58 +0200
Subject: [PATCH] refactor(scheduler): route DynSchedulerExecutor through
 DynExecutor erasure path

Delete the hand-maintained Arc<SchedulerExecutor> ToolExecutor wrapper,
which forwarded only 9 of the trait's 13 methods and was structurally
identical to the Arc<ShellExecutor> anti-pattern that caused #5985. The
ACP scheduler tool executor now flows through the existing
zeph_tools::DynExecutor/ErasedToolExecutor erasure path, which forwards
all methods by construction instead of requiring manual per-method
overrides that can silently drift out of parity.

Closes #6000
---
 CHANGELOG.md              | 10 +++++++
 src/acp.rs                |  3 +--
 src/scheduler_executor.rs | 57 ++++++++-------------------------------
 3 files changed, 22 insertions(+), 48 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d32d1afd1..6707a6499 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -72,6 +72,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   before. `--migrate-config` drops any leftover `require_tls`/`ssrf_protection` keys from an
   existing `[a2a]` table, warning the user, without erroring (#5885).
 
+- **ACP**: removed `DynSchedulerExecutor`, a hand-maintained `ToolExecutor` wrapper around
+  `Arc<SchedulerExecutor>` that forwarded 9 of the trait's 13 methods (omitting
+  `execute_confirmed`, `set_skill_env`, `set_effective_trust`, `is_tool_retryable`) — the 8th+
+  recurrence of the "leaf type gains a trait method, some Arc wrapper silently drifts" defect
+  class (#5899/#5905/#5906/#5985). The scheduler's ACP tool executor is now wired through the
+  existing `zeph_tools::DynExecutor`/`ErasedToolExecutor` erasure path (already used by every
+  other dynamically-composed executor), which forwards all 13 methods by construction — no
+  bespoke wrapper to drift. `DynSchedulerExecutor` was `pub(crate)`, not a public API, so this
+  is an internal refactor with no behavior change (#6000).
+
 ### Docs
 
 - **LLM**: `AnyProvider`/`Router`/`Triage`'s `capability_delegation_advisory()` rustdoc comments
diff --git a/src/acp.rs b/src/acp.rs
index 46fd24741..594bf88a8 100644
--- a/src/acp.rs
+++ b/src/acp.rs
@@ -1869,8 +1869,7 @@ async fn spawn_acp_agent(
             agent = agent.with_custom_task_rx(rx);
         }
         if let Some(sched_exec) = scheduler_executor {
-            agent = agent
-                .add_tool_executor(crate::scheduler_executor::DynSchedulerExecutor(sched_exec));
+            agent = agent.add_tool_executor(zeph_tools::DynExecutor(sched_exec));
         }
     }
 
diff --git a/src/scheduler_executor.rs b/src/scheduler_executor.rs
index 10485e34c..4916957c6 100644
--- a/src/scheduler_executor.rs
+++ b/src/scheduler_executor.rs
@@ -516,52 +516,6 @@ impl ToolExecutor for SchedulerExecutor {
     zeph_tools::tool_executor_no_inner_defaults!();
 }
 
-/// `Arc`-wrapper so `SchedulerExecutor` can be shared across ACP sessions without `Clone`.
-#[cfg(feature = "acp")]
-pub(crate) struct DynSchedulerExecutor(pub(crate) std::sync::Arc<SchedulerExecutor>);
-
-#[cfg(feature = "acp")]
-impl ToolExecutor for DynSchedulerExecutor {
-    async fn execute(&self, response: &str) -> Result<Option<ToolOutput>, ToolError> {
-        self.0.execute(response).await
-    }
-
-    fn tool_definitions(&self) -> Vec<ToolDef> {
-        self.0.tool_definitions()
-    }
-
-    async fn execute_tool_call(&self, call: &ToolCall) -> Result<Option<ToolOutput>, ToolError> {
-        self.0.execute_tool_call(call).await
-    }
-
-    fn requires_confirmation(&self, call: &ToolCall) -> bool {
-        self.0.requires_confirmation(call)
-    }
-
-    async fn execute_tool_call_confirmed(
-        &self,
-        call: &ToolCall,
-    ) -> Result<Option<ToolOutput>, ToolError> {
-        self.0.execute_tool_call_confirmed(call).await
-    }
-
-    fn checkpoint_undo(&self, n: usize) -> zeph_tools::CheckpointActionResult {
-        self.0.checkpoint_undo(n)
-    }
-
-    fn checkpoint_redo(&self) -> zeph_tools::CheckpointActionResult {
-        self.0.checkpoint_redo()
-    }
-
-    fn checkpoint_list(&self) -> zeph_tools::CheckpointListResult {
-        self.0.checkpoint_list()
-    }
-
-    fn is_tool_speculatable(&self, tool_id: &str) -> bool {
-        self.0.is_tool_speculatable(tool_id)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use std::sync::Arc;
@@ -756,6 +710,17 @@ mod tests {
         assert_eq!(exec.tool_definitions().len(), 4);
     }
 
+    #[cfg(feature = "acp")]
+    #[tokio::test]
+    async fn dyn_executor_erasure_path_dispatches() {
+        let (exec, _rx) = make_executor().await;
+        let dyn_exec = zeph_tools::DynExecutor(Arc::new(exec));
+        assert_eq!(dyn_exec.tool_definitions().len(), 4);
+        let call = make_call("list_tasks", serde_json::json!({}));
+        let result = dyn_exec.execute_tool_call(&call).await.unwrap().unwrap();
+        assert!(result.summary.contains("No active scheduled tasks"));
+    }
+
     #[tokio::test]
     async fn list_tasks_empty() {
         let (exec, _rx) = make_executor().await;