Skip to content

Fix get-focused ownership for Sandbox controls without a process ID - #880

Merged
Zach Teutsch (zateutsch) merged 13 commits into
mainfrom
nmetulev-sandbox-focused-control
Sep 22, 2026
Merged

Zach Teutsch (zateutsch) merged 13 commits into
mainfrom
nmetulev-sandbox-focused-control

Conversation

@nmetulev

@nmetulev Nikola Metulev (nmetulev) commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Description

Fix ui get-focused reporting no focus when a Sandbox control omits its process ID. The exact CsWin32 CUIAutomation8 query returned Daylight's focused title control with PID 0, while native ancestor windows independently identified the app.

The command now verifies missing ownership through a bounded, cancellable ancestor search. It rejects foreign process identities and requires the exact top-level window for -w, excluding owned popups. Actual query failures return an error rather than a successful no-focus result. The positive-own-PID process-target path is preserved. Command help, generated CLI schema and npm help explain the window scope.

Usage Example

winapp ui get-focused -a <PID> --on sandbox --json
winapp ui get-focused -w <HWND> --on sandbox --json

Observed with the focused TaskTitleInput: installed prerelease.52 returned {"hasFocus":false} in six trials; the candidate returned hasFocus:true with that control in all six, by both PID and main HWND. Explicit owned-popup and foreign-window targets returned false as expected. Use fresh IDs from ui list-windows.

Related Issue

Fixes #876. Focus activation (#877), empty-value transport (#875), input guards and sandbox lifecycle are outside this change. Main's independently merged empty-value fix is preserved.

Type of Change

  • Bug fix
  • Documentation
  • Test update

Checklist

  • Regression tests cover missing, foreign and stale identity, explicit-window scope, traversal bounds and query failures.
  • Windows non-UI validation: 49 service/pure tests and 13 command tests passed with the help fix, including a regression demonstrated failing before correction. The 13 command tests also passed after integrating main's unrelated changes.
  • Canonical guide, shipped skill/reference, command help and generated schema/npm descriptions updated.
  • Local scripts\build-cli.ps1 -SkipTests and generated-doc/plugin validation passed with the help fix; subsequent CLI build -OnlyDocs passed. Published ARM64 ui get-focused --help checked.
  • Current-head integration CI passed on e66235f8: all four test shards, UI E2E, docs, 14 samples and required aggregate. TRX: 7,711 passed, 37 intentional skips.

Screenshots / Demo

Textual CLI output above demonstrates this nonvisual query correction. No screenshots required.

Additional Notes

Live ARM64 candidate evidence belongs to build head e63e39921b37d95b58adfba9012c9130580a3814, SHA256 DF19FC7248FBF33FD3D2FB818E4C40970881707AD899D57BB2F43EFC2F077942: five focus operations and one guarded click, with keyboard-focus property confirmation and side-by-side baseline queries. The blank editor was canceled without entering or saving text; only this issue's staged artifacts were cleaned. The prior local app installation's exact revision was not verified.

Current main was merged without rewriting branch history; overlapping focus and empty-value guidance were both preserved. The focus ownership algorithm is unchanged from the live-tested implementation. Prior live evidence is not a claim that a newly built merge-head binary was live-tested. No host UI or new Sandbox interaction was performed for this update. Local builds skipped full shared-host UI tests; targeted non-UI tests and full repository CI provide integration coverage.

The native checks validate ownership of a provider-supplied HWND; they do not authenticate a malicious UIA provider's claimed relationship to that window. This read-only query is not an input-authorization boundary, and input safety guards are unchanged.

Verify native ancestor process and explicit root window ownership, preserve query errors, and cover missing/foreign/stale identity cases.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

🔵 Needs a closer look

The Sandbox-specific native ownership path still needs live candidate validation and latest-head CI.

Pull request overview

Fixes Sandbox get-focused ownership detection when controls omit process IDs.

Changes:

  • Verifies ownership through bounded native-ancestor traversal.
  • Propagates query failures and adds regression coverage.
  • Documents process- and window-scoped behavior.
File summaries
File Description
src/winapp-CLI/WinApp.UIAutomation/Services/UiAutomationService.cs Adds native ownership verification.
src/winapp-CLI/WinApp.UIAutomation.Tests/RealUiAutomationTests.Coverage.cs Verifies query failures propagate.
src/winapp-CLI/WinApp.UIAutomation.Tests/FocusedElementTests.cs Covers ownership and traversal scenarios.
src/winapp-CLI/WinApp.Cli.Tests/UiCommandTests.SimpleVerbs.cs Tests command-level JSON errors.
docs/ui-automation.md Documents updated get-focused behavior.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/ui-automation.md Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

🔵 Needs a closer look

Native UI Automation ownership remains platform-dependent, and current repository checks are pending.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread docs/ui-automation.md
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

🔵 Needs a closer look

Native UIA and Win32 ownership behavior remains platform-specific, with latest-head checks still pending.

Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

github-actions Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Build Metrics Report

Validation passed. All required build and validation jobs succeeded.

Binary Sizes

Artifact Baseline Current Delta
CLI (ARM64) 57.05 MB 57.06 MB 📈 +2.0 KB (+0.00%)
CLI (x64) 57.10 MB 57.10 MB 📈 +1.5 KB (+0.00%)
MSIX (ARM64) 23.73 MB 23.73 MB 📈 +2.9 KB (+0.01%)
MSIX (x64) 25.17 MB 25.17 MB 📈 +1.4 KB (+0.01%)
NPM Package 49.48 MB 49.48 MB 📈 +0.7 KB (+0.00%)
NuGet Package 49.59 MB 49.59 MB 📉 -1.9 KB (-0.00%)

.NET Test Results (TRX reports)

Other suites are reflected in the overall validation status above.

✅ 7711 passed, 37 skipped out of 7748 tests in 1155.5s (+30 tests, -211.2s vs. baseline)

Test Coverage

✅ 85.7% line coverage, 80.1% branch coverage · ✅ no change vs. baseline

CLI Startup Time

64ms median (x64, winapp --version) · ✅ no change vs. baseline

Try This Build

Installs 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))) 880
Switching between builds often?

Put the tool on your PATH once:

& ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPath

Then this build is just:

winapp-pr 880

Run winapp-pr with no arguments to pick from a list of open PRs.


Updated 2026-09-22 00:22:36 UTC · commit e66235f · workflow run

@nmetulev
Nikola Metulev (nmetulev) marked this pull request as ready for review September 18, 2026 21:39
@nmetulev Nikola Metulev (nmetulev) added this to the 0.7 milestone Sep 21, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@nmetulev Nikola Metulev (nmetulev) added the agent-preparing Agent is addressing feedback or completing required validation and CI label Sep 21, 2026
@nmetulev

Nikola Metulev (nmetulev) commented Sep 21, 2026 •

Copy link
Copy Markdown
Member Author

State: agent-preparing
Assessed: head a4413db38ee2abee74493334fd265f7f2f380c06 against main@e2602300.
Evidence: Normal merge preserved BOTH focus and empty-value guidance. Help regression fails before correction, passes after.49service/pure+13CLI tests passed for2a5c89c9;13CLI repeated on latesthead, relevant code unchanged. Latest build -OnlyDocs running; prior build-cli -SkipTests and generated-doc/plugin validation pass; published ARM64 help shows exact top-level scope. Historical6/6liveproof is e63, not new merge binaries; no hostUI/Sandbox interaction. Static code-quality pass on ownership and description changes finds nothing obvious (approximation, not engine).
Review: Both zateutsch threads answered/resolved. Help fixed in2a5c89c9 with regenerated schema/npm descriptions. PID0provider-HWND authentication suggestion retained as explicitly nonblocking defense-in-depth follow-up, with limitation documented and rationale in its thread. Human zateutsch re-review and current-head Copilot requested. No claimed human approval.
Pending: Current integration CI35666737596, independent current-head review, actual-main/no-conflict final checks. Addressed human comments awaiting re-review alone will not block ready once other technical gates pass.
Blocker / owner / next step: None; ordinary CI/review waiting.
**Resume:**10-minute monitoring remains active including after ready-for-review while requested re-review is pending. User-policy label rename adopted; old agent-ready-for-review will not be recreated. agent-blocked reserved for inability to finish feedback/CI or need for author input.

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-process UI Automation and native window ownership behavior remains OS-sensitive and warrants final human review.

Review effort: Balanced
Findings: None

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Native UI Automation behavior is platform-sensitive, and current-head build, CI, and live validation remain incomplete.

Review effort: Balanced
Findings: None

@nmetulev Nikola Metulev (nmetulev) added ready-for-review Agent work and technical checks complete; awaiting review or re-review, not approval or merge agent-preparing Agent is addressing feedback or completing required validation and CI and removed agent-preparing Agent is addressing feedback or completing required validation and CI ready-for-review Agent work and technical checks complete; awaiting review or re-review, not approval or merge labels Sep 21, 2026

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Native cross-process UI Automation ownership traversal warrants final human review despite strong regression and runtime evidence.

Review effort: Balanced
Findings: None

@nmetulev Nikola Metulev (nmetulev) added ready-for-review Agent work and technical checks complete; awaiting review or re-review, not approval or merge and removed agent-preparing Agent is addressing feedback or completing required validation and CI labels Sep 21, 2026
Comment thread src/winapp-CLI/WinApp.UIAutomation/Services/UiAutomationService.cs
Comment thread docs/ui-automation.md
@nmetulev Nikola Metulev (nmetulev) added agent-preparing Agent is addressing feedback or completing required validation and CI and removed ready-for-review Agent work and technical checks complete; awaiting review or re-review, not approval or merge labels Sep 21, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

# Conflicts:
#	plugins/winapp/skills/winapp-ui-automation/SKILL.md
Add CLI help regression and regenerate schema/npm help surfaces.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Native Sandbox ownership behavior still requires completed current-head integration validation and final human review.

Review effort: Balanced
Findings: None

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-process UI Automation ownership changed, and current-head integration validation remains pending.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Comment thread src/winapp-CLI/WinApp.Cli/Commands/UiGetFocusedCommand.cs
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Native cross-process UI Automation behavior still requires completed current-head integration and live Windows validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@nmetulev Nikola Metulev (nmetulev) added ready-for-review Agent work and technical checks complete; awaiting review or re-review, not approval or merge and removed agent-preparing Agent is addressing feedback or completing required validation and CI labels Sep 22, 2026
@zateutsch
Zach Teutsch (zateutsch) merged commit d87165f into main Sep 22, 2026
42 checks passed
@zateutsch
Zach Teutsch (zateutsch) deleted the nmetulev-sandbox-focused-control branch September 22, 2026 00:59
@nmetulev Nikola Metulev (nmetulev) removed the ready-for-review Agent work and technical checks complete; awaiting review or re-review, not approval or merge label Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ui get-focused reports no focus in Sandbox while the target text box has keyboard focus

3 participants