From efde9f906fe7b426f20c5e477127a5f7e58e2ad6 Mon Sep 17 00:00:00 2001 From: Tomas Grasl Date: Sun, 16 Aug 2026 11:01:11 +0200 Subject: [PATCH 1/3] feat: add press_key and type_text tools The input module had no keyboard capability at all. The only sendKeys calls in src/ were bound to a specific element inside fill_by_uid and upload_file_by_uid, plus one on a prompt dialog, so an agent could not submit a form with Enter, close a modal with Escape, walk an autocomplete list with the arrow keys, or move focus with Tab. I added two tools to the input module, which is part of the slim preset, so they are available in every configuration: - press_key presses a key or combination on the focused element, e.g. "Enter", "Escape" or "Control+Shift+R". - type_text types into the focused element with an optional submitKey, for elements that only react to real typing such as autocomplete widgets and rich text editors. fill_by_uid stays the right tool for replacing the value of a known input. Key names follow the DOM KeyboardEvent.key vocabulary ("Enter", "ArrowDown", "a") so a model can name keys the same way it would in page code, and src/utils/keyboard.ts maps them onto the Selenium Key constants. Unknown names fail with an error that lists what is accepted rather than silently doing nothing. Modifiers are held only for the duration of the key press. If the action sequence throws part way through, the recovery path issues a release actions command, otherwise the modifiers would stay logically held down for the rest of the session and corrupt every later key press. Tested with 32 unit tests covering the parser and the handlers, plus 6 integration tests against a real Firefox that assert typing, Enter submitting a form, Tab moving focus, modifiers reaching the page as ctrlKey/shiftKey, and modifiers being released afterwards. --- README.md | 2 +- src/firefox/dom.ts | 74 ++++++++++ src/firefox/index.ts | 16 +++ src/tools/input.ts | 89 ++++++++++++ src/utils/keyboard.ts | 130 ++++++++++++++++++ .../integration/keyboard.integration.test.ts | 124 +++++++++++++++++ tests/tools/input.test.ts | 109 ++++++++++++++- tests/utils/keyboard.test.ts | 73 ++++++++++ 8 files changed, 615 insertions(+), 2 deletions(-) create mode 100644 src/utils/keyboard.ts create mode 100644 tests/integration/keyboard.integration.test.ts create mode 100644 tests/utils/keyboard.test.ts diff --git a/README.md b/README.md index d0d5d113..9822ec0c 100644 --- a/README.md +++ b/README.md @@ -220,7 +220,7 @@ Both flags are required because the MCP uses both WebDriver Classic (`--marionet - Pages: list/new/navigate/select/close/get_page_text (get_page_text supports optional `saveTo`) - Snapshot/UID: take/resolve/clear (take supports optional `saveTo`) -- Input: click/hover/fill/drag/upload/form fill +- Input: click/hover/fill/drag/upload/form fill, press_key (key combinations on the focused element), type_text (typing with an optional submit key) - Network: list/get (ID‑first, filters, always‑on capture; both support optional `saveTo`) - Downloads: list_downloads/clear_downloads (always‑on capture), set_download_behavior (allow/deny/default) - Console: list/clear (list supports optional `saveTo`) diff --git a/src/firefox/dom.ts b/src/firefox/dom.ts index 707b1161..2250cf3a 100644 --- a/src/firefox/dom.ts +++ b/src/firefox/dom.ts @@ -3,6 +3,7 @@ */ import { By, Key, WebDriver, WebElement } from 'selenium-webdriver'; +import { parseKeyCombination } from '../utils/keyboard.js'; export class DomInteractions { constructor( @@ -288,6 +289,79 @@ export class DomInteractions { await this.waitForEventsAfterAction(); } + // ============================================================================ + // Keyboard input + // ============================================================================ + + /** + * Press a key or key combination on the focused element, e.g. "Enter" or + * "Control+Shift+R". Modifiers are held for the duration of the key press + * and released afterwards. + */ + async pressKey(combination: string): Promise { + const { modifiers, key } = parseKeyCombination(combination); + + const actions = this.driver.actions(); + for (const modifier of modifiers) { + actions.keyDown(modifier); + } + actions.sendKeys(key); + for (const modifier of [...modifiers].reverse()) { + actions.keyUp(modifier); + } + + try { + await actions.perform(); + } catch (error) { + // A sequence that fails part way through can leave modifiers logically + // held down for the rest of the session, so release them explicitly. + await this.releaseHeldKeys(); + throw error; + } + + await this.waitForEventsAfterAction(); + } + + /** + * Type text into the focused element, optionally followed by a single key + * such as "Enter" or "Tab". + */ + async typeText(text: string, submitKey?: string): Promise { + // Parse before typing so an invalid submitKey fails without leaving the + // page half-filled. + if (submitKey) { + parseKeyCombination(submitKey); + } + + if (text.length > 0) { + try { + await this.driver.actions().sendKeys(text).perform(); + } catch (error) { + await this.releaseHeldKeys(); + throw error; + } + } + + if (submitKey) { + await this.pressKey(submitKey); + return; + } + + await this.waitForEventsAfterAction(); + } + + /** + * Release every key and button the session is currently holding down. + * Best effort: this runs while recovering from a failed action sequence. + */ + private async releaseHeldKeys(): Promise { + try { + await this.driver.actions().clear(); + } catch { + // Session may already be gone; the original error is the useful one. + } + } + /** * Wait for events to propagate after user action * Gives the page time to respond to interactions diff --git a/src/firefox/index.ts b/src/firefox/index.ts index 652d6ec9..e3db1472 100644 --- a/src/firefox/index.ts +++ b/src/firefox/index.ts @@ -225,6 +225,22 @@ export class FirefoxClient { return await this.dom.uploadFileByUid(uid, filePath); } + // Keyboard methods, targeting whatever element currently has focus + + async pressKey(combination: string): Promise { + if (!this.dom) { + throw new Error('Not connected'); + } + return await this.dom.pressKey(combination); + } + + async typeText(text: string, submitKey?: string): Promise { + if (!this.dom) { + throw new Error('Not connected'); + } + return await this.dom.typeText(text, submitKey); + } + // ============================================================================ // Console // ============================================================================ diff --git a/src/tools/input.ts b/src/tools/input.ts index 37ff43e1..be904c8d 100644 --- a/src/tools/input.ts +++ b/src/tools/input.ts @@ -147,6 +147,49 @@ export const uploadFileByUidTool = { }, }; +export const pressKeyTool = { + name: 'press_key', + description: + 'Press a key or key combination on the focused element, e.g. "Enter", "Escape" or "Control+Shift+R". Use click_by_uid or fill_by_uid first to move focus.', + annotations: { + readOnlyHint: false, + }, + inputSchema: { + type: 'object', + properties: { + key: { + type: 'string', + description: + 'Key or combination. Single characters ("a", "/"), named keys ("Enter", "Escape", "Tab", "Backspace", "Delete", "Home", "End", "PageUp", "PageDown", "ArrowUp", "ArrowDown", "ArrowLeft", "ArrowRight", "F1"-"F12", "Space"), optionally prefixed with "Control+", "Shift+", "Alt+" or "Meta+".', + }, + }, + required: ['key'], + }, +}; + +export const typeTextTool = { + name: 'type_text', + description: + 'Type text into the focused element, optionally followed by a key such as Enter. Use fill_by_uid to replace the value of a known input; use this for elements that only react to real typing, such as autocomplete and rich text editors.', + annotations: { + readOnlyHint: false, + }, + inputSchema: { + type: 'object', + properties: { + text: { + type: 'string', + description: 'Text to type', + }, + submitKey: { + type: 'string', + description: 'Key to press after typing, e.g. "Enter" or "Tab"', + }, + }, + required: ['text'], + }, +}; + // Handlers export async function handleClickByUid(args: unknown): Promise { try { @@ -327,6 +370,50 @@ export async function handleUploadFileByUid(args: unknown): Promise { + try { + const { key } = (args as { key: string }) || {}; + + if (!key || typeof key !== 'string') { + throw new Error('key parameter is required and must be a string'); + } + + const { getFirefox } = await import('../index.js'); + const firefox = await getFirefox(); + + await firefox.pressKey(key); + + return successResponse(`press ${key}`); + } catch (error) { + return errorResponse(error as Error); + } +} + +export async function handleTypeText(args: unknown): Promise { + try { + const { text, submitKey } = (args as { text: string; submitKey?: string }) || {}; + + if (typeof text !== 'string') { + throw new Error('text parameter is required and must be a string'); + } + + if (submitKey !== undefined && typeof submitKey !== 'string') { + throw new Error('submitKey parameter must be a string'); + } + + const { getFirefox } = await import('../index.js'); + const firefox = await getFirefox(); + + await firefox.typeText(text, submitKey); + + return successResponse( + submitKey ? `typed ${text.length} chars, then ${submitKey}` : `typed ${text.length} chars` + ); + } catch (error) { + return errorResponse(error as Error); + } +} + export const module = defineModule({ name: 'input', description: 'Interact with the page via UID-based clicks, typing, drag, and uploads.', @@ -337,5 +424,7 @@ export const module = defineModule({ [dragByUidToUidTool, handleDragByUidToUid], [fillFormByUidTool, handleFillFormByUid], [uploadFileByUidTool, handleUploadFileByUid], + [pressKeyTool, handlePressKey], + [typeTextTool, handleTypeText], ], }); diff --git a/src/utils/keyboard.ts b/src/utils/keyboard.ts new file mode 100644 index 00000000..6fcc4d89 --- /dev/null +++ b/src/utils/keyboard.ts @@ -0,0 +1,130 @@ +/** + * Parsing of key combinations for the keyboard input tools. + * + * Key names follow the DOM KeyboardEvent.key vocabulary ("Enter", "ArrowDown", + * "a") so that a model can name keys the same way it would in page code, and + * are mapped onto the Selenium Key constants used by the actions API. + */ + +import { Key } from 'selenium-webdriver'; + +/** Modifier names, lowercased, mapped to their Selenium constant. */ +const MODIFIERS: Record = { + control: Key.CONTROL, + ctrl: Key.CONTROL, + shift: Key.SHIFT, + alt: Key.ALT, + option: Key.ALT, + meta: Key.META, + command: Key.COMMAND, + cmd: Key.COMMAND, +}; + +/** Non-printable key names, lowercased, mapped to their Selenium constant. */ +const NAMED_KEYS: Record = { + enter: Key.ENTER, + return: Key.RETURN, + escape: Key.ESCAPE, + esc: Key.ESCAPE, + tab: Key.TAB, + backspace: Key.BACK_SPACE, + delete: Key.DELETE, + del: Key.DELETE, + insert: Key.INSERT, + space: Key.SPACE, + home: Key.HOME, + end: Key.END, + pageup: Key.PAGE_UP, + pagedown: Key.PAGE_DOWN, + arrowup: Key.ARROW_UP, + arrowdown: Key.ARROW_DOWN, + arrowleft: Key.ARROW_LEFT, + arrowright: Key.ARROW_RIGHT, + f1: Key.F1, + f2: Key.F2, + f3: Key.F3, + f4: Key.F4, + f5: Key.F5, + f6: Key.F6, + f7: Key.F7, + f8: Key.F8, + f9: Key.F9, + f10: Key.F10, + f11: Key.F11, + f12: Key.F12, +}; + +export interface ParsedKeyCombination { + /** Selenium constants for the modifiers to hold down, in the given order. */ + modifiers: string[]; + /** Selenium constant or single character for the key to press. */ + key: string; +} + +function supportedKeyNames(): string { + return Object.keys(NAMED_KEYS).join(', '); +} + +/** + * Split a combination into its modifier names and the final key name. + * A trailing "+" is treated as the literal plus key, so "Control++" and "+" + * both work. + */ +function splitCombination(combination: string): { modifierNames: string[]; keyName: string } { + if (combination === '+') { + return { modifierNames: [], keyName: '+' }; + } + if (combination.endsWith('++')) { + return { + modifierNames: combination.slice(0, -2).split('+'), + keyName: '+', + }; + } + const parts = combination.split('+'); + const keyName = parts.pop() ?? ''; + return { modifierNames: parts, keyName }; +} + +/** + * Parse a combination such as "Enter", "Control+a" or "Control+Shift+R" into + * the modifiers to hold and the key to press. Throws with a descriptive error + * when a name is not recognised. + */ +export function parseKeyCombination(combination: string): ParsedKeyCombination { + if (!combination || typeof combination !== 'string') { + throw new Error('key parameter is required and must be a string'); + } + + const { modifierNames, keyName } = splitCombination(combination.trim()); + + const modifiers: string[] = []; + for (const name of modifierNames) { + const modifier = MODIFIERS[name.trim().toLowerCase()]; + if (!modifier) { + throw new Error( + `Unknown modifier "${name}" in key combination "${combination}". ` + + `Supported modifiers: ${Object.keys(MODIFIERS).join(', ')}.` + ); + } + modifiers.push(modifier); + } + + if (keyName.length === 0) { + throw new Error(`Missing key in combination "${combination}".`); + } + + const named = NAMED_KEYS[keyName.toLowerCase()]; + if (named) { + return { modifiers, key: named }; + } + + // Anything else must be a single printable character, e.g. "a" or "/". + if ([...keyName].length === 1) { + return { modifiers, key: keyName }; + } + + throw new Error( + `Unknown key "${keyName}" in key combination "${combination}". ` + + `Use a single character or one of: ${supportedKeyNames()}.` + ); +} diff --git a/tests/integration/keyboard.integration.test.ts b/tests/integration/keyboard.integration.test.ts new file mode 100644 index 00000000..c6bdead4 --- /dev/null +++ b/tests/integration/keyboard.integration.test.ts @@ -0,0 +1,124 @@ +/** + * Integration tests for keyboard input + * Tests with real Firefox browser in headless mode + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { + createTestFirefox, + closeFirefox, + waitForElementInSnapshot, + waitForPageLoad, +} from '../helpers/firefox.js'; +import type { FirefoxClient } from '@/firefox/index.js'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = fileURLToPath(new URL('.', import.meta.url)); +const fixturesPath = resolve(__dirname, '../fixtures'); + +describe('Keyboard Integration Tests', () => { + let firefox: FirefoxClient; + + beforeAll(async () => { + firefox = await createTestFirefox(); + }, 30000); + + afterAll(async () => { + await closeFirefox(firefox); + }); + + async function openForm(): Promise { + await firefox.navigate(`file://${fixturesPath}/form.html`); + await waitForPageLoad(); + } + + async function focusNameField(): Promise { + const nameInput = await waitForElementInSnapshot(firefox, (node) => node.id === 'name', 10000); + await firefox.clickByUid(nameInput.uid); + } + + it('should type text into the focused element', async () => { + await openForm(); + await focusNameField(); + + await firefox.typeText('Ada Lovelace'); + + const value = await firefox.evaluate('document.getElementById("name").value'); + expect(value).toBe('Ada Lovelace'); + }, 20000); + + it('should submit a form with the Enter submit key', async () => { + await openForm(); + await focusNameField(); + + await firefox.typeText('Grace Hopper', 'Enter'); + + const result = await firefox.evaluate('document.getElementById("formResult").textContent'); + expect(result).toContain('Grace Hopper'); + }, 20000); + + it('should move focus with Tab', async () => { + await openForm(); + await focusNameField(); + + await firefox.pressKey('Tab'); + + const focusedId = await firefox.evaluate('document.activeElement.id'); + expect(focusedId).toBe('email'); + }, 20000); + + it('should apply modifiers to the key press', async () => { + await openForm(); + await focusNameField(); + + await firefox.evaluate(` + window.__lastKey = null; + document.addEventListener('keydown', (event) => { + window.__lastKey = { key: event.key, ctrl: event.ctrlKey, shift: event.shiftKey }; + }); + `); + + await firefox.pressKey('Control+Shift+b'); + + const lastKey = (await firefox.evaluate('window.__lastKey')) as { + key: string; + ctrl: boolean; + shift: boolean; + }; + expect(lastKey.ctrl).toBe(true); + expect(lastKey.shift).toBe(true); + }, 20000); + + it('should release modifiers after the key press', async () => { + await openForm(); + await focusNameField(); + + await firefox.pressKey('Control+Shift+b'); + + // A plain character typed afterwards must not inherit the modifiers. + await firefox.evaluate(` + window.__plainKey = null; + document.addEventListener('keydown', (event) => { + window.__plainKey = { key: event.key, ctrl: event.ctrlKey, shift: event.shiftKey }; + }); + `); + await firefox.pressKey('x'); + + const plainKey = (await firefox.evaluate('window.__plainKey')) as { + key: string; + ctrl: boolean; + shift: boolean; + }; + expect(plainKey.key).toBe('x'); + expect(plainKey.ctrl).toBe(false); + expect(plainKey.shift).toBe(false); + }, 20000); + + it('should reject an unknown key without touching the page', async () => { + await openForm(); + await focusNameField(); + + await expect(firefox.pressKey('Enterr')).rejects.toThrow('Unknown key'); + }, 20000); +}); diff --git a/tests/tools/input.test.ts b/tests/tools/input.test.ts index af6ec86d..5ae36b67 100644 --- a/tests/tools/input.test.ts +++ b/tests/tools/input.test.ts @@ -2,7 +2,7 @@ * Unit tests for input tools */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import { clickByUidTool, hoverByUidTool, @@ -10,6 +10,8 @@ import { dragByUidToUidTool, fillFormByUidTool, uploadFileByUidTool, + pressKeyTool, + typeTextTool, } from '../../src/tools/input.js'; describe('Input Tools', () => { @@ -92,5 +94,110 @@ describe('Input Tools', () => { expect(required).toContain('uid'); expect(required).toContain('filePath'); }); + + it('pressKeyTool should require key', () => { + const { properties, required } = pressKeyTool.inputSchema; + expect(properties).toBeDefined(); + expect(properties?.key).toBeDefined(); + expect(properties?.key.type).toBe('string'); + expect(required).toContain('key'); + }); + + it('typeTextTool should require text and accept submitKey', () => { + const { properties, required } = typeTextTool.inputSchema; + expect(properties).toBeDefined(); + expect(properties?.text).toBeDefined(); + expect(properties?.submitKey).toBeDefined(); + expect(required).toContain('text'); + expect(required).not.toContain('submitKey'); + }); + }); + + describe('Keyboard tools: handler behavior', () => { + const pressKey = vi.fn().mockResolvedValue(undefined); + const typeText = vi.fn().mockResolvedValue(undefined); + + function mockFirefox() { + pressKey.mockClear(); + typeText.mockClear(); + vi.doMock('../../src/index.js', () => ({ + args: {}, + getFirefox: vi.fn().mockResolvedValue({ pressKey, typeText }), + })); + } + + afterEach(() => { + vi.resetModules(); + vi.restoreAllMocks(); + }); + + it('should forward the key combination to the client', async () => { + mockFirefox(); + const { handlePressKey } = await import('../../src/tools/input.js'); + const result = await handlePressKey({ key: 'Control+Shift+R' }); + + expect(result.isError).toBeUndefined(); + expect(pressKey).toHaveBeenCalledWith('Control+Shift+R'); + }); + + it('should reject a missing key without calling the client', async () => { + mockFirefox(); + const { handlePressKey } = await import('../../src/tools/input.js'); + const result = await handlePressKey({}); + + expect(result.isError).toBe(true); + expect(pressKey).not.toHaveBeenCalled(); + }); + + it('should surface an invalid key combination as a tool error', async () => { + pressKey.mockClear(); + typeText.mockClear(); + pressKey.mockRejectedValueOnce(new Error('Unknown key "Enterr"')); + vi.doMock('../../src/index.js', () => ({ + args: {}, + getFirefox: vi.fn().mockResolvedValue({ pressKey, typeText }), + })); + const { handlePressKey } = await import('../../src/tools/input.js'); + const result = await handlePressKey({ key: 'Enterr' }); + + expect(result.isError).toBe(true); + expect((result.content[0] as { type: 'text'; text: string }).text).toContain('Unknown key'); + }); + + it('should forward text and submitKey', async () => { + mockFirefox(); + const { handleTypeText } = await import('../../src/tools/input.js'); + const result = await handleTypeText({ text: 'hello', submitKey: 'Enter' }); + + expect(result.isError).toBeUndefined(); + expect(typeText).toHaveBeenCalledWith('hello', 'Enter'); + }); + + it('should allow typing without a submitKey', async () => { + mockFirefox(); + const { handleTypeText } = await import('../../src/tools/input.js'); + const result = await handleTypeText({ text: 'hello' }); + + expect(result.isError).toBeUndefined(); + expect(typeText).toHaveBeenCalledWith('hello', undefined); + }); + + it('should accept an empty string as text', async () => { + mockFirefox(); + const { handleTypeText } = await import('../../src/tools/input.js'); + const result = await handleTypeText({ text: '' }); + + expect(result.isError).toBeUndefined(); + expect(typeText).toHaveBeenCalledWith('', undefined); + }); + + it('should reject a non-string text', async () => { + mockFirefox(); + const { handleTypeText } = await import('../../src/tools/input.js'); + const result = await handleTypeText({ text: 42 }); + + expect(result.isError).toBe(true); + expect(typeText).not.toHaveBeenCalled(); + }); }); }); diff --git a/tests/utils/keyboard.test.ts b/tests/utils/keyboard.test.ts new file mode 100644 index 00000000..ce927b2f --- /dev/null +++ b/tests/utils/keyboard.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect } from 'vitest'; +import { Key } from 'selenium-webdriver'; +import { parseKeyCombination } from '../../src/utils/keyboard.js'; + +describe('parseKeyCombination', () => { + it('parses a named key without modifiers', () => { + expect(parseKeyCombination('Enter')).toEqual({ modifiers: [], key: Key.ENTER }); + }); + + it('is case insensitive for key names', () => { + expect(parseKeyCombination('escape').key).toBe(Key.ESCAPE); + expect(parseKeyCombination('ESCAPE').key).toBe(Key.ESCAPE); + expect(parseKeyCombination('ArrowDown').key).toBe(Key.ARROW_DOWN); + }); + + it('parses a single printable character', () => { + expect(parseKeyCombination('a')).toEqual({ modifiers: [], key: 'a' }); + expect(parseKeyCombination('/')).toEqual({ modifiers: [], key: '/' }); + }); + + it('keeps the case of printable characters', () => { + expect(parseKeyCombination('A').key).toBe('A'); + }); + + it('parses a single modifier', () => { + expect(parseKeyCombination('Control+a')).toEqual({ + modifiers: [Key.CONTROL], + key: 'a', + }); + }); + + it('parses several modifiers in order', () => { + expect(parseKeyCombination('Control+Shift+R')).toEqual({ + modifiers: [Key.CONTROL, Key.SHIFT], + key: 'R', + }); + }); + + it('accepts modifier aliases', () => { + expect(parseKeyCombination('Ctrl+a').modifiers).toEqual([Key.CONTROL]); + expect(parseKeyCombination('Cmd+a').modifiers).toEqual([Key.COMMAND]); + expect(parseKeyCombination('Option+a').modifiers).toEqual([Key.ALT]); + }); + + it('treats a trailing plus as the plus key', () => { + expect(parseKeyCombination('+')).toEqual({ modifiers: [], key: '+' }); + expect(parseKeyCombination('Control++')).toEqual({ modifiers: [Key.CONTROL], key: '+' }); + }); + + it('trims surrounding whitespace', () => { + expect(parseKeyCombination(' Enter ').key).toBe(Key.ENTER); + }); + + it('handles a multi-byte character as a single key', () => { + expect(parseKeyCombination('é')).toEqual({ modifiers: [], key: 'é' }); + }); + + it('rejects an empty combination', () => { + expect(() => parseKeyCombination('')).toThrow('key parameter is required'); + }); + + it('rejects an unknown modifier', () => { + expect(() => parseKeyCombination('Hyper+a')).toThrow('Unknown modifier "Hyper"'); + }); + + it('rejects an unknown key name', () => { + expect(() => parseKeyCombination('Control+Enterr')).toThrow('Unknown key "Enterr"'); + }); + + it('rejects a combination with no key', () => { + expect(() => parseKeyCombination('Control+')).toThrow('Missing key'); + }); +}); From 106683104facd604d1a24c725d05de632a8d634e Mon Sep 17 00:00:00 2001 From: Tomas Grasl Date: Sun, 16 Aug 2026 11:41:34 +0200 Subject: [PATCH 2/3] fix: make type_text atomic and cover the action sequences with unit tests Review feedback on this PR. - typeText built two separate action sequences, one for the text and one for the submit key. MCP tool handlers are not serialised, so a concurrent call could move focus between the two and the submit key would land on a different element. Both now go into a single Actions object performed once. - An empty submitKey passed the handler but was dropped by a truthy check, so the tool reported success without pressing anything. It is now rejected, and the schema declares minLength. - The handler tests mocked pressKey/typeText wholesale, so nothing asserted the keyDown/keyUp ordering, the release-on-failure path or how many times perform() runs. Added tests/firefox/dom-keyboard.test.ts, which drives DomInteractions against a mocked actions API. These run in PR Check, unlike the integration tests. I also checked the review's concern that the requestAnimationFrame ping in waitForEventsAfterAction races a navigation triggered by Enter. I could not reproduce it: with a form that really navigates (no preventDefault), press_key and type_text both completed and landed on the target page in 24 out of 24 runs, and clickByUid on the same form behaved identically. The wait is shared by every existing input tool rather than introduced here, so I left it alone. Making it navigation-aware is worth doing on its own. --- src/firefox/dom.ts | 73 +++++++------ src/tools/input.ts | 16 ++- tests/firefox/dom-keyboard.test.ts | 165 +++++++++++++++++++++++++++++ tests/tools/input.test.ts | 12 +++ 4 files changed, 232 insertions(+), 34 deletions(-) create mode 100644 tests/firefox/dom-keyboard.test.ts diff --git a/src/firefox/dom.ts b/src/firefox/dom.ts index 2250cf3a..747bdca0 100644 --- a/src/firefox/dom.ts +++ b/src/firefox/dom.ts @@ -3,7 +3,22 @@ */ import { By, Key, WebDriver, WebElement } from 'selenium-webdriver'; -import { parseKeyCombination } from '../utils/keyboard.js'; +import type { Actions } from 'selenium-webdriver/lib/input.js'; +import { parseKeyCombination, type ParsedKeyCombination } from '../utils/keyboard.js'; + +/** + * Append a key press to an action sequence: hold the modifiers, press the key, + * then release the modifiers in reverse order. + */ +function appendKeyCombination(actions: Actions, { modifiers, key }: ParsedKeyCombination): void { + for (const modifier of modifiers) { + actions.keyDown(modifier); + } + actions.sendKeys(key); + for (const modifier of [...modifiers].reverse()) { + actions.keyUp(modifier); + } +} export class DomInteractions { constructor( @@ -299,25 +314,11 @@ export class DomInteractions { * and released afterwards. */ async pressKey(combination: string): Promise { - const { modifiers, key } = parseKeyCombination(combination); + const parsed = parseKeyCombination(combination); const actions = this.driver.actions(); - for (const modifier of modifiers) { - actions.keyDown(modifier); - } - actions.sendKeys(key); - for (const modifier of [...modifiers].reverse()) { - actions.keyUp(modifier); - } - - try { - await actions.perform(); - } catch (error) { - // A sequence that fails part way through can leave modifiers logically - // held down for the rest of the session, so release them explicitly. - await this.releaseHeldKeys(); - throw error; - } + appendKeyCombination(actions, parsed); + await this.performKeyActions(actions); await this.waitForEventsAfterAction(); } @@ -329,27 +330,37 @@ export class DomInteractions { async typeText(text: string, submitKey?: string): Promise { // Parse before typing so an invalid submitKey fails without leaving the // page half-filled. - if (submitKey) { - parseKeyCombination(submitKey); - } + const parsed = submitKey === undefined ? null : parseKeyCombination(submitKey); + // Text and submit key go into a single sequence: performed separately, a + // concurrent tool call could move focus in between and the submit key + // would land on a different element. + const actions = this.driver.actions(); if (text.length > 0) { - try { - await this.driver.actions().sendKeys(text).perform(); - } catch (error) { - await this.releaseHeldKeys(); - throw error; - } + actions.sendKeys(text); } - - if (submitKey) { - await this.pressKey(submitKey); - return; + if (parsed) { + appendKeyCombination(actions, parsed); } + await this.performKeyActions(actions); await this.waitForEventsAfterAction(); } + /** + * Perform a keyboard action sequence, releasing anything left held down if + * the sequence fails part way through. Without this a failed combination + * would leave its modifiers logically pressed for the rest of the session. + */ + private async performKeyActions(actions: Actions): Promise { + try { + await actions.perform(); + } catch (error) { + await this.releaseHeldKeys(); + throw error; + } + } + /** * Release every key and button the session is currently holding down. * Best effort: this runs while recovering from a failed action sequence. diff --git a/src/tools/input.ts b/src/tools/input.ts index be904c8d..9ba6115e 100644 --- a/src/tools/input.ts +++ b/src/tools/input.ts @@ -183,6 +183,7 @@ export const typeTextTool = { }, submitKey: { type: 'string', + minLength: 1, description: 'Key to press after typing, e.g. "Enter" or "Tab"', }, }, @@ -397,8 +398,15 @@ export async function handleTypeText(args: unknown): Promise { throw new Error('text parameter is required and must be a string'); } - if (submitKey !== undefined && typeof submitKey !== 'string') { - throw new Error('submitKey parameter must be a string'); + if (submitKey !== undefined) { + if (typeof submitKey !== 'string') { + throw new Error('submitKey parameter must be a string'); + } + // An empty string would otherwise be silently dropped and reported as a + // success that never pressed anything. + if (submitKey.length === 0) { + throw new Error('submitKey parameter must not be empty'); + } } const { getFirefox } = await import('../index.js'); @@ -407,7 +415,9 @@ export async function handleTypeText(args: unknown): Promise { await firefox.typeText(text, submitKey); return successResponse( - submitKey ? `typed ${text.length} chars, then ${submitKey}` : `typed ${text.length} chars` + submitKey !== undefined + ? `typed ${text.length} chars, then ${submitKey}` + : `typed ${text.length} chars` ); } catch (error) { return errorResponse(error as Error); diff --git a/tests/firefox/dom-keyboard.test.ts b/tests/firefox/dom-keyboard.test.ts new file mode 100644 index 00000000..0069bf22 --- /dev/null +++ b/tests/firefox/dom-keyboard.test.ts @@ -0,0 +1,165 @@ +/** + * Unit tests for the keyboard action sequences built by DomInteractions. + * The Selenium actions API is mocked so ordering, atomicity and the recovery + * path can be asserted without launching a browser. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { Key } from 'selenium-webdriver'; +import { DomInteractions } from '@/firefox/dom.js'; + +interface ActionsRecorder { + calls: string[]; + performCount: number; + clearCount: number; + failPerform: (error: Error) => void; +} + +function createDriver(): { driver: any; recorder: ActionsRecorder } { + const calls: string[] = []; + let performError: Error | null = null; + let performCount = 0; + let clearCount = 0; + + const actions = { + keyDown: (key: string) => { + calls.push(`keyDown:${key}`); + return actions; + }, + keyUp: (key: string) => { + calls.push(`keyUp:${key}`); + return actions; + }, + sendKeys: (keys: string) => { + calls.push(`sendKeys:${keys}`); + return actions; + }, + perform: async () => { + performCount++; + if (performError) { + throw performError; + } + }, + clear: async () => { + clearCount++; + }, + }; + + const driver = { + actions: () => actions, + // waitForEventsAfterAction pings the page after every action + executeScript: vi.fn().mockResolvedValue(undefined), + }; + + return { + driver, + recorder: { + calls, + get performCount() { + return performCount; + }, + get clearCount() { + return clearCount; + }, + failPerform: (error: Error) => { + performError = error; + }, + } as ActionsRecorder, + }; +} + +describe('DomInteractions keyboard actions', () => { + let driver: any; + let recorder: ActionsRecorder; + let dom: DomInteractions; + + beforeEach(() => { + ({ driver, recorder } = createDriver()); + dom = new DomInteractions(driver); + }); + + describe('pressKey', () => { + it('sends a named key without modifiers', async () => { + await dom.pressKey('Enter'); + + expect(recorder.calls).toEqual([`sendKeys:${Key.ENTER}`]); + expect(recorder.performCount).toBe(1); + }); + + it('holds modifiers around the key and releases them in reverse order', async () => { + await dom.pressKey('Control+Shift+R'); + + expect(recorder.calls).toEqual([ + `keyDown:${Key.CONTROL}`, + `keyDown:${Key.SHIFT}`, + 'sendKeys:R', + `keyUp:${Key.SHIFT}`, + `keyUp:${Key.CONTROL}`, + ]); + expect(recorder.performCount).toBe(1); + }); + + it('releases held keys when the sequence fails', async () => { + recorder.failPerform(new Error('actions failed')); + + await expect(dom.pressKey('Control+a')).rejects.toThrow('actions failed'); + expect(recorder.clearCount).toBe(1); + }); + + it('rejects an unknown key before touching the driver', async () => { + await expect(dom.pressKey('Enterr')).rejects.toThrow('Unknown key'); + expect(recorder.performCount).toBe(0); + expect(recorder.calls).toEqual([]); + }); + }); + + describe('typeText', () => { + it('types text in a single sequence', async () => { + await dom.typeText('hello'); + + expect(recorder.calls).toEqual(['sendKeys:hello']); + expect(recorder.performCount).toBe(1); + }); + + it('performs text and submit key atomically', async () => { + await dom.typeText('hello', 'Enter'); + + expect(recorder.calls).toEqual(['sendKeys:hello', `sendKeys:${Key.ENTER}`]); + // One perform: a second call could be interleaved with another tool call + // that moves focus, sending the submit key to the wrong element. + expect(recorder.performCount).toBe(1); + }); + + it('applies modifiers of the submit key inside the same sequence', async () => { + await dom.typeText('hi', 'Control+Enter'); + + expect(recorder.calls).toEqual([ + 'sendKeys:hi', + `keyDown:${Key.CONTROL}`, + `sendKeys:${Key.ENTER}`, + `keyUp:${Key.CONTROL}`, + ]); + expect(recorder.performCount).toBe(1); + }); + + it('rejects an invalid submit key before typing anything', async () => { + await expect(dom.typeText('hello', 'Enterr')).rejects.toThrow('Unknown key'); + expect(recorder.calls).toEqual([]); + expect(recorder.performCount).toBe(0); + }); + + it('accepts empty text', async () => { + await dom.typeText(''); + + expect(recorder.calls).toEqual([]); + expect(recorder.performCount).toBe(1); + }); + + it('releases held keys when the sequence fails', async () => { + recorder.failPerform(new Error('actions failed')); + + await expect(dom.typeText('hello', 'Control+Enter')).rejects.toThrow('actions failed'); + expect(recorder.clearCount).toBe(1); + }); + }); +}); diff --git a/tests/tools/input.test.ts b/tests/tools/input.test.ts index 5ae36b67..b5b04ae6 100644 --- a/tests/tools/input.test.ts +++ b/tests/tools/input.test.ts @@ -191,6 +191,18 @@ describe('Input Tools', () => { expect(typeText).toHaveBeenCalledWith('', undefined); }); + it('should reject an empty submitKey instead of silently ignoring it', async () => { + mockFirefox(); + const { handleTypeText } = await import('../../src/tools/input.js'); + const result = await handleTypeText({ text: 'hello', submitKey: '' }); + + expect(result.isError).toBe(true); + expect((result.content[0] as { type: 'text'; text: string }).text).toContain( + 'must not be empty' + ); + expect(typeText).not.toHaveBeenCalled(); + }); + it('should reject a non-string text', async () => { mockFirefox(); const { handleTypeText } = await import('../../src/tools/input.js'); From 2eb59565225c74b12546128101f3382ae024b0fb Mon Sep 17 00:00:00 2001 From: Tomas Grasl Date: Sun, 16 Aug 2026 12:37:09 +0200 Subject: [PATCH 3/3] test: cover keyboard tools against a form that really navigates Review feedback on this PR: form.html cancels its submit with preventDefault, so nothing here exercised a key press that tears the document away while the tools are still finishing up. Added nav-form.html and nav-target.html, a form with no preventDefault, and two integration tests asserting that press_key("Enter") and type_text(text, "Enter") both complete and land on the target page. The second also checks the query string, so it fails if the typed text and the submit key were separated by the navigation. This replaces the manual check described in the previous commit with something CI can verify. Ran the file three times, green each time. --- tests/fixtures/nav-form.html | 20 ++++++++++ tests/fixtures/nav-target.html | 10 +++++ .../integration/keyboard.integration.test.ts | 37 +++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 tests/fixtures/nav-form.html create mode 100644 tests/fixtures/nav-target.html diff --git a/tests/fixtures/nav-form.html b/tests/fixtures/nav-form.html new file mode 100644 index 00000000..e27d89a6 --- /dev/null +++ b/tests/fixtures/nav-form.html @@ -0,0 +1,20 @@ + + + + + Navigating Form Test Page + + +

Navigating Form

+ + + + + diff --git a/tests/fixtures/nav-target.html b/tests/fixtures/nav-target.html new file mode 100644 index 00000000..0a44d821 --- /dev/null +++ b/tests/fixtures/nav-target.html @@ -0,0 +1,10 @@ + + + + + Navigation Target + + +

Landed

+ + diff --git a/tests/integration/keyboard.integration.test.ts b/tests/integration/keyboard.integration.test.ts index c6bdead4..a84b5447 100644 --- a/tests/integration/keyboard.integration.test.ts +++ b/tests/integration/keyboard.integration.test.ts @@ -121,4 +121,41 @@ describe('Keyboard Integration Tests', () => { await expect(firefox.pressKey('Enterr')).rejects.toThrow('Unknown key'); }, 20000); + + // form.html cancels its submit, so these use a fixture that really navigates: + // the document is torn away while the tools are still finishing up. + describe('with a form that performs a real navigation', () => { + async function openNavForm(): Promise { + await firefox.navigate(`file://${fixturesPath}/nav-form.html`); + await waitForPageLoad(); + const queryInput = await waitForElementInSnapshot( + firefox, + (node) => node.id === 'query', + 10000 + ); + await firefox.clickByUid(queryInput.uid); + } + + it('should survive a navigation triggered by press_key', async () => { + await openNavForm(); + + await expect(firefox.pressKey('Enter')).resolves.not.toThrow(); + + const pathname = await firefox.evaluate('location.pathname'); + expect(pathname).toContain('nav-target.html'); + }, 20000); + + it('should survive a navigation triggered by the type_text submit key', async () => { + await openNavForm(); + + await expect(firefox.typeText('mcp', 'Enter')).resolves.not.toThrow(); + + const pathname = await firefox.evaluate('location.pathname'); + expect(pathname).toContain('nav-target.html'); + // The typed text made it into the submitted form, so the two halves of + // the sequence were not separated by the navigation. + const search = await firefox.evaluate('location.search'); + expect(search).toBe('?q=mcp'); + }, 20000); + }); });