Skip to content

checkpoint_undo/redo/list not forwarded by ScopedToolExecutor/ShadowProbeExecutor — breaks /undo even after #5899, since these sit outside the gated chain #5905

Description

@bug-ops

Description

Same defect class as #5899 (checkpoint methods silently dropped at a wrapper boundary), but on the two outermost wrapper layers, which means fixing #5899 alone will not restore /undo//redo whenever capability_scopes or shadow_sentinel is enabled.

ScopedToolExecutor (crates/zeph-tools/src/scope.rs:435-614) and ShadowProbeExecutor (crates/zeph-tools/src/shadow_probe.rs:205-391) both implement ToolExecutor for a generic inner executor E/T and forward most cross-cutting methods (set_skill_env, set_effective_trust, is_tool_retryable, is_tool_speculatable) to self.inner. Neither overrides checkpoint_undo, checkpoint_redo, or checkpoint_list — grepping both files for checkpoint returns zero matches outside doc/comment text.

Per scope.rs's own module doc, the production wiring order is:

ScopedToolExecutor          ← outermost (this crate)
  → ShadowProbeExecutor     ← wired when shadow_sentinel.enabled = true
      → PolicyGateExecutor
          → TrustGateExecutor
              → CompositeExecutor
                  → ToolFilter, AuditedExecutor, ...

confirmed in src/runner.rs:2680-2797, where tool_executor is progressively rewrapped: build_scoped_executor(tool_executor, ...) (line 2705) produces the ScopedToolExecutor, then (if shadow_sentinel.enabled) ShadowProbeExecutor::new(tool_executor, ...) (line 2783) wraps that again. The resulting tool_executor is the top-level Arc<dyn ErasedToolExecutor> that /undo, /redo, /undo list call via checkpoint_undo_erased/checkpoint_redo_erased/checkpoint_list_erased (crates/zeph-core/src/agent/agent_access_impl.rs:1692,1727,1746).

Because neither wrapper overrides the three checkpoint methods, any checkpoint call on the fully composed executor falls through to ToolExecutor's default (CheckpointActionResult::unsupported() / CheckpointListResult { supported: false, .. }) as soon as capability_scopes (any scopes configured) or shadow_sentinel.enabled = true is set — even after #5899's fix lands for TrustGateExecutor/PolicyGateExecutor/AdversarialPolicyGateExecutor/CompositeExecutor, because those inner gates are never reached; the call is swallowed one or two layers before it gets there.

Both config knobs are opt-in (capability_scopes.scopes empty and shadow_sentinel.enabled = false by default), so this is dormant in the default config, but both are documented, spec-driven security features (spec 050) intended for production use, not edge cases.

Reproduction Steps

  1. Configure [tools.shell] checkpoints_enabled = true.
  2. Configure at least one [security.capability_scopes.<name>] scope (or set [security.shadow_sentinel] enabled = true).
  3. Run a shell command that mutates a file (checkpoint capture occurs inside ShellExecutor).
  4. Run /undo list.
  5. Observe: "Checkpoints are not enabled..." is returned, even though checkpoints_enabled = true and a checkpoint was captured — because ScopedToolExecutor/ShadowProbeExecutor sit in front of the chain and never forward the call.

Expected Behavior

/undo, /redo, /undo list should operate on the checkpoint stack regardless of which optional security wrappers are active — these wrappers should be transparent to checkpoint operations, exactly like CompositeExecutor already is.

Actual Behavior

Checkpoint commands report "unsupported" whenever ScopedToolExecutor or ShadowProbeExecutor wraps the executor chain, independent of the inner gates' own forwarding correctness.

Environment

  • HEAD: 2616f7b (zeph-tools rotation sweep, CI cycle 1285)
  • Crate: zeph-toolsscope.rs, shadow_probe.rs

Suggested Fix

Add to impl<E: ToolExecutor> ToolExecutor for ScopedToolExecutor<E> (scope.rs) and impl<T: ToolExecutor> ToolExecutor for ShadowProbeExecutor<T> (shadow_probe.rs):

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()
}

Add a regression test in each file mirroring composite.rs's state_forwarding test module, asserting checkpoint_list().supported == true propagates through both wrappers when the inner executor supports checkpoints.

Related: #5899 (same class, inner gate trio), #5900 (requires_confirmation/is_tool_speculatable forwarding gaps), #3869 (original defect class — set_skill_env/set_effective_trust across CompositeExecutor).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1High ROI, low complexity — do next sprintbugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions