Skip to content

fix(maestro): typed failure reasons for target-resolution misses - #3341

Merged
thymikee merged 1 commit into
mainfrom
fix/maestro-test-system-sheet-2560
Oct 9, 2026
Merged

thymikee merged 1 commit into
mainfrom
fix/maestro-test-system-sheet-2560

Conversation

@thymikee

@thymikee thymikee commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Summary

Fixes the live defect behind #2560 for Maestro index selectors. An out-of-range index over otherwise-visible matches produced resolution evidence with matched: true, visible: true and no target, so every miss — selector miss, invisible matches, out-of-range index, degenerate geometry — was reported as the generic "Maestro target did not resolve to a visible element.", and assertVisible accepted the vacuous matched/visible pair (asserting an element its selector never selected).

Adds a typed MaestroTargetFailureReason computed at the resolution site, threaded through the daemon match projection to the reporter. Messages now name the selector and the actual miss; the reason rides in error details.targetFailureReason; observation conditions hold only for an actionable resolution.

The other two #2560 symptoms (plain test/replay/manual divergence, silent repeat) do not reproduce on current main and were already addressed by the optional-step warnings (#2571) and the earlier dispatch fixes.

Validation

  • pnpm check:affected --run: format, lint, typecheck, layering, fallow, build, 3183 tests pass.
  • New regression tests: resolution-level typed reasons, daemon match projection, observation-complement semantics, reporter message + typed details.
  • Device repro on iPad Pro 13-inch (M5) with an ASWebAuthenticationSession fixture hosting com.apple.SafariViewService: out-of-range index now fails with "matched 1 element(s); its index is out of range" (hard fail and precise optional-skip warning); index: 0 and plain dismissal still pass; assertVisible with out-of-range index now correctly fails instead of passing vacuously.

View guided diff Turn on auto-fix

…eason

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.
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.13 MB 5.13 MB +1.2 kB
Package (unpacked) 5.13 MB 5.13 MB +1.2 kB
Package (download) 1.54 MB 1.54 MB +276 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 23.2 ms 23.4 ms +0.2 ms
CLI --help 68.7 ms 65.2 ms -3.5 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 10 files

View guided diff | Turn on auto-fix | Re-trigger cubic

@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Re-ran the failed 'Smoke Tests' job (run 37843216989 / job 113537548137). Its only failure was the step 'Preflight iOS runner through public CLI' with typed details.kind: daemon_startup_failed, which fails before any scenario runs — this is the tracked lane-flake class of #3342 (five instances tonight across unrelated diffs; same step, same typed kind). No diff here can reach it: all 10 changed files are under packages/maestro/, which the daemon loads only via lazy await import('@agent-device/maestro') edges (src/daemon/replay-device-selection.ts, src/commands/replay/script-source-bundle.ts), so this change is not on the daemon's boot import path. pnpm check:affected --run passes at head 2964115. If the re-run fails a second time at this same head I'll pull the log rather than touch code.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 8, 2026
@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

The PR is ready for human review at 2964115. The code looks right to me. All 19 checks pass. The earlier Smoke Tests failure was the "Preflight iOS runner through public CLI" step with a typed daemon_startup_failed. This diff touches only packages/maestro, which the daemon loads lazily, and the re-run passed, so the two do not overlap. I did not run the tests or the iPad ASWebAuthenticationSession repro, so the live-validation claim rests on the PR body. I also did not check third-party MaestroRuntimeOperations implementers outside this repo. Their matches without failureReason fall back to the default message and the old matched/visible rules, which is fine for in-repo code. Not blocking, and you can take or leave these: the new message strings on the ok:false resolutions in https://github.com/callstack/agent-device/blob/2964115/packages/maestro/src/internal/runtime-targets.ts#L190 are read only by tests, because the reporter in runtime-port-observation.ts builds its own wording in maestroTargetFailureMessage, so dropping message and asserting failureReason would keep one table of wording. Also, an element that matches and is visible but has no usable geometry (no-usable-geometry) used to pass assertVisible and fail notVisible, and now does the opposite (https://github.com/callstack/agent-device/blob/2964115/packages/maestro/src/internal/runtime-port-observation.ts#L52), so one line in the PR body or Maestro docs saying that degenerate geometry and an out-of-range index now count as not visible would help. No conflicts.

@thymikee
thymikee merged commit c7b534f into main Oct 9, 2026
19 of 20 checks passed
@thymikee
thymikee deleted the fix/maestro-test-system-sheet-2560 branch October 9, 2026 06:59
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-09 06:59 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant