Add scoped and typed UI Automation queries - #848
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
Cross-window root resolution can report ambiguity despite a unique exact AutomationId match.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds scoped, typed UI Automation queries across CLI read commands, backed by authoritative Control View traversal.
Changes:
- Adds
--root,--type, and--class-namequery predicates. - Adds control-type mapping, ambiguity handling, and refreshed root polling.
- Adds real-provider tests, documentation, skill guidance, and generated schema updates.
File summaries
| File | Description |
|---|---|
UiAutomationService.Query.cs |
Implements constrained traversal and root resolution. |
UiAutomationService.cs |
Routes constrained queries and expands slug traversal. |
UiSelector.cs |
Models query predicates. |
UiControlTypes.cs |
Maps official UIA types and aliases. |
FakeUiServices.cs |
Records selectors for tests. |
UiControlTypesTests.cs |
Tests type mappings. |
UiAutomationServicePureTests.cs |
Updates invalid-type coverage. |
RealUiAutomationTests.Query.cs |
Adds real-provider query coverage. |
UiErrors.cs |
Supports redirected ambiguity errors. |
UiWaitForCommand.cs |
Adds scoped wait queries. |
UiSearchCommand.cs |
Adds scoped search queries. |
UiQueryOptions.cs |
Defines shared query options. |
UiGetValueCommand.cs |
Adds query predicates. |
UiGetPropertyCommand.cs |
Adds query predicates. |
UiCommandTests.Query.Integration.cs |
Tests real CLI query workflows. |
UiCommandTests.Query.cs |
Tests option composition and validation. |
SKILL.md |
Documents query workflows for the shipped skill. |
docs/ui-automation.md |
Documents semantics and supported types. |
docs/cli-schema.json |
Regenerates command schema. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Build Metrics ReportBinary Sizes
Test Results✅ 7139 passed, 37 skipped out of 7176 tests in 1432.0s (+288 tests, +484.4s vs. baseline) Test Coverage✅ 85.9% line coverage, 80% branch coverage · ✅ no change vs. baseline CLI Startup Time64ms median (x64, Try This BuildInstalls the MSIX for your architecture, replacing any previously installed build. Needs the GitHub CLI — the command offers to install it and sign you in if it is missing. & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) 848Switching between builds often?Put the tool on your PATH once: & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPathThen this build is just: winapp-pr 848Run Updated 2026-09-17 04:31:52 UTC · commit |
There was a problem hiding this comment.
🟡 Changes recommended
Traversal and wait races can misreport disappearance, while npm wrappers can silently omit invalid query values.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 3
- Review effort level: Balanced
013abad to
69c4264
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Bulk searches can include Raw View nodes, identity enforcement has a selectorless gap, and one replacement regression failed intermittently.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 25/25 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Predicate reads currently swallow provider faults and cannot match explicitly empty ClassName values, producing incorrect query and disappearance results.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
src/winapp-CLI/WinApp.UIAutomation/Services/UiAutomationService.Query.cs:170
- Empty literal ClassName values never match, and read faults become false matches. What is wrong:
SafeGetBstrnormalizes an empty BSTR tonulland swallows getter exceptions. Show me: an element whose provider ClassName is empty with--class-name ""→ no match (andwait-for --gonesucceeds); expected: an exact empty-string match because this option is intentionally accepted and preserved. Why it matters: all four new query surfaces return incorrect results for valid literal values, while transient provider faults can imply disappearance. Smallest fix: compare the raw BSTR string and let provider faults propagate.
&& (selector.ClassName is null || string.Equals(
SafeGetBstr(() => element.get_CurrentClassName()), selector.ClassName, StringComparison.OrdinalIgnoreCase));
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Root-selector control types bypass validation in the public .NET API, allowing invalid input to be reported as absence.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 29/29 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The COM-sensitive implementation spans multiple public surfaces, and the stated pre-human-review gate remains in progress.
Review details
- Files reviewed: 29/29 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Follow-up review context for unchanged head bc46bda: the last Copilot review (5207909539) reported zero new comments but retained "Needs a closer look," citing the COM-sensitive scope and an in-progress gate. Since that review, all 30 executing checks have succeeded (one additional CodeQL aggregate is neutral), including required build-and-package and e2e-test-ui. All 24 inline threads are answered and resolved; the PR is conflict-free. No code changes were made to obtain another review. Requesting one follow-up assessment with this completed CI context. Please identify any substantive remaining correctness or verification reservation; zero comments alone is not being treated as explicit sign-off. Native stack 849 remains #842 -> #848; neither PR is being merged. |
There was a problem hiding this comment.
🔵 Needs a closer look
The COM identity, traversal, and retry changes span multiple public surfaces and warrant final human review despite extensive regression coverage.
Review details
- Files reviewed: 33/33 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Follow-up assessment context for unchanged head 1c328c0: all CI is now complete, including required build-and-package and e2e-test-ui. All review threads have been answered/resolved. Independent fix-delta review and separate Opus review found no remaining critical/high issue; normal packaging build and both NativeAOT architectures passed owned, nonactivating general-property and wait checks. The previous review generated zero new comments but retained Needs a closer look due to COM scope. Requesting one follow-up assessment with completed validation context: please identify any substantive remaining reservation. No code changes were made to seek favorable wording, and human approval remains separate. |
Support composable root, control type and literal class predicates for UI read and wait commands, with complete scoped traversal and regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve global exact-ID precedence, skip unrelated runtime slugs, keep provider errors from satisfying gone waits, and cover the intended control-type package API. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep strict provider failures for constrained queries while allowing unrelated stale candidates during ordinary slug reads. Reject Root.Root before window resolution and cover stable-target churn and Inbox/Archive root boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the cancellation token last in the merged completion helper and make the ambiguity proxy exercise the single substring bulk query after an exact FindFirst miss. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep scoped read races visible after retained-provider validation, exercise live and serialized identity paths independently, and validate full-query retries with nonactivating UI fixtures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adapt lower traversal seams and pass cancellation through slug reads. Keep reconstructed slug reads strict without exposing internal metadata, and propagate acquired pattern getter failures for strict reads while preserving ordinary fallbacks. Add source-generated JSON roundtrip and retained ValuePattern regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep ordinary property reads best-effort, but preserve scoped provider faults for get-property and wait-for retry/error handling. Cover BOOL and BSTR failures, empty strings, and source-generated JSON restoration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1c328c0 to
4a6843e
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The implementation matches #819, covers all affected shipping surfaces, and includes comprehensive regression and real-app validation.
Review details
- Files reviewed: 36/36 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
Adds composable
--root,--type, and--class-namepredicates toui search,get-property,get-value, andwait-for.TextBox→EditandTextBlock→Text. Unknown outer/root types are rejected. ClassName matches literal strings, including empty values, exactly and case-insensitively.Stack
Depends on #842 and reuses its complete traversal helpers. Native stack 849 has order #842 → #848, ultimate base
main. No batching, persistent handles, explicit-action changes, or formatting changes.Validation and performance
Normal build, generated schema/docs, npm, and published ARM64/x64 real-app integrations passed. Regression coverage includes partial provider results, global root uniqueness/precedence, deep boundaries, all types/aliases, exact/empty class names, root replacement, explicit HWND/popup isolation, and identity/property/traversal failures. Actionable review regressions were demonstrated failing before fixes.
Final real-app measurement (20 iterations after 3 warmups): median baseline 217.865ms, type 154.439ms, class 148.711ms, root+type+class 150.149ms. Fixture measurements are not a universal performance guarantee.
Pre-human-review gate complete
At head
bc46bda29e48c9893b00ea178cc0f98d009ce1b6:build-and-packageand real-appe2e-test-uiboth passed.Fixes #819