diff --git a/CHANGELOG.md b/CHANGELOG.md index b1d61686..ee304ec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,42 @@ - New Bunosh tasks for feature worktrees: `worktree:create ` opens a new branch off main in a sibling directory, `worktree:fetch ` opens an existing branch there, both symlinking the main checkout's `node_modules`; `worktree:delete [branch]` removes a worktree when merged. + ## 2026-08-21 +### Actions report what the app said and did, not just how the page moved + +A click that fired a request the server rejected used to look like a success. The page diff described +DOM and accessibility-tree movement only, so a toast reading "Operation against a key holding the wrong +kind of value" was either buried in raw markup or dropped altogether, and the rejected request surfaced +much later as a session-wide count. Every action now also carries: + +- **messages** — text the app put on the page in response: toasts, alerts, banners and inline errors, + including ones built without any accessibility markup. An action that navigated reports only what its + live regions announced, so the content of the page that opened is not read back as a reply +- **requests** — the calls the action made to the application, each with its status, capped per action + with rejected calls kept ahead of successful ones +- **consoleErrors** — what the page logged while the action ran + +When a request comes back 400 or 500 the result leads with that, and points at the message the user was +shown, rather than leaving the model to read the click as successful and repeat it. + +Elements are no longer compared across two pages. An action that navigated used to report one ARIA diff +whose halves belonged to different pages — the elements of the page left behind listed as removed next to +the elements of the page arrived at, with the chrome common to both cancelled out and nothing saying which +side was which, so elements that no longer exist read as available. Such an action now reports the move +itself, the message the app announced in transit, and its requests; the page arrived at is described in +full by the context that follows it. + +The Pilot's review reads the same evidence attached to the action that caused it, narrowed to what +indicates failure: rejected requests, the first two messages, one console error. It used to get a bare +`POST /api/… → 400` with no page context, which reads as a missing value, and would send the tester back +to fill in a form that was already filled. + ### Changes +- Console messages from the browser were dropped before they were ever recorded, so console errors always + showed as none and a page that logged its own failure reported nothing. - AI models that emit channel markers in tool names (e.g. `click<|channel|>commentary`, common with gpt-oss and gemma) no longer waste a turn: the provider now repairs the name to the real tool and executes it, instead of rejecting the call and telling the AI to retry. In a 24h CI sample this diff --git a/boat/prima/src/prima.ts b/boat/prima/src/prima.ts index 990ea397..b53a9796 100644 --- a/boat/prima/src/prima.ts +++ b/boat/prima/src/prima.ts @@ -1043,7 +1043,12 @@ export class Prima { private async pageChanges(result: ActionResult, previousState: WebPageState | null, code: string): Promise { if (!previousState) return 'no snapshot was captured before this command, so nothing could be compared'; const toolResult = await result.toToolResult(ActionResult.fromState(previousState), code); - return toolResult.pageDiff?.ariaChanges || 'no change'; + const pageDiff = toolResult.pageDiff; + if (!pageDiff?.urlChanged) return pageDiff?.ariaChanges || 'no change'; + + const lines = [`left ${previousState.url} for ${result.url}`]; + for (const message of pageDiff.messages ?? []) lines.push(`- ${message}`); + return lines.join('\n'); } async status(hash: string): Promise { diff --git a/boat/prima/tests/prima.test.ts b/boat/prima/tests/prima.test.ts index 13f86682..d3b31655 100644 --- a/boat/prima/tests/prima.test.ts +++ b/boat/prima/tests/prima.test.ts @@ -75,7 +75,7 @@ function fakePrima(options: Record = {}) { title: 'Dashboard', hash: 'dashboard_h1_dashboard', getStateHash: () => 'dashboard_h1_dashboard', - toToolResult: async () => ({ pageDiff: { urlChanged: true, ariaChanges: 'added:\n - heading "Dashboard"' } }), + toToolResult: async () => ({ pageDiff: { urlChanged: true, messages: ['Signed in as admin'] } }), }); (prima as any).bot = { getExplorer: () => ({ @@ -130,7 +130,8 @@ describe('Prima.pw', () => { test('reports page changes from the action pipeline diff and writes artifacts', async () => { const { prima } = fakePrima(); const envelope = await prima.pw("({ page }) => page.click('text=Login')"); - expect(envelope.changes).toContain('heading "Dashboard"'); + expect(envelope.changes).toContain('left https://app.example.com/login for https://app.example.com/dashboard'); + expect(envelope.changes).toContain('Signed in as admin'); expect(envelope.status).toMatch(/^[0-9a-f]{15}$/); expect(existsSync(path.join(artifactsRoot, envelope.status!, 'aria.yml'))).toBe(true); expect(existsSync(path.join(artifactsRoot, envelope.status!, 'page.html'))).toBe(true); diff --git a/src/action-result.ts b/src/action-result.ts index e5ca7d59..3e4bdf84 100644 --- a/src/action-result.ts +++ b/src/action-result.ts @@ -4,7 +4,7 @@ import { ConfigParser, type HtmlConfig, outputPath } from './config.ts'; import type { Link, WebPageState } from './state-manager.ts'; import { LARGE_ARIA_CHANGE_THRESHOLD, compactAriaSnapshot, diffAriaSnapshots } from './utils/aria.ts'; import { TTLCache } from './utils/cache.ts'; -import { type HtmlDiffPart, type HtmlDiffResult, htmlDiff } from './utils/html-diff.ts'; +import { type HtmlDiffPart, type HtmlDiffResult, htmlDiff, liveRegionMessages } from './utils/html-diff.ts'; import { extractHeadings, extractLinks, extractTargetedHtml, htmlCombinedSnapshot, htmlMinimalUISnapshot, htmlTextSnapshot, minifyHtml } from './utils/html.ts'; import { createDebug } from './utils/logger.ts'; import { slugify } from './utils/strings.ts'; @@ -27,6 +27,7 @@ interface ActionResultData extends WebPageState { h3?: string | undefined; h4?: string | undefined; browserLogs?: any[]; + networkRequests?: NetworkCall[]; iframeSnapshots?: Array<{ src: string; html: string; id?: string }>; ariaSnapshot?: string | null; ariaSnapshotFile?: string; @@ -41,6 +42,9 @@ export interface PageDiff { currentUrl: string; ariaChanges?: string | null; ariaChangeCount?: number; + messages?: string[]; + requests?: NetworkCall[]; + consoleErrors?: string[]; htmlParts?: HtmlDiffPart[]; iframes?: string; } @@ -66,6 +70,7 @@ export class ActionResult implements ActionResultData { public url = ''; public fullUrl: string | undefined = undefined; public browserLogs: any[] = []; + public networkRequests: NetworkCall[] = []; public iframeSnapshots: Array<{ src: string; html: string; id?: string }> = []; public iframeURL: string | undefined = undefined; readonly screenshotFile: string | undefined = undefined; @@ -91,6 +96,7 @@ export class ActionResult implements ActionResultData { this.httpStatus = data.httpStatus; this.error = data.error ?? null; this.browserLogs = data.browserLogs ?? []; + this.networkRequests = data.networkRequests ?? []; this.iframeSnapshots = data.iframeSnapshots ?? []; this.iframeURL = data.iframeURL; this.notes = data.notes ?? []; @@ -508,23 +514,30 @@ export class ActionResult implements ActionResultData { return result; } - const urlChanged = previousState ? !this.isSameUrl({ url: previousState.url }) : true; + const pageDiff: PageDiff = { + urlChanged: previousState ? !this.isSameUrl({ url: previousState.url }) : true, + currentUrl: this.url, + }; + result.pageDiff = pageDiff; - if (!previousState) { - result.pageDiff = { - urlChanged: true, - currentUrl: this.url, - }; - return result; + if (this.networkRequests.length > 0) { + pageDiff.requests = this.networkRequests; + } + + const consoleErrors = this.consoleErrors(); + if (consoleErrors.length > 0) { + pageDiff.consoleErrors = consoleErrors; } + if (!previousState) return result; + + pageDiff.previousUrl = previousState.url; + const diff = await this.diff(previousState); - const pageDiff: PageDiff = { - urlChanged, - previousUrl: previousState.url, - currentUrl: this.url, - }; + if (diff.messages.length > 0) { + pageDiff.messages = diff.messages; + } if (diff.ariaChanged) { pageDiff.ariaChanges = diff.ariaChanged; @@ -552,11 +565,28 @@ export class ActionResult implements ActionResultData { } } - result.pageDiff = pageDiff; return result; } + + private consoleErrors(): string[] { + const errors: string[] = []; + + for (const log of this.browserLogs) { + if ((log.type || log.level) !== 'error') continue; + const text = String(log.text || log.message || log).trim(); + if (!text) continue; + if (errors.includes(text)) continue; + errors.push(text.slice(0, CONSOLE_ERROR_MAX_LENGTH)); + if (errors.length === CONSOLE_ERROR_LIMIT) break; + } + + return errors; + } } +const CONSOLE_ERROR_MAX_LENGTH = 300; +const CONSOLE_ERROR_LIMIT = 3; + const HTML_PARTS_TOTAL_BUDGET = 8000; const HTML_PARTS_COUNT_LIMIT = 8; const HTML_PART_SUBTREE_BUDGET = 2000; @@ -584,6 +614,7 @@ function collapseHtmlParts(parts: HtmlDiffPart[]): HtmlDiffPart[] { export class Diff { private _htmlDiffResult: HtmlDiffResult | null = null; + private _messages: string[] = []; private _ariaDiffResult: string | null = null; private _ariaChangeCount = 0; private _isSameUrl: boolean; @@ -636,19 +667,33 @@ export class Diff { return this._htmlDiffResult; } + get messages(): string[] { + return this._messages; + } + async calculate(): Promise { if (!this.previous) return; - if (this._isSameUrl) { - this._htmlDiffResult = await htmlDiff(this.previous.html, this.current.html, ConfigParser.getInstance().getConfig().html); + if (!this._isSameUrl) { + this._messages = liveRegionMessages(this.previous.html, this.current.html); + return; } + this._htmlDiffResult = await htmlDiff(this.previous.html, this.current.html, ConfigParser.getInstance().getConfig().html); + this._messages = this._htmlDiffResult.messages; + const ariaDiff = diffAriaSnapshots(this.previous.ariaSnapshot, this.current.ariaSnapshot); this._ariaDiffResult = ariaDiff.text; this._ariaChangeCount = ariaDiff.count; } } +export interface NetworkCall { + method: string; + path: string; + status: number; +} + export interface FocusedElement { role: string; name: string; diff --git a/src/action.ts b/src/action.ts index 8eff4d54..3ef6769b 100644 --- a/src/action.ts +++ b/src/action.ts @@ -3,7 +3,7 @@ import { join } from 'node:path'; import { context, trace } from '@opentelemetry/api'; import { container, recorder } from 'codeceptjs'; import * as codeceptjs from 'codeceptjs'; -import { ActionResult, type FocusedElement } from './action-result.js'; +import { ActionResult, type FocusedElement, type NetworkCall } from './action-result.js'; import { clearActivity, setActivity } from './activity.ts'; import { ConfigParser, outputPath } from './config.js'; import type { ExplorbotConfig } from './config.js'; @@ -21,6 +21,8 @@ const debugLog = createDebug('explorbot:action'); const CAPTURE_NAVIGATION_TRANSITION_ATTEMPTS = 3; const DEFAULT_ACTION_TIMEOUT = 3000; const DEFAULT_PAGE_TIMEOUT = 3000; +const MAX_NETWORK_CALLS = 10; +const IMPORTANT_LOG_LEVELS = new Set(['info', 'error', 'warning', 'warn']); class Action { private actor: CodeceptJS.I; @@ -38,6 +40,8 @@ class Action { private recorder?: PlaywrightRecorder; private recovery: RecoveryRunner; private mainDocumentStatus: number | undefined = undefined; + private networkRequests: NetworkCall[] = []; + private baseOrigin: string; constructor(actor: CodeceptJS.I, stateManager: StateManager, recorder?: PlaywrightRecorder, recovery?: RecoveryRunner) { this.actor = actor; @@ -46,6 +50,7 @@ class Action { this.playwrightHelper = container.helpers('Playwright'); this.recorder = recorder; this.recovery = recovery || ((fn) => fn()); + this.baseOrigin = URL.parse(this.config.playwright?.url || '')?.origin || ''; } async saveScreenshot(): Promise { @@ -127,9 +132,7 @@ class Action { const logPath = join(statesDir, logFile); const formattedLogs = browserLogs.map((log: any) => { const logTimestamp = new Date().toISOString(); - const level = (log.type || log.level || 'LOG').toUpperCase(); - const message = log.text || log.message || String(log); - return `[${logTimestamp}] ${level}: ${message}`; + return `[${logTimestamp}] ${log.type.toUpperCase()}: ${log.text}`; }); fs.writeFileSync(logPath, `${formattedLogs.join('\n')}\n`, 'utf8'); @@ -157,12 +160,16 @@ class Action { ariaSnapshotFile = ariaFileName; } + const networkRequests = this.networkRequests; + this.networkRequests = []; + const result = new ActionResult({ html, title, httpStatus: await this.captureMainDocumentStatus(), url, browserLogs, + networkRequests, htmlFile, logFile, screenshotFile, @@ -202,26 +209,53 @@ class Action { } } - private captureMainDocumentResponse(): () => void { + private captureResponses(): () => void { const page = this.playwrightHelper.page; if (!page?.on || !page?.off) return () => {}; this.mainDocumentStatus = undefined; + this.networkRequests = []; const handler = (response: any) => { const request = response.request(); - if (request.resourceType() !== 'document') return; - if (response.frame() !== page.mainFrame()) return; const status = response.status(); if (typeof status !== 'number') return; if (status <= 0) return; - this.mainDocumentStatus = status; + + if (request.resourceType() === 'document') { + if (response.frame() !== page.mainFrame()) return; + this.mainDocumentStatus = status; + return; + } + + this.recordNetworkCall(request, status); }; page.on('response', handler); return () => page.off('response', handler); } + private recordNetworkCall(request: any, status: number): void { + const resourceType = request.resourceType(); + if (resourceType !== 'xhr' && resourceType !== 'fetch') return; + + const url = URL.parse(request.url()); + if (!url) return; + if (url.origin !== this.baseOrigin) return; + + const call: NetworkCall = { method: request.method(), path: url.pathname, status }; + if (this.networkRequests.some((r) => r.method === call.method && r.path === call.path && r.status === call.status)) return; + + if (this.networkRequests.length >= MAX_NETWORK_CALLS) { + if (status < 400) return; + const succeeded = this.networkRequests.findIndex((r) => r.status < 400); + if (succeeded === -1) return; + this.networkRequests.splice(succeeded, 1); + } + + this.networkRequests.push(call); + } + /** * Capture HTML snapshots of all iframes on the page */ @@ -267,13 +301,7 @@ class Action { try { const logs = await (this.actor as any).grabBrowserLogs(); - // Filter for important logs (info, error, warning) - const importantLogs = logs.filter((log: any) => { - const level = log.type || log.level; - return ['info', 'error', 'warning', 'warn'].includes(level); - }); - - return importantLogs; + return logs.map(toBrowserLog).filter((log: any) => IMPORTANT_LOG_LEVELS.has(log.type)); } catch (error) { debugLog('Failed to capture browser logs:', error); return []; @@ -292,7 +320,7 @@ class Action { const stepListener = attachStepLogger(executedSteps, assertionSteps); const groupId = this.recorder ? await this.recorder.beginAction(codeString) : null; this.playwrightGroupId = groupId; - const detachMainDocumentResponse = this.captureMainDocumentResponse(); + const detachResponses = this.captureResponses(); const activeSpan = Observability.getSpan(); const tracer = trace.getTracer('ai'); const stepSpan = activeSpan ? tracer.startSpan('codeceptjs.step', undefined, trace.setSpan(context.active(), activeSpan)) : null; @@ -341,7 +369,7 @@ class Action { throw err; } finally { this.restorePageTimeout(); - detachMainDocumentResponse(); + detachResponses(); if (groupId) await this.recorder!.endAction(); detachStepLogger(stepListener); if (stepSpan) { @@ -429,6 +457,12 @@ async function captureHtml(page: any, frame: any, actor: any): Promise { throw new Error('Playwright page is unavailable for HTML capture'); } +function toBrowserLog(log: any): { type: string; text: string } { + const type = typeof log.type === 'function' ? log.type() : log.type || log.level || 'log'; + const text = typeof log.text === 'function' ? log.text() : log.text || log.message || String(log); + return { type, text: text.replace(/\s+/g, ' ').trim() }; +} + async function captureTitle(page: any, actor: any): Promise { if (page?.title) return page.title(); if (actor?.grabTitle) return actor.grabTitle(); diff --git a/src/ai/pilot.ts b/src/ai/pilot.ts index 7c95ca28..ebc45adc 100644 --- a/src/ai/pilot.ts +++ b/src/ai/pilot.ts @@ -27,6 +27,8 @@ import { withdrawVisionTools } from './tools.ts'; const CHECK_TOOLS = ['verify', 'see', 'research', 'context']; const META_TOOLS = ['record', 'reset', 'stop', 'finish']; +const PILOT_MESSAGE_LIMIT = 2; +const PILOT_MESSAGE_MAX_LENGTH = 160; export class Pilot implements Agent { emoji = '🧭'; @@ -1049,6 +1051,21 @@ export class Pilot implements Agent { const ariaDiff = t.output?.pageDiff?.ariaChanges; if (ariaDiff) line += `\n ${ariaDiff}`; + if (t.output?.pageDiff?.urlChanged) line += `\n moved: ${t.output.pageDiff.previousUrl} → ${t.output.pageDiff.currentUrl}`; + + const failedRequests = (t.output?.pageDiff?.requests ?? []).filter((r: any) => r.status >= 400); + if (failedRequests.length > 0) { + line += `\n requests: ${failedRequests.map((r: any) => `${r.method} ${r.path} → ${r.status}`).join(', ')}`; + } + + const messages = (t.output?.pageDiff?.messages ?? []).slice(0, PILOT_MESSAGE_LIMIT); + if (messages.length > 0) { + line += `\n messages: ${messages.map((m: string) => m.slice(0, PILOT_MESSAGE_MAX_LENGTH)).join(' | ')}`; + } + + const consoleError = t.output?.pageDiff?.consoleErrors?.[0]; + if (consoleError) line += `\n console: ${consoleError.slice(0, PILOT_MESSAGE_MAX_LENGTH)}`; + return line; }) .join('\n\n'); diff --git a/src/ai/tools.ts b/src/ai/tools.ts index b24089ef..89e97477 100644 --- a/src/ai/tools.ts +++ b/src/ai/tools.ts @@ -1111,7 +1111,12 @@ export function createAgentTools({ explorer, stateManager, ai, researcher, navig return tools; } -const PAGE_DIFF_SUGGESTION = 'Analyze page diff. htmlParts shows what changed and WHERE — each part has a container selector. Use the container as context when clicking elements from the diff.'; +const PAGE_DIFF_SUGGESTION = + 'Analyze page diff. htmlParts shows what changed and WHERE — each part has a container selector. Use the container as context when clicking elements from the diff. messages holds text the app showed in response, requests the calls it made and consoleErrors what it logged.'; + +const FAILED_REQUEST_SUGGESTION = 'The server rejected a request made by this action (see requests). The UI accepted the interaction but the operation did not complete — read messages and consoleErrors for the reason and report it instead of repeating the action.'; + +const NAVIGATED_SUGGESTION = 'The action left the page. Elements are never compared across pages, so this diff carries the move itself and what the app announced in transit — an empty element diff does not mean nothing happened.'; const ARIA_OUTPUT_CAP = 4000; const HTML_OUTPUT_CAP = 6000; @@ -1196,7 +1201,11 @@ export function successToolResult(action: string, data?: Record, so const ariaChanges = data.pageDiff.ariaChanges || ''; const urlChanged = data.pageDiff.urlChanged === true; const hasHtmlParts = Array.isArray(data.pageDiff.htmlParts) && data.pageDiff.htmlParts.length > 0; - if (isMajorPageChange(data.pageDiff)) { + if (hasFailedRequest(data.pageDiff)) { + suggestion = `${FAILED_REQUEST_SUGGESTION} ${suggestion}`; + } else if (urlChanged) { + suggestion = `${NAVIGATED_SUGGESTION} ${suggestion}`; + } else if (isMajorPageChange(data.pageDiff)) { suggestion = `MAJOR PAGE CHANGE. Page entered a different mode. Check htmlParts and iframes in pageDiff before next action. ${suggestion}`; } else if (!urlChanged && !ariaChanges && !hasHtmlParts) { suggestion = 'Action ran without error but produced no observable change (URL, ARIA and HTML all unchanged). The locator likely matched a non-interactive ancestor or an element outside the intended control. Re-locate via xpathCheck() or verify with see() before treating this as success.'; @@ -1212,10 +1221,15 @@ export function isMajorPageChange(pageDiff: PageDiff): boolean { return pageDiff.urlChanged !== true && (pageDiff.ariaChangeCount ?? 0) >= LARGE_ARIA_CHANGE_THRESHOLD; } +export function hasFailedRequest(pageDiff: PageDiff): boolean { + return (pageDiff.requests ?? []).some((request) => request.status >= 400); +} + function hasObservablePageChange(data?: Record): boolean { if (!data?.pageDiff) return false; if (data.pageDiff.urlChanged === true) return true; if (data.pageDiff.ariaChanges) return true; + if (data.pageDiff.messages?.length) return true; return Array.isArray(data.pageDiff.htmlParts) && data.pageDiff.htmlParts.length > 0; } diff --git a/src/utils/html-diff.ts b/src/utils/html-diff.ts index 2d90c981..1771dbe2 100644 --- a/src/utils/html-diff.ts +++ b/src/utils/html-diff.ts @@ -17,6 +17,7 @@ export interface HtmlDiffResult { removed: string[]; similarity: number; summary: string; + messages: string[]; } interface HtmlNode { @@ -29,6 +30,11 @@ interface HtmlNode { const IGNORED_PATHS = new Set(['html[1]', 'html[1]/head[1]', 'html[1]/body[1]']); +const LIVE_REGION_ROLES = new Set(['alert', 'alertdialog', 'status', 'log']); +const TEXT_LINE_PREFIX = 'TEXT:'; +const MESSAGE_MAX_LENGTH = 200; +const MESSAGE_LIMIT = 8; + type DocumentNode = parse5TreeAdapter.Document; type ElementNode = parse5TreeAdapter.Element; type ParentNode = parse5TreeAdapter.Document | parse5TreeAdapter.Element; @@ -203,7 +209,9 @@ export async function htmlDiff(originalHtml: string, modifiedHtml: string, htmlC const similarity = calculateSimilarity(originalLines, modifiedLines); const { added, removed } = findDifferences(originalLines, modifiedLines); - const parts = await buildDiffParts(originalDocument, modifiedDocument); + const originalMap = collectElementMap(originalDocument); + const modifiedMap = collectElementMap(modifiedDocument); + const parts = await buildDiffParts(originalMap, modifiedMap); const structuralAdditions = parts.flatMap((p) => p.added.filter((a) => a.startsWith('ELEMENT:'))); const allAdded = [...added, ...structuralAdditions]; @@ -216,9 +224,69 @@ export async function htmlDiff(originalHtml: string, modifiedHtml: string, htmlC removed, similarity, summary, + messages: collectMessages(originalMap, modifiedMap, allAdded), }; } +/** + * Text the app announced while the page stayed the same: live region content first, then any other text that appeared. + */ +function collectMessages(originalMap: NodeMap, modifiedMap: NodeMap, added: string[]): string[] { + const appearedText = added.filter((line) => line.startsWith(TEXT_LINE_PREFIX)).map((line) => line.slice(TEXT_LINE_PREFIX.length)); + + return limitMessages([...collectLiveRegionTexts(originalMap, modifiedMap), ...appearedText]); +} + +/** + * Text the app announced across a navigation. Only live regions: everything else on a new page is its content, not a message. + */ +export function liveRegionMessages(originalHtml: string, modifiedHtml: string): string[] { + const originalMap = collectElementMap(parseDocument(originalHtml)); + const modifiedMap = collectElementMap(parseDocument(modifiedHtml)); + + return limitMessages(collectLiveRegionTexts(originalMap, modifiedMap)); +} + +function limitMessages(candidates: string[]): string[] { + const messages: string[] = []; + + for (const candidate of candidates) { + const text = candidate.replace(/\s+/g, ' ').trim().slice(0, MESSAGE_MAX_LENGTH); + if (!text) continue; + if (messages.some((message) => message.includes(text))) continue; + messages.push(text); + if (messages.length === MESSAGE_LIMIT) break; + } + + return messages; +} + +function collectLiveRegionTexts(originalMap: NodeMap, modifiedMap: NodeMap): string[] { + const texts: string[] = []; + + for (const [path, element] of modifiedMap) { + if (!isLiveRegion(element)) continue; + const text = getTextContent(element).trim(); + if (!text) continue; + const previous = originalMap.get(path); + if (previous && getTextContent(previous).trim() === text) continue; + texts.push(text); + } + + return texts; +} + +function isLiveRegion(element: ElementNode): boolean { + if (element.tagName?.toLowerCase() === 'output') return true; + + const attrs = element.attrs ?? []; + const role = attrs.find((attr) => attr.name === 'role')?.value.toLowerCase(); + if (role && LIVE_REGION_ROLES.has(role)) return true; + + const live = attrs.find((attr) => attr.name === 'aria-live')?.value.toLowerCase(); + return live === 'polite' || live === 'assertive'; +} + /** * Parse HTML into a document, wrapping fragments with html/body for consistency. * Uses custom sanitization that removes iframes for diff purposes. @@ -448,10 +516,7 @@ function findStableContainer(topLevelPath: string, originalMap: NodeMap, modifie return { path: 'html[1]/body[1]', selector: 'body' }; } -async function buildDiffParts(originalDocument: DocumentNode, modifiedDocument: DocumentNode): Promise { - const originalMap = collectElementMap(originalDocument); - const modifiedMap = collectElementMap(modifiedDocument); - +async function buildDiffParts(originalMap: NodeMap, modifiedMap: NodeMap): Promise { const addedPaths: string[] = []; const changedPaths: string[] = []; @@ -774,7 +839,7 @@ function flattenHtml(node: HtmlNode): string[] { function process(n: HtmlNode): void { if (n.type === 'text' && n.content) { if (n.content.length >= 5) { - lines.push(`TEXT:${n.content}`); + lines.push(`${TEXT_LINE_PREFIX}${n.content}`); } return; } @@ -795,7 +860,7 @@ function flattenHtml(node: HtmlNode): string[] { } if (n.content && n.content.length >= 5) { - lines.push(`TEXT:${n.content}`); + lines.push(`${TEXT_LINE_PREFIX}${n.content}`); } if (n.children) { diff --git a/tests/integration/prima-smoke.test.ts b/tests/integration/prima-smoke.test.ts index 8ab7d669..fb5982b3 100644 --- a/tests/integration/prima-smoke.test.ts +++ b/tests/integration/prima-smoke.test.ts @@ -110,7 +110,7 @@ describe('Prima drives a real page', () => { expect(envelope.used).toEqual(["({ page }) => page.click('text=Submit')"]); expect(envelope.page.title).toBe('Widget Depot Thanks'); expect(envelope.page.previousUrl).not.toBe(envelope.page.url); - expect(envelope.changes).toContain('Submit'); + expect(envelope.changes).toContain('/thanks'); const rendered = renderEnvelope(envelope); expect(rendered).toContain('### Changes'); diff --git a/tests/unit/action-browser-logs.test.ts b/tests/unit/action-browser-logs.test.ts new file mode 100644 index 00000000..b262bd39 --- /dev/null +++ b/tests/unit/action-browser-logs.test.ts @@ -0,0 +1,34 @@ +import { beforeEach, describe, expect, it } from 'bun:test'; +import Action from '../../src/action.ts'; +import { ConfigParser } from '../../src/config.ts'; + +function buildAction(logs: any[]): Action { + const actor: any = { grabBrowserLogs: async () => logs }; + return new Action(actor, {} as any); +} + +describe('Action browser logs', () => { + beforeEach(() => { + ConfigParser.resetForTesting(); + ConfigParser.setupTestConfig(); + }); + + it('reads playwright console messages, whose type and text are methods', async () => { + const action = buildAction([ + { type: () => 'error', text: () => 'ReferenceError: foo is not defined' }, + { type: () => 'debug', text: () => 'ignored' }, + ]); + + const logs = await (action as any).captureBrowserLogs(); + + expect(logs).toEqual([{ type: 'error', text: 'ReferenceError: foo is not defined' }]); + }); + + it('reads plain log objects and collapses their whitespace', async () => { + const action = buildAction([{ level: 'warning', message: 'slow\n response' }]); + + const logs = await (action as any).captureBrowserLogs(); + + expect(logs).toEqual([{ type: 'warning', text: 'slow response' }]); + }); +}); diff --git a/tests/unit/action-network-calls.test.ts b/tests/unit/action-network-calls.test.ts new file mode 100644 index 00000000..f6c26de5 --- /dev/null +++ b/tests/unit/action-network-calls.test.ts @@ -0,0 +1,54 @@ +import { beforeEach, describe, expect, it } from 'bun:test'; +import Action from '../../src/action.ts'; +import { ConfigParser } from '../../src/config.ts'; + +function buildAction(): Action { + return new Action({} as any, {} as any); +} + +function request(path: string, method = 'GET') { + return { method: () => method, resourceType: () => 'xhr', url: () => `https://example.com${path}` }; +} + +function record(action: Action, count: number, status: number, prefix: string): void { + for (let i = 0; i < count; i++) { + (action as any).recordNetworkCall(request(`/api/${prefix}/${i}`), status); + } +} + +describe('Action network calls', () => { + beforeEach(() => { + ConfigParser.resetForTesting(); + ConfigParser.setupTestConfig(); + }); + + it('stops collecting successful calls once the list is full', () => { + const action = buildAction(); + + record(action, 12, 200, 'ok'); + + expect((action as any).networkRequests).toHaveLength(10); + }); + + it('makes room for a rejected call by dropping a successful one', () => { + const action = buildAction(); + record(action, 10, 200, 'ok'); + + (action as any).recordNetworkCall(request('/api/runs', 'POST'), 500); + + const calls = (action as any).networkRequests; + expect(calls).toHaveLength(10); + expect(calls).toContainEqual({ method: 'POST', path: '/api/runs', status: 500 }); + }); + + it('keeps the earliest failures when every call was rejected', () => { + const action = buildAction(); + record(action, 10, 500, 'failed'); + + (action as any).recordNetworkCall(request('/api/runs', 'POST'), 500); + + const calls = (action as any).networkRequests; + expect(calls).toHaveLength(10); + expect(calls).not.toContainEqual({ method: 'POST', path: '/api/runs', status: 500 }); + }); +}); diff --git a/tests/unit/action-result-diff.test.ts b/tests/unit/action-result-diff.test.ts index e6c88678..e4bde0df 100644 --- a/tests/unit/action-result-diff.test.ts +++ b/tests/unit/action-result-diff.test.ts @@ -79,17 +79,21 @@ describe('ActionResult Diff', () => { const previous = new ActionResult({ url: '/page1', html: '

Page 1

', + ariaSnapshot: '- button "Click me"', }); const current = new ActionResult({ url: '/page2', html: '

Page 2

', + ariaSnapshot: '- button "Other button"', }); const diff = await Diff.create(current, previous); expect(diff.htmlDiff).toBeNull(); expect(diff.htmlParts).toEqual([]); + expect(diff.ariaChanged).toBeNull(); + expect(diff.ariaChangeCount).toBe(0); }); test('should calculate aria diff', async () => { diff --git a/tests/unit/page-diff-evidence.test.ts b/tests/unit/page-diff-evidence.test.ts new file mode 100644 index 00000000..0370cd22 --- /dev/null +++ b/tests/unit/page-diff-evidence.test.ts @@ -0,0 +1,159 @@ +import { beforeEach, describe, expect, test } from 'bun:test'; +import { ActionResult } from '../../src/action-result.ts'; +import { successToolResult } from '../../src/ai/tools.ts'; +import { ConfigParser } from '../../src/config.ts'; +import { htmlDiff, liveRegionMessages } from '../../src/utils/html-diff.ts'; + +const page = (body: string) => `
${body}
`; + +describe('html diff messages', () => { + test('reports a notification that carries no aria role', async () => { + const before = page('
'); + const after = page('
Operation against a key holding the wrong kind of value
'); + + const diff = await htmlDiff(before, after); + + expect(diff.messages).toContain('Operation against a key holding the wrong kind of value'); + }); + + test('reports live region text and keeps it out of the list once it stops changing', async () => { + const before = page('
'); + const after = page('
Payment declined
'); + + expect((await htmlDiff(before, after)).messages).toEqual(['Payment declined']); + expect((await htmlDiff(after, after)).messages).toEqual([]); + }); + + test('reads aria-live regions and output elements', async () => { + const before = page('
'); + const after = page('
Saved
3 results'); + + expect((await htmlDiff(before, after)).messages).toEqual(['Saved', '3 results']); + }); + + test('reports live region text across a navigation and leaves the new page content out', async () => { + const before = page('

Sign in

'); + const after = page('
Welcome back, admin

Dashboard

Nothing scheduled for today

'); + + expect(liveRegionMessages(before, after)).toEqual(['Welcome back, admin']); + }); + + test('reports nothing when the page did not change', async () => { + const html = page('
Idle
'); + + expect((await htmlDiff(html, html)).messages).toEqual([]); + }); +}); + +describe('pageDiff evidence', () => { + beforeEach(() => { + ConfigParser.resetForTesting(); + ConfigParser.setupTestConfig(); + }); + + test('carries messages, requests and console errors of the action', async () => { + const previous = new ActionResult({ id: 1, url: '/runs', html: page('
') }); + const current = new ActionResult({ + id: 2, + url: '/runs', + html: page('
Save failed
'), + networkRequests: [{ method: 'POST', path: '/api/runs', status: 400 }], + browserLogs: [ + { type: 'error', text: 'POST /api/runs returned a 400' }, + { type: 'info', text: 'noise' }, + ], + }); + + const { pageDiff } = await current.toToolResult(previous, 'Save'); + + expect(pageDiff?.messages).toContain('Save failed'); + expect(pageDiff?.requests).toEqual([{ method: 'POST', path: '/api/runs', status: 400 }]); + expect(pageDiff?.consoleErrors).toEqual(['POST /api/runs returned a 400']); + }); + + test('keeps the content of a page it navigated to out of the messages', async () => { + const previous = new ActionResult({ id: 1, url: '/runs', html: page('

Runs

Projects') }); + const current = new ActionResult({ + id: 2, + url: '/projects', + html: page('
Project archived

Projects list

Choose a project to continue

'), + }); + + const { pageDiff } = await current.toToolResult(previous, 'Projects'); + + expect(pageDiff?.urlChanged).toBe(true); + expect(pageDiff?.messages).toEqual(['Project archived']); + }); + + test('does not compare elements across two pages', async () => { + const previous = new ActionResult({ id: 1, url: '/runs', html: page('

Runs

'), ariaSnapshot: '- button "New run"\n- link "Projects"' }); + const current = new ActionResult({ id: 2, url: '/projects', html: page('

Projects list

'), ariaSnapshot: '- button "Create project"\n- searchbox "Filter projects"' }); + + const { pageDiff } = await current.toToolResult(previous, 'Projects'); + + expect(pageDiff?.ariaChanges).toBeUndefined(); + expect(pageDiff?.htmlParts).toBeUndefined(); + }); + + test('omits evidence the action did not produce', async () => { + const previous = new ActionResult({ id: 1, url: '/runs', html: page('') }); + const current = new ActionResult({ id: 2, url: '/runs', html: page('') }); + + const { pageDiff } = await current.toToolResult(previous, 'Save'); + + expect(pageDiff?.messages).toBeUndefined(); + expect(pageDiff?.requests).toBeUndefined(); + expect(pageDiff?.consoleErrors).toBeUndefined(); + }); + + test('survives the state rebuild every tool does before reading the diff', async () => { + const previous = new ActionResult({ id: 1, url: '/runs', html: page('') }); + const current = new ActionResult({ + id: 2, + url: '/runs', + html: page(''), + networkRequests: [{ method: 'POST', path: '/api/runs', status: 400 }], + }); + + const { pageDiff } = await ActionResult.fromState(current).toToolResult(previous, 'Save'); + + expect(pageDiff?.requests).toEqual([{ method: 'POST', path: '/api/runs', status: 400 }]); + }); + + test('reports requests of a first-ever capture with no previous state', async () => { + const current = new ActionResult({ + id: 1, + url: '/runs', + html: page(''), + networkRequests: [{ method: 'GET', path: '/api/runs', status: 200 }], + }); + + const { pageDiff } = await current.toToolResult(null, 'Save'); + + expect(pageDiff?.requests).toEqual([{ method: 'GET', path: '/api/runs', status: 200 }]); + }); +}); + +describe('tool suggestion for rejected requests', () => { + test('leads with the server rejection when a request failed', () => { + const result = successToolResult('click', { + pageDiff: { urlChanged: false, currentUrl: '/runs', requests: [{ method: 'POST', path: '/api/runs', status: 500 }] }, + }); + + expect(result.suggestion).toStartWith('The server rejected a request made by this action'); + }); + + test('keeps the plain page diff suggestion when every request succeeded', () => { + const result = successToolResult('click', { + pageDiff: { urlChanged: false, currentUrl: '/runs', ariaChanges: 'ariaDiff:\n added:\n - alert "Run started"', requests: [{ method: 'GET', path: '/api/runs', status: 200 }] }, + }); + + expect(result.suggestion).toStartWith('Analyze page diff.'); + }); + + test('says why an action that left the page carries no element diff', () => { + const result = successToolResult('click', { pageDiff: { urlChanged: true, previousUrl: '/runs', currentUrl: '/projects' } }); + + expect(result.suggestion).toStartWith('The action left the page.'); + }); +}); diff --git a/tests/unit/pilot-action-evidence.test.ts b/tests/unit/pilot-action-evidence.test.ts new file mode 100644 index 00000000..1a104b86 --- /dev/null +++ b/tests/unit/pilot-action-evidence.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from 'bun:test'; +import { Pilot } from '../../src/ai/pilot.ts'; +import { RequestStore } from '../../src/api/request-store.ts'; + +function buildPilot(): Pilot { + const deps: any = { + ai: {}, + explorer: {}, + stateManager: { otherTabs: [] }, + requestStore: new RequestStore('/tmp/pilot-actions-test'), + playwrightRecorder: {}, + }; + return new Pilot(deps, {}, {} as any); +} + +function clickWith(pageDiff: any) { + return [{ toolName: 'click', wasSuccessful: true, input: { explanation: 'Save the run' }, output: { pageDiff } }]; +} + +function format(pageDiff: any): string { + return (buildPilot() as any).formatActions(clickWith(pageDiff)); +} + +describe('Pilot recent_actions evidence', () => { + it('names the rejected request and the message the app showed', () => { + const line = format({ + urlChanged: false, + currentUrl: '/runs', + requests: [{ method: 'POST', path: '/api/runs', status: 400 }], + messages: ['Operation against a key holding the wrong kind of value'], + consoleErrors: ['Ember Data Request POST /api/runs returned a 400'], + }); + + expect(line).toContain('requests: POST /api/runs → 400'); + expect(line).toContain('messages: Operation against a key holding the wrong kind of value'); + expect(line).toContain('console: Ember Data Request POST /api/runs returned a 400'); + }); + + it('names the page the action moved to, which is not described by an element diff', () => { + const line = format({ urlChanged: true, previousUrl: '/runs', currentUrl: '/projects' }); + + expect(line).toContain('moved: /runs → /projects'); + }); + + it('drops requests that succeeded', () => { + const line = format({ + urlChanged: false, + currentUrl: '/runs', + requests: [ + { method: 'GET', path: '/api/runs', status: 200 }, + { method: 'POST', path: '/api/runs', status: 500 }, + ], + }); + + expect(line).toContain('requests: POST /api/runs → 500'); + expect(line).not.toContain('/api/runs → 200'); + }); + + it('keeps at most two messages and one console error', () => { + const line = format({ + urlChanged: false, + currentUrl: '/runs', + messages: ['first', 'second', 'third'], + consoleErrors: ['one', 'two'], + }); + + expect(line).toContain('messages: first | second'); + expect(line).not.toContain('third'); + expect(line).toContain('console: one'); + expect(line).not.toContain('two'); + }); + + it('adds no evidence lines when the action produced none', () => { + const line = format({ urlChanged: false, currentUrl: '/runs' }); + + expect(line).not.toContain('requests:'); + expect(line).not.toContain('messages:'); + expect(line).not.toContain('console:'); + }); +});