fix(extension): scope reusable-tab selection to owned group members#1816
Open
Benjamin-eecs wants to merge 1 commit into
Open
fix(extension): scope reusable-tab selection to owned group members#1816Benjamin-eecs wants to merge 1 commit into
Benjamin-eecs wants to merge 1 commit into
Conversation
Stops resolveTab from overwriting a user http(s) tab when the canonical group has converged into a user window. Fixes jackwener#1760
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR tightens the logic for reusing “owned container” tabs to avoid accidentally reusing user HTTP(S) tabs when an owned tab group converges into a user window.
Changes:
- Extend
findReusableOwnedContainerTabto accept an optional owned group id and filter out HTTP(S) tabs that are not in that group. - Update callers to pass the owned group id when looking for a reusable tab.
- Add a regression test to ensure a user HTTP tab outside the canonical group is not reused in a converged user window scenario.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| extension/src/background.ts | Adds owned-group-aware filtering to reusable tab selection and updates call sites. |
| extension/src/background.test.ts | Adds test coverage for not reusing user HTTP tabs outside the canonical group; adjusts an existing test setup. |
| extension/dist/background.js | Compiled output reflecting the source changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+859
to
872
| // When a canonical owned group lives in a user window (cross-window | ||
| // convergence can land it there), an http(s) tab outside the group is | ||
| // user content and must not be reused. Group members and non-http tabs | ||
| // (about:blank / data: / fresh container) stay eligible. | ||
| const reusable = tabs.find(tab => | ||
| tab.id !== undefined && | ||
| initialTabIsAvailable(tab.id) && | ||
| isDebuggableUrl(tab.url), | ||
| isDebuggableUrl(tab.url) && | ||
| ( | ||
| ownedGroupId === undefined || | ||
| tab.groupId === ownedGroupId || | ||
| !isSafeNavigationUrl(tab.url ?? '') | ||
| ), | ||
| ); |
Comment on lines
+866
to
+871
| isDebuggableUrl(tab.url) && | ||
| ( | ||
| ownedGroupId === undefined || | ||
| tab.groupId === ownedGroupId || | ||
| !isSafeNavigationUrl(tab.url ?? '') | ||
| ), |
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.
Description
Re-attempt of #1762 against the post-#1794 model, following the direction left in the close comment:
findReusableOwnedContainerTab(windowId)inextension/src/background.tsqueries every debuggable tab in the candidate window. Under the v1.0.17 convergence model the canonical OpenCLI Adapter / Browser group can land in the user's main Chrome window, where ungrouped http(s) tabs are user content.resolveTabthen happily reuses one of those, overwriting the user's open page (#1760).Add an
ownedGroupIdparameter tofindReusableOwnedContainerTaband three of its four callers that already have the group id. The filter accepts tabs that are in our group, plusabout:blank/data:/ empty-URL tabs (safe-to-overwrite container blanks), and rejects http(s) tabs that live outside the group. The fourth caller (just-created window, group not yet assigned) keeps the unscoped behavior because that path is reached only with a freshly minted window whose tabs are extension-created.Related issue: fixes #1760. The previous attempt #1762 was the wrong abstraction and is preserved as the educational close.
Type of Change
Checklist
Screenshots / Output
New unit test
does not reuse a user http tab outside the canonical group when the group has converged into a user windowreproduces the #1760 scenario: a window with one ungroupedhttps://example.com/user-pagetab and one in-groupabout:blanktab, an OpenCLI Adapter group registered against the in-group tab. The test asserts thatresolveTabIddoes not return the user tab id, does not touch the user tab's url or groupId, and that the chosen tab is in the canonical group.The pre-existing
discovers and reuses an existing OpenCLI Adapter group after service worker restarttest mocked the post-restart state with the adapter tab outside the canonical group (tabs[0].groupId = -1). Under the v1.0.17 invariant that Chrome group state is source of truth, the realistic state is the tab inside the group, so the test setup is updated totabs[0].groupId = 99and the now-redundantexpect(chrome.tabs.group).toHaveBeenCalledWith({ groupId: 99, tabIds: [1] })assertion is dropped. Every other test in the file continues to pass unchanged. Full suite: 80/80.Live verified on Chrome for Testing 149 with the patched extension loaded. A CDP probe set up the bug state in a single window (one ungrouped
https://example.com/users-real-tab, one in-groupabout:blanknamedOpenCLI Adapter) and compared the two filter implementations againstchrome.tabs.query:https://example.com/users-real-tab(groupId -1, the user tab)about:blank(groupId of the canonical group, the in-group tab)