Feat/28 direct mode#33
Merged
Merged
Conversation
added 7 commits
April 20, 2026 15:22
- New SET_DIRECT_MODE message routes to chrome.proxy.settings.set({mode:'direct'}),
giving users a true "bypass everything" option that overrides the OS/IE proxy
that System mode otherwise honors.
- Storage schema v2: introduce top-level mode field
('direct' | 'system' | 'profile'). One-way migrateData() infers mode from the
existing activeProfileId for v1 payloads; stale ids are dropped safely.
- Popup gets a dedicated Direct button next to System, with a matching status
indicator state (green dot, no pulse).
- Tests: 10 new Vitest cases for migrateData edge cases and 4 new Playwright
specs exercising the Direct button end-to-end against real Chromium.
- Options dark-mode polish (bundled): restore the missing --border-radius /
--transition tokens that were silently falling back to 0, add a
prefers-color-scheme: dark block, align focus-ring and danger-hover colors
with the iOS palette.
- Version: 1.5.2 -> 1.6.0 in manifest, package*, options About, docs JSON-LD,
STORE_LISTING, README, CHANGELOG.
Closes #28.
… popup
Activating a profile in the popup did not turn the toolbar icon to the
profile's color until the user interacted with the address bar. The popup
call path is: popup click -> ACTIVATE_PROFILE -> activateProxy() ->
updateIconForActiveTab() -> chrome.tabs.query({currentWindow:true}).
At that moment the popup still has focus and the extension popup is its
own windowType:'popup' with no browsing tabs, so the query returned
empty, tab.url fell through to '', and the icon was set to the inactive
gray. Only after the popup closed (typically when the user clicked into
the address bar) did onUpdated refire and pick up the real active tab.
Fix: resolve the active tab via chrome.windows.getLastFocused with
windowTypes:['normal'], which excludes our popup window, then query
active tab by that window id. Applied in both updateIconForActiveTab()
and the onUpdated listener's sibling lookup.
The earlier "icon stays gray until address-bar interaction" bug wasn't
caught by any automated test. Two gaps:
1. Our e2e fixture loads popup.html into a *regular* browser tab, so
chrome.tabs.query({currentWindow:true}) always found that tab — the
broken codepath (currentWindow = windowType:'popup') was never
exercised. Manual testing with the real extension popup was the only
way to trigger it.
2. Tests only inspected DOM (status text, class) and storage, never the
toolbar icon — the most user-visible signal.
Fix both:
- Expose last-applied icon color through GET_STATE. background.js now
tracks `lastIconColor` in module state and returns it in the GET_STATE
response, giving tests an observable surface for icon repaint without
needing a chrome.action.getIcon API (which doesn't exist).
- New e2e spec popup-window-icon.spec.ts creates a real popup-type window
via chrome.windows.create({type:'popup'}), activates a red-colored
profile from inside it, and asserts lastIconColor matches the profile
color. Verified the spec does detect the bug: reverting the
getActiveBrowserTab() fix makes it fail with Received: null,
Expected: "#F44336" — the exact symptom the user reported.
Total: 81 unit + 55 e2e, all green.
Real root cause of the "icon stays gray until address-bar interaction" bug, which survived the earlier windowType:'popup' fix. updateIconForActiveTab was designed to be per-tab: it required the active tab's URL to start with http(s):// before painting the profile color. On chrome://newtab, about:blank, or any extension page, the check failed and the icon stayed gray even with a profile active. Users open the extension popup from the new-tab page constantly, so they saw the gray icon until navigating. The per-tab indicator is useful when routing rules are enabled — it tells you whether the current site is actually going through the proxy or bypassing it. With no routing rules, every http request goes through the proxy unconditionally, so the nuance only confuses. Fix: split the branches. Profile without routing rules → paint profile color unconditionally. Profile with routing rules → keep existing per-tab check. Extended e2e/popup-window-icon.spec.ts from 1 to 3 tests, including the critical non-http-tab case (about:blank browsing tab) that reproduces the user's scenario. Verified the new test fails without this fix (Expected: "#F44336", Received: null) and passes with it. Also added a test that locks in the per-tab behavior for the routing-rules case so the fix doesn't regress that feature. 81 unit + 57 e2e all green.
…Popup + raw CDP
The existing popup-window-icon.spec.ts simulates the popup via
chrome.windows.create({type:'popup'}) — semantically close but not
identical to the toolbar popup a user actually clicks. The real popup
uses chrome.action.openPopup() and renders as an internal Chromium
widget that Playwright's Page API does not expose (verified: target
exists in CDP but never surfaces as a context.pages() entry).
Work around Playwright's limitation by:
1. Opening a browser-level CDPSession (browser.newBrowserCDPSession()).
2. Triggering chrome.action.openPopup() from the service worker.
3. Polling Target.getTargets for the popup.html page target.
4. Target.attachToTarget with flatten:false, then piping Runtime.evaluate
through Target.sendMessageToTarget — Playwright doesn't expose a
public CDPSession for non-Page targets, so messages are routed
manually with our own id/sessionId dispatcher.
Test leaves the active browsing tab on about:blank so it exercises the
exact scenario the user hit from the new-tab page. Verified the spec
does detect the bug: reverting the no-routing-rules branch makes it
fail with Expected: "#F44336", Received: null.
58 e2e all green (was 57).
Extract the CDP bridge into e2e/real-popup.ts (openRealPopup, seedProfiles, makeProfile) so popup-click specs stay readable, then add five more click-interaction cases against the REAL toolbar popup: - Click Direct -> mode='direct', icon cleared - Click System after a profile is active -> mode='system', icon cleared - Switch between three color-coded profiles -> icon tracks each click - profile -> Direct -> profile round-trip leaves state consistent - Popup DOM (status text, .selected / .active classes) mirrors mode All driven via chrome.action.openPopup() + Target.sendMessageToTarget, same plumbing as the original spec. The earlier monolithic test is now one of six members of the `Real browser-action popup — click interactions` describe block. 81 unit + 63 e2e all green.
…d to system
Codex review flagged that migrateData trusted already-v2 payloads with
mode='profile' without re-verifying that activeProfileId still points
to a real profile. Reachable window: options.js deleteProfile writes
activeProfileId=undefined then asks background to DEACTIVATE_PROFILE.
If the SW restarts between those two writes, initializeProxyState
reads {mode:'profile', activeProfileId:undefined}, and would end up
with currentMode='profile' + activeProfile=null — UI state flags out
of sync until the next user action.
Fix: unify v1 and v2 paths through a single activeIdValid check.
Already-v2 data is still respected for 'direct' / 'system' modes, but
'profile' is demoted to 'system' whenever activeProfileId doesn't
resolve. v1 inference unchanged.
Added two vitest cases covering the demote path (stale id; undefined
id). 83 unit + 63 e2e all green.
Owner
Author
Code Review — PR #33: Feat/28 Direct ModeSummaryThis PR introduces Direct Connection mode (closes #28), fixes the toolbar icon repaint bug, adds options page dark mode, and bumps the version to 1.6.0. 23 files changed across core logic, UI, tests, and documentation. 1. PR Description vs Implementation Consistency ✅All items described in the PR are fully implemented:
2. Multi-Dimensional AnalysisCorrectness ✅
Standards & Conventions ✅
Security ✅
Effectiveness ✅
3. Test Coverage ✅Excellent coverage across all new functionality:
4. Regression Verification ✅
Verdict: Approved ✅Well-structured PR with thorough test coverage, clean code, and no security concerns. The implementation faithfully delivers on all described features. Merging now. Generated by Claude Code |
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
1 participant
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.
No description provided.