From a0802b1863c386e7f8ed1b9e6fd8c0e2df76ad76 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 12 Jan 2026 00:29:53 -0600 Subject: [PATCH] fix locator issues (#14) * fix locator issues * address feedback --- src/actions.ts | 119 ++++++++++++++++++++++++++++++++++++++++-------- src/browser.ts | 14 ++++-- src/snapshot.ts | 94 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 202 insertions(+), 25 deletions(-) diff --git a/src/actions.ts b/src/actions.ts index f797c07..43730d0 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -111,6 +111,47 @@ interface SnapshotData { refs?: Record; } +/** + * Convert Playwright errors to AI-friendly messages + */ +function toAIFriendlyError(error: unknown, selector: string): Error { + const message = error instanceof Error ? error.message : String(error); + + // Handle strict mode violation (multiple elements match) + if (message.includes('strict mode violation')) { + // Extract count if available + const countMatch = message.match(/resolved to (\d+) elements/); + const count = countMatch ? countMatch[1] : 'multiple'; + + return new Error( + `Selector "${selector}" matched ${count} elements. ` + + `Run 'snapshot' to get updated refs, or use a more specific CSS selector.` + ); + } + + // Handle element not found + if ( + message.includes('waiting for') && + (message.includes('to be visible') || message.includes('Timeout')) + ) { + return new Error( + `Element "${selector}" not found or not visible. ` + + `Run 'snapshot' to see current page elements.` + ); + } + + // Handle element not interactable + if (message.includes('intercepts pointer events') || message.includes('not visible')) { + return new Error( + `Element "${selector}" is not interactable (may be hidden or covered). ` + + `Try scrolling it into view or check if a modal/overlay is blocking it.` + ); + } + + // Return original error for unknown cases + return error instanceof Error ? error : new Error(message); +} + /** * Execute a command and return a response */ @@ -384,11 +425,15 @@ async function handleClick(command: ClickCommand, browser: BrowserManager): Prom // Support both refs (@e1) and regular selectors const locator = browser.getLocator(command.selector); - await locator.click({ - button: command.button, - clickCount: command.clickCount, - delay: command.delay, - }); + try { + await locator.click({ + button: command.button, + clickCount: command.clickCount, + delay: command.delay, + }); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { clicked: true }); } @@ -396,13 +441,17 @@ async function handleClick(command: ClickCommand, browser: BrowserManager): Prom async function handleType(command: TypeCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); - if (command.clear) { - await locator.fill(''); - } + try { + if (command.clear) { + await locator.fill(''); + } - await locator.pressSequentially(command.text, { - delay: command.delay, - }); + await locator.pressSequentially(command.text, { + delay: command.delay, + }); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { typed: true }); } @@ -556,14 +605,22 @@ async function handleSelect(command: SelectCommand, browser: BrowserManager): Pr const locator = browser.getLocator(command.selector); const values = Array.isArray(command.values) ? command.values : [command.values]; - await locator.selectOption(values); + try { + await locator.selectOption(values); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { selected: values }); } async function handleHover(command: HoverCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); - await locator.hover(); + try { + await locator.hover(); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { hovered: true }); } @@ -643,26 +700,42 @@ async function handleWindowNew( async function handleFill(command: FillCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); - await locator.fill(command.value); + try { + await locator.fill(command.value); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { filled: true }); } async function handleCheck(command: CheckCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); - await locator.check(); + try { + await locator.check(); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { checked: true }); } async function handleUncheck(command: UncheckCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); - await locator.uncheck(); + try { + await locator.uncheck(); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { unchecked: true }); } async function handleUpload(command: UploadCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); const files = Array.isArray(command.files) ? command.files : [command.files]; - await locator.setInputFiles(files); + try { + await locator.setInputFiles(files); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { uploaded: files }); } @@ -671,13 +744,21 @@ async function handleDoubleClick( browser: BrowserManager ): Promise { const locator = browser.getLocator(command.selector); - await locator.dblclick(); + try { + await locator.dblclick(); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { clicked: true }); } async function handleFocus(command: FocusCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); - await locator.focus(); + try { + await locator.focus(); + } catch (error) { + throw toAIFriendlyError(error, command.selector); + } return successResponse(command.id, { focused: true }); } diff --git a/src/browser.ts b/src/browser.ts index 0081f39..f51e2bc 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -95,12 +95,20 @@ export class BrowserManager { const page = this.getPage(); - // Parse the selector and create locator + // Build locator with exact: true to avoid substring matches + let locator: Locator; if (refData.name) { - return page.getByRole(refData.role as any, { name: refData.name }); + locator = page.getByRole(refData.role as any, { name: refData.name, exact: true }); } else { - return page.getByRole(refData.role as any); + locator = page.getByRole(refData.role as any); } + + // If an nth index is stored (for disambiguation), use it + if (refData.nth !== undefined) { + locator = locator.nth(refData.nth); + } + + return locator; } /** diff --git a/src/snapshot.ts b/src/snapshot.ts index 072b22a..de81aa9 100644 --- a/src/snapshot.ts +++ b/src/snapshot.ts @@ -24,6 +24,8 @@ export interface RefMap { selector: string; role: string; name?: string; + /** Index for disambiguation when multiple elements have same role+name */ + nth?: number; }; } @@ -129,7 +131,7 @@ const STRUCTURAL_ROLES = new Set([ function buildSelector(role: string, name?: string): string { if (name) { const escapedName = name.replace(/"/g, '\\"'); - return `getByRole('${role}', { name: "${escapedName}" })`; + return `getByRole('${role}', { name: "${escapedName}", exact: true })`; } return `getByRole('${role}')`; } @@ -161,12 +163,60 @@ export async function getEnhancedSnapshot( return { tree: enhancedTree, refs }; } +/** + * Track role+name combinations to detect duplicates + */ +interface RoleNameTracker { + counts: Map; + /** Maps role+name key to array of ref IDs that use it */ + refsByKey: Map; + getKey(role: string, name?: string): string; + getNextIndex(role: string, name?: string): number; + trackRef(role: string, name: string | undefined, ref: string): void; + /** Get all role+name keys that have duplicates */ + getDuplicateKeys(): Set; +} + +function createRoleNameTracker(): RoleNameTracker { + const counts = new Map(); + const refsByKey = new Map(); + return { + counts, + refsByKey, + getKey(role: string, name?: string): string { + return `${role}:${name ?? ''}`; + }, + getNextIndex(role: string, name?: string): number { + const key = this.getKey(role, name); + const current = counts.get(key) ?? 0; + counts.set(key, current + 1); + return current; + }, + trackRef(role: string, name: string | undefined, ref: string): void { + const key = this.getKey(role, name); + const refs = refsByKey.get(key) ?? []; + refs.push(ref); + refsByKey.set(key, refs); + }, + getDuplicateKeys(): Set { + const duplicates = new Set(); + for (const [key, refs] of refsByKey) { + if (refs.length > 1) { + duplicates.add(key); + } + } + return duplicates; + }, + }; +} + /** * Process ARIA snapshot: add refs and apply filters */ function processAriaTree(ariaTree: string, refs: RefMap, options: SnapshotOptions): string { const lines = ariaTree.split('\n'); const result: string[] = []; + const tracker = createRoleNameTracker(); // For interactive-only mode, we collect just interactive elements if (options.interactive) { @@ -179,31 +229,43 @@ function processAriaTree(ariaTree: string, refs: RefMap, options: SnapshotOption if (INTERACTIVE_ROLES.has(roleLower)) { const ref = nextRef(); + const nth = tracker.getNextIndex(roleLower, name); + tracker.trackRef(roleLower, name, ref); refs[ref] = { selector: buildSelector(roleLower, name), role: roleLower, name, + nth, // Always store nth, we'll use it for duplicates }; let enhanced = `- ${role}`; if (name) enhanced += ` "${name}"`; enhanced += ` [ref=${ref}]`; + // Only show nth in output if it's > 0 (for readability) + if (nth > 0) enhanced += ` [nth=${nth}]`; if (suffix && suffix.includes('[')) enhanced += suffix; result.push(enhanced); } } + + // Post-process: remove nth from refs that don't have duplicates + removeNthFromNonDuplicates(refs, tracker); + return result.join('\n') || '(no interactive elements)'; } // Normal processing with depth/compact filters for (const line of lines) { - const processed = processLine(line, refs, options); + const processed = processLine(line, refs, options, tracker); if (processed !== null) { result.push(processed); } } + // Post-process: remove nth from refs that don't have duplicates + removeNthFromNonDuplicates(refs, tracker); + // If compact mode, remove empty structural elements if (options.compact) { return compactTree(result.join('\n')); @@ -212,6 +274,22 @@ function processAriaTree(ariaTree: string, refs: RefMap, options: SnapshotOption return result.join('\n'); } +/** + * Remove nth from refs that ended up not having duplicates + * This keeps single-element locators simple (no unnecessary .nth(0)) + */ +function removeNthFromNonDuplicates(refs: RefMap, tracker: RoleNameTracker): void { + const duplicateKeys = tracker.getDuplicateKeys(); + + for (const [ref, data] of Object.entries(refs)) { + const key = tracker.getKey(data.role, data.name); + if (!duplicateKeys.has(key)) { + // Not a duplicate, remove nth to keep locator simple + delete refs[ref].nth; + } + } +} + /** * Get indentation level (number of spaces / 2) */ @@ -223,7 +301,12 @@ function getIndentLevel(line: string): number { /** * Process a single line: add ref if needed, filter if requested */ -function processLine(line: string, refs: RefMap, options: SnapshotOptions): string | null { +function processLine( + line: string, + refs: RefMap, + options: SnapshotOptions, + tracker: RoleNameTracker +): string | null { const depth = getIndentLevel(line); // Check max depth @@ -273,17 +356,22 @@ function processLine(line: string, refs: RefMap, options: SnapshotOptions): stri if (shouldHaveRef) { const ref = nextRef(); + const nth = tracker.getNextIndex(roleLower, name); + tracker.trackRef(roleLower, name, ref); refs[ref] = { selector: buildSelector(roleLower, name), role: roleLower, name, + nth, // Always store nth, we'll clean up non-duplicates later }; // Build enhanced line with ref let enhanced = `${prefix}${role}`; if (name) enhanced += ` "${name}"`; enhanced += ` [ref=${ref}]`; + // Only show nth in output if it's > 0 (for readability) + if (nth > 0) enhanced += ` [nth=${nth}]`; if (suffix) enhanced += suffix; return enhanced;