Merge pull request #56 from angiejones/feat/new-tools

fix: improve switch_to_frame API, add missing docs/tests, fix test reliability

Author: angiejones

README.md

@@ -505,6 +505,7 @@ Executes JavaScript in the browser and returns the result.
 | Parameter | Type | Required | Description |
 |-----------|------|----------|-------------|
 | script | string | Yes | JavaScript code to execute |
+| args | array | No | Arguments to pass to the script (accessible via `arguments[0]`, `arguments[1]`, etc.) |
 
 **Example:**
 ```json
@@ -516,6 +517,17 @@ Executes JavaScript in the browser and returns the result.
 }
 ```
 
+**Example with arguments:**
+```json
+{
+  "tool": "execute_script",
+  "parameters": {
+    "script": "return arguments[0] + arguments[1];",
+    "args": [10, 32]
+  }
+}
+```
+
 ### get_window_handles
 Returns a list of all window/tab handles in the current session.
 
@@ -577,18 +589,17 @@ None required
 ```
 
 ### switch_to_frame
-Switches focus to an iframe within the page.
+Switches focus to an iframe or frame within the page. Provide either `by`/`value` to locate the frame by element, or `index` to switch by position.
 
 **Parameters:**
 | Parameter | Type | Required | Description |
 |-----------|------|----------|-------------|
 | by | string | No | Locator strategy (id, css, xpath, name, tag, class) |
 | value | string | No | Value for the locator strategy |
 | index | number | No | Frame index (0-based) |
+| timeout | number | No | Max wait time in ms (default: 10000) |
 
-Provide either `by`/`value` to locate the frame by element, or `index` to switch by position. Omit all parameters to switch to the parent frame.
-
-**Example:**
+**Example (by locator):**
 ```json
 {
   "tool": "switch_to_frame",
@@ -599,6 +610,16 @@ Provide either `by`/`value` to locate the frame by element, or `index` to switch
 }
 ```
 
+**Example (by index):**
+```json
+{
+  "tool": "switch_to_frame",
+  "parameters": {
+    "index": 0
+  }
+}
+```
+
 ### switch_to_default_content
 Switches focus back to the main page from an iframe.
 

src/lib/server.js

@@ -652,8 +652,17 @@ server.tool(
                     content: [{ type: 'text', text: `Window closed. Switched to: ${handles[0]}` }]
                 };
             }
+            // Last window closed — quit the driver and clean up the session
+            const sessionId = state.currentSession;
+            try {
+                await driver.quit();
+            } catch (quitError) {
+                console.error(`Error quitting driver for session ${sessionId}:`, quitError);
+            }
+            state.drivers.delete(sessionId);
+            state.currentSession = null;
             return {
-                content: [{ type: 'text', text: 'Last window closed' }]
+                content: [{ type: 'text', text: 'Last window closed. Session ended.' }]
             };
         } catch (e) {
             return {
@@ -667,25 +676,24 @@ server.tool(
 // Frame Management Tools
 server.tool(
     "switch_to_frame",
-    "switches focus to an iframe or frame within the page",
+    "switches focus to an iframe or frame within the page. Provide either by/value to locate by element, or index to switch by position.",
     {
-        by: z.enum(["id", "css", "xpath", "name", "tag", "class", "index"]).describe("Locator strategy or 'index' to find frame"),
-        value: z.string().describe("Value for the locator strategy, or frame index number"),
+        by: z.enum(["id", "css", "xpath", "name", "tag", "class"]).optional().describe("Locator strategy to find frame element"),
+        value: z.string().optional().describe("Value for the locator strategy"),
+        index: z.number().optional().describe("Frame index (0-based) to switch to by position"),
         timeout: z.number().optional().describe("Maximum time to wait for frame in milliseconds")
     },
-    async ({ by, value, timeout = 10000 }) => {
+    async ({ by, value, index, timeout = 10000 }) => {
         try {
             const driver = getDriver();
-            if (by === 'index') {
-                const index = parseInt(value, 10);
-                if (isNaN(index)) {
-                    throw new Error(`Invalid frame index: ${value}`);
-                }
+            if (index !== undefined) {
                 await driver.switchTo().frame(index);
-            } else {
+            } else if (by && value) {
                 const locator = getLocator(by, value);
                 const element = await driver.wait(until.elementLocated(locator), timeout);
                 await driver.switchTo().frame(element);
+            } else {
+                throw new Error('Provide either by/value to locate frame by element, or index to switch by position');
             }
             return {
                 content: [{ type: 'text', text: `Switched to frame` }]
@@ -794,7 +802,7 @@ server.tool(
 
 server.tool(
     "send_alert_text",
-    "types text into a browser prompt dialog",
+    "types text into a browser prompt dialog and accepts it",
     {
         text: z.string().describe("Text to enter into the prompt"),
         timeout: z.number().optional().describe("Maximum time to wait for alert in milliseconds")

test/new-tools.test.mjs

@@ -142,6 +142,11 @@ describe('execute_script', () => {
     assert.deepEqual(parsed, { a: 1, b: 2 });
   });
 
+  it('executes script with arguments', async () => {
+    const result = await client.callTool('execute_script', { script: 'return arguments[0] + arguments[1];', args: [10, 32] });
+    assert.equal(getResponseText(result), '42');
+  });
+
   it('returns error for invalid script', async () => {
     const result = await client.callTool('execute_script', { script: 'return undefinedVariable.property;' });
     assert.equal(result.isError, true);
@@ -266,7 +271,7 @@ describe('frame management', () => {
   });
 
   it('switch_to_frame by index', async () => {
-    let result = await client.callTool('switch_to_frame', { by: 'index', value: '0' });
+    let result = await client.callTool('switch_to_frame', { index: 0 });
     assert.equal(getResponseText(result), 'Switched to frame');
 
     result = await client.callTool('get_element_text', { by: 'id', value: 'frame-text' });
@@ -276,8 +281,8 @@ describe('frame management', () => {
     await client.callTool('switch_to_default_content');
   });
 
-  it('switch_to_frame returns error for invalid index', async () => {
-    const result = await client.callTool('switch_to_frame', { by: 'index', value: 'abc' });
+  it('switch_to_frame returns error when no locator or index provided', async () => {
+    const result = await client.callTool('switch_to_frame', {});
     assert.equal(result.isError, true);
   });
 });
@@ -299,14 +304,18 @@ describe('alert handling', () => {
     await client.stop();
   });
 
-  it('get_alert_text reads alert message', async () => {
+  it('get_alert_text reads alert message and accept_alert closes it', async () => {
     await client.callTool('click_element', { by: 'id', value: 'alert-btn' });
     const result = await client.callTool('get_alert_text');
     assert.equal(getResponseText(result), 'Hello from alert!');
+
+    // Accept the alert to close it
+    const acceptResult = await client.callTool('accept_alert');
+    assert.equal(getResponseText(acceptResult), 'Alert accepted');
   });
 
-  it('accept_alert dismisses the alert', async () => {
-    // Alert is still open from previous test
+  it('accept_alert accepts a fresh alert', async () => {
+    await client.callTool('click_element', { by: 'id', value: 'alert-btn' });
     const result = await client.callTool('accept_alert');
     assert.equal(getResponseText(result), 'Alert accepted');
   });