Skip to content

feat(maestro): support optional on scrollUntilVisible and extendedWaitUntil (#1291)#1339

Merged
thymikee merged 3 commits into
mainfrom
devin/1784538620-maestro-1291-optional
Jul 20, 2026
Merged

feat(maestro): support optional on scrollUntilVisible and extendedWaitUntil (#1291)#1339
thymikee merged 3 commits into
mainfrom
devin/1784538620-maestro-1291-optional

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Closes #1291.

Upstream Maestro accepts optional on scrollUntilVisible and extendedWaitUntil at both the command level and the element selector level. This PR extends the existing optional-command execution boundary to those commands.

Changes:

  • MaestroSelectorMap now carries optional?: boolean and the shared selector parser reads it.
  • parseScrollUntilVisible and parseExtendedWaitUntil accept optional inside element / visible / notVisible selectors, strip it from the selector used for matching, and propagate it to the command-level optional flag.
  • The existing executeOptionalCommand / isOptionalCommand path in replay-plan-step-execution.ts already downgrades maestroTestFailure errors to warnings for optional commands, so a timed-out optional lookup now continues the flow with skipped incremented.
  • FLOW_DIVERGENCES['upstream/076_optional_assertion'] is narrowed: optional on scrollUntilVisible/extendedWaitUntil is no longer listed as unsupported; only assertTrue remains.
  • Docs and support matrix are updated to advertise the new capability.

Example now accepted:

- scrollUntilVisible:
    element:
      id: listItem
      optional: true
- extendedWaitUntil:
    visible:
      text: Ready
      optional: true
    timeout: 5000

Validation:

  • pnpm format:check
  • pnpm lint
  • pnpm typecheck
  • pnpm test:unit
  • pnpm maestro:conformance
  • pnpm check:layering

All green.

Link to Devin session: https://app.devin.ai/sessions/8bec5e78313446e180020b22e479c7ac
Requested by: @thymikee

@thymikee thymikee self-assigned this Jul 20, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.8 MB 1.8 MB +530 B
JS gzip 572.3 kB 572.4 kB +161 B
npm tarball 686.6 kB 686.8 kB +160 B
npm unpacked 2.4 MB 2.4 MB +530 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.5 ms 28.3 ms -0.3 ms
CLI --help 58.1 ms 59.1 ms +1.0 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/session.js +490 B +154 B
dist/src/registry.js +40 B +7 B

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-20 12:01 UTC

@thymikee

Copy link
Copy Markdown
Member

Two parser/semantic blockers remain:

  1. Selector-level optional can satisfy the parser’s non-empty-selector check by itself. parseMaestroSelectorMapEntries counts { optional: true } as a selector, then the command parser strips optional, so scrollUntilVisible.element, extendedWaitUntil.visible, or .notVisible can reach execution as {} and be silently treated as an optional miss instead of failing fast. Preserve the invariant that at least one real matching criterion remains after extracting optional, with rejection tests for all three forms.

  2. extendedWaitUntil ORs optionality from both visible and notVisible, but execution evaluates visible first when both exist. Therefore an unused notVisible: { ..., optional: true } can make a failing required visible condition optional. Reject simultaneous conditions or derive optionality only from the condition that is actually executed, and add a regression.

After those fixes, provide live device evidence that missing optional scrollUntilVisible and extendedWaitUntil steps each warn/continue to a succeeding downstream command. The existing positive parser/engine tests are otherwise revert-sensitive, and current CI is green.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Addressed both blockers and pushed:

  1. Bare optional selectorsparseMaestroSelectorMapEntries now requires at least one non-optional matching key (id/text/label/enabled/selected) and rejects { optional: true } alone. Added parser rejection tests for scrollUntilVisible.element, extendedWaitUntil.visible, and extendedWaitUntil.notVisible.

  2. Simultaneous visible + notVisibleextendedWaitUntil now rejects having both conditions and derives optional only from the single condition that will execute, fixing the notVisible.optional leak when visible is the active branch. Added a regression test.

Also added a layer-3 differential scenario optional-warned-scroll-and-wait that targets the fixture app, uses element-level optional on scrollUntilVisible and command-level optional on extendedWaitUntil, and ends with a succeeding assertVisible so outcome parity proves the warning/continue behavior.

Local validation passes:

  • pnpm format:check, pnpm lint, pnpm typecheck
  • pnpm test:unit
  • pnpm maestro:conformance
  • pnpm check:layering

I cannot trigger the Conformance Differential workflow from this environment (gh workflow run returns 403). Could you run the device evidence for the new scenario?

gh workflow run "Conformance Differential" --ref devin/1784538620-maestro-1291-optional --raw-field only=optional-warned-scroll-and-wait

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Pushed the fallow refactor; all 21 CI checks are now green (including Fallow Code Quality, Maestro Conformance Oracle, Layering Guard).

The new optional-warned-scroll-and-wait differential scenario is registered and validated structurally. I still cannot trigger the live Conformance Differential workflow from this environment (gh workflow run returns 403). When you have a moment, could you run the device evidence for it?

gh workflow run "Conformance Differential" --ref devin/1784538620-maestro-1291-optional --raw-field only=optional-warned-scroll-and-wait

That will compare real Maestro vs agent-device on the fixture app for both scrollUntilVisible and extendedWaitUntil optional warning/continue behavior.

thymikee and others added 3 commits July 20, 2026 10:46
…tUntil

Add optional support at command level and element level for
scrollUntilVisible and extendedWaitUntil. The parser now accepts
optional in both positions and propagates it to the command so the
existing optional-command execution boundary downgrades a timed-out
lookup to a warning and continues the flow.

Update the upstream/076_optional_assertion divergence entry so only
assertTrue remains unsupported, and keep the docs/support matrix in sync.

Closes #1291

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… optionality, and add device differential scenario

- parseMaestroSelectorMapEntries now rejects selectors that contain only
  optional and no real matching criteria, with rejection tests for
  scrollUntilVisible.element, extendedWaitUntil.visible, and .notVisible.
- extendedWaitUntil now rejects simultaneous visible and notVisible conditions
  and derives optionality only from the single condition that will execute.
- Add layer-3 differential flow/scenario optional-warned-scroll-and-wait that
  exercises both command-level and element-level optional on a missing target
  and verifies the flow continues to the final assertion.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…plexity gate

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/1784538620-maestro-1291-optional branch from f39973b to 009b8f4 Compare July 20, 2026 10:47
@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 20, 2026
@thymikee

Copy link
Copy Markdown
Member

Re-review of 009b8f47: code is clean. Both prior blockers are fixed: selector maps containing only optional now fail after excluding metadata from the matching-key check, and extendedWaitUntil rejects simultaneous visible/notVisible, preventing optionality from leaking from an unused branch. The new rejection tests and differential scenario are revert-sensitive, the production route still uses the shared optional-command boundary, and all checks are green. Remaining readiness evidence: please run optional-warned-scroll-and-wait through the live Conformance Differential on a device and attach the pass/pass result.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Replying to @thymikee re-review (comment 5021515681):

Thanks — glad the blockers are resolved.

This environment has no booted device or Maestro CLI (which maestro/adb/emulator are all missing), and gh workflow run "Conformance Differential" still returns HTTP 403 (Resource not accessible by integration), so I cannot dispatch or run the live differential from here.

Could you trigger the workflow for optional-warned-scroll-and-wait and attach the pass/pass result? The command should be:

gh workflow run "Conformance Differential" --ref devin/1784538620-maestro-1291-optional --raw-field only=optional-warned-scroll-and-wait

The code is otherwise ready to merge once the device evidence is green.

@thymikee
thymikee merged commit ab80434 into main Jul 20, 2026
24 checks passed
@thymikee
thymikee deleted the devin/1784538620-maestro-1291-optional branch July 20, 2026 12:01
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.

Maestro compat: support optional on scrollUntilVisible and extendedWaitUntil (command- and element-level)

1 participant