Conversation
DynExecutor was missing the requires_confirmation method, causing it to fall through to the ToolExecutor trait default (false) and silently ignore the inner executor's confirmation policy. Adds the delegation method and two unit tests covering both the true and false paths, consistent with the existing is_tool_retryable and is_tool_speculatable delegation pattern. Closes #3650
When SpeculationEngine::try_commit returns a cached result for a tool
call, emit ToolStartEvent{speculative:true} and inject the result as a
ready future, bypassing normal dispatch for that index.
Changes:
- Clone speculation_engine Arc before the tier loop to avoid borrow
conflicts across await points
- Add commit_speculative_tier: pre-scans tier indices via try_commit,
stamps tool_started_ats for committed calls, warns on Err results,
emits ToolStartEvent{speculative:true} for each committed call
- Filter committed indices out of stamp_and_send_tier_start and
build_tier_call_futures; inject committed results as ready futures
Until #3641 (try_dispatch wiring) lands, try_commit always returns None
and this path is a no-op. The wiring is in place to activate
automatically when #3641 is implemented.
Closes #3643
4164890 to
a20753c
Compare
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
test(tools): add direct delegation test for DynExecutor::requires_confirmation #3650
DynExecutor::requires_confirmationwas missing the delegation method, silently returningfalsevia the trait default and ignoring the inner executor's confirmation policy. Adds the one-line delegation and two unit tests covering both thetrueandfalsepaths.feat(core): set ToolStartEvent.speculative = true when committing a speculative result #3643 Wires
ToolStartEvent.speculative = truewhenSpeculationEngine::try_commitreturns a cached result. Addscommit_speculative_tierhelper that pre-scans tier indices, stampstool_started_ats, warns onErrcommitted results, and emits the correct speculative event before filtering committed indices out of normal stamp/dispatch. Until feat(core): wire try_dispatch into decoding-level SSE ToolStream streaming path #3641 (try_dispatch wiring) lands,try_commitalways returnsNone— the path is a no-op today and activates automatically when feat(core): wire try_dispatch into decoding-level SSE ToolStream streaming path #3641 is done.Changes
crates/zeph-tools/src/executor.rs: addDynExecutor::requires_confirmationdelegation + 2 testscrates/zeph-core/src/agent/tool_execution/native.rs: addcommit_speculative_tier, update tier loopTest plan
cargo nextest run -p zeph-tools --lib— 1132 tests passcargo nextest run -p zeph-core --lib— 1353 tests passcargo +nightly fmt --check— cleancargo clippy --workspace -- -D warnings— cleandyn_executor_requires_confirmation_delegates,dyn_executor_requires_confirmation_default_falseFollow-up
commit_speculative_tier(4 cases: engine=None, no commits, Ok commit, Err commit)debug_assert!incommit_speculative_tierguarding againstConfirmationRequiredin committed Err resultsCloses #3650
Closes #3643