feat(extension): default tab selector to the window's active tab - #4756
Merged
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of the persist-effect rewrite in Files Reviewed (1 file)
Previous Review Summaries (2 snapshots, latest commit e3482d8)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e3482d8)Status: No Issues Found | Recommendation: Merge Executive SummaryFull review of the active-tab default logic and its test coverage found no new high-confidence CRITICAL/WARNING issues; the previously flagged re-entrancy no-op on Files Reviewed (8 files)
Previous review (commit c2b4808)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe re-entrancy guard on Overview
Issue Details (click to expand)WARNING
Files Reviewed (8 files)
Reviewed by claude-sonnet-5 · Input: 14 · Output: 3.7K · Cached: 246.8K Review guidance: REVIEW.md from base branch |
Sample the side panel window's active tab in the shared tab-list query and resolve selection as stored > active > first. Persist-effect gains empty-list and hydration guards so a valid stored selection is never wiped. New conversations no longer inherit the selection: they are created synchronously without selectedTabId, then patched with a fresh click-time active-tab sample when still unset.
Probe established that locator.click() on New conversation leaves the activated content tab active, so tests use the UI-click recipe. Cover the create-path active default with three-tab legs whose activation target always differs from both the frozen and first-listed labels, manual-pick persistence across poll cycles, single-tab fallback, the hydration-race and empty-list wipe guards, and rewrite the inheritance test for the R3 contract.
Rewrite the Firefox mirror of the inverted inheritance scenario under R3 and rename it for Chrome parity. The A9 probe confirmed that Selenium's switchTo() re-activates the panel tab, so the create-time active-tab sample sees the non-inspectable panel: the scenario asserts the deterministic first-listed default instead of the active-tab default (harness-limited), with first-listed read dynamically.
iscekic
force-pushed
the
feat/ext-default-active-tab
branch
from
July 24, 2026 16:59
c2b4808 to
e3482d8
Compare
Prevent a stale fallback write from clobbering a manual target-tab pick when the create path leaves selectedTabId unset (R3) and React batches the effect update after the user pick. Found by CI e2e-chrome on the merged head (conversation controls stay tied to the selected conversation).
iscekic
enabled auto-merge (squash)
July 24, 2026 18:15
jeanduplessis
approved these changes
Jul 25, 2026
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
2 participants
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.
What
The side panel's target-tab selector now defaults to the browser window's active tab instead of the first-listed tab, whenever the active conversation has no valid stored selection (unset, or pointing at a tab that is no longer inspectable). Resolution priority: valid stored selection > active tab (when inspectable) > first inspectable tab.
New conversations no longer inherit the current conversation's selection. They are created synchronously with no
selectedTabId(preserving the create-then-type-immediately contract), then patched with a fresh click-time sample of the active tab — but only if the selection is still unset at patch time, so a manual pick always wins.Why
Previously the selector defaulted to the first tab returned by
chrome.debugger.getTargets()(arbitrary order, never the tab the user is looking at), and new conversations copied the previous selection even when the user had switched context.How
use-tab-debugger.ts: exportedgetActiveTabId()(never throws; degrades toundefined) and the existing tab-list query now atomically returns{ tabs, activeTabId }so both inputs always settle together — the persist effect can never freeze a first-tab default before the active tab arrives.agent-chat-panel.tsx:getSelectedInspectableTabIdgains the active-tab priority and is passed at all four call sites. The persist effect gains empty-list and hydration guards (a momentarily empty list or a pre-hydration fallback can no longer wipe a valid stored selection) and is suppressed while a create-default patch is in flight.createConversation(): synchronous store switch withoutselectedTabId; re-entrancy-guarded fire-and-forget patch that writes only when the selection is still unset.No manifest or permission changes. Chrome and Firefox share the same code path.
Testing
getActiveTabIdedge cases (no active tab, non-numeric id, query rejection).locator.click()leaves the activated content tab active; create-path defaults to the activated tab across three-tab legs (activation target always differs from both frozen and first-listed labels); manual pick survives poll cycles; single-tab fallback; hydration-race + freeze reload; empty-list wipe guard; the inverted inheritance test rewritten for the new contract (all distinguishing labels read dynamically).switchTo()re-activates the panel tab, so the active-default leg is harness-limited there; the scenario asserts the deterministic first-listed default instead (shared create/resolver code is E2E-covered on Chrome).verify,build,build:firefox,e2e:chrome(57 passed / 10 env-gated skips),e2e:firefox(25/25).Known harness blind spot (recorded in the approved plan): the production first-conversation active-default needs real side-panel geometry (the panel is not a tab); it is covered by the unit priority tests plus the create-path E2E, not reproducible in the panel-as-tab harness.