-
Notifications
You must be signed in to change notification settings - Fork 53
feat: Add complete W3C WebDriver API support with automatic driver ma… #23
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
…nagement - Implement all W3C Selenium methods including advanced actions, session management, and browser controls - Add automatic browser version mismatch detection and resolution with fallback strategies - Include comprehensive JSDoc annotations for all tools explaining W3C mapping and use cases - Add enhanced error handling with specific ChromeDriver/GeckoDriver version compatibility - Implement Selenium Manager integration for automatic driver downloads - Add support for advanced mouse/keyboard actions, timeouts, capabilities, and form submission - Ensure full compatibility with Chrome 115+ and Firefox using Chrome for Testing API
WalkthroughThe update significantly expands the Selenium-based MCP server's browser automation capabilities. It introduces a comprehensive suite of new tools for browser, window, tab, frame, alert, navigation, element, cookie, wait, script execution, scrolling, dropdown, pointer, keyboard, session, and timeout management. Browser startup is enhanced with robust retry and fallback strategies. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPServer
participant SeleniumWebDriver
Client->>MCPServer: start_browser(browser)
MCPServer->>SeleniumWebDriver: Attempt startup (with fallback)
alt Success
SeleniumWebDriver-->>MCPServer: WebDriver instance
MCPServer-->>Client: Success response
else Failure (retry)
MCPServer->>SeleniumWebDriver: Retry with fallback
alt All attempts fail
MCPServer-->>Client: Detailed error message
end
end
sequenceDiagram
participant Client
participant MCPServer
participant SeleniumWebDriver
Client->>MCPServer: get_window_handle()
MCPServer->>SeleniumWebDriver: Retrieve window handle
SeleniumWebDriver-->>MCPServer: Window handle
MCPServer-->>Client: Window handle
sequenceDiagram
participant Client
participant MCPServer
participant SeleniumWebDriver
Client->>MCPServer: find_elements(by, value, timeout)
MCPServer->>SeleniumWebDriver: Locate elements with provided strategy
SeleniumWebDriver-->>MCPServer: Element(s) reference(s)
MCPServer-->>Client: Element(s) data
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (4)
src/lib/server.js (4)
94-98
: Remove unnecessary continue statement.The
continue
statement is redundant here as it's the last statement in the loop iteration.- if (isVersionMismatchError(e.message)) { - console.log(`Detected version mismatch on attempt ${attempt}, trying fallback strategies...`); - // Continue to next attempt with different strategy - continue; + if (isVersionMismatchError(e.message)) { + console.log(`Detected version mismatch on attempt ${attempt}, trying fallback strategies...`); + // Will continue to next attempt with different strategy
2058-2066
: Remove unnecessary continue statement in findChromeBinary.The
continue
statement in the catch block is redundant.try { if (fs.existsSync(path)) { return path; } } catch (e) { - continue; + // Skip to next path }
2113-2121
: Remove unnecessary continue statement in findFirefoxBinary.The
continue
statement in the catch block is redundant.try { if (fs.existsSync(path)) { return path; } } catch (e) { - continue; + // Skip to next path }
1611-1880
: Consider moving documentation comments to proper JSDoc positions.The W3C mapping documentation appears after the tool definitions. For better maintainability and documentation generation, consider placing these as JSDoc comments above each tool definition.
For example, instead of:
server.tool("click_and_hold", ...); /** * Advanced Actions: Click and Hold * ... */Use:
/** * Advanced Actions: Click and Hold * Simulates pressing and holding the mouse button on an element. * W3C Mapping: Actions API - pointer actions. */ server.tool("click_and_hold", ...);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/server.js
(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/server.js
[error] 97-97: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2064-2064: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2119-2119: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2191-2191: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2192-2192: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2193-2193: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2209-2209: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2210-2210: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2222-2222: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (2)
src/lib/server.js (2)
41-42
: LGTM! Locator strategy additions are correct.The addition of
linkText
andpartialLinkText
locator strategies properly extends the W3C WebDriver support as described in the PR objectives.
60-2392
: Excellent implementation of comprehensive W3C WebDriver support!The addition of extensive browser automation tools, retry mechanisms with fallback strategies, and proper error handling significantly enhances the MCP server's capabilities. The implementation is well-structured and follows consistent patterns throughout.
The minor issues identified (unnecessary continue statements and switch case scoping) are easily addressable and don't impact the overall quality of the implementation.
case "all": | ||
await clearDirectory(cacheDir); | ||
clearedItems.push("entire cache"); | ||
break; | ||
case "drivers": | ||
const driversDir = path.join(cacheDir, 'chromedriver'); | ||
const geckodriverDir = path.join(cacheDir, 'geckodriver'); | ||
const edgedriverDir = path.join(cacheDir, 'msedgedriver'); | ||
|
||
if (fs.existsSync(driversDir)) { | ||
await clearDirectory(driversDir); | ||
clearedItems.push("chromedriver cache"); | ||
} | ||
if (fs.existsSync(geckodriverDir)) { | ||
await clearDirectory(geckodriverDir); | ||
clearedItems.push("geckodriver cache"); | ||
} | ||
if (fs.existsSync(edgedriverDir)) { | ||
await clearDirectory(edgedriverDir); | ||
clearedItems.push("edgedriver cache"); | ||
} | ||
break; | ||
case "browsers": | ||
const chromeDir = path.join(cacheDir, 'chrome'); | ||
const firefoxDir = path.join(cacheDir, 'firefox'); | ||
|
||
if (fs.existsSync(chromeDir)) { | ||
await clearDirectory(chromeDir); | ||
clearedItems.push("Chrome browser cache"); | ||
} | ||
if (fs.existsSync(firefoxDir)) { | ||
await clearDirectory(firefoxDir); | ||
clearedItems.push("Firefox browser cache"); | ||
} | ||
break; | ||
case "metadata": | ||
const metadataFile = path.join(cacheDir, 'se-metadata.json'); | ||
if (fs.existsSync(metadataFile)) { | ||
fs.unlinkSync(metadataFile); | ||
clearedItems.push("metadata file"); | ||
} | ||
break; | ||
} |
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.
Add block scoping to switch cases to prevent variable hoisting issues.
Variable declarations in switch cases can be accessed by other cases due to JavaScript's variable hoisting. Wrap each case in a block to restrict scope.
switch (cache_type) {
case "all":
await clearDirectory(cacheDir);
clearedItems.push("entire cache");
break;
- case "drivers":
+ case "drivers": {
const driversDir = path.join(cacheDir, 'chromedriver');
const geckodriverDir = path.join(cacheDir, 'geckodriver');
const edgedriverDir = path.join(cacheDir, 'msedgedriver');
if (fs.existsSync(driversDir)) {
await clearDirectory(driversDir);
clearedItems.push("chromedriver cache");
}
if (fs.existsSync(geckodriverDir)) {
await clearDirectory(geckodriverDir);
clearedItems.push("geckodriver cache");
}
if (fs.existsSync(edgedriverDir)) {
await clearDirectory(edgedriverDir);
clearedItems.push("edgedriver cache");
}
break;
- case "browsers":
+ }
+ case "browsers": {
const chromeDir = path.join(cacheDir, 'chrome');
const firefoxDir = path.join(cacheDir, 'firefox');
if (fs.existsSync(chromeDir)) {
await clearDirectory(chromeDir);
clearedItems.push("Chrome browser cache");
}
if (fs.existsSync(firefoxDir)) {
await clearDirectory(firefoxDir);
clearedItems.push("Firefox browser cache");
}
break;
- case "metadata":
+ }
+ case "metadata": {
const metadataFile = path.join(cacheDir, 'se-metadata.json');
if (fs.existsSync(metadataFile)) {
fs.unlinkSync(metadataFile);
clearedItems.push("metadata file");
}
break;
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
switch (cache_type) { | |
case "all": | |
await clearDirectory(cacheDir); | |
clearedItems.push("entire cache"); | |
break; | |
case "drivers": | |
const driversDir = path.join(cacheDir, 'chromedriver'); | |
const geckodriverDir = path.join(cacheDir, 'geckodriver'); | |
const edgedriverDir = path.join(cacheDir, 'msedgedriver'); | |
if (fs.existsSync(driversDir)) { | |
await clearDirectory(driversDir); | |
clearedItems.push("chromedriver cache"); | |
} | |
if (fs.existsSync(geckodriverDir)) { | |
await clearDirectory(geckodriverDir); | |
clearedItems.push("geckodriver cache"); | |
} | |
if (fs.existsSync(edgedriverDir)) { | |
await clearDirectory(edgedriverDir); | |
clearedItems.push("edgedriver cache"); | |
} | |
break; | |
case "browsers": | |
const chromeDir = path.join(cacheDir, 'chrome'); | |
const firefoxDir = path.join(cacheDir, 'firefox'); | |
if (fs.existsSync(chromeDir)) { | |
await clearDirectory(chromeDir); | |
clearedItems.push("Chrome browser cache"); | |
} | |
if (fs.existsSync(firefoxDir)) { | |
await clearDirectory(firefoxDir); | |
clearedItems.push("Firefox browser cache"); | |
} | |
break; | |
case "metadata": | |
const metadataFile = path.join(cacheDir, 'se-metadata.json'); | |
if (fs.existsSync(metadataFile)) { | |
fs.unlinkSync(metadataFile); | |
clearedItems.push("metadata file"); | |
} | |
break; | |
} | |
switch (cache_type) { | |
case "all": | |
await clearDirectory(cacheDir); | |
clearedItems.push("entire cache"); | |
break; | |
case "drivers": { | |
const driversDir = path.join(cacheDir, 'chromedriver'); | |
const geckodriverDir = path.join(cacheDir, 'geckodriver'); | |
const edgedriverDir = path.join(cacheDir, 'msedgedriver'); | |
if (fs.existsSync(driversDir)) { | |
await clearDirectory(driversDir); | |
clearedItems.push("chromedriver cache"); | |
} | |
if (fs.existsSync(geckodriverDir)) { | |
await clearDirectory(geckodriverDir); | |
clearedItems.push("geckodriver cache"); | |
} | |
if (fs.existsSync(edgedriverDir)) { | |
await clearDirectory(edgedriverDir); | |
clearedItems.push("edgedriver cache"); | |
} | |
break; | |
} | |
case "browsers": { | |
const chromeDir = path.join(cacheDir, 'chrome'); | |
const firefoxDir = path.join(cacheDir, 'firefox'); | |
if (fs.existsSync(chromeDir)) { | |
await clearDirectory(chromeDir); | |
clearedItems.push("Chrome browser cache"); | |
} | |
if (fs.existsSync(firefoxDir)) { | |
await clearDirectory(firefoxDir); | |
clearedItems.push("Firefox browser cache"); | |
} | |
break; | |
} | |
case "metadata": { | |
const metadataFile = path.join(cacheDir, 'se-metadata.json'); | |
if (fs.existsSync(metadataFile)) { | |
fs.unlinkSync(metadataFile); | |
clearedItems.push("metadata file"); | |
} | |
break; | |
} | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 2191-2191: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2192-2192: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2193-2193: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2209-2209: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2210-2210: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2222-2222: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/lib/server.js between lines 2185 and 2228, the switch cases declare
variables without block scoping, which can cause variable hoisting issues. To
fix this, wrap the code inside each case statement with curly braces {} to
create a block scope, ensuring variables declared in one case do not leak into
others.
…nagement
Summary by CodeRabbit
New Features
Improvements