-
Notifications
You must be signed in to change notification settings - Fork 91
feat: multi-window support - closes #37 #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds five public window-management server tools for multi-tab/window handling ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server
participant Driver
participant Browser
rect rgba(220,240,230,0.25)
User->>Server: start_browser()
Server->>Driver: initialize WebDriver
Driver->>Browser: launch initial window (A)
Driver-->>Server: driver ready
end
rect rgba(230,240,255,0.18)
User->>Server: get_window_handles()
Server->>Driver: getAllWindowHandles()
Driver-->>Server: [handleA, handleB, ...]
Server-->>User: handles list
end
rect rgba(255,245,220,0.18)
User->>Server: switch_to_latest_window()
Server->>Driver: switchTo().window(latestHandle)
Driver->>Browser: focus latest tab
Driver-->>Server: switched
Server-->>User: confirmation
end
rect rgba(255,230,230,0.12)
User->>Server: close_current_window()
Server->>Driver: driver.close()
Driver->>Browser: close focused tab
Driver-->>Server: closed (focus may change)
Server-->>User: confirmation
end
rect rgba(240,230,255,0.12)
User->>Server: close_session()
Server->>Driver: driver.quit()
Driver->>Browser: terminate all windows
Server->>Server: delete session, clear state
Server-->>User: session closed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 LanguageTooldocs/CHANGELOG_TAB_SUPPORT.md[uncategorized] ~7-~7: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [uncategorized] ~8-~8: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [uncategorized] ~9-~9: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [uncategorized] ~10-~10: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) 🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
IMPLEMENTATION_SUMMARY.md(1 hunks)README.md(2 hunks)docs/CHANGELOG_TAB_SUPPORT.md(1 hunks)docs/MULTI_TAB_USAGE.md(1 hunks)src/lib/server.js(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/CHANGELOG_TAB_SUPPORT.md
6-6: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
10-10: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (14)
src/lib/server.js (7)
426-443: LGTM!The
get_window_handlestool correctly retrieves all window handles and formats them appropriately. Error handling is consistent with the existing patterns.
445-462: LGTM!The
get_current_window_handletool is implemented correctly with appropriate error handling.
464-483: LGTM!The
switch_to_windowtool correctly switches to the specified window handle. Invalid handles will be caught by Selenium and reported through the error handling.
485-510: LGTM!The
switch_to_latest_windowtool properly handles the edge case when no windows are available and correctly switches to the most recently opened window.
512-529: Verify behavior when closing the last window.The implementation correctly closes the current window. However, consider the scenario where a user closes the last remaining window: the driver will be in an invalid state, but
state.currentSessionwill still reference the session ID. Subsequent tool calls will fail with Selenium errors rather than the clearer "No active browser session" message fromgetDriver().This behavior may be acceptable, as users should use
close_sessionto properly terminate the session. The error handling will catch issues, though the error messages might be less clear.Consider documenting this behavior in the usage guide or adding a check to detect when the last window is closed.
531-551: LGTM!The
close_sessionupdate correctly usesdriver.quit()to terminate the entire browser session and properly cleans up the state by removing the session from the drivers map and clearingcurrentSession. The implementation maintains proper cleanup order.
586-586: LGTM!The server connection is correctly awaited, ensuring the server is properly initialized before handling requests.
README.md (1)
22-22: LGTM!The documentation for the new window management tools is comprehensive and accurate. The feature is properly listed, and each tool is documented with clear examples that match the implementation.
Also applies to: 437-508
docs/CHANGELOG_TAB_SUPPORT.md (1)
1-5: LGTM!The changelog accurately documents the new features and correctly explains backward compatibility. The notes about
close_sessionand element interaction tools are accurate and helpful.Also applies to: 12-19
docs/MULTI_TAB_USAGE.md (3)
7-11: LGTM!The tool descriptions accurately match the implementation.
13-72: LGTM!The example workflow is comprehensive and demonstrates proper usage of all the new window management tools. The JSON examples are accurate and follow the tool signatures correctly.
74-78: LGTM!The best practices are well-chosen and accurate. The advice about maintaining valid window context after closing tabs is particularly important and aligns with proper Selenium window management.
IMPLEMENTATION_SUMMARY.md (2)
7-22: LGTM!The changes are accurately documented and provide a clear summary of the new features and documentation updates.
23-33: LGTM!The testing guidance is comprehensive and provides a clear workflow to validate all the new window management functionality. The steps cover the key scenarios and edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/CHANGELOG_TAB_SUPPORT.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/CHANGELOG_TAB_SUPPORT.md
6-6: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 2
(MD005, list-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
7-7: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 2
(MD005, list-indent)
7-7: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
8-8: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 2
(MD005, list-indent)
8-8: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
9-9: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 2
(MD005, list-indent)
9-9: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
10-10: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 2
(MD005, list-indent)
10-10: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
Multi-Tab/Window Support
New Features
get_window_handles: Retrieves all active window handles.get_current_window_handle: Gets the handle of the currently focused window.switch_to_window: Switches focus to a specific window by its handle.switch_to_latest_window: Switches to the most recently opened window.close_current_window: Closes the currently active window.Backward Compatibility
This update is fully backward compatible. Existing tools are unaffected.
close_sessiontool still closes the entire browser session, including all tabs.click_element,send_keys, etc.) operate on the currently focused tab, preserving existing behavior.Workflows that do not involve multiple tabs will continue to function as before without any changes.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes