fix(tools): forward checkpoint and confirmation methods through executor wrappers - #5930
Merged
Merged
Conversation
…tor wrappers TrustGateExecutor, PolicyGateExecutor, and AdversarialPolicyGateExecutor never overrode checkpoint_undo/checkpoint_redo/checkpoint_list, so calls fell through to the ToolExecutor trait's no-op default instead of reaching ShellExecutor. ScopedToolExecutor and ShadowProbeExecutor had the same gap on the outer layers of the production wrapper chain. As a result, /undo, /redo, and /undo list always reported checkpoints as unsupported whenever trust/policy/adversarial gating, capability_scopes, or shadow_sentinel wrapped the executor chain, even with checkpoints_enabled = true set. ScopedToolExecutor also never overrode requires_confirmation, defaulting to false regardless of the real policy underneath, which would silently disable the speculative-dispatch engine's confirmation gate once any tool opts into speculative execution. Add one-line forwards to self.inner in all five wrapper files, mirroring the existing pass-through style used for is_tool_retryable/set_skill_env. Add regression tests per file proving delegation with distinguishable non-default return values, including an argument round-trip check on checkpoint_undo. Closes #5899, #5905, #5906
bug-ops
force-pushed
the
fix/5899-checkpoint-confirm-forwarding
branch
from
July 10, 2026 21:31
d9cdec9 to
72ca18f
Compare
bug-ops
enabled auto-merge (squash)
July 10, 2026 21:32
This was referenced Jul 10, 2026
bug-ops
added a commit
that referenced
this pull request
Jul 11, 2026
…e_tool_call_confirmed through remaining ToolExecutor wrappers (#6011) CompositeExecutor did not forward requires_confirmation or execute_tool_call_confirmed, and AdversarialPolicyGateExecutor did not forward is_tool_speculatable, so all three fell through to the trait's default and silently dropped whatever the wrapped chain actually reported. PolicyGateExecutor and AdversarialPolicyGateExecutor also did not forward requires_confirmation, masking TrustGateExecutor's real policy whenever tools.policy or tools.adversarial_policy is enabled. Add the missing forwards: OR-forwarding for requires_confirmation and first-match-wins for execute_tool_call_confirmed in CompositeExecutor (mirroring the existing is_tool_retryable/is_tool_speculatable and execute_confirmed overrides), plain delegation to self.inner in the two gate executors. Same defect class as #5899/#5905/#5906 (fixed by #5930), scoped out of that PR to keep it minimal. Closes #5900, #5938, #5931
bug-ops
added a commit
that referenced
this pull request
Jul 11, 2026
…ckpoint drop, add checkpoint-stack tests CompressedExecutor, ToolFilter, and Arc<ShellExecutor> now forward every cross-cutting ToolExecutor method to their inner/wrapped executor instead of silently falling through to trait defaults, closing the same shadow-impl forwarding gap fixed for other wrappers in #5930 and #6011. capture_snapshot_for no longer drops a checkpoint for a newly-created file under a symlinked allowed_paths prefix on macOS: a new canonicalize_or_nearest_ancestor helper resolves symlinks via the nearest existing ancestor when the target path itself does not exist yet. Adds regression coverage for the ShellExecutor checkpoint stack: redo without a prior undo, checkpoint_list ordering, and multi-step undo/redo depth bookkeeping.
bug-ops
added a commit
that referenced
this pull request
Jul 11, 2026
…ckpoint drop, add checkpoint-stack tests CompressedExecutor, ToolFilter, and Arc<ShellExecutor> now forward every cross-cutting ToolExecutor method to their inner/wrapped executor instead of silently falling through to trait defaults, closing the same shadow-impl forwarding gap fixed for other wrappers in #5930 and #6011. capture_snapshot_for no longer drops a checkpoint for a newly-created file under a symlinked allowed_paths prefix on macOS: a new canonicalize_or_nearest_ancestor helper resolves symlinks via the nearest existing ancestor when the target path itself does not exist yet. Adds regression coverage for the ShellExecutor checkpoint stack: redo without a prior undo, checkpoint_list ordering, and multi-step undo/redo depth bookkeeping.
bug-ops
added a commit
that referenced
this pull request
Jul 11, 2026
…ckpoint drop, add checkpoint-stack tests (#6036) CompressedExecutor, ToolFilter, and Arc<ShellExecutor> now forward every cross-cutting ToolExecutor method to their inner/wrapped executor instead of silently falling through to trait defaults, closing the same shadow-impl forwarding gap fixed for other wrappers in #5930 and #6011. capture_snapshot_for no longer drops a checkpoint for a newly-created file under a symlinked allowed_paths prefix on macOS: a new canonicalize_or_nearest_ancestor helper resolves symlinks via the nearest existing ancestor when the target path itself does not exist yet. Adds regression coverage for the ShellExecutor checkpoint stack: redo without a prior undo, checkpoint_list ordering, and multi-step undo/redo depth bookkeeping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TrustGateExecutor,PolicyGateExecutor, andAdversarialPolicyGateExecutornever overrodecheckpoint_undo/checkpoint_redo/checkpoint_list, so calls fell through to theToolExecutortrait's no-op default instead of reachingShellExecutor.ScopedToolExecutorandShadowProbeExecutorhad the same gap on the outer wrapper layers. Net effect:/undo,/redo, and/undo listalways reported checkpoints as unsupported whenever trust/policy/adversarial gating,capability_scopes, orshadow_sentinelwrapped the executor chain — the standard, default-recommended production configuration — even withcheckpoints_enabled = trueset.ScopedToolExecutoralso never overroderequires_confirmation, defaulting tofalseregardless of the real policy underneath, which would silently disable the speculative-dispatch engine's confirmation gate once any tool opts into speculative execution.self.inner.<method>(...)forwards to all five wrapper files, mirroring each file's existing pass-through style foris_tool_retryable/set_skill_env. Adds 6 regression tests (one per file,scope.rshas 2) using local stub inner executors that return distinguishable non-default values, including an argument round-trip check oncheckpoint_undo'snparameter.Closes #5899, #5905, #5906
Test plan
cargo +nightly fmt --check— cleancargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings— cleancargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins— 12579 passed, 0 failed, 35 skippedRUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --features "desktop,ide,server,chat,pdf,scheduler"— cleancheckpoint_undoto hardcode the forwardedn, confirmed the new test fails, then confirmed the file was restored byte-identical.local/testing/coverage-status.mdand.local/testing/playbooks/undo-redo.mdupdated with a new regression scenario for gate/scope wrapper transparencyNote: local
cargo build/rustdoc runs on this machine requireCARGO_BUILD_WARNINGS=allowdue to pre-existing, unrelated issues (#5894, #5895) from thebuild.warningsmigration in #5891 — not touched by this PR.