Skip to content

fix(engine): bind shared cancel slot to the owning turn - #38

Open
asto18089 wants to merge 1 commit into
Pinvou:pinvou3-cleanfrom
asto18089:fix/254-turn-bound-cancel
Open

fix(engine): bind shared cancel slot to the owning turn#38
asto18089 wants to merge 1 commit into
Pinvou:pinvou3-cleanfrom
asto18089:fix/254-turn-bound-cancel

Conversation

@asto18089

Copy link
Copy Markdown

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_mode fired 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_message inside 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

  • The shared slot becomes 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 the token under that identity.
  • New host entry point 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_mode keeps 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_cancelled false, provider future not dropped), the id observed from TurnStarted interrupts exactly that turn with the usual TurnComplete(Interrupted).

cargo test -p codewhale-tui --lib core::engine:: (504) and forkguard (58) slices pass; full suite runs in the paired parent-repo PR.

The paired parent-repo PR wires EnginePool::cancel and the forwarder's pending-cancel replay to this API.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

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 CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant