added clear method in server file #28
Conversation
WalkthroughAdds a new public "clear" tool to the MCP Selenium server in src/lib/server.js. It locates an element, waits up to a timeout, and clears its content via element.clear(). Uses locatorSchema plus a text field in input, returns success/error content messages. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP Server
participant WebDriver
participant Element
Client->>MCP Server: tool("clear", {by, value, text, timeout})
MCP Server->>WebDriver: findElement(by, value) with wait(timeout)
WebDriver-->>MCP Server: Element reference
MCP Server->>Element: clear()
Element-->>MCP Server: result
MCP Server-->>Client: content: "Element value reset"
alt on error
MCP Server-->>Client: content: "Error in reset of the element: <message>"
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/server.js (2)
236-236: Add a fallback for cases where element.clear() is unreliable.Some elements (e.g., contenteditable divs, certain custom inputs) don’t clear reliably with clear(). Provide a key-chord fallback to select-all + backspace.
- await element.clear(); + try { + await element.clear(); + } catch (clearErr) { + // Fallback for elements where clear() is unreliable + await element.sendKeys(Key.chord(Key.CONTROL, 'a'), Key.BACK_SPACE); + }
226-226: Tighten wording for clarity and consistency.Use consistent, action-oriented phrasing.
- "resets content of the element", + "clears the value of an element", @@ - content: [{ type: 'text', text: 'Element value reset' }] + content: [{ type: 'text', text: 'Element cleared' }] @@ - content: [{ type: 'text', text: `Error in reset of the element: ${e.message}` }] + content: [{ type: 'text', text: `Error clearing element: ${e.message}` }]Also applies to: 238-238, 242-242
| ...locatorSchema, | ||
| text: z.string().describe("Resets content of the element") | ||
| }, | ||
| async ({ by, value, text, timeout = 10000 }) => { | ||
| try { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused required "text" input and param from the clear tool.
The tool ignores the provided "text" but still requires it via zod, forcing clients to supply meaningless data and creating API friction.
Apply this diff to drop the unused field and param:
server.tool(
"clear",
"resets content of the element",
{
- ...locatorSchema,
- text: z.string().describe("Resets content of the element")
+ ...locatorSchema
},
- async ({ by, value, text, timeout = 10000 }) => {
+ async ({ by, value, timeout = 10000 }) => {📝 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.
| ...locatorSchema, | |
| text: z.string().describe("Resets content of the element") | |
| }, | |
| async ({ by, value, text, timeout = 10000 }) => { | |
| try { | |
| ...locatorSchema | |
| }, | |
| async ({ by, value, timeout = 10000 }) => { | |
| try { |
🤖 Prompt for AI Agents
In src/lib/server.js around lines 228 to 232, the clear tool currently declares
and requires a "text" field in the zod schema and function signature but never
uses it; remove the unused "text" from the schema, its .describe(), and from the
async function parameter list (including defaulting and any references), and
update any callsites and TypeScript types/usages that expect that parameter so
clients are no longer forced to provide meaningless data.
| const element = await driver.wait(until.elementLocated(locator), timeout); | ||
| await element.clear(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wait for element to be visible and enabled before clearing.
elementLocated only ensures presence in the DOM; clear() can fail on hidden/disabled elements. Guard with visibility/enabled waits for robustness.
- const element = await driver.wait(until.elementLocated(locator), timeout);
- await element.clear();
+ const element = await driver.wait(until.elementLocated(locator), timeout);
+ await driver.wait(until.elementIsVisible(element), timeout);
+ await driver.wait(until.elementIsEnabled(element), timeout);
+ await element.clear();📝 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.
| const element = await driver.wait(until.elementLocated(locator), timeout); | |
| await element.clear(); | |
| const element = await driver.wait(until.elementLocated(locator), timeout); | |
| await driver.wait(until.elementIsVisible(element), timeout); | |
| await driver.wait(until.elementIsEnabled(element), timeout); | |
| await element.clear(); |
🤖 Prompt for AI Agents
In src/lib/server.js around lines 235-236, the code locates an element then
calls clear() but only ensures presence in the DOM; change the flow to wait for
the element to be visible and enabled before clearing. After obtaining the
element with driver.wait(until.elementLocated(locator), timeout), call
driver.wait(until.elementIsVisible(element), timeout) and
driver.wait(until.elementIsEnabled(element), timeout) (or an equivalent combined
visibility/enabled check) and only then call element.clear(); this makes clear()
robust against hidden/disabled elements.
|
@angiejones , requesting PR for merge. Kindly consider. |
Added clear method to the server file.
Summary by CodeRabbit