Skip to content

Maestro compat: support childOf on assertVisible/assertNotVisible (#1294)#1334

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1784400801-maestro-1294-assert-childOf
Open

Maestro compat: support childOf on assertVisible/assertNotVisible (#1294)#1334
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1784400801-maestro-1294-assert-childOf

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Extends assertVisible/assertNotVisible to accept the same childOf ancestor-scoping option already supported on tapOn, so Maestro flows like upstream/114_child_of_selector.yaml parse and run identically to upstream.

What changed

  • IR/parser: MaestroAssertVisibleCommand / MaestroAssertNotVisibleCommand now carry an optional childOf?: MaestroSelector. parseAssertion accepts the childOf key and parses it the same way as tapOn.
  • Runtime: MaestroObservationCondition includes childOf, and assertVisible/assertNotVisible pass it through requireObservationport.observeobserveTypedMaestroCondition → the existing resolveTargetFromSnapshot ancestor-scoping path (scopeMatchesByAncestor).
  • Evidence: MaestroObservationEvidence also carries childOf, so observation results retain the selector context.
  • Conformance: normalize.ts projects the new command-level childOf into the canonical selector (nested under selector.childOf, matching upstream's shape), and the stale upstream/114_child_of_selector divergence declaration is removed.

Example

- assertVisible:
    text: "child_id"
    childOf:
      text: "parent_id_1"
- assertNotVisible:
    text: "child_id"
    childOf:
      text: "parent_id_3"

Validation

  • pnpm format:check, pnpm lint, pnpm typecheck pass.
  • pnpm test:unit passes (480 files / 4254 tests).
  • pnpm maestro:conformance passes.
  • pnpm check:layering passes.

Closes #1294.

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

)

- Accept childOf at command level in the Maestro IR and parser.
- Thread childOf through the observation condition to the snapshot
  target resolver, reusing the existing ancestor-scoping path.
- Project childOf into the conformance canonical selector so
  upstream/114_child_of_selector matches.
- Remove the stale divergence declaration for 114 and update docs.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@thymikee thymikee self-assigned this Jul 18, 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 18, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-1334/

Built to branch gh-pages at 2026-07-18 19:54 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.8 MB 1.8 MB +288 B
JS gzip 569.7 kB 569.8 kB +77 B
npm tarball 684.3 kB 684.4 kB +98 B
npm unpacked 2.4 MB 2.4 MB +288 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.8 ms 27.6 ms +0.8 ms
CLI --help 56.6 ms 58.1 ms +1.5 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/session.js +270 B +72 B
dist/src/registry.js +18 B +5 B

@thymikee

Copy link
Copy Markdown
Member

The implementation route looks correct, but the motivating runtime behavior is not regression-covered. The conformance fixture proves parsing/normalization and existing tests prove the generic resolver; reverting either childOf forwarding hop in replay-plan-step-execution.ts or daemon-runtime-port-observation.ts would still leave CI green. Please add a production-path/interpreter observation test for both assertVisible and assertNotVisible, using duplicate matching children inside and outside different parents, and assert the scoped result/evidence. That test should fail if either forwarding hop is removed.

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

Copy link
Copy Markdown
Contributor Author

@thymikee Added a production-path/interpreter observation regression test in src/compat/maestro/__tests__/daemon-runtime-port-observation.test.ts.

It runs a Maestro program with both assertVisible and assertNotVisible against a snapshot that has duplicate child_id children inside and outside different parents, and asserts the scoped evidence (childOf, candidateCount, ref, visible) plus matched.

port.observe is instrumented while executeMaestroProgram drives the replay plan, so removing either childOf forwarding hop breaks the test:

  • assertVisible + childOf: parent_id_1 must resolve to the child under that parent (candidateCount: 1, ref: 'e3').
  • assertNotVisible + childOf: parent_id_3 must see no matching child under that parent (candidateCount: 0, matched: true).

Validation:

  • pnpm format:check, pnpm lint, pnpm typecheck pass.
  • pnpm test:unit passes (480 files / 4255 tests).
  • pnpm maestro:conformance passes.
  • pnpm check:layering passes.

@thymikee

Copy link
Copy Markdown
Member

Re-reviewed a4dd027f: the added interpreter → daemon runtime-port regression fully addresses the blocker. It covers both assertVisible and assertNotVisible with duplicate matches across parent scopes, and reverting either childOf forwarding hop makes the test fail. The implementation remains aligned with issue #1294 and ADR 0015. Code review is clean; ready for human review.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 18, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Thanks for the re-review. The PR is green and ready for the next human reviewer.

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 childOf on assertVisible/assertNotVisible (parity with tapOn)

1 participant