fix(engine): bind shared cancel slot to the owning turn - #38
Open
asto18089 wants to merge 1 commit into
Open
Conversation
|
Thanks @asto18089 for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
Hosts cancel through a shared token slot that used to carry no turn
identity: `cancel_with_mode` fired whatever token occupied the slot at
call time. For host-driven turns the app-side generation gate made that
safe, because the host reserves the next turn (advancing its epoch under
the same lock the cancel checks) before the engine can start one. The
three runtime self-start paths — idle sub-agent completion, background
shell completion wake, and goal continuation — call handle_send_message
inside the engine and swap the shared token before any host reserve, so
a host cancel whose generation view was still on the finished turn
passed the epoch check and fired the follow-up turn's token (Pinvou
pinvou-agent#254).
The slot is now a TurnCancelSlot { turn_id, token } swapped atomically at
every turn start (handle_send_message and the user shell-command turn
mint the turn id first, then install). Hosts get
EngineHandle::cancel_turn(turn_id, reason, mode): the identity check and
token clone happen under the same slot lock the install uses, so the
decision is atomic against a concurrent turn start and the cloned token
only ever fires the named turn. On identity mismatch nothing is
cancelled and no steer disposition or cancel reason is published — the
target turn is already gone. cancel_with_mode keeps its exact
fire-current-token semantics for single-user frontends.
Tests: a slot-contract unit test (unnamed slot skips, stale id skips
without firing the follow-up, matching id fires) and a forkguard
regression driving a real turn through an injected blocking client,
asserting a foreign turn id leaves the in-flight request untouched while
the observed TurnStarted id interrupts it.
asto18089
force-pushed
the
fix/254-turn-bound-cancel
branch
from
September 2, 2026 11:00
83edde1 to
2239d54
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
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.
Fixes the foundation half of Pinvou/pinvou-agent#254.
Problem
Hosts cancel through a shared token slot that carried no turn identity:
EngineHandle::cancel_with_modefired whatever token occupied the slot at call time. For host-driven turns the host-side generation gate made that safe — the host reserves the next turn (advancing its epoch under the same lock the cancel checks) before the engine can start one.The three runtime self-start paths — idle sub-agent completion, background shell completion wake, and goal continuation — call
handle_send_messageinside the engine and swap the shared token before any host reserve. A host cancel whose generation view was still on the finished turn passed the epoch check and fired the follow-up turn's token: the C1 issued to cancel turn N killed the engine's self-started turn N+1.Fix
TurnCancelSlot { turn_id, token }, swapped atomically at every turn start.handle_send_messageand the user!shell-command turn mint the turn id first, then install the token under that identity.EngineHandle::cancel_turn(turn_id, reason, mode) -> bool. The identity check and the token clone happen under the same slot lock the install uses, so the decision is atomic against a concurrent turn start, and the cloned token can only ever fire the named turn. On identity mismatch nothing is cancelled and no steer disposition or cancel reason is published — the target turn is already gone.cancel_with_modekeeps its exact fire-current-token semantics for single-user frontends (TUI stop button,Op::CancelRequest).Tests
engine_handle_cancel_turn_only_fires_the_named_turns_token— slot contract: unnamed slot skips, stale id skips without firing the follow-up turn's token, matching id fires.forkguard_cancel_turn_binding_spares_unnamed_turns_and_hits_the_observed_turn— regression through a real engine with an injected blocking model client: a foreign turn id leaves the in-flight request untouched (is_cancelledfalse, provider future not dropped), the id observed fromTurnStartedinterrupts exactly that turn with the usualTurnComplete(Interrupted).cargo test -p codewhale-tui --lib core::engine::(504) andforkguard(58) slices pass; full suite runs in the paired parent-repo PR.The paired parent-repo PR wires
EnginePool::canceland the forwarder's pending-cancel replay to this API.