Skip to content

fix(extension): reconcile-based tab group management#40324

Merged
yury-s merged 10 commits into
microsoft:mainfrom
yury-s:fix-extension-groupid-reset
Apr 20, 2026
Merged

fix(extension): reconcile-based tab group management#40324
yury-s merged 10 commits into
microsoft:mainfrom
yury-s:fix-extension-groupid-reset

Conversation

@yury-s
Copy link
Copy Markdown
Member

@yury-s yury-s commented Apr 20, 2026

Summary

  • Replace scattered _addTabToGroup / _ungroupTab / _selectorTabId tracking with a single _reconcile() that drives Chrome's Playwright group from _connectedTabIds. Serialized via a promise queue, retries with 0/100/200ms ramp then 400ms steady on drag errors, and detects dissolved groups via chrome.tabGroups.get.
  • Reset _groupId on connection close so reconnecting starts fresh instead of targeting a dissolved group.
  • Ungroup non-debuggable tabs (chrome:, edge:, devtools:) dragged into the group without attempting a round-trip attach that would fail.
  • Clean up stale Playwright groups on service worker startup (via the same reconcile queue to avoid racing with new connections).
  • Connect page is no longer part of the Playwright group; updated the corresponding test.

Tests: new tab is re-added to Playwright group after reconnecting and chrome:// tab dragged into group is automatically ungrouped; updated connected tab is in green Playwright group, connect page is not.

yury-s added 9 commits April 20, 2026 11:19
When the MCP client disconnects, Chrome eventually dissolves the
Playwright group because all tabs are removed from it. The extension
was not clearing _groupId on close, so on reconnect it tried to add
the newly-attached tab to the stale (now dissolved) group. The
fallback creates a new group, but during the async window the
_onTabUpdated handler observes the new groupId != the stale _groupId
and detaches the tab.

Reset _groupId in the onclose callback so the next connection starts
from a clean slate.
When a user drags a tab with a chrome://, edge:// or devtools:// URL
into the Playwright group, chrome.debugger.attach cannot be attached,
so the tab would remain in the group without being controlled. Detect
these schemes in _onTabUpdated and ungroup the tab immediately.

chrome.tabs.ungroup also rejects with "user may be dragging a tab"
during an active drag, so _ungroupTab retries with a short delay,
mirroring _addTabToGroup.
Service worker restarts lose all connection state, so any Playwright
tab groups from a previous session are stale. Query for groups with
that title on startup and ungroup their tabs so the user does not
end up with orphan groups after a reload.
Run _cleanupStaleGroups through _groupQueue so a new connection's
_addTabToGroup call waits for cleanup to finish, preventing cleanup
from ungrouping freshly-added tabs.
…of truth

Replace the scattered _addTabToGroup/_ungroupTab calls with a single
_reconcile() method that brings Chrome's Playwright group in line with
the desired members (_connectedTabIds + _selectorTabId).

- Serialized via _reconcileQueue so concurrent calls can't race.
- Retries on drag errors with 0/100/200ms ramp, then 400ms steady.
- Suppresses _onTabUpdated handling while reconcile mutates the group,
  so Chrome's synthetic onUpdated events don't fight the reconciler.
- Detects dissolved stale groups via chrome.tabGroups.get and resets
  _groupId so the next reconcile creates a fresh group.
…roup

The selector tab was previously tracked separately and added to the
Playwright group via _selectorTabId. Remove that field — the connect
page is no longer part of the group.

Update the corresponding test to assert the connected tab is grouped
and the connect page is not.

Also make the reconnect-group test's evaluate resilient to a
momentarily-unavailable chrome.tabs binding during page transitions.
…rhead

- Reuse _disconnect in _connectTab instead of duplicating close/cleanup.
  The onclose callback handles state clearing, so _disconnect is now a
  thin wrapper taking an optional reason.
- Extract PLAYWRIGHT_GROUP_TITLE/_COLOR constants and an
  isNonDebuggableUrl helper; de-duplicate scheme list and title string.
- Skip _reconcile in _onTabUpdated when Chrome's view already matches
  our desired state (inOurGroup === connected).
- Parallelize _updateBadge's three chrome.action calls.
- Parallelize per-group tab queries in _cleanupStaleGroups and the
  group existence + membership checks in _reconcileOnce.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Chrome resets per-tab action state on navigation, so connected tabs
lose the ✓ badge when they navigate (e.g., after browser_tabs new).
Re-apply the badge in _onTabUpdated for tabs that are in
_connectedTabIds.

Extracted the badge payload to a CONNECTED_BADGE constant to keep the
badge in sync between ontabattached and _onTabUpdated.
@yury-s yury-s merged commit 38a46bf into microsoft:main Apr 20, 2026
36 of 37 checks passed
@yury-s yury-s deleted the fix-extension-groupid-reset branch April 20, 2026 23:50
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

6434 passed, 976 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

Test results for "tests 1"

3 flaky ⚠️ [chromium-library] › library/beforeunload.spec.ts:130 › should support dismissing the dialog multiple times `@chromium-ubuntu-22.04-arm-node20`
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1080 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:433 › should work behind reverse proxy `@windows-latest-node20`

39248 passed, 847 skipped


Merge workflow run.

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