From 29641150cea9f8e41119ba1dead9368442c19993 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 8 Oct 2026 22:56:24 +0200 Subject: [PATCH] fix(maestro): give target-resolution misses their own typed failure reason An out-of-range selector index over otherwise-visible matches produced a resolution with matched/visible evidence but no target, and every miss was reported with the same generic 'did not resolve to a visible element' text. assertVisible then accepted the vacuous matched/visible pair, so a flow could assert an element its selector never selected. Carry a MaestroTargetFailureReason from the resolution site through the daemon match projection to the reporter: messages now name the selector and the actual miss (no match, none visible, index out of range, no usable geometry), the typed reason rides along in error details, and observation conditions hold only for an actionable resolution. --- .../daemon-runtime-port-observation.test.ts | 40 +++++++++++++ .../daemon-runtime-port-observation.ts | 1 + packages/maestro/src/index.ts | 1 + .../runtime-port-observation.test.ts | 51 +++++++++++++++++ .../internal/__tests__/runtime-port.test.ts | 29 ++++++++++ .../__tests__/runtime-targets-typed.test.ts | 56 +++++++++++++++++++ .../src/internal/facade-runtime-port.ts | 2 + .../src/internal/runtime-port-observation.ts | 35 ++++++++++-- .../src/internal/runtime-port-types.ts | 8 +++ .../maestro/src/internal/runtime-targets.ts | 52 +++++++++++++++-- 10 files changed, 266 insertions(+), 9 deletions(-) create mode 100644 packages/maestro/src/internal/__tests__/runtime-port-observation.test.ts diff --git a/packages/maestro/src/daemon-port/__tests__/daemon-runtime-port-observation.test.ts b/packages/maestro/src/daemon-port/__tests__/daemon-runtime-port-observation.test.ts index a7177dd643..0874714e21 100644 --- a/packages/maestro/src/daemon-port/__tests__/daemon-runtime-port-observation.test.ts +++ b/packages/maestro/src/daemon-port/__tests__/daemon-runtime-port-observation.test.ts @@ -150,6 +150,46 @@ test('computes expensive target evidence only for the command policies that cons expect(relational).not.toHaveProperty('dispatchSelector'); }); +test('projects an out-of-range index miss onto target evidence with its typed failure reason', () => { + const snapshot = makeSnapshot([ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 402, height: 874 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + kind: formatRole('Button'), + identifier: 'not-now', + label: 'Not Now', + rect: { x: 20, y: 700, width: 120, height: 44 }, + }, + { + index: 2, + parentIndex: 0, + type: 'Button', + kind: formatRole('Button'), + identifier: 'not-now', + label: 'Not Now', + rect: { x: 20, y: 760, width: 120, height: 44 }, + }, + ]); + + const miss = resolveTypedMaestroTarget({ + context: { generation: 3, env: {} }, + snapshot, + platform: 'ios', + query: { selector: { id: 'not-now', index: 2 }, purpose: 'tap', timeoutMs: 0 }, + }); + + expect(miss).toMatchObject({ + generation: 3, + matched: true, + visible: true, + candidateCount: 2, + failureReason: 'index-out-of-range', + }); + expect(miss).not.toHaveProperty('rect'); +}); + test('matches iOS Maestro ids on semantic nodes suppressed from interactive presentation', async () => { const snapshot = makeSnapshot([ { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 402, height: 874 } }, diff --git a/packages/maestro/src/daemon-port/daemon-runtime-port-observation.ts b/packages/maestro/src/daemon-port/daemon-runtime-port-observation.ts index f6ce5ffd6d..19eaf8f1a8 100644 --- a/packages/maestro/src/daemon-port/daemon-runtime-port-observation.ts +++ b/packages/maestro/src/daemon-port/daemon-runtime-port-observation.ts @@ -272,6 +272,7 @@ function targetMatchFromResolution( matched: resolution.evidence.matched, visible: resolution.evidence.visible, candidateCount: resolution.evidence.candidateCount, + failureReason: resolution.failureReason, ...(resolution.evidence.ref ? { ref: resolution.evidence.ref } : {}), ...(viewport ? { viewport } : {}), }; diff --git a/packages/maestro/src/index.ts b/packages/maestro/src/index.ts index d0acefb8bc..e49ac229a8 100644 --- a/packages/maestro/src/index.ts +++ b/packages/maestro/src/index.ts @@ -61,6 +61,7 @@ export { type MaestroSelector, type MaestroSinglePointerGestureInput, type MaestroSnapshotTargetQuery, + type MaestroTargetFailureReason, type MaestroTargetMatch, type MaestroTargetQuery, type MaestroTargetResolution, diff --git a/packages/maestro/src/internal/__tests__/runtime-port-observation.test.ts b/packages/maestro/src/internal/__tests__/runtime-port-observation.test.ts new file mode 100644 index 0000000000..4f5083c641 --- /dev/null +++ b/packages/maestro/src/internal/__tests__/runtime-port-observation.test.ts @@ -0,0 +1,51 @@ +import { expect, test } from 'vitest'; +import { maestroObservationMatches } from '../runtime-port-observation.ts'; + +const visibleCondition = { kind: 'visible' as const, selector: { text: 'Not Now' } }; +const notVisibleCondition = { + kind: 'notVisible' as const, + selector: { text: 'Not Now' }, + timeoutMs: 0, +}; + +test('a visible condition requires an actionable resolution, not just matched evidence', () => { + const resolved = { generation: 0, matched: true, visible: true, candidateCount: 1 }; + + expect(maestroObservationMatches(visibleCondition, resolved)).toBe(true); + // An out-of-range `index` over otherwise visible matches leaves matched and + // visible evidence with no selected element; the condition must not hold. + expect( + maestroObservationMatches(visibleCondition, { + ...resolved, + failureReason: 'index-out-of-range', + }), + ).toBe(false); + expect( + maestroObservationMatches(visibleCondition, { + ...resolved, + matched: false, + visible: false, + failureReason: 'selector-did-not-match', + }), + ).toBe(false); +}); + +test('a notVisible condition is the strict complement of an actionable resolution', () => { + const resolved = { generation: 0, matched: true, visible: true, candidateCount: 1 }; + + expect(maestroObservationMatches(notVisibleCondition, resolved)).toBe(false); + expect( + maestroObservationMatches(notVisibleCondition, { + ...resolved, + failureReason: 'index-out-of-range', + }), + ).toBe(true); + expect( + maestroObservationMatches(notVisibleCondition, { + ...resolved, + matched: false, + visible: false, + failureReason: 'selector-did-not-match', + }), + ).toBe(true); +}); diff --git a/packages/maestro/src/internal/__tests__/runtime-port.test.ts b/packages/maestro/src/internal/__tests__/runtime-port.test.ts index 9da89973c2..582978de93 100644 --- a/packages/maestro/src/internal/__tests__/runtime-port.test.ts +++ b/packages/maestro/src/internal/__tests__/runtime-port.test.ts @@ -351,6 +351,35 @@ describe('MaestroRuntimePort', () => { expect(operations.resolveGestureViewport).not.toHaveBeenCalled(); }); + test('reports an out-of-range index with its own message and typed reason', async () => { + const operations = makeOperations({ + resolveTarget: async (_input, context) => ({ + generation: context.generation, + matched: true, + visible: true, + candidateCount: 2, + failureReason: 'index-out-of-range', + ref: undefined, + }), + tapOn: async () => undefined, + }); + const command = parseMaestroProgram('---\n- tapOn:\n text: Not Now\n index: 2\n') + .commands[0]!; + + await expect( + createMaestroRuntimePort(operations).execute({ + command: command as Extract, + generation: 0, + env: {}, + invalidateObservation: vi.fn(), + }), + ).rejects.toMatchObject({ + message: + 'Maestro target "text":"Not Now", "index":2 matched 2 element(s); its index is out of range.', + details: { reason: 'maestro-test-failure', targetFailureReason: 'index-out-of-range' }, + }); + }); + test('reports optional tap misses to the interpreter without hiding infrastructure failures', async () => { const tapOn = vi.fn(async () => undefined); const missingOperations = makeOperations({ diff --git a/packages/maestro/src/internal/__tests__/runtime-targets-typed.test.ts b/packages/maestro/src/internal/__tests__/runtime-targets-typed.test.ts index 9ed8f55c37..457bc2afb3 100644 --- a/packages/maestro/src/internal/__tests__/runtime-targets-typed.test.ts +++ b/packages/maestro/src/internal/__tests__/runtime-targets-typed.test.ts @@ -124,6 +124,62 @@ test('typed childOf reports a scoped miss when only an outside child matches', ( }); }); +test('reports an out-of-range index as its own typed failure instead of a visible miss', () => { + const snapshot = makeSnapshot([ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 320, height: 640 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Not Now', + rect: { x: 16, y: 500, width: 120, height: 44 }, + }, + { + index: 2, + parentIndex: 0, + type: 'Button', + label: 'Not Now', + rect: { x: 16, y: 560, width: 120, height: 44 }, + }, + ]); + + expect( + resolveMaestroTargetFromSnapshot( + snapshot, + { selector: { text: 'Not Now', index: 2 } }, + 'android', + ), + ).toMatchObject({ + ok: false, + failureReason: 'index-out-of-range', + message: 'Maestro selector matched 2 visible element(s); index 2 is out of range.', + evidence: { matched: true, visible: true, candidateCount: 2 }, + }); +}); + +test('reports matched-but-invisible targets with a typed no-visible-match reason', () => { + const snapshot = makeSnapshot([ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 320, height: 640 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Hidden', + rect: { x: 16, y: 500, width: 120, height: 44 }, + visibleToUser: false, + }, + ]); + + expect( + resolveMaestroTargetFromSnapshot(snapshot, { selector: { text: 'Hidden' } }, 'android'), + ).toMatchObject({ + ok: false, + failureReason: 'no-visible-match', + message: 'Maestro selector matched 1 element(s), but none were visible.', + evidence: { matched: true, visible: false, candidateCount: 1 }, + }); +}); + test('keeps Maestro-visible app content matchable while a React Native overlay is present', () => { const snapshot = makeSnapshot([ { diff --git a/packages/maestro/src/internal/facade-runtime-port.ts b/packages/maestro/src/internal/facade-runtime-port.ts index 725266c215..1aa7381889 100644 --- a/packages/maestro/src/internal/facade-runtime-port.ts +++ b/packages/maestro/src/internal/facade-runtime-port.ts @@ -33,6 +33,7 @@ import { literalFromMaestroRegex } from './selector-regex.ts'; import { resolveMaestroTargetFromSnapshot, hasMaestroRecursiveRelations, + type MaestroTargetFailureReason, type MaestroTargetQuery as MaestroSnapshotTargetQuery, type MaestroTargetResolution, } from './runtime-targets.ts'; @@ -91,6 +92,7 @@ export type { MaestroSelector, MaestroSinglePointerGestureInput, MaestroSnapshotTargetQuery, + MaestroTargetFailureReason, MaestroTargetMatch, MaestroTargetQuery, MaestroTargetResolution, diff --git a/packages/maestro/src/internal/runtime-port-observation.ts b/packages/maestro/src/internal/runtime-port-observation.ts index 7655ce4ca6..fc673be486 100644 --- a/packages/maestro/src/internal/runtime-port-observation.ts +++ b/packages/maestro/src/internal/runtime-port-observation.ts @@ -44,11 +44,13 @@ export async function observeMaestroCondition( export function maestroObservationMatches( condition: MaestroObservationCondition, - match: Pick, + match: Pick, ): boolean { - return condition.kind === 'visible' - ? match.matched && match.visible - : !match.matched || !match.visible; + // A typed failure reason means no element was selected (for example an + // out-of-range `index` over matches that are otherwise visible), so the + // observed selector holds only when a target actually resolved. + const holds = match.matched && match.visible && match.failureReason === undefined; + return condition.kind === 'visible' ? holds : !holds; } export async function resolveMaestroTarget( @@ -63,9 +65,10 @@ export async function resolveMaestroTarget( ); const validated = validateTargetMatch(match, request.generation); if (!validated.matched || !validated.visible || !validated.rect) { - throw maestroTestFailure('Maestro target did not resolve to a visible element.', { + throw maestroTestFailure(maestroTargetFailureMessage(selector, validated), { selector, candidateCount: validated.candidateCount, + ...(validated.failureReason ? { targetFailureReason: validated.failureReason } : {}), }); } return { @@ -93,6 +96,28 @@ export function observationForTarget(target: MaestroTargetResolution): MaestroOb }; } +function maestroTargetFailureMessage(selector: MaestroSelector, match: MaestroTargetMatch): string { + const selectorText = formatMaestroSelector(selector); + switch (match.failureReason) { + case 'selector-did-not-match': + return `Maestro target ${selectorText} did not match any element.`; + case 'no-visible-match': + return `Maestro target ${selectorText} matched ${match.candidateCount} element(s), but none were visible.`; + case 'index-out-of-range': + return `Maestro target ${selectorText} matched ${match.candidateCount} element(s); its index is out of range.`; + case 'no-usable-geometry': + return `Maestro target ${selectorText} did not provide usable geometry.`; + default: + return 'Maestro target did not resolve to a visible element.'; + } +} + +function formatMaestroSelector(selector: MaestroSelector): string { + const entries = Object.entries(selector).filter(([, value]) => value !== undefined); + if (entries.length === 0) return 'selector'; + return entries.map(([key, value]) => `"${key}":${JSON.stringify(value)}`).join(', '); +} + function isRect(value: unknown): value is { x: number; y: number; width: number; height: number } { if (!value || typeof value !== 'object') return false; const rect = value as Record; diff --git a/packages/maestro/src/internal/runtime-port-types.ts b/packages/maestro/src/internal/runtime-port-types.ts index 4c82492049..1cbc56dea0 100644 --- a/packages/maestro/src/internal/runtime-port-types.ts +++ b/packages/maestro/src/internal/runtime-port-types.ts @@ -13,6 +13,7 @@ import type { MaestroObservationCondition, MaestroObservationEvidence, } from './engine-types.ts'; +import type { MaestroTargetFailureReason } from './runtime-targets.ts'; export type MaestroRuntimeReadContext = { readonly appId?: string; @@ -40,6 +41,13 @@ export type MaestroTargetMatch = { readonly ref?: string; readonly dispatchSelector?: MaestroDispatchSelector; readonly surfaceSignature?: string; + /** + * The typed reason for a failed resolution. An out-of-range `index` or a degenerate + * candidate geometry leaves a resolution matched and visible with no target, and + * `MaestroTargetMatch` is observation evidence too, where `matched`/`visible` ARE the + * outcome — so the absence of a dispatch target travels as its own reason. + */ + readonly failureReason?: MaestroTargetFailureReason; }; export type MaestroDispatchSelector = { diff --git a/packages/maestro/src/internal/runtime-targets.ts b/packages/maestro/src/internal/runtime-targets.ts index fdc69ae8d0..030781e097 100644 --- a/packages/maestro/src/internal/runtime-targets.ts +++ b/packages/maestro/src/internal/runtime-targets.ts @@ -30,6 +30,13 @@ export type MaestroTargetEvidence = { ref?: string; }; +/** Why a target resolution produced no actionable element, kept distinct from the matched/visible observation outcome. */ +export type MaestroTargetFailureReason = + | 'selector-did-not-match' + | 'no-visible-match' + | 'index-out-of-range' + | 'no-usable-geometry'; + type MaestroInteractivePresentation = { snapshot: SnapshotState; presentedIndexesBySourceIndex: ReadonlyMap; @@ -44,7 +51,12 @@ export type MaestroTargetResolution = dispatchCandidates: number; evidence: MaestroTargetEvidence; } - | { ok: false; message: string; evidence: MaestroTargetEvidence }; + | { + ok: false; + message: string; + failureReason: MaestroTargetFailureReason; + evidence: MaestroTargetEvidence; + }; export function resolveMaestroTargetFromSnapshot( snapshot: SnapshotState, @@ -66,6 +78,7 @@ export function resolveMaestroTargetFromSnapshot( return { ok: false, message: 'Maestro childOf parent did not match.', + failureReason: 'selector-did-not-match', evidence: buildMaestroTargetEvidence(query, candidates.matches, [], undefined), }; } @@ -84,7 +97,14 @@ export function resolveMaestroTargetFromSnapshot( const rect = options.interactiveBounds ? (presentedTarget?.rect ?? semanticRect) : (semanticRect ?? presentedTarget?.rect); - if (!rect) return failedTargetResolution(query, matches, rankedMatches, evidence); + if (!rect) { + return { + ok: false, + message: 'Maestro target did not provide usable geometry.', + failureReason: 'no-usable-geometry', + evidence, + }; + } return { ok: true, @@ -168,11 +188,35 @@ function failedTargetResolution( return { ok: false, message: `Maestro selector matched ${matches.length} element(s), but none were visible.`, + failureReason: 'no-visible-match', evidence, }; } - const index = query.selector.index === undefined ? '' : ` index ${query.selector.index}`; - return { ok: false, message: `Maestro selector did not match${index}.`, evidence }; + const index = query.selector.index; + if (rankedMatches.length > 0) { + // Visible matches exist, so the miss comes from applying `index` or from + // degenerate candidate geometry rather than from the selector itself. + if (index !== undefined && selectMaestroSnapshotNode(rankedMatches, index) === undefined) { + return { + ok: false, + message: `Maestro selector matched ${rankedMatches.length} visible element(s); index ${index} is out of range.`, + failureReason: 'index-out-of-range', + evidence, + }; + } + return { + ok: false, + message: 'Maestro target did not provide usable geometry.', + failureReason: 'no-usable-geometry', + evidence, + }; + } + return { + ok: false, + message: 'Maestro selector did not match.', + failureReason: 'selector-did-not-match', + evidence, + }; } function countInteractionDispatchCandidates(