Skip to content

fix(extension): make tool cancellation cooperative - #89

Merged
BB-fat merged 1 commit into
Tencent:mainfrom
NianJiuZst:codex/cooperative-tool-cancellation
Aug 13, 2026
Merged

fix(extension): make tool cancellation cooperative#89
BB-fat merged 1 commit into
Tencent:mainfrom
NianJiuZst:codex/cooperative-tool-cancellation

Conversation

@NianJiuZst

Copy link
Copy Markdown
Contributor

Summary

This is a current-main rewrite of #18, based on 37fbaacc94a4e7c1b4198fc0a72756fe0b1e663a.

  • remove the dispatcher-level Promise.race that reported cancellation while the original handler was still running;
  • acknowledge the separate cancel RPC immediately, while keeping the original RPC pending until handler work and compensation settle;
  • thread AbortSignal through session startup, tab/window operations, observations, console/network reads, and emulation;
  • make session_start, tab creation/borrowing, and fallback-window handling transactional where compensation is possible;
  • keep the daemon session queue busy until the extension returns its final result or a bounded two-second cleanup timeout expires;
  • surface compensation failures explicitly as cleanup_failed, including the orphan resource id.

The obsolete port-TOCTOU and CI-only commits from the original branch are intentionally not included because those changes already landed on main.

Review feedback addressed

  • Queue ownership: a forwarded cancellation no longer releases the per-session queue immediately. A second same-session RPC remains session_busy until the extension finishes cleanup; a wedged handler is bounded by cancel_cleanup_timeout.
  • Cooperative checks: tab_return and multi-stage observation paths check cancellation after awaited steps and before later side effects or state publication.
  • Fallback compensation: a fallback window created by tab_return is removed when cancellation wins before the tab move, and is also removed when an attempted move fails.
  • Visible cleanup failures: failed cleanup is not swallowed. The final RPC returns cleanup_failed with the affected tab/window id, while retryable borrowed-tab state is retained.
  • Delayed-compensation E2E: the Rust integration test gates the extension's final cancellation response, proves the session stays busy during that delay, then proves the queue reopens after cleanup.

Cancellation boundary

Chrome operations that have already completed are not presented as rolled back. The guarantee is instead lifecycle honesty: the extension does not send the original response while its handler is still running invisibly, and the daemon does not release queue ownership before that response or the cleanup deadline.

For reversible allocations, cancellation performs compensation before the final response. In particular, cancelled or partially failed session startup removes its newly created Agent Window, and a cancelled observation cannot replace the session RefStore after a newer operation has moved on.

Validation

  • corepack pnpm --filter @browser-skill/extension test — 52 files, 647 tests passed
  • corepack pnpm --filter @browser-skill/extension compile
  • corepack pnpm lint
  • corepack pnpm ext:build
  • node --test scripts/*.test.mjs
  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --locked -- -D warnings
  • cargo test -p bsk --test cancel_forwarding --locked -- --nocapture — 5 tests passed
  • cargo test --workspace --locked
  • git diff --check

Replaces #18.

@BB-fat

BB-fat commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Thank you @NianJiuZst for the quick rewrite on top of current main — this version is in great shape.

All four review points from 7/18 landed cleanly and with high quality: the queue now stays busy until compensation fully completes, cancellation checks are cooperative throughout, the fallback window is compensated as well, and cleanup_failed is now explicit. The gating in the delayed-compensation E2E test is especially solid.

We have two tiny follow-ups of our own (wrapping stopAll's remove in try/catch, adding CLI error copy for cleanup_failed / cancel_cleanup_timeout, plus a one-line cancellation check in the hover latch) — no action needed from you; we'll pick those up separately.

Merging now. Contributions like this are very welcome — looking forward to the next one!

@BB-fat
BB-fat merged commit 4549ed0 into Tencent:main Aug 13, 2026
4 checks passed
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.

2 participants