fix(extension): roll back CDP attach when a domain enable fails#33
Open
hobostay wants to merge 1 commit into
Open
fix(extension): roll back CDP attach when a domain enable fails#33hobostay wants to merge 1 commit into
hobostay wants to merge 1 commit into
Conversation
If `chrome.debugger.attach` succeeds but a subsequent CDP domain enable rejects — most notably `Page.enable`, when the tab navigates to a chrome:// URL or the Web Store in the gap between the two calls — the driver left the tab in an unrecoverable state. `attachedTabs` never recorded the tabId, so the next `ensureAttached` re-attached and failed permanently with "Another debugger is already attached" until the extension was reloaded. Detach the debuggee directly in the failure path (the public `detach()` is a no-op here because the id is not cached yet) so the next attempt starts from a clean slate. Adds a regression test that fails on the previous code, where the rollback never ran and `detach` was never called. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Problem
ChromiumCdp.ensureAttachedcan leave a tab permanently stuck when a CDP domain enable fails after the raw attach succeeds.The attach sequence was:
enablePageDomainis the only domain enable that is neither best-effort nor guarded. If it rejects — e.g. the tab navigates to achrome://URL or the Web Store in the gap betweenattachandPage.enable, or a transient CDP error during attach — the.catchre-throws but performs no cleanup, andattachedTabsnever records the tabId.On the next tool call,
ensureAttachedsees the tabId is not cached, has no in-flight attach, and callsapi.attachagain → Chrome rejects it with "Another debugger is already attached". Every subsequent operation on that tab fails until the extension is reloaded. Note the publicdetach()cannot self-heal this, because it no-ops whenattachedTabslacks the id.Fix
Detach the debuggee directly in the failure path (before re-throwing) so the next attempt starts from a clean slate:
api.attachfailures are untouched (no detach attempted, since nothing was attached) — only failures after a successful attach roll back.Test
Adds a regression test that mocks
Page.enableto throw afterattachsucceeds, then asserts:ensureAttachedrejects with the original error,api.detach({ tabId }),ensureAttachedsucceeds afterwards.This test fails on the previous code (0 detach calls) and passes with the fix.
Validation
Ran the frontend CI gates locally against this branch:
pnpm --filter @browser-skill/extension exec wxt prepare✅pnpm ext:test— 388 passed ✅pnpm --filter @browser-skill/extension compile(tsc --noEmit) ✅pnpm lint(biome + stylelint) ✅pnpm ext:build✅🤖 Generated with Claude Code