Skip to content

fix(extension): make tool cancellation cooperative#18

Open
NianJiuZst wants to merge 5 commits into
Tencent:mainfrom
NianJiuZst:codex/cooperative-tool-cancellation
Open

fix(extension): make tool cancellation cooperative#18
NianJiuZst wants to merge 5 commits into
Tencent:mainfrom
NianJiuZst:codex/cooperative-tool-cancellation

Conversation

@NianJiuZst

@NianJiuZst NianJiuZst commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What problem this solves

The extension previously implemented cancellation by racing every tool handler against a promise that rejected as soon as an AbortSignal fired. That made the RPC appear cancelled immediately, but it did not stop the handler that lost the race.

For read-only work this wasted CDP work and could corrupt later state: a cancelled snapshot could finish after a newer snapshot and replace the session RefStore with stale element ids. For mutating work the impact was more visible. Cancelling tool.session_start while chrome.windows.create was pending caused the daemon to discard its reservation, but the unresolved handler later created and registered an Agent Window that the daemon did not know about.

Several tab handlers already contained abort checks and compensation logic, but the dispatcher did not pass their AbortSignal. The result was a cancellation protocol that acknowledged cancellation without establishing that the underlying browser operation had stopped or rolled back.

How this fixes it

The dispatcher no longer uses Promise.race to manufacture an early cancelled response. It still acknowledges the separate cancel request immediately, but the original tool RPC now waits for its handler to observe the signal and finish any necessary compensation before sending its final response.

Cancellation is wired through the previously disconnected paths:

  • session_start receives the AbortSignal;
  • tab create, close, select, borrow, and return receive the AbortSignal;
  • screenshot, console, get-html, and snapshot receive cancellation checks around their awaited browser/CDP work;
  • navigation, interaction, evaluate, wait, and human-loop handlers retain their existing signal plumbing.

SessionManager.start now treats Agent Window creation as a transactional operation. If cancellation arrives after windows.create, or if active-tab initialization fails, it removes the incomplete window and does not register the session. The session handler maps this AbortError to the structured cancelled result.

Snapshot checks the signal before replacing RefStore contents. A cancelled older snapshot can therefore finish its underlying CDP request without overwriting refs produced by later work.

Cancellation boundary

Some Chrome APIs are irreversible after dispatch, such as removing a tab that Chrome has already accepted. The fix does not pretend those operations can be rolled back. Instead, it guarantees that BrowserSkill does not send the original RPC response while a handler is still running invisibly. Handlers with a reversible side effect, such as creating a tab or Agent Window, perform compensation before returning cancelled.

User impact

Cancelling a slow command no longer leaves hidden Agent Windows or silently mutates the element-ref state after the CLI has moved on. The cancel control remains responsive because its own acknowledgement is immediate, while the cancelled tool's final lifecycle is now truthful and deterministic.

The change also fixes a non-cancellation leak: if Agent Window active-tab setup fails after window creation, the partial window is removed.

Validation

  • corepack pnpm --filter @browser-skill/extension test
    • 35 test files passed
    • 376 tests passed
  • corepack pnpm --filter @browser-skill/extension compile
  • corepack pnpm ext:build
  • node --test scripts/*.test.mjs
  • corepack pnpm exec stylelint "**/*.css"
  • git diff --check

Regression tests verify that:

  • cancel acknowledgement is immediate but the original session-start RPC remains pending until cleanup finishes;
  • the created Agent Window is removed and no SessionContext is registered;
  • startup setup failures also remove incomplete windows;
  • a snapshot cancelled during CDP capture leaves the previous RefStore untouched.

CI Baseline Compatibility

This branch also carries the one-line Biome 2.4 formatter output for apps/extension/vitest.config.ts. The same formatting mismatch currently fails the Frontend job on upstream/main; this minimal compatibility commit is intentionally separate from the functional fix and lets this PR run the repository CI suite to completion.

CI Reliability Follow-up

GitHub Actions exposed a pre-existing test-only port allocation race: integration helpers probed an ephemeral port, released it, and then asked the daemon to bind the same number, allowing another process to claim it in between. The resulting Address already in use failure was unrelated to the functional changes. The PR now lets each daemon bind port 0 directly and reads the assigned address from its handle, removing the TOCTOU window across all affected integration helpers.

@NianJiuZst NianJiuZst marked this pull request as ready for review July 10, 2026 14:58
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