Skip to content

Activate target windows and verify ui focus outcomes - #881

Merged
Nikola Metulev (nmetulev) merged 19 commits into
mainfrom
nmetulev-activate-and-verify-ui-focus
Sep 22, 2026
Merged

Nikola Metulev (nmetulev) merged 19 commits into
mainfrom
nmetulev-activate-and-verify-ui-focus

Conversation

@nmetulev

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

Copy link
Copy Markdown
Member

Description

Corrects ui focus reporting success for a control in an inactive window. The command requests activation of the selected control's top-level window when needed, focuses that control, and confirms both strict foreground ownership and live keyboard focus before reporting success.

Focus confirmation allows up to 500 ms for asynchronous UI Automation providers, observing the same retained control at 50 ms intervals. It does not repeat activation or SetFocus. Every observation rechecks cancellation, target/root/process validity, and strict foreground ownership before and after the provider read.

Activation refusal, lost foreground, unconfirmed focus, stale targets, and provider failures return nonzero. Existing desktop coordination, explicit-window isolation, and wrong-window input guards remain in place. An owned popup is not accepted as foreground for a control in its owner. The selector is accurately required in help, generated schema, and npm types.

Usage Example

winapp ui focus SearchTasksBox -a Daylight --on sandbox --json

Before: the original Sandbox reproduction returned success and HasKeyboardFocus=True while Daylight's main window remained inactive. An immediate one-shot confirmation could also report failure before a provider published a successful focus change.

After: success requires the selected control's window to be foreground and that control to confirm keyboard focus within the bounded observation period. Refused activation returns foreground_not_target; unconfirmed keyboard focus returns focus_not_acquired. Check for a blocking dialog and activate the intended window manually before retrying. Locked sessions receive unlock-and-retry guidance. There is no forced activation or local-target fallback.

Related Issue

Fixes #877.

Type of Change

  • 🐛 Bug fix
  • 📝 Documentation
  • 🧪 Test update

Checklist

  • Regression tests cover activation, delayed focus, bounded failure, cancellation, provider errors, stale targets, popup behavior, and explicit-window isolation
  • Tested on Windows with non-UI regressions and allocated Windows Sandbox interaction checks, with limitations below
  • Canonical docs/ui-automation.md, generated CLI/npm references, shipped skill, and Copilot agent reference updated

Screenshots / Demo

Textual outcome evidence is used; no interaction recording was captured. Before the asynchronous polling change, a guest-only fixture verified native foreground HWND and actual control focus together:

Scenario Observed outcome
Already active target Exit 0, target foreground and control focused
Eligible inactive target with stable decoy foreground Exit 0, target became foreground and control focused
Disabled owner behind owned popup Nonzero foreground_not_target
Selected control inside popup Exit 0, popup foreground and its control focused
Focus handler activates decoy Nonzero foreground_not_target

These results identify source 1f9fe8331dfc0fefd07b9545c13edab4fed8e619, ARM64 SHA256 81C0F7E2FDF0EA8B6F07C6FB3B38B1EDE27B3765461C7076AD8674C7494B1064, not the later polling implementation. Original Daylight WinUI control focus and explicit-window refusal were validated on earlier source 27eda8ff3193ee133284cd94d663c183bfd97eca (ARM64 SHA256 6D321BDEB62E154B1D943BCBEE01349F4BB1D64E33FCE2968365D98EC5B815AA).

Additional Notes

The asynchronous-focus and activation-delay regressions were run failing before their fixes. Focused ownership/encoder tests pass 54/54; the broader command/ownership/routing/encoder suite passes 855, with one opt-in recording test skipped. Local NativeAOT x64/ARM64 and npm/NuGet builds passed with -SkipTests -SkipMsix. Full repository validation also passed on the current integration, including both CLI shards, UI Automation, UI end-to-end tests, auxiliary checks, documentation, and all sample jobs.

Live timing-validation limitation: Notepad was absent from the allocated guest. Two bounded custom-provider fixture attempts against polling candidate 708ed58bafc85e0f711dc8e175cce4c3cd72d9c6 (ARM64 SHA256 66B7EC6C8F4C0DF4335ECB3942DA5338E6FC50869EFD1697130FAC9A0DBA60BA) did not faithfully exercise delayed focus: UIA focused the native host before the provider callback. Those attempts are explicitly not counted as live confirmation of the timing fix. The fixture windows and exact test artifacts were cleaned; no host UI, installation, user-data edits, or guest reset was used. The reported Notepad scenario was not independently reproduced by these fixtures.

Windows may legitimately refuse activation; the fix detects refusal rather than bypassing policy.

Fixes #877.

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

Final live Sandbox smoke and CI are still pending for Windows activation and popup-sensitive behavior.

Pull request overview

Corrects ui focus false positives by activating the selected control’s window and verifying the resulting foreground and keyboard focus.

Changes:

  • Adds strict activation, identity, foreground, and focus verification.
  • Makes the selector required across CLI and npm surfaces.
  • Adds regression coverage and updates user guidance.
File summaries
File Description
src/winapp-npm/src/winapp-commands.ts Requires a focus selector and updates API documentation.
src/winapp-CLI/WinApp.UIAutomation/Services/UiAutomationService.cs Strengthens identity and cancellation checks before focusing.
src/winapp-CLI/WinApp.UIAutomation.TestSupport/FakeUiServices.cs Adds focus-verification test hooks.
src/winapp-CLI/WinApp.Cli/Helpers/UiJsonError.cs Adds the focus_not_acquired error code.
src/winapp-CLI/WinApp.Cli/Helpers/UiInjectionReporting.cs Supports routed error output and improved recovery guidance.
src/winapp-CLI/WinApp.Cli/Commands/UiFocusCommand.cs Implements activation and verified focus behavior.
src/winapp-CLI/WinApp.Cli.Tests/UiCommandTests.SimpleVerbs.cs Updates existing focus failure fixtures.
src/winapp-CLI/WinApp.Cli.Tests/UiCommandTests.Focus.cs Covers activation, focus loss, stale targets, popups, and cancellation.
src/winapp-CLI/WinApp.Cli.Tests/UiCommandTests.cs Updates existing success and JSON tests.
src/winapp-CLI/WinApp.Cli.Tests/UiCommandTests.Coordination.cs Updates coordinated focus verification.
src/winapp-CLI/WinApp.Cli.Tests/FakeDesktopForegroundService.cs Adds configurable foreground callbacks.
plugins/winapp/skills/winapp-ui-automation/SKILL.md Documents focus workflow and recovery.
docs/ui-automation.md Documents activation, verification, and failure codes.
docs/npm-usage.md Updates generated npm API documentation.
docs/cli-schema.json Records required selector and revised command description.
Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@nmetulev

Copy link
Copy Markdown
Member Author

Copilot reviewed the current head 27eda8ff3193ee133284cd94d663c183bfd97eca and generated no inline findings. The final live-smoke item mentioned in the review summary is now complete (details and exact final binary hash are in the PR validation section): all five controlled Sandbox outcomes, original Daylight control focus, and unrelated explicit-window refusal passed on final bytes. Repository CI is still running; this PR remains draft until it finishes.

@github-actions

github-actions Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

⏳ Build in progress — metrics below are from a previous commit and will update when the current build finishes.

Build Metrics Report

Validation did not pass. Artifacts were uploaded before validation finished; check the workflow run before using them.

Binary Sizes

Artifact Baseline Current Delta
CLI (ARM64) 57.28 MB 57.26 MB 📉 -19.5 KB (-0.03%)
CLI (x64) 57.33 MB 57.31 MB 📉 -19.0 KB (-0.03%)
MSIX (ARM64) 23.78 MB 23.79 MB 📈 +7.6 KB (+0.03%)
MSIX (x64) 25.25 MB 25.24 MB 📉 -10.5 KB (-0.04%)
NPM Package 49.62 MB 49.61 MB 📉 -6.4 KB (-0.01%)
NuGet Package 49.72 MB 49.71 MB 📉 -13.2 KB (-0.03%)

.NET Test Results (TRX reports)

Other suites are reflected in the overall validation status above.

❌ 7832 passed, 1 failed, 37 skipped out of 7870 tests in 1175.3s (+38 tests, -10.1s vs. baseline)

Test Coverage

✅ 86% line coverage, 80.7% branch coverage · ✅ +0.4% vs. baseline

CLI Startup Time

63ms median (x64, winapp --version) · ✅ +7ms 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))) 881
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 881

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


Updated 2026-09-22 18:54:34 UTC · commit 7c44ae5 · workflow run

@nmetulev
Nikola Metulev (nmetulev) marked this pull request as ready for review September 18, 2026 21:13
@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 0ea3d1536313f544a5627eb283656a2eff98dff7 against main@3520ee6ff3525c9af1eb5b279930225eae8e8876; normal merges, no rewritten history. Existing non-draft state preserved.
Evidence: Current runtime code passed 779 command/guard/routing tests (one opt-in recording skip), 311 npm tests and a subsequent 40/40 focus/ownership run at 5f6b6e09. NativeAOT x64/ARM64/npm/NuGet build passed at 20cd2edb with -SkipTests -SkipMsix; subsequent changes are independently verified comments and a single shipped Markdown line. Plugin validation and diff-check pass. All checks at 8c9e09ca subsequently passed, but main advanced with screenshot/empty-value fixes (#884/#878), so strict-base integration was normally updated. New current-head command/ownership/routing suite: 826 passed, one opt-in recording skip. Fresh local NativeAOT/package build and current-head CI/review are running. No old run is counted as the required current-head gate.
Review: Independent complete feature coverage at afdf9c28, message-fix delta at 742f0ae4, and each subsequent merge/docs delta through current 8c9e09ca completed with no remaining actionable findings. Before the last one-line Markdown correction, all 15 feature files had exactly identical added/deleted text across merged ranges (stable patch ID bf012d5c4dff6c41d1190e5e9e540dec19869711); final one-line correction separately reviewed. Static precheck covered four production C# files plus one TestSupport file, with no obvious violations (not a substitute for CodeQL).
Feedback disposition: Locked-session wording fixed with a red/green regression and replied/resolved. Shipped Copilot reference corrected and replied/resolved. The intervening top-level popup-PID allegation was refuted by 12 passing targeted tests and live owner-chain analysis, independently reassessed; test/guard unchanged. Current-head Copilot completed with no findings and acknowledges resolved feedback. This is technical review evidence, not formal human approval.
Limits: Previous Sandbox bytes (27eda8ff, ARM64 SHA256 6D321BDEB62E154B1D943BCBEE01349F4BB1D64E33FCE2968365D98EC5B815AA) are historical feature evidence only; no fresh host/Sandbox UI was run in this update. Current-head CI UI E2E is distinct evidence. Main e9ab2ea introduced unrelated certInfo generated-description drift in npm docs/type comments; removed only our self-generated unrelated hunks with parent agreement. Required CI gates are not waived.
Pending: narrow independent latest-merge assessment, new current-head build/review/strict required build-and-package check (GitHub Actions integration 15368) and remaining integration checks, then fresh head/base/discussion/policy/conflict assessment. Human/codeowner/last-push approvals remain separate and pending.
Blocker / owner / next step: No external intervention known; awaiting CI normally. Worktree clean; no unresolved actionable findings known.
Resume: Same-session 10-minute automation active. New pr-lifecycle/code-quality instructions were read from merged local files because the old registry could not load pr-lifecycle. This is the only lifecycle comment.
Updated user policy: Lifecycle ready label is now ready-for-review (renamed globally). agent-blocked only means feedback/CI cannot be completed or author input is needed; ordinary waiting for re-review after addressed feedback does not block ready when other technical gates pass. Current CI remains pending, so this PR stays agent-preparing.
Fresh bounded live proof: On 1f9fe833 ARM64 SHA256 81C0F7E2FDF0EA8B6F07C6FB3B38B1EDE27B3765461C7076AD8674C7494B1064 (host/guest match), a new parent-allocated guest-only WinForms fixture verified all five native-foreground/control-focus scenarios: active/inactive success, modal-owner refusal, selected-popup success, foreground-loss refusal. Exact test artifacts cleaned and slot released, guest healthy. Daylight absent: no fresh WinUI claim. Main then advanced to publisher-DN/README changes; current cd6f8190 normally integrates them, 40/40 focus/ownership tests pass, fresh CI/review and mergerange review pending. No runtime focus changes after tested bytes.
Latest strict-base update: Main added unrelated control-search sample fixes; normal merge 0ea3d153 preserves focus code. Current-head focus/ownership tests40/40passed; prior publisher-integration NativeAOT/packages passed at cd6f8190. Fresh current-head CI/Copilot and narrow independent range-equivalence pending; still preparing.

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

🟡 Changes recommended

Locked-session recovery currently gives ui focus users contradictory guidance to use another UIA-pattern verb.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Use action-neutral guidance and cover the focus error path.

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

The owned-popup success test assigns the popup to the wrong process, causing it to fail with stale_element.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Popup success test uses mismatched target process ID

src/​winapp-CLI/​WinApp.Cli.Tests/​UiCommandTests.Focus.cs:171

The popup success test configures the popup as another process. ConfigureVerifiedFocus targets PID 1234, but this line makes HWND 5000 report PID 8888. ConfirmTarget() therefore classifies the handle as recycled and returns stale_element, so the test cannot reach its expected success result. Keep the popup in the target process (PID 1234) so this test exercises popup foreground handling rather than the stale-target guard.

@nmetulev

Copy link
Copy Markdown
Member Author

Responding to the current-head Copilot review's “Previously missed” popup PID note: this is intentional cross-process owned-dialog coverage, not a failing fixture. In Focus_ControlInOwnedPopup_VerifiesPopupNotMainWindow, HWND 5000 has PID 8888 and owner HWND 4242; that live owner has the requested PID 1234. DesktopTargetValidation.ClassifyTargetWindow rejects a mismatched PID only when IsOwnedByExpectedProcess is also false (lines 103–105). Its owner-chain check validates the live owner's PID (lines 160–173). This is required for app-owned system dialogs running in a different process and does not broaden the strict foreground window check.

On current head 742f0ae4d341de02a97f5f984a76eb700131b40c, reran:

dotnet run --project src\winapp-CLI\WinApp.Cli.Tests\WinApp.Cli.Tests.csproj -c Debug --no-build -- --filter "FullyQualifiedName~Focus_ControlInOwnedPopup_VerifiesPopupNotMainWindow|FullyQualifiedName~DesktopTargetValidation"

12 passed, 0 failed, 0 skipped. The broader current-source suite also passed 779 with one unrelated opt-in skip. Changing the popup to PID 1234 would remove useful cross-process ownership coverage, so it is intentionally unchanged. Independent reassessment of this specific note is also being recorded in the lifecycle status. The prior locked-session wording finding was fixed and its inline thread answered/resolved.

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

🟢 Approval recommended

The solution builds cleanly, all 29 targeted focus tests and 311 npm tests pass, and real inactive-window activation succeeded.

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

🟡 Changes recommended

The shipped Copilot agent command reference still documents the old focus contract.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@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 22, 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.

Comment thread src/winapp-npm/src/winapp-commands.ts Resolved
Comment thread src/winapp-npm/src/winapp-commands.ts Resolved

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.

Cover activation settle ordering with an injected-delay regression and remove an obsolete selector comment.

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

Two moderate issues affecting focus reliability and live verification remain unresolved.

Review effort: Balanced
Findings: None

Resolved since last review (6)
Previously missed (1)

In code that hasn't changed since last review

Low severity Update documentation for focus activation and failure behavior

docs/​ui-automation.md:811

The canonical CLI command list still says focus only “Move keyboard focus” in docs/usage.md:2177, even though this change now activates the window, verifies foreground ownership and keyboard focus, and can fail with recovery-required errors. Users who start from docs/usage.md will therefore get an outdated contract; update that hand-authored list together with this focus reference.

@nmetulev Nikola Metulev (nmetulev) added agent-blocked Agent cannot address feedback or fix CI without help, or needs author input 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 agent-blocked Agent cannot address feedback or fix CI without help, or needs author input labels Sep 22, 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

Production activation and delayed UIA focus behavior still lacks controlled live integration validation.

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 22, 2026
@nmetulev
Nikola Metulev (nmetulev) merged commit bf04de1 into main Sep 22, 2026
49 checks passed
@nmetulev
Nikola Metulev (nmetulev) deleted the nmetulev-activate-and-verify-ui-focus branch September 22, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review Agent work and technical checks complete; awaiting review or re-review, not approval or merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ui focus should activate the target window and verify focus before reporting success

3 participants