fix(tools): forward checkpoint and confirmation methods through executor wrappers

CHANGELOG.md

@@ -5,6 +5,21 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
+### Fixed
+
+- `fix(tools)`: `checkpoint_undo`/`checkpoint_redo`/`checkpoint_list` are now forwarded to the
+  wrapped inner executor by `TrustGateExecutor`, `PolicyGateExecutor`,
+  `AdversarialPolicyGateExecutor` (#5899), `ScopedToolExecutor`, and `ShadowProbeExecutor`
+  (#5905). Previously none of these `ToolExecutor` wrappers overrode the three checkpoint
+  methods, so calls fell through to the trait's no-op default instead of reaching
+  `ShellExecutor` — `/undo`, `/redo`, and `/undo list` always reported "Checkpoints are not
+  enabled" whenever trust/policy/adversarial gating, `capability_scopes`, or `shadow_sentinel`
+  wrapped the executor chain, even with `[tools.shell] checkpoints_enabled = true` set — the
+  standard, default-recommended production configuration, not an edge case. Also fixes
+  `ScopedToolExecutor::requires_confirmation` (#5906), previously hardcoded to the trait
+  default `false` regardless of the real policy underneath, affecting the (currently dormant)
+  speculative-dispatch engine.
+
 ### Added
 
 - `feat(acp)`: `[[acp.auth_clients]]` — named bearer-token clients for the ACP HTTP/WS

crates/zeph-tools/src/adversarial_gate.rs

@@ -243,6 +243,18 @@ impl<T: ToolExecutor> ToolExecutor for AdversarialPolicyGateExecutor<T> {
     fn is_tool_retryable(&self, tool_id: &str) -> bool {
         self.inner.is_tool_retryable(tool_id)
     }
+
+    fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_undo(n)
+    }
+
+    fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_redo()
+    }
+
+    fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+        self.inner.checkpoint_list()
+    }
 }
 
 fn params_summary(params: &serde_json::Map<String, serde_json::Value>) -> String {
@@ -525,6 +537,57 @@ mod tests {
         assert!(!retryable, "MockInner returns false for is_tool_retryable");
     }
 
+    #[derive(Debug)]
+    struct CheckpointingInner;
+
+    impl ToolExecutor for CheckpointingInner {
+        async fn execute(&self, _: &str) -> Result<Option<ToolOutput>, ToolError> {
+            Ok(None)
+        }
+        async fn execute_tool_call(&self, _: &ToolCall) -> Result<Option<ToolOutput>, ToolError> {
+            Ok(None)
+        }
+        fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                reverted_commands: n,
+                ..Default::default()
+            }
+        }
+        fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                ..Default::default()
+            }
+        }
+        fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+            crate::executor::CheckpointListResult {
+                supported: true,
+                ..Default::default()
+            }
+        }
+    }
+
+    #[tokio::test]
+    async fn delegation_checkpoint_methods() {
+        let (_, llm) = MockLlm::new("ALLOW");
+        let gate = AdversarialPolicyGateExecutor::new(
+            CheckpointingInner,
+            make_validator(false),
+            Arc::new(llm),
+        );
+        let undo_result = gate.checkpoint_undo(7);
+        assert!(undo_result.supported);
+        assert_eq!(
+            undo_result.reverted_commands, 7,
+            "n must be forwarded, not hardcoded"
+        );
+        assert!(gate.checkpoint_redo().supported);
+        assert!(gate.checkpoint_list().supported);
+    }
+
     #[tokio::test]
     async fn delegation_tool_definitions() {
         let (_, llm) = MockLlm::new("ALLOW");

crates/zeph-tools/src/policy_gate.rs

@@ -380,6 +380,18 @@ impl<T: ToolExecutor> ToolExecutor for PolicyGateExecutor<T> {
     fn is_tool_speculatable(&self, tool_id: &str) -> bool {
         self.inner.is_tool_speculatable(tool_id)
     }
+
+    fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_undo(n)
+    }
+
+    fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_redo()
+    }
+
+    fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+        self.inner.checkpoint_list()
+    }
 }
 
 fn truncate_params(params: &serde_json::Map<String, serde_json::Value>) -> String {
@@ -467,6 +479,64 @@ mod tests {
         }
     }
 
+    #[derive(Debug)]
+    struct CheckpointingExecutor;
+
+    impl ToolExecutor for CheckpointingExecutor {
+        async fn execute(&self, _: &str) -> Result<Option<ToolOutput>, ToolError> {
+            Ok(None)
+        }
+        async fn execute_tool_call(&self, _: &ToolCall) -> Result<Option<ToolOutput>, ToolError> {
+            Ok(None)
+        }
+        fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                reverted_commands: n,
+                ..Default::default()
+            }
+        }
+        fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                ..Default::default()
+            }
+        }
+        fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+            crate::executor::CheckpointListResult {
+                supported: true,
+                ..Default::default()
+            }
+        }
+    }
+
+    #[test]
+    fn checkpoint_methods_delegated_to_inner() {
+        let config = PolicyConfig {
+            enabled: false,
+            default_effect: DefaultEffect::Allow,
+            rules: vec![],
+            policy_file: None,
+            policy_provider: ProviderName::default(),
+        };
+        let enforcer = Arc::new(PolicyEnforcer::compile(&config).unwrap());
+        let context = Arc::new(RwLock::new(PolicyContext {
+            trust_level: SkillTrustLevel::Trusted,
+            env: HashMap::new(),
+        }));
+        let gate = PolicyGateExecutor::new(CheckpointingExecutor, enforcer, context);
+        let undo_result = gate.checkpoint_undo(7);
+        assert!(undo_result.supported);
+        assert_eq!(
+            undo_result.reverted_commands, 7,
+            "n must be forwarded, not hardcoded"
+        );
+        assert!(gate.checkpoint_redo().supported);
+        assert!(gate.checkpoint_list().supported);
+    }
+
     #[tokio::test]
     async fn allow_by_default_when_default_allow() {
         let config = PolicyConfig {

crates/zeph-tools/src/scope.rs

@@ -611,6 +611,22 @@ impl<E: ToolExecutor> ToolExecutor for ScopedToolExecutor<E> {
     fn is_tool_speculatable(&self, tool_id: &str) -> bool {
         self.inner.is_tool_speculatable(tool_id)
     }
+
+    fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_undo(n)
+    }
+
+    fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_redo()
+    }
+
+    fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+        self.inner.checkpoint_list()
+    }
+
+    fn requires_confirmation(&self, call: &ToolCall) -> bool {
+        self.inner.requires_confirmation(call)
+    }
 }
 
 // ── Config-driven builder ──────────────────────────────────────────────────────
@@ -724,6 +740,38 @@ mod tests {
         }
     }
 
+    struct CheckpointingExecutor;
+
+    impl ToolExecutor for CheckpointingExecutor {
+        async fn execute(&self, _: &str) -> Result<Option<ToolOutput>, ToolError> {
+            Ok(None)
+        }
+        fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                reverted_commands: n,
+                ..Default::default()
+            }
+        }
+        fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                ..Default::default()
+            }
+        }
+        fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+            crate::executor::CheckpointListResult {
+                supported: true,
+                ..Default::default()
+            }
+        }
+        fn requires_confirmation(&self, _call: &ToolCall) -> bool {
+            true
+        }
+    }
+
     fn null_def(id: &str) -> ToolDef {
         ToolDef {
             id: id.to_owned().into(),
@@ -1060,6 +1108,25 @@ mod tests {
         assert!(!updated.admits("builtin:write"));
     }
 
+    #[test]
+    fn checkpoint_methods_delegated_to_inner() {
+        let executor = ScopedToolExecutor::new(CheckpointingExecutor, ToolScope::full());
+        let undo_result = executor.checkpoint_undo(7);
+        assert!(undo_result.supported);
+        assert_eq!(
+            undo_result.reverted_commands, 7,
+            "n must be forwarded, not hardcoded"
+        );
+        assert!(executor.checkpoint_redo().supported);
+        assert!(executor.checkpoint_list().supported);
+    }
+
+    #[test]
+    fn requires_confirmation_delegated_to_inner() {
+        let executor = ScopedToolExecutor::new(CheckpointingExecutor, ToolScope::full());
+        assert!(executor.requires_confirmation(&make_call("builtin:shell")));
+    }
+
     #[test]
     fn build_from_config_with_scopes() {
         let mut scopes = std::collections::HashMap::new();

crates/zeph-tools/src/shadow_probe.rs

@@ -388,6 +388,18 @@ impl<T: ToolExecutor> ToolExecutor for ShadowProbeExecutor<T> {
     fn requires_confirmation(&self, call: &ToolCall) -> bool {
         self.inner.requires_confirmation(call)
     }
+
+    fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_undo(n)
+    }
+
+    fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+        self.inner.checkpoint_redo()
+    }
+
+    fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+        self.inner.checkpoint_list()
+    }
 }
 
 #[cfg(test)]
@@ -784,6 +796,52 @@ mod tests {
         assert_eq!(probe.recorded.lock().unwrap().len(), 1);
     }
 
+    struct CheckpointingInner;
+    impl ToolExecutor for CheckpointingInner {
+        async fn execute(&self, _: &str) -> Result<Option<ToolOutput>, ToolError> {
+            Ok(None)
+        }
+        fn checkpoint_undo(&self, n: usize) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                reverted_commands: n,
+                ..Default::default()
+            }
+        }
+        fn checkpoint_redo(&self) -> crate::executor::CheckpointActionResult {
+            crate::executor::CheckpointActionResult {
+                supported: true,
+                message: "stub".into(),
+                ..Default::default()
+            }
+        }
+        fn checkpoint_list(&self) -> crate::executor::CheckpointListResult {
+            crate::executor::CheckpointListResult {
+                supported: true,
+                ..Default::default()
+            }
+        }
+    }
+
+    #[test]
+    fn checkpoint_methods_delegated_to_inner() {
+        let exec = ShadowProbeExecutor::new(
+            CheckpointingInner,
+            Arc::new(AllowProbe),
+            Arc::new(std::sync::atomic::AtomicU64::new(1)),
+            Arc::new(parking_lot::RwLock::new("calm".to_owned())),
+        );
+        let undo_result = exec.checkpoint_undo(7);
+        assert!(undo_result.supported);
+        assert_eq!(
+            undo_result.reverted_commands, 7,
+            "n must be forwarded, not hardcoded"
+        );
+        assert!(exec.checkpoint_redo().supported);
+        assert!(exec.checkpoint_list().supported);
+    }
+
     #[test]
     fn is_tool_speculatable_always_false() {
         let exec = make_executor(AllowProbe);