From 017a36df3fd70c2f2c71d57e45721fff2a17224b Mon Sep 17 00:00:00 2001 From: Stefan Broenner Date: Wed, 30 Sep 2026 18:12:12 +0200 Subject: [PATCH 1/2] fix: enforce shared UI search deadlines Stop scans and retries at the shared timeout without claiming partial absence or uniqueness. Preserve timeout diagnostics and missing-dialog details. Refs #253 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- FEATURES.md | 12 ++ .../Automation/BoundedSearchTraversal.cs | 14 +- .../Automation/Tools/UIFindTool.cs | 5 +- .../Automation/Tools/UIWaitTool.cs | 3 + .../Automation/UIAutomationService.Find.cs | 138 +++++++++++++----- .../Automation/UIAutomationService.Tree.cs | 6 +- .../Models/UIAutomationDiagnostics.cs | 3 +- .../UIAutomationAdvancedSearchTests.cs | 21 +-- .../Integration/UIFindToolIntegrationTests.cs | 47 ++++++ .../Unit/BoundedSearchTraversalTests.cs | 65 +++++++++ .../Unit/FindDeadlineTests.cs | 100 +++++++++---- 11 files changed, 336 insertions(+), 78 deletions(-) diff --git a/FEATURES.md b/FEATURES.md index 4c60406..42006f4 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -228,6 +228,14 @@ of claiming the target is absent or unique. Narrow the search using an exact name, `automationId`, `controlType`, or `className`, or a `parentElementId` from the same MCP session. A longer timeout does not increase this limit. +With a positive `timeoutMs` (default: 5,000), retries and scans share one time +budget. No new scan starts at or after the deadline, and scanning stops between +Windows accessibility calls when time runs out. A Windows call already in +progress can still take longer to return. An interrupted scan reports `timeout` +without claiming the target is absent or unique. Set `includeDiagnostics=true` +to see elapsed time and the last scan's element count. `timeoutMs=0` performs +one node-bounded scan without a time deadline. + --- ## 🖱️ UI Click (`ui_click`) @@ -470,6 +478,10 @@ Wait until a UI condition is met before continuing - no blind sleeps or screensh - Wait for a specific element to become enabled/visible/toggled - State mode rejects selectors; appear/disappear reject `elementId` and `desiredState` +Appear/disappear waits use the same shared retry-and-scan time budget as +`ui_find`. An interrupted scan reports `timeout`, not a successful disappearance. +Windows accessibility calls already in progress can still overrun the time budget. + --- ## 🧩 UI Batch (`ui_batch`) diff --git a/src/Sbroenne.WindowsMcp/Automation/BoundedSearchTraversal.cs b/src/Sbroenne.WindowsMcp/Automation/BoundedSearchTraversal.cs index 074b56d..b8258ae 100644 --- a/src/Sbroenne.WindowsMcp/Automation/BoundedSearchTraversal.cs +++ b/src/Sbroenne.WindowsMcp/Automation/BoundedSearchTraversal.cs @@ -13,7 +13,8 @@ internal static Outcome Walk( Func visit, int maxNodes, int maxDepth, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + Action? checkDeadline = null) where T : class { ArgumentNullException.ThrowIfNull(root); @@ -23,6 +24,7 @@ internal static Outcome Walk( ArgumentNullException.ThrowIfNull(visit); ArgumentOutOfRangeException.ThrowIfNegative(maxNodes); cancellationToken.ThrowIfCancellationRequested(); + checkDeadline?.Invoke(); if (maxDepth <= 0) { return new(0, false); @@ -35,14 +37,18 @@ internal static Outcome Walk( var ancestors = new Stack<(T Node, int ParentDepth)>(); var current = firstChild(root); + checkDeadline?.Invoke(); var parentDepth = 0; var scanned = 0; while (current is not null) { cancellationToken.ThrowIfCancellationRequested(); + checkDeadline?.Invoke(); scanned++; var depth = parentDepth + (countsForDepth(current) ? 1 : 0); - if (visit(current, depth)) + var satisfied = visit(current, depth); + checkDeadline?.Invoke(); + if (satisfied) { return new(scanned, false); } @@ -55,7 +61,9 @@ internal static Outcome Walk( } cancellationToken.ThrowIfCancellationRequested(); + checkDeadline?.Invoke(); var child = depth < maxDepth ? firstChild(current) : null; + checkDeadline?.Invoke(); if (child is not null) { ancestors.Push((current, parentDepth)); @@ -67,7 +75,9 @@ internal static Outcome Walk( while (true) { cancellationToken.ThrowIfCancellationRequested(); + checkDeadline?.Invoke(); var sibling = nextSibling(current); + checkDeadline?.Invoke(); if (sibling is not null) { current = sibling; diff --git a/src/Sbroenne.WindowsMcp/Automation/Tools/UIFindTool.cs b/src/Sbroenne.WindowsMcp/Automation/Tools/UIFindTool.cs index a28d040..92fc874 100644 --- a/src/Sbroenne.WindowsMcp/Automation/Tools/UIFindTool.cs +++ b/src/Sbroenne.WindowsMcp/Automation/Tools/UIFindTool.cs @@ -29,6 +29,9 @@ public static partial class UIFindTool /// If unvisited nodes remain at the scan limit, search_incomplete means absence or uniqueness could not be established. /// Narrow with exact name, automationId, controlType, className or a known parentElementId; /// increasing timeoutMs does not increase this limit. + /// Positive timeoutMs limits both retries and scanning. No new scan starts at or after the deadline; + /// an in-progress Windows accessibility call can still overrun it. An interrupted scan reports timeout, + /// not absence or uniqueness. timeoutMs=0 performs one node-bounded scan without a time deadline. /// /// Window handle as decimal string (from window_management 'find' or 'list'). REQUIRED. /// Element name (exact match, case-insensitive). For Electron apps and Chromium browsers, this is often the visible label or ARIA label. @@ -49,7 +52,7 @@ public static partial class UIFindTool /// Search root: window (default) or active_dialog. Use active_dialog after opening a modal or native file dialog. /// Fail with ambiguity details when more than one element matches. Default: false. /// Exclude disabled elements when true. - /// Timeout in milliseconds (default: 5000). + /// Search time budget in milliseconds (default: 5000). Stops between Windows accessibility calls; an in-progress call can overrun the budget. Zero performs one scan without a time deadline. /// Include diagnostics (timing, query, elements scanned) in response. Default: false. /// Cancellation token. /// A call result containing a text content block with the JSON payload listing found elements and their properties (including element IDs). IsError reflects operation success. diff --git a/src/Sbroenne.WindowsMcp/Automation/Tools/UIWaitTool.cs b/src/Sbroenne.WindowsMcp/Automation/Tools/UIWaitTool.cs index bf5569f..7fc7b27 100644 --- a/src/Sbroenne.WindowsMcp/Automation/Tools/UIWaitTool.cs +++ b/src/Sbroenne.WindowsMcp/Automation/Tools/UIWaitTool.cs @@ -27,6 +27,9 @@ public static partial class UIWaitTool /// - 'state': wait until the element with the given elementId reaches desiredState. Provide elementId + desiredState. /// Uses efficient exponential backoff polling internally. Returns success as soon as the condition holds, /// or a timeout failure with diagnostics. + /// Appear/disappear waits share one time budget across retries and scanning; no new scan starts + /// at or after the deadline. An in-progress Windows accessibility call can overrun the budget. + /// An interrupted scan cannot establish absence, uniqueness, or disappearance. /// /// Window handle as decimal string (from window_management 'find'/'list' or app). Used to scope 'appear'/'disappear'. /// Condition to wait for: 'appear', 'disappear', or 'state'. Default: 'appear'. diff --git a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs index 6852c16..48062a8 100644 --- a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs +++ b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs @@ -13,7 +13,7 @@ public sealed partial class UIAutomationService { /// /// Shares the production deadline policy with deterministic clock/probe regressions. - /// One probe must start at/after the deadline, even if the preceding probe crossed it. + /// No new probe starts at or after the deadline. /// internal static Task WaitForFindResultAsync( ElementQuery query, @@ -44,11 +44,51 @@ private static async Task WaitForSearchResultAsync( { var delay = 50; var action = disappear ? "wait_for_disappear" : "wait_for"; + UIAutomationResult? lastResult = null; while (true) { cancellationToken.ThrowIfCancellationRequested(); - var finalProbe = elapsedMilliseconds() >= timeoutMs; + if (elapsedMilliseconds() >= timeoutMs) + { + var elapsed = elapsedMilliseconds(); + var message = lastResult switch + { + null => $"Search timed out after {timeoutMs}ms before it could start. The expected UI state could not be checked.", + { ErrorType: UIAutomationErrorType.WindowNotFound } => + $"{lastResult.ErrorMessage} Search timed out after {timeoutMs}ms.", + _ when disappear => + $"Element still present after {timeoutMs}ms timeout. Expected it to disappear.", + _ => $"Element not found within {timeoutMs}ms timeout." + }; + return UIAutomationResult.CreateFailure( + action, + UIAutomationErrorType.Timeout, + message, + (lastResult?.Diagnostics ?? new UIAutomationDiagnostics { DurationMs = elapsed }) with + { + DurationMs = elapsed, + Query = query, + ElapsedBeforeTimeout = elapsed + }); + } + var result = await probe().ConfigureAwait(false); + if (result.Diagnostics is not null) + { + var elapsed = elapsedMilliseconds(); + result = result with + { + Diagnostics = result.Diagnostics with + { + DurationMs = elapsed, + Query = query, + ElapsedBeforeTimeout = result.ErrorType == UIAutomationErrorType.Timeout + ? elapsed + : result.Diagnostics.ElapsedBeforeTimeout + } + }; + } + lastResult = result; if (disappear) { if ((!result.Success && IsSatisfiedDisappearAbsence(result.ErrorType)) || @@ -68,23 +108,6 @@ private static async Task WaitForSearchResultAsync( return result with { Action = action }; } - if (finalProbe) - { - var elapsed = elapsedMilliseconds(); - return UIAutomationResult.CreateFailure( - action, - UIAutomationErrorType.Timeout, - disappear - ? $"Element still present after {timeoutMs}ms timeout. Expected it to disappear." - : $"Element not found within {timeoutMs}ms timeout.", - new UIAutomationDiagnostics - { - DurationMs = elapsed, - Query = query, - ElapsedBeforeTimeout = elapsed - }); - } - var remaining = timeoutMs - elapsedMilliseconds(); if (remaining > 0) { @@ -99,15 +122,6 @@ public async Task FindElementsAsync(ElementQuery query, Canc { ArgumentNullException.ThrowIfNull(query); - if (query.RequireUnique && query.FoundIndex != 1) - { - return UIAutomationResult.CreateFailure( - "find", - UIAutomationErrorType.InvalidParameter, - "requireUnique cannot be combined with foundIndex other than 1. Refine the selector instead.", - CreateDiagnostics(Stopwatch.StartNew(), query)); - } - if (query.TimeoutMs <= 0) { return await FindElementsOnceAsync(query, cancellationToken).ConfigureAwait(false); @@ -129,14 +143,26 @@ public async Task FindElementsAsync(ElementQuery query, Canc private async Task FindElementsOnceAsync( ElementQuery query, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + Action? checkDeadline = null) { var stopwatch = Stopwatch.StartNew(); + var elementsScanned = 0; + + if (query.RequireUnique && query.FoundIndex != 1) + { + return UIAutomationResult.CreateFailure( + "find", + UIAutomationErrorType.InvalidParameter, + "requireUnique cannot be combined with foundIndex other than 1. Refine the selector instead.", + CreateDiagnostics(stopwatch, query)); + } try { return await _staThread.ExecuteAsync(() => { + checkDeadline?.Invoke(); // Get root element UIA.IUIAutomationElement? rootElement; if (!string.IsNullOrEmpty(query.ParentElementId)) @@ -150,6 +176,7 @@ private async Task FindElementsOnceAsync( rootElement = ElementIdGenerator.ResolveToAutomationElement( query.ParentElementId); + checkDeadline?.Invoke(); if (rootElement == null) { return UIAutomationResult.CreateFailure( @@ -178,6 +205,7 @@ private async Task FindElementsOnceAsync( : GetRootElement(query.WindowHandle); } + checkDeadline?.Invoke(); if (rootElement == null) { var requestedActiveDialog = string.Equals( @@ -235,13 +263,13 @@ private async Task FindElementsOnceAsync( } var elementInfos = new List(); - var elementsScanned = 0; var matchCount = 0; var scanLimitReached = false; var maxResults = query.RequireUnique ? 2 : query.FoundIndex > 1 ? query.FoundIndex : 100; // Detect framework and get optimal search strategy var strategy = GetFrameworkStrategy(rootElement); + checkDeadline?.Invoke(); // Resolve visibility filtering: explicit caller value wins; otherwise exclude // off-screen nodes for Chromium/Electron (huge hidden/virtualized trees), include elsewhere. @@ -261,6 +289,7 @@ private async Task FindElementsOnceAsync( // passes. Exact/native conditions must not bypass the provider traversal cap. void RunScan(UIA.IUIAutomationCondition scanCondition) { + checkDeadline?.Invoke(); elementInfos.Clear(); matchCount = 0; scanLimitReached = false; @@ -269,7 +298,7 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) rootElement, scanCondition, query, elementInfos, ref elementsScanned, ref matchCount, maxResults, query.ExactDepth.HasValue ? effectiveMaxDepth : query.MaxDepth ?? int.MaxValue, - visibleOnly, regionFilter, cancellationToken); + visibleOnly, regionFilter, cancellationToken, checkDeadline); // Exclude off-screen elements when visibility filtering is in effect. if (visibleOnly && elementInfos.Count > 0) @@ -300,10 +329,8 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) RunScan(condition); } - stopwatch.Stop(); - LogSearchPerformance(_logger, "find", elementsScanned, stopwatch.ElapsedMilliseconds, elementInfos.Count); - string? windowTitle = rootElement.GetName(); + checkDeadline?.Invoke(); // AUTO-RECOVERY: If exact name match failed, automatically try partial match if (!scanLimitReached && elementInfos.Count == 0 && @@ -322,7 +349,7 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) rootElement, relaxedCondition, relaxedQuery, elementInfos, ref elementsScanned, ref matchCount, maxResults, query.ExactDepth.HasValue ? effectiveMaxDepth : query.MaxDepth ?? int.MaxValue, - visibleOnly, regionFilter, cancellationToken); + visibleOnly, regionFilter, cancellationToken, checkDeadline); if (visibleOnly && elementInfos.Count > 0) { @@ -340,6 +367,9 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) } } + checkDeadline?.Invoke(); + LogSearchPerformance(_logger, "find", elementsScanned, stopwatch.ElapsedMilliseconds, elementInfos.Count); + if (scanLimitReached) { return UIAutomationResult.CreateFailure( @@ -418,6 +448,19 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) return UIAutomationResult.CreateSuccessCompact("find", [.. elementInfos], CreateDiagnosticsWithContext(stopwatch, rootElement, query, elementsScanned, windowTitle, query.WindowHandle, usedContentView)); }, cancellationToken); } + catch (SearchDeadlineExceededException ex) + { + return UIAutomationResult.CreateFailure( + "find", + UIAutomationErrorType.Timeout, + ex.Message, + CreateDiagnostics(stopwatch, query) with + { + ElementsScanned = elementsScanned, + WindowHandle = query.WindowHandle, + ElapsedBeforeTimeout = stopwatch.ElapsedMilliseconds + }); + } catch (COMException ex) { LogFindElementsError(_logger, ex); @@ -453,12 +496,14 @@ private bool FindElementsWithCachedFilter( int maxDepth, bool visibleOnly, BoundingRect? regionFilter, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + Action? checkDeadline = null) { var scanned = 0; var matches = matchCount; try { + checkDeadline?.Invoke(); var cacheRequest = Uia.CreateElementCacheRequest(UIA.TreeScope.TreeScope_Element); cacheRequest.AddProperty(UIA3PropertyIds.IsControlElement); cacheRequest.TreeFilter = Uia.TrueCondition; @@ -504,12 +549,14 @@ bool Visit(UIA.IUIAutomationElement element, int depth) if (query.ExactDepth.HasValue) { cancellationToken.ThrowIfCancellationRequested(); + checkDeadline?.Invoke(); if (elementsScanned >= MaxElementsToScan) { return true; } Visit(rootElement.BuildUpdatedCache(cacheRequest), 0); + checkDeadline?.Invoke(); if (query.ExactDepth.Value == 0) { return false; // Root-only scope is provably complete without navigation. @@ -526,7 +573,8 @@ bool Visit(UIA.IUIAutomationElement element, int depth) query.ExactDepth.HasValue ? Math.Min(query.ExactDepth.Value, maxDepth) : Math.Max(1, maxDepth), - cancellationToken); + cancellationToken, + checkDeadline); return outcome.LimitReached; } catch (Exception ex) when (COMExceptionHelper.IsExpectedElementTraversalFailure(ex)) @@ -541,6 +589,22 @@ bool Visit(UIA.IUIAutomationElement element, int depth) } } + private sealed class SearchDeadlineExceededException(int timeoutMs) : Exception( + $"Search stopped after {timeoutMs}ms before it completed. " + + "Absence or uniqueness could not be established. Narrow the search or increase timeoutMs."); + + private static Action CreateSearchDeadlineCheck( + Stopwatch stopwatch, + int timeoutMs, + CancellationToken cancellationToken) => () => + { + cancellationToken.ThrowIfCancellationRequested(); + if (stopwatch.ElapsedMilliseconds >= timeoutMs) + { + throw new SearchDeadlineExceededException(timeoutMs); + } + }; + /// /// Evaluates nameContains/namePattern/className against an element's cached properties. /// diff --git a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Tree.cs b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Tree.cs index 411cbc5..1e1ceb3 100644 --- a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Tree.cs +++ b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Tree.cs @@ -208,6 +208,7 @@ public async Task WaitForElementAsync(ElementQuery query, in ArgumentNullException.ThrowIfNull(query); var stopwatch = Stopwatch.StartNew(); + var checkDeadline = CreateSearchDeadlineCheck(stopwatch, timeoutMs, cancellationToken); // Subscribe before the first probe, so a change that lands between probing and sleeping // still wakes us instead of being lost. var signal = await TrySubscribeToStructureChangesAsync(query, cancellationToken).ConfigureAwait(false); @@ -217,7 +218,7 @@ public async Task WaitForElementAsync(ElementQuery query, in return await WaitForFindResultAsync( query, timeoutMs, - () => FindElementsAsync(query with { TimeoutMs = 0 }, cancellationToken), + () => FindElementsOnceAsync(query with { TimeoutMs = 0 }, cancellationToken, checkDeadline), (delay, token) => DelayOrUntilStructureChangedAsync(signal, delay, token), () => stopwatch.ElapsedMilliseconds, cancellationToken).ConfigureAwait(false); @@ -239,6 +240,7 @@ public async Task WaitForElementDisappearAsync(ElementQuery ArgumentNullException.ThrowIfNull(query); var stopwatch = Stopwatch.StartNew(); + var checkDeadline = CreateSearchDeadlineCheck(stopwatch, timeoutMs, cancellationToken); var signal = await TrySubscribeToStructureChangesAsync(query, cancellationToken).ConfigureAwait(false); try @@ -246,7 +248,7 @@ public async Task WaitForElementDisappearAsync(ElementQuery return await WaitForDisappearResultAsync( query, timeoutMs, - () => FindElementsAsync(query with { TimeoutMs = 0 }, cancellationToken), + () => FindElementsOnceAsync(query with { TimeoutMs = 0 }, cancellationToken, checkDeadline), (delay, token) => DelayOrUntilStructureChangedAsync(signal, delay, token), () => stopwatch.ElapsedMilliseconds, cancellationToken).ConfigureAwait(false); diff --git a/src/Sbroenne.WindowsMcp/Models/UIAutomationDiagnostics.cs b/src/Sbroenne.WindowsMcp/Models/UIAutomationDiagnostics.cs index 0275628..4f9fb15 100644 --- a/src/Sbroenne.WindowsMcp/Models/UIAutomationDiagnostics.cs +++ b/src/Sbroenne.WindowsMcp/Models/UIAutomationDiagnostics.cs @@ -167,7 +167,8 @@ public sealed record ElementQuery public bool IncludeChildren { get; init; } /// - /// Timeout in milliseconds for implicit wait (0 = no wait). + /// Shared scan/retry time budget in milliseconds, checked between Windows accessibility calls. + /// Zero performs one node-bounded scan without a time deadline. /// public int TimeoutMs { get; init; } diff --git a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationAdvancedSearchTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationAdvancedSearchTests.cs index 6e91029..546a12a 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationAdvancedSearchTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationAdvancedSearchTests.cs @@ -104,7 +104,7 @@ public async Task Find_WithTimeout_WaitsForElementToAppear() } [Fact] - public async Task Find_WithTimeout_PerformsFinalProbeAtDeadline() + public async Task Find_WithTimeout_DoesNotProbeAfterDeadline() { const string DelayedAutomationId = "DelayedSubmitButton"; @@ -135,13 +135,14 @@ public async Task Find_WithTimeout_PerformsFinalProbeAtDeadline() return observed; }, - (_, _) => throw new InvalidOperationException("The deadline has expired; probe without delay."), + (_, _) => throw new InvalidOperationException("The deadline has expired; do not delay."), () => elapsed, CancellationToken.None); - Assert.True(result.Success, result.ErrorMessage); - Assert.Single(result.Items!); - Assert.Equal(2, probes); + Assert.False(result.Success); + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.Null(result.Items); + Assert.Equal(1, probes); } finally { @@ -152,7 +153,7 @@ public async Task Find_WithTimeout_PerformsFinalProbeAtDeadline() [Fact] [Trait("Category", "RequiresDesktop")] - public async Task Disappear_FinalProbeObservesVisibilityChangeWithoutStructureSignal() + public async Task Disappear_DoesNotProbeVisibilityChangeAfterDeadline() { var query = new ElementQuery { @@ -173,8 +174,7 @@ public async Task Disappear_FinalProbeObservesVisibilityChangeWithoutStructureSi if (probes == 1) { Assert.True(observed.Success, observed.ErrorMessage); - // No event signal is wired to this wait. Visibility must be re-probed - // even when the first provider call used the entire deadline. + // A visibility change after the deadline must not trigger another scan. _fixture.Form!.Invoke(() => _fixture.Form.SetSubmitButtonVisibleForTesting(false)); elapsed = 2001; @@ -185,8 +185,9 @@ public async Task Disappear_FinalProbeObservesVisibilityChangeWithoutStructureSi (_, _) => throw new InvalidOperationException("No sleep is allowed after the deadline."), () => elapsed, CancellationToken.None); - Assert.True(result.Success, result.ErrorMessage); - Assert.Equal(2, probes); + Assert.False(result.Success); + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.Equal(1, probes); } finally { diff --git a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs index 61a4d53..b7ad21a 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs @@ -83,6 +83,53 @@ public async Task Find_ButtonByName_ReturnsButton() Assert.Contains("Submit", result.Items![0].Name ?? string.Empty); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Find_DeadlineExpiresWhileQueued_ReturnsUnfinishedSearch(bool disappear) + { + var started = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var release = new ManualResetEventSlim(); + var blocker = _staThread.ExecuteAsync(() => + { + started.SetResult(); + if (!release.Wait(TimeSpan.FromSeconds(10))) + { + throw new TimeoutException("Test did not release the automation queue."); + } + }); + await started.Task; + Task search; + try + { + var query = new ElementQuery + { + WindowHandle = _windowHandle, + Name = "Submit", + ControlType = "Button", + RequireUnique = true, + TimeoutMs = 25 + }; + search = disappear + ? _automationService.WaitForElementDisappearAsync(query, query.TimeoutMs) + : _automationService.FindElementsAsync(query); + await Task.Delay(100); + } + finally + { + release.Set(); + await blocker; + } + + var result = await search; + Assert.False(result.Success); + Assert.Equal(disappear ? "wait_for_disappear" : "find", result.Action); + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.Contains("before it completed", result.ErrorMessage, StringComparison.Ordinal); + Assert.Equal(0, result.Diagnostics?.ElementsScanned); + Assert.Null(result.Items); + } + [Fact] public async Task Find_ButtonByName_PartialMatch_ReturnsButton() { diff --git a/tests/Sbroenne.WindowsMcp.Tests/Unit/BoundedSearchTraversalTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Unit/BoundedSearchTraversalTests.cs index 131f96c..8578a17 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Unit/BoundedSearchTraversalTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Unit/BoundedSearchTraversalTests.cs @@ -193,5 +193,70 @@ public void ExhaustedSmallTree_IsProvablyCompleteBeforeBudget() Assert.False(outcome.LimitReached); } + [Fact] + public void ExpiredDeadline_DoesNotFetchAnyProviderNode() + { + Assert.Throws(() => BoundedSearchTraversal.Walk( + new Node(-1), + _ => throw new InvalidOperationException("Expired search must not fetch children."), + _ => throw new InvalidOperationException("Expired search must not fetch siblings."), + _ => true, (_, _) => false, + 2000, 20, CancellationToken.None, + checkDeadline: () => throw new TimeoutException())); + } + + [Fact] + public void SlowProvider_StopsBeforeVisitingOrFetchingMoreNodes() + { + var expired = false; + var fetched = 0; + Assert.Throws(() => BoundedSearchTraversal.Walk( + new Node(-1), + _ => + { + fetched++; + expired = true; + return new Node(0); + }, + _ => throw new InvalidOperationException("Expired search must not fetch siblings."), + _ => true, + (_, _) => throw new InvalidOperationException("Expired search must not visit a candidate."), + 2000, 20, CancellationToken.None, + checkDeadline: CheckDeadline)); + Assert.Equal(1, fetched); + + void CheckDeadline() + { + if (expired) + { + throw new TimeoutException(); + } + } + } + + [Fact] + public void DeadlineDuringVisit_DoesNotReturnPartialSuccess() + { + var expired = false; + Assert.Throws(() => BoundedSearchTraversal.Walk( + new Node(-1), + _ => new Node(0), + _ => throw new InvalidOperationException("Expired search must not fetch siblings."), + _ => true, + (_, _) => + { + expired = true; + return true; + }, + 2000, 20, CancellationToken.None, + checkDeadline: () => + { + if (expired) + { + throw new TimeoutException(); + } + })); + } + private sealed record Node(int Index, bool IsControl = true); } diff --git a/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs index c8118fe..fcf170b 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs @@ -8,7 +8,7 @@ public sealed class FindDeadlineTests private static readonly int[] ExpectedDelays = [50, 75]; [Fact] - public async Task ProbeCrossingDeadline_ReceivesOneFreshFinalProbe() + public async Task ProbeCrossingDeadline_DoesNotStartAnotherProbe() { long elapsed = 0; var probes = 0; @@ -26,12 +26,13 @@ public async Task ProbeCrossingDeadline_ReceivesOneFreshFinalProbe() () => elapsed, CancellationToken.None); - Assert.True(result.Success); - Assert.Equal(2, probes); + Assert.False(result.Success); + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.Equal(1, probes); } [Fact] - public async Task DelayIsClamped_AndFinalProbeStartsAtDeadline() + public async Task DelayIsClamped_AndNoProbeStartsAtDeadline() { long elapsed = 0; var probeTimes = new List(); @@ -53,29 +54,34 @@ public async Task DelayIsClamped_AndFinalProbeStartsAtDeadline() CancellationToken.None); Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); - Assert.Equal(new long[] { 0, 50, 125 }, probeTimes); + Assert.Equal(new long[] { 0, 50 }, probeTimes); Assert.Equal(ExpectedDelays, delays); } - [Fact] - public async Task SlowFinalProbe_DoesNotStartAdditionalProbes() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task AlreadyExpiredDeadline_DoesNotStartProviderCall(bool disappear) { long elapsed = 2000; var probes = 0; - var result = await UIAutomationService.WaitForFindResultAsync( - new ElementQuery(), 2000, - () => - { - probes++; - elapsed += 500; - return Task.FromResult(Missing()); - }, - (_, _) => throw new InvalidOperationException("Must not delay after final probe."), - () => elapsed, - CancellationToken.None); + Task Probe() + { + probes++; + elapsed += 500; + return Task.FromResult(Missing()); + } + Task Wait(int _, CancellationToken token) => + throw new InvalidOperationException("Must not delay after deadline."); + var result = disappear + ? await UIAutomationService.WaitForDisappearResultAsync( + new ElementQuery(), 2000, Probe, Wait, () => elapsed, CancellationToken.None) + : await UIAutomationService.WaitForFindResultAsync( + new ElementQuery(), 2000, Probe, Wait, () => elapsed, CancellationToken.None); Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); - Assert.Equal(1, probes); + Assert.Equal(0, probes); + Assert.Contains("before it could start", result.ErrorMessage, StringComparison.Ordinal); } [Fact] @@ -115,7 +121,7 @@ public async Task NonRetryableResult_IsNotHiddenByFinalProbe(string errorType) return Task.FromResult(UIAutomationResult.CreateFailure("find", errorType, "Stop.")); }, (_, _) => throw new InvalidOperationException("Must not retry this result."), - () => 2000, + () => 0, CancellationToken.None); Assert.Equal(errorType, result.ErrorType); @@ -126,7 +132,7 @@ private static UIAutomationResult Missing() => UIAutomationResult.CreateFailure("find", UIAutomationErrorType.ElementNotFound, "Not present."); [Fact] - public async Task Disappearance_FinalProbeObservesChangeWithoutEvent() + public async Task Disappearance_ObservesChangeBeforeDeadlineWithoutEvent() { long elapsed = 0; var probes = 0; @@ -135,7 +141,7 @@ public async Task Disappearance_FinalProbeObservesChangeWithoutEvent() () => { probes++; - return Task.FromResult(elapsed < 125 + return Task.FromResult(elapsed < 50 ? new UIAutomationResult { Success = true, @@ -154,8 +160,8 @@ public async Task Disappearance_FinalProbeObservesChangeWithoutEvent() Assert.True(result.Success); Assert.Equal("wait_for_disappear", result.Action); - Assert.Equal(125, elapsed); - Assert.Equal(3, probes); + Assert.Equal(50, elapsed); + Assert.Equal(2, probes); } [Fact] @@ -166,10 +172,54 @@ public async Task Disappearance_IncompleteSearchDoesNotBecomeSuccessAtDeadline() () => Task.FromResult(UIAutomationResult.CreateFailure( "find", UIAutomationErrorType.SearchIncomplete, "Unknown remaining elements.")), (_, _) => throw new InvalidOperationException("Incomplete search must not be retried."), - () => 125, + () => 0, CancellationToken.None); Assert.False(result.Success); Assert.Equal(UIAutomationErrorType.SearchIncomplete, result.ErrorType); } + + [Fact] + public async Task Timeout_PreservesLastSearchDiagnostics() + { + long elapsed = 0; + var result = await UIAutomationService.WaitForFindResultAsync( + new ElementQuery(), 125, + () => Task.FromResult(UIAutomationResult.CreateFailure( + "find", UIAutomationErrorType.ElementNotFound, "Not present.", + new UIAutomationDiagnostics { DurationMs = 10, ElementsScanned = 42, WindowTitle = "Installer" })), + (delay, _) => + { + elapsed += delay; + return Task.CompletedTask; + }, + () => elapsed, + CancellationToken.None); + + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.Equal(125, result.Diagnostics?.DurationMs); + Assert.Equal(42, result.Diagnostics?.ElementsScanned); + Assert.Equal("Installer", result.Diagnostics?.WindowTitle); + } + + [Fact] + public async Task Timeout_DistinguishesMissingDialogFromMissingElement() + { + long elapsed = 0; + var result = await UIAutomationService.WaitForFindResultAsync( + new ElementQuery { Scope = "active_dialog" }, 125, + () => Task.FromResult(UIAutomationResult.CreateFailure( + "find", UIAutomationErrorType.WindowNotFound, + "No visible enabled dialog is currently owned by the requested window.")), + (delay, _) => + { + elapsed += delay; + return Task.CompletedTask; + }, + () => elapsed, + CancellationToken.None); + + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.Contains("No visible enabled dialog", result.ErrorMessage, StringComparison.Ordinal); + } } From a32c50edbff5dbf477dc3573be452b40c99053a8 Mon Sep 17 00:00:00 2001 From: Stefan Broenner Date: Wed, 30 Sep 2026 18:40:56 +0200 Subject: [PATCH 2/2] fix: check search deadlines throughout provider operations Bound modal lookup, matching, framework discovery, child conversion and element identity reads. Preserve normalized dialog diagnostics and isolate node-budget tests from elapsed time. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Automation/ElementIdGenerator.cs | 43 ++++++++--- .../Automation/UIA3Extensions.cs | 43 ++++++++--- .../Automation/UIAutomationService.Find.cs | 73 +++++++++++------- .../Automation/UIAutomationService.Helpers.cs | 75 ++++++++++++------- .../Automation/UIAutomationService.Text.cs | 13 ++-- .../Integration/UIAutomationWinFormsTests.cs | 10 ++- .../Integration/UIFindToolIntegrationTests.cs | 18 +++++ .../UISearchLimitIntegrationTests.cs | 2 +- .../Unit/FindDeadlineTests.cs | 38 ++++++++++ 9 files changed, 230 insertions(+), 85 deletions(-) diff --git a/src/Sbroenne.WindowsMcp/Automation/ElementIdGenerator.cs b/src/Sbroenne.WindowsMcp/Automation/ElementIdGenerator.cs index d7520d2..7c36936 100644 --- a/src/Sbroenne.WindowsMcp/Automation/ElementIdGenerator.cs +++ b/src/Sbroenne.WindowsMcp/Automation/ElementIdGenerator.cs @@ -30,31 +30,45 @@ public static string GenerateId(UIA.IUIAutomationElement element, UIA.IUIAutomat Generate(element, rootElement, cached: false); /// Registers a live element using cached identity properties when available. - public static string GenerateFastId(UIA.IUIAutomationElement element, UIA.IUIAutomationElement rootElement) => - Generate(element, rootElement, cached: true); + public static string GenerateFastId( + UIA.IUIAutomationElement element, + UIA.IUIAutomationElement rootElement, + Action? checkDeadline = null) => + Generate(element, rootElement, cached: true, checkDeadline); /// Registers a live element when no property cache was requested. - public static string GenerateFastIdFromCurrent(UIA.IUIAutomationElement element, UIA.IUIAutomationElement rootElement) => - Generate(element, rootElement, cached: false); - - private static string Generate(UIA.IUIAutomationElement element, UIA.IUIAutomationElement rootElement, bool cached) + public static string GenerateFastIdFromCurrent( + UIA.IUIAutomationElement element, + UIA.IUIAutomationElement rootElement, + Action? checkDeadline = null) => + Generate(element, rootElement, cached: false, checkDeadline); + + private static string Generate( + UIA.IUIAutomationElement element, + UIA.IUIAutomationElement rootElement, + bool cached, + Action? checkDeadline = null) { ArgumentNullException.ThrowIfNull(element); ArgumentNullException.ThrowIfNull(rootElement); try { + checkDeadline?.Invoke(); nint handle; try { handle = cached ? rootElement.GetCachedNativeWindowHandle() : rootElement.GetNativeWindowHandle(); + checkDeadline?.Invoke(); } catch (Exception ex) when (COMExceptionHelper.IsExpectedElementFailure(ex)) { + checkDeadline?.Invoke(); handle = rootElement.GetNativeWindowHandle(); + checkDeadline?.Invoke(); } if (handle == nint.Zero) { - handle = GetTopLevelWindowHandle(element); + handle = GetTopLevelWindowHandle(element, checkDeadline); } else { @@ -65,17 +79,23 @@ private static string Generate(UIA.IUIAutomationElement element, UIA.IUIAutomati int providerProcessId; try { + checkDeadline?.Invoke(); runtimeId = cached ? (int[]?)element.GetCachedPropertyValue(UIA3PropertyIds.RuntimeId) : element.GetRuntimeId(); + checkDeadline?.Invoke(); providerProcessId = cached ? (int)element.GetCachedPropertyValue(UIA3PropertyIds.ProcessId) : element.CurrentProcessId; + checkDeadline?.Invoke(); } catch (Exception ex) when (COMExceptionHelper.IsExpectedElementFailure(ex)) { + checkDeadline?.Invoke(); runtimeId = element.GetRuntimeId(); + checkDeadline?.Invoke(); providerProcessId = element.CurrentProcessId; + checkDeadline?.Invoke(); } var runtime = runtimeId is { Length: > 0 } ? string.Join(".", runtimeId) : "0"; @@ -133,19 +153,24 @@ private static string Generate(UIA.IUIAutomationElement element, UIA.IUIAutomati return null; } - private static nint GetTopLevelWindowHandle(UIA.IUIAutomationElement element) + private static nint GetTopLevelWindowHandle(UIA.IUIAutomationElement element, Action? checkDeadline = null) { var current = element; + checkDeadline?.Invoke(); var desktop = UIA3Automation.Instance.RootElement; - while (current is not null && !current.IsSameElement(desktop)) + checkDeadline?.Invoke(); + while (current is not null && !current.IsSameElement(desktop, checkDeadline)) { + checkDeadline?.Invoke(); var currentHandle = current.GetNativeWindowHandle(); + checkDeadline?.Invoke(); if (currentHandle != nint.Zero) { return NativeMethods.GetAncestor(currentHandle, NativeConstants.GA_ROOT); } current = current.GetParent(); + checkDeadline?.Invoke(); } return nint.Zero; diff --git a/src/Sbroenne.WindowsMcp/Automation/UIA3Extensions.cs b/src/Sbroenne.WindowsMcp/Automation/UIA3Extensions.cs index 64be005..3525021 100644 --- a/src/Sbroenne.WindowsMcp/Automation/UIA3Extensions.cs +++ b/src/Sbroenne.WindowsMcp/Automation/UIA3Extensions.cs @@ -410,7 +410,7 @@ public static bool SupportsPattern(this UIA.IUIAutomationElement element, int pa /// /// Gets all supported pattern IDs by checking each known pattern. /// - public static int[] GetSupportedPatternIds(this UIA.IUIAutomationElement element) + public static int[] GetSupportedPatternIds(this UIA.IUIAutomationElement element, Action? checkDeadline = null) { ArgumentNullException.ThrowIfNull(element); var supportedPatterns = new List(); @@ -456,7 +456,10 @@ public static int[] GetSupportedPatternIds(this UIA.IUIAutomationElement element foreach (var patternId in allPatternIds) { - if (element.SupportsPattern(patternId)) + checkDeadline?.Invoke(); + var supported = element.SupportsPattern(patternId); + checkDeadline?.Invoke(); + if (supported) { supportedPatterns.Add(patternId); } @@ -468,10 +471,10 @@ public static int[] GetSupportedPatternIds(this UIA.IUIAutomationElement element /// /// Gets all supported pattern names. /// - public static string[] GetSupportedPatternNames(this UIA.IUIAutomationElement element) + public static string[] GetSupportedPatternNames(this UIA.IUIAutomationElement element, Action? checkDeadline = null) { ArgumentNullException.ThrowIfNull(element); - var ids = element.GetSupportedPatternIds(); + var ids = element.GetSupportedPatternIds(checkDeadline); return ids.Select(UIA3PatternIds.ToName).ToArray(); } @@ -590,13 +593,17 @@ public static string[] GetSupportedPatternNames(this UIA.IUIAutomationElement el /// /// Tries to get the Value pattern's current value. /// - public static string? TryGetValue(this UIA.IUIAutomationElement element) + public static string? TryGetValue(this UIA.IUIAutomationElement element, Action? checkDeadline = null) { ArgumentNullException.ThrowIfNull(element); try { + checkDeadline?.Invoke(); var pattern = element.GetPattern(UIA3PatternIds.Value); - return pattern?.CurrentValue; + checkDeadline?.Invoke(); + var value = pattern?.CurrentValue; + checkDeadline?.Invoke(); + return value; } catch (COMException ex) when (COMExceptionHelper.IsExpectedElementFailure(ex)) @@ -677,18 +684,22 @@ public static bool TryToggle(this UIA.IUIAutomationElement element) /// /// Gets the toggle state. /// - public static string? GetToggleState(this UIA.IUIAutomationElement element) + public static string? GetToggleState(this UIA.IUIAutomationElement element, Action? checkDeadline = null) { ArgumentNullException.ThrowIfNull(element); try { + checkDeadline?.Invoke(); var pattern = element.GetPattern(UIA3PatternIds.Toggle); + checkDeadline?.Invoke(); if (pattern == null) { return null; } - return pattern.CurrentToggleState switch + var state = pattern.CurrentToggleState; + checkDeadline?.Invoke(); + return state switch { UIA.ToggleState.ToggleState_Off => "Off", UIA.ToggleState.ToggleState_On => "On", @@ -705,13 +716,17 @@ public static bool TryToggle(this UIA.IUIAutomationElement element) /// /// Gets selection state using the SelectionItem pattern. /// - public static string? GetSelectionStateName(this UIA.IUIAutomationElement element) + public static string? GetSelectionStateName(this UIA.IUIAutomationElement element, Action? checkDeadline = null) { ArgumentNullException.ThrowIfNull(element); try { + checkDeadline?.Invoke(); var pattern = element.GetPattern(UIA3PatternIds.SelectionItem); - return pattern is null ? null : pattern.CurrentIsSelected != 0 ? "On" : "Off"; + checkDeadline?.Invoke(); + var state = pattern is null ? null : pattern.CurrentIsSelected != 0 ? "On" : "Off"; + checkDeadline?.Invoke(); + return state; } catch (COMException ex) when (COMExceptionHelper.IsExpectedElementFailure(ex)) { @@ -850,7 +865,10 @@ public static bool TryScrollIntoView(this UIA.IUIAutomationElement element) /// /// Compares two elements by runtime ID. /// - public static bool IsSameElement(this UIA.IUIAutomationElement element, UIA.IUIAutomationElement? other) + public static bool IsSameElement( + this UIA.IUIAutomationElement element, + UIA.IUIAutomationElement? other, + Action? checkDeadline = null) { ArgumentNullException.ThrowIfNull(element); if (other == null) @@ -860,8 +878,11 @@ public static bool IsSameElement(this UIA.IUIAutomationElement element, UIA.IUIA try { + checkDeadline?.Invoke(); var id1 = element.GetRuntimeId(); + checkDeadline?.Invoke(); var id2 = other.GetRuntimeId(); + checkDeadline?.Invoke(); if (id1 == null || id2 == null) { diff --git a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs index 2a4d9ec..f89d1e2 100644 --- a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs +++ b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Find.cs @@ -148,6 +148,9 @@ private async Task FindElementsOnceAsync( { var stopwatch = Stopwatch.StartNew(); var elementsScanned = 0; + var normalizedScope = string.IsNullOrWhiteSpace(query.Scope) + ? "window" + : query.Scope.Trim().ToLowerInvariant(); if (query.RequireUnique && query.FoundIndex != 1) { @@ -188,9 +191,6 @@ private async Task FindElementsOnceAsync( } else { - var normalizedScope = string.IsNullOrWhiteSpace(query.Scope) - ? "window" - : query.Scope.Trim().ToLowerInvariant(); if (normalizedScope is not ("window" or "active_dialog")) { return UIAutomationResult.CreateFailure( @@ -201,17 +201,14 @@ private async Task FindElementsOnceAsync( } rootElement = normalizedScope == "active_dialog" - ? GetActiveDialogRoot(query.WindowHandle) + ? GetActiveDialogRoot(query.WindowHandle, checkDeadline) : GetRootElement(query.WindowHandle); } checkDeadline?.Invoke(); if (rootElement == null) { - var requestedActiveDialog = string.Equals( - query.Scope, - "active_dialog", - StringComparison.OrdinalIgnoreCase); + var requestedActiveDialog = normalizedScope == "active_dialog"; return UIAutomationResult.CreateFailure( "find", UIAutomationErrorType.WindowNotFound, @@ -269,7 +266,8 @@ private async Task FindElementsOnceAsync( var maxResults = query.RequireUnique ? 2 : query.FoundIndex > 1 ? query.FoundIndex : 100; // Detect framework and get optimal search strategy - var strategy = GetFrameworkStrategy(rootElement); + var detectedFramework = DetectFramework(rootElement, checkDeadline); + var strategy = GetFrameworkStrategy(detectedFramework); checkDeadline?.Invoke(); // Resolve visibility filtering: explicit caller value wins; otherwise exclude @@ -379,7 +377,7 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) $"Search incomplete: checked {elementsScanned} candidates within the {MaxElementsToScan}-element scan budget. " + "The scan budget was reached or the provider changed before the search request was resolved. " + "Remaining candidates were not checked.", - CreateDiagnosticsWithContext(stopwatch, rootElement, query, elementsScanned, windowTitle, query.WindowHandle, usedContentView)); + CreateDiagnosticsWithContext(stopwatch, rootElement, query, elementsScanned, windowTitle, query.WindowHandle, usedContentView, checkDeadline, detectedFramework)); } if (elementInfos.Count == 0) @@ -388,7 +386,7 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) "find", UIAutomationErrorType.ElementNotFound, BuildNotFoundMessage(query), - CreateDiagnosticsWithContext(stopwatch, rootElement, query, elementsScanned, windowTitle, query.WindowHandle, usedContentView)); + CreateDiagnosticsWithContext(stopwatch, rootElement, query, elementsScanned, windowTitle, query.WindowHandle, usedContentView, checkDeadline, detectedFramework)); } // Sort by proximity to reference element if nearElement specified @@ -428,7 +426,9 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) elementsScanned, windowTitle, query.WindowHandle, - usedContentView) with + usedContentView, + checkDeadline, + detectedFramework) with { MultipleMatches = elementInfos .Take(10) @@ -446,7 +446,7 @@ void RunScan(UIA.IUIAutomationCondition scanCondition) } // Always use compact format for Find to reduce token count by ~70% - return UIAutomationResult.CreateSuccessCompact("find", [.. elementInfos], CreateDiagnosticsWithContext(stopwatch, rootElement, query, elementsScanned, windowTitle, query.WindowHandle, usedContentView)); + return UIAutomationResult.CreateSuccessCompact("find", [.. elementInfos], CreateDiagnosticsWithContext(stopwatch, rootElement, query, elementsScanned, windowTitle, query.WindowHandle, usedContentView, checkDeadline, detectedFramework)); }, cancellationToken); } catch (SearchDeadlineExceededException ex) @@ -515,12 +515,12 @@ bool Visit(UIA.IUIAutomationElement element, int depth) if ((query.ExactDepth.HasValue && (depth != query.ExactDepth.Value || (depth > 0 && element.GetCachedPropertyValue(UIA3PropertyIds.IsControlElement) is not true))) || - !MatchesCondition(element, condition) || !MatchesAdvancedCriteriaCached(element, query)) + !MatchesCondition(element, condition, checkDeadline) || !MatchesAdvancedCriteriaCached(element, query)) { return false; } - var info = ConvertToElementInfo(element, rootElement, _coordinateConverter, fromCachedElement: true); + var info = ConvertToElementInfo(element, rootElement, _coordinateConverter, fromCachedElement: true, checkDeadline: checkDeadline); if (info is null || (visibleOnly && info.IsOffscreen) || (query.EnabledOnly == true && !info.IsEnabled) || (regionFilter is not null && !IntersectsRegion(info.BoundingRect, regionFilter))) @@ -536,7 +536,7 @@ bool Visit(UIA.IUIAutomationElement element, int depth) if (query.IncludeChildren) { - info = info with { Children = GetChildren(element, rootElement) }; + info = info with { Children = GetChildren(element, rootElement, checkDeadline: checkDeadline) }; } results.Add(info); @@ -606,6 +606,14 @@ private static Action CreateSearchDeadlineCheck( } }; + internal static T ExecuteSearchProviderCall(Func providerCall, Action? checkDeadline) + { + checkDeadline?.Invoke(); + var result = providerCall(); + checkDeadline?.Invoke(); + return result; + } + /// /// Evaluates nameContains/namePattern/className against an element's cached properties. /// @@ -685,12 +693,16 @@ internal static bool MatchesNativeSearchProperties( } /// Tests only the candidate itself, never a provider-normalized ancestor. - private static bool MatchesCondition(UIA.IUIAutomationElement element, UIA.IUIAutomationCondition condition) + private static bool MatchesCondition( + UIA.IUIAutomationElement element, + UIA.IUIAutomationCondition condition, + Action? checkDeadline = null) { try { - var result = element.FindFirst(UIA.TreeScope.TreeScope_Element, condition); - return result != null && element.IsSameElement(result); + var result = ExecuteSearchProviderCall( + () => element.FindFirst(UIA.TreeScope.TreeScope_Element, condition), checkDeadline); + return result != null && element.IsSameElement(result, checkDeadline); } catch (Exception ex) when (COMExceptionHelper.IsExpectedElementTraversalFailure(ex)) { @@ -735,35 +747,38 @@ private static string BuildNotFoundMessage(ElementQuery query) return $"No element found matching: {string.Join(", ", criteria)}"; } - private UIA.IUIAutomationElement? GetActiveDialogRoot(string? windowHandle) + private UIA.IUIAutomationElement? GetActiveDialogRoot(string? windowHandle, Action? checkDeadline = null) { if (!WindowHandleParser.TryParse(windowHandle, out var parentHandle)) { return null; } + checkDeadline?.Invoke(); var popupHandle = NativeMethods.GetWindow(parentHandle, NativeConstants.GW_ENABLEDPOPUP); if (popupHandle != IntPtr.Zero && popupHandle != parentHandle && NativeMethods.IsWindowVisible(popupHandle)) { - return Uia.ElementFromHandle(popupHandle); + return ExecuteSearchProviderCall(() => Uia.ElementFromHandle(popupHandle), checkDeadline); } - var parent = Uia.ElementFromHandle(parentHandle); + var parent = ExecuteSearchProviderCall(() => Uia.ElementFromHandle(parentHandle), checkDeadline); if (parent == null) { return null; } - var windows = parent.FindAll( - UIA.TreeScope.TreeScope_Children, - Uia.CreatePropertyCondition(UIA3PropertyIds.ControlType, UIA3ControlTypeIds.Window)); - for (var index = 0; index < (windows?.Length ?? 0); index++) + var condition = Uia.CreatePropertyCondition(UIA3PropertyIds.ControlType, UIA3ControlTypeIds.Window); + var windows = ExecuteSearchProviderCall( + () => parent.FindAll(UIA.TreeScope.TreeScope_Children, condition), checkDeadline); + var count = ExecuteSearchProviderCall(() => windows?.Length ?? 0, checkDeadline); + for (var index = 0; index < count; index++) { - var candidate = windows!.GetElement(index); - var pattern = candidate.GetPattern(UIA3PatternIds.Window); - if (pattern?.CurrentIsModal != 0) + var candidate = ExecuteSearchProviderCall(() => windows!.GetElement(index), checkDeadline); + var pattern = ExecuteSearchProviderCall( + () => candidate.GetPattern(UIA3PatternIds.Window), checkDeadline); + if (ExecuteSearchProviderCall(() => pattern?.CurrentIsModal != 0, checkDeadline)) { return candidate; } diff --git a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Helpers.cs b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Helpers.cs index 806a2d6..dc37528 100644 --- a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Helpers.cs +++ b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Helpers.cs @@ -249,6 +249,7 @@ private static int GetControlTypeId(string controlTypeName) /// Optional child elements. /// If true, element was retrieved with a cache request (use cached properties). If false, use current properties. /// Whether to retain Chromium layout containers that expose a direct action. + /// Optional shared search deadline check. /// The element info, or null if conversion fails. internal static UIElementInfo? ConvertToElementInfo( UIA.IUIAutomationElement element, @@ -256,14 +257,16 @@ private static int GetControlTypeId(string controlTypeName) CoordinateConverter coordinateConverter, UIElementInfo[]? children = null, bool fromCachedElement = false, - bool detectSemanticLayoutActions = false) + bool detectSemanticLayoutActions = false, + Action? checkDeadline = null) { try { + checkDeadline?.Invoke(); // Use cached rect for tree/find operations (elements may go stale), current for actions var rect = fromCachedElement ? element.CachedBoundingRectangle - : element.CurrentBoundingRectangle; + : ExecuteSearchProviderCall(() => element.CurrentBoundingRectangle, checkDeadline); var boundingRect = new BoundingRect { @@ -278,19 +281,19 @@ private static int GetControlTypeId(string controlTypeName) // Use centralized element ID generation with short IDs var elementId = fromCachedElement - ? ElementIdGenerator.GenerateFastId(element, rootElement) - : ElementIdGenerator.GenerateFastIdFromCurrent(element, rootElement); + ? ElementIdGenerator.GenerateFastId(element, rootElement, checkDeadline) + : ElementIdGenerator.GenerateFastIdFromCurrent(element, rootElement, checkDeadline); // Get properties - cached when tree walking/finding, current for actions - string? name = fromCachedElement ? element.GetCachedName() : element.GetName(); - string? automationId = fromCachedElement ? element.GetCachedAutomationId() : element.GetAutomationId(); - string? controlType = fromCachedElement ? element.GetCachedControlTypeName() : element.GetControlTypeName(); + string? name = fromCachedElement ? element.GetCachedName() : ExecuteSearchProviderCall(element.GetName, checkDeadline); + string? automationId = fromCachedElement ? element.GetCachedAutomationId() : ExecuteSearchProviderCall(element.GetAutomationId, checkDeadline); + string? controlType = fromCachedElement ? element.GetCachedControlTypeName() : ExecuteSearchProviderCall(element.GetControlTypeName, checkDeadline); bool isEnabled = fromCachedElement ? element.GetCachedIsEnabled() - : element.CurrentIsEnabled != 0; + : ExecuteSearchProviderCall(() => element.CurrentIsEnabled != 0, checkDeadline); bool isOffscreen = fromCachedElement ? element.GetCachedIsOffscreen() - : element.CurrentIsOffscreen != 0; + : ExecuteSearchProviderCall(() => element.CurrentIsOffscreen != 0, checkDeadline); var isSemanticLayoutCandidate = fromCachedElement && detectSemanticLayoutActions && @@ -309,25 +312,26 @@ controlType is "Pane" or "Group" && MonitorRelativeRect = monitorRelativeRect, MonitorIndex = monitorIndex, ClickablePoint = ClickablePoint.FromCenter(monitorRelativeRect, monitorIndex), - SupportedPatterns = fromCachedElement ? [] : element.GetSupportedPatternNames(), + SupportedPatterns = fromCachedElement ? [] : element.GetSupportedPatternNames(checkDeadline), IsSemanticLayoutOnly = isSemanticLayoutCandidate && !isDirectlyActionable, IsDirectlyActionable = isDirectlyActionable, HasDeveloperIdentifier = !string.IsNullOrWhiteSpace(automationId), Value = (!fromCachedElement || controlType is "Edit" or "Spinner") && - (controlType != "Edit" || CanReadDirectFieldValue(element)) - ? element.TryGetValue() + (controlType != "Edit" || CanReadDirectFieldValue(element, checkDeadline)) + ? element.TryGetValue(checkDeadline) : null, ToggleState = string.Equals(controlType, "RadioButton", StringComparison.Ordinal) - ? element.GetSelectionStateName() + ? element.GetSelectionStateName(checkDeadline) : !fromCachedElement || string.Equals(controlType, "CheckBox", StringComparison.Ordinal) - ? element.GetToggleState() + ? element.GetToggleState(checkDeadline) : null, IsEnabled = isEnabled, IsOffscreen = isOffscreen, Children = children }; + checkDeadline?.Invoke(); return info; } @@ -349,25 +353,33 @@ private static bool HasCachedDirectAction(UIA.IUIAutomationElement element) element.GetCachedPropertyValue(UIA3PropertyIds.IsValuePatternAvailable) is true; } - private UIElementInfo[]? GetChildren(UIA.IUIAutomationElement element, UIA.IUIAutomationElement rootElement, int maxChildren = 100) + private UIElementInfo[]? GetChildren( + UIA.IUIAutomationElement element, + UIA.IUIAutomationElement rootElement, + int maxChildren = 100, + Action? checkDeadline = null) { var children = new List(); var walker = Uia.ControlViewWalker; - var child = walker.GetFirstChildElement(element); + var child = ExecuteSearchProviderCall( + () => walker.GetFirstChildElement(element), checkDeadline); var count = 0; while (child != null && count < maxChildren) { try { - var childInfo = ConvertToElementInfo(child, rootElement, _coordinateConverter); + var childInfo = ConvertToElementInfo( + child, rootElement, _coordinateConverter, checkDeadline: checkDeadline); if (childInfo != null) { children.Add(childInfo); } count++; - child = walker.GetNextSiblingElement(child); + child = count < maxChildren + ? ExecuteSearchProviderCall(() => walker.GetNextSiblingElement(child), checkDeadline) + : null; } catch (Exception ex) when (COMExceptionHelper.IsExpectedElementFailure(ex)) { @@ -435,7 +447,9 @@ private static UIAutomationDiagnostics CreateDiagnosticsWithContext( int elementsScanned, string? windowTitle, string? windowHandle, - bool? usedContentView = null) + bool? usedContentView = null, + Action? checkDeadline = null, + string? detectedFramework = null) { return new UIAutomationDiagnostics { @@ -444,7 +458,7 @@ private static UIAutomationDiagnostics CreateDiagnosticsWithContext( ElementsScanned = elementsScanned, WindowTitle = windowTitle, WindowHandle = windowHandle, - DetectedFramework = DetectFramework(rootElement), + DetectedFramework = detectedFramework ?? DetectFramework(rootElement, checkDeadline), UsedContentView = usedContentView }; } @@ -452,12 +466,12 @@ private static UIAutomationDiagnostics CreateDiagnosticsWithContext( /// /// Detects the UI framework of the given element. /// - private static string? DetectFramework(UIA.IUIAutomationElement element) + private static string? DetectFramework(UIA.IUIAutomationElement element, Action? checkDeadline = null) { try { - var frameworkId = element.GetFrameworkId(); - var className = element.GetClassName(); + var frameworkId = ExecuteSearchProviderCall(element.GetFrameworkId, checkDeadline); + var className = ExecuteSearchProviderCall(element.GetClassName, checkDeadline); // Check for Chromium-based apps (Electron, Chrome, Edge, etc.) if (className?.StartsWith("Chrome", StringComparison.OrdinalIgnoreCase) == true || @@ -487,7 +501,7 @@ private static UIAutomationDiagnostics CreateDiagnosticsWithContext( if (frameworkId == "Win32" && className != null) { var walker = UIA3Automation.Instance.ControlViewWalker; - var child = walker.GetFirstChildElement(element); + var child = ExecuteSearchProviderCall(() => walker.GetFirstChildElement(element), checkDeadline); var maxChildren = 10; // Limit scan depth for performance var childCount = 0; @@ -495,8 +509,8 @@ private static UIAutomationDiagnostics CreateDiagnosticsWithContext( { try { - var childClassName = child.GetClassName(); - var childFramework = child.GetFrameworkId(); + var childClassName = ExecuteSearchProviderCall(child.GetClassName, checkDeadline); + var childFramework = ExecuteSearchProviderCall(child.GetFrameworkId, checkDeadline); // Check for Chromium if (childClassName?.StartsWith("Chrome", StringComparison.OrdinalIgnoreCase) == true || @@ -524,8 +538,10 @@ private static UIAutomationDiagnostics CreateDiagnosticsWithContext( return "WinUI"; } - child = walker.GetNextSiblingElement(child); childCount++; + child = childCount < maxChildren + ? ExecuteSearchProviderCall(() => walker.GetNextSiblingElement(child), checkDeadline) + : null; } catch (Exception ex) when (COMExceptionHelper.IsExpectedElementFailure(ex)) { @@ -549,8 +565,11 @@ private static UIAutomationDiagnostics CreateDiagnosticsWithContext( /// A framework strategy with optimal search parameters. internal static FrameworkStrategy GetFrameworkStrategy(UIA.IUIAutomationElement element) { - var framework = DetectFramework(element); + return GetFrameworkStrategy(DetectFramework(element)); + } + private static FrameworkStrategy GetFrameworkStrategy(string? framework) + { return framework switch { "Chromium/Electron" => FrameworkStrategy.Electron, diff --git a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Text.cs b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Text.cs index f53cc46..5df3ac0 100644 --- a/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Text.cs +++ b/src/Sbroenne.WindowsMcp/Automation/UIAutomationService.Text.cs @@ -119,10 +119,10 @@ public async Task GetTextAsync(string? elementId, string? wi } } - private static bool CanReadDirectFieldValue(UIA.IUIAutomationElement element) + private static bool CanReadDirectFieldValue(UIA.IUIAutomationElement element, Action? checkDeadline = null) { - if (element.CurrentControlType != UIA3ControlTypeIds.Edit || - !string.Equals(element.CurrentFrameworkId, "Chrome", StringComparison.Ordinal)) + if (ExecuteSearchProviderCall(() => element.CurrentControlType, checkDeadline) != UIA3ControlTypeIds.Edit || + !string.Equals(ExecuteSearchProviderCall(() => element.CurrentFrameworkId, checkDeadline), "Chrome", StringComparison.Ordinal)) { return true; } @@ -135,11 +135,12 @@ private static bool CanReadDirectFieldValue(UIA.IUIAutomationElement element) element, walker.GetFirstChildElement, walker.GetNextSiblingElement, - child => child.CurrentIsControlElement != 0, - (child, _) => segmented = child.CurrentControlType == UIA3ControlTypeIds.Spinner, + child => ExecuteSearchProviderCall(() => child.CurrentIsControlElement != 0, checkDeadline), + (child, _) => segmented = ExecuteSearchProviderCall(() => child.CurrentControlType == UIA3ControlTypeIds.Spinner, checkDeadline), maxNodes: 16, maxDepth: 1, - CancellationToken.None); + CancellationToken.None, + checkDeadline); return !segmented && !outcome.LimitReached; } diff --git a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationWinFormsTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationWinFormsTests.cs index 84bfe56..a741e73 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationWinFormsTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIAutomationWinFormsTests.cs @@ -951,7 +951,15 @@ public async Task WaitForDisappear_ExistingElement_TimesOut() 500); Assert.False(result.Success); - Assert.Contains("still present", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.NotNull(result.Diagnostics); + Assert.True(result.Diagnostics.DurationMs >= 500); + Assert.Null(result.Items); + Assert.True( + result.ErrorMessage?.Contains("still present", StringComparison.OrdinalIgnoreCase) == true || + result.ErrorMessage?.Contains("before it completed", StringComparison.OrdinalIgnoreCase) == true || + result.ErrorMessage?.Contains("before it could start", StringComparison.OrdinalIgnoreCase) == true, + $"Unexpected timeout explanation: {result.ErrorMessage}"); } #endregion diff --git a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs index b7ad21a..826efc4 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Integration/UIFindToolIntegrationTests.cs @@ -148,6 +148,24 @@ public async Task Find_ButtonByName_PartialMatch_ReturnsButton() Assert.Contains("Cancel", result.Items![0].Name ?? string.Empty); } + [Fact] + public async Task Find_WhitespacePaddedDialogScope_PreservesMissingDialogExplanation() + { + var result = await _automationService.FindElementsAsync(new ElementQuery + { + WindowHandle = _windowHandle, + Name = "Install", + ControlType = "Button", + Scope = " active_dialog ", + TimeoutMs = 500 + }); + + Assert.False(result.Success); + Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); + Assert.Contains("No visible enabled dialog", result.ErrorMessage, StringComparison.Ordinal); + Assert.Null(result.Items); + } + [Fact] public async Task Find_TextBox_ReturnsEdit() { diff --git a/tests/Sbroenne.WindowsMcp.Tests/Integration/UISearchLimitIntegrationTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Integration/UISearchLimitIntegrationTests.cs index 948e9af..910f6f4 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Integration/UISearchLimitIntegrationTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Integration/UISearchLimitIntegrationTests.cs @@ -370,7 +370,7 @@ private Task FindAsync( string? name = null, string? contains = null, string? pattern = null, string? parent = null) => UIFindTool.ExecuteAsync( fixture.WindowHandle, name, contains, pattern, "Text", null, null, null, 1, false, - false, null, null, false, false, parent, "window", false, null, 5000, false, CancellationToken.None); + false, null, null, false, false, parent, "window", false, null, 0, false, CancellationToken.None); private static JsonDocument Parse(CallToolResult result) => JsonDocument.Parse(Assert.IsType(Assert.Single(result.Content)).Text); diff --git a/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs b/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs index fcf170b..e70cec6 100644 --- a/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs +++ b/tests/Sbroenne.WindowsMcp.Tests/Unit/FindDeadlineTests.cs @@ -222,4 +222,42 @@ public async Task Timeout_DistinguishesMissingDialogFromMissingElement() Assert.Equal(UIAutomationErrorType.Timeout, result.ErrorType); Assert.Contains("No visible enabled dialog", result.ErrorMessage, StringComparison.Ordinal); } + + [Fact] + public void ProviderCall_ExpiredBudget_DoesNotInvokeProvider() + { + Assert.Throws(() => UIAutomationService.ExecuteSearchProviderCall( + () => throw new InvalidOperationException("Expired search must not call the provider."), + () => throw new TimeoutException())); + } + + [Fact] + public void ProviderCall_CrossingDeadline_DoesNotStartNextCall() + { + var expired = false; + var calls = 0; + Assert.Throws(() => + { + UIAutomationService.ExecuteSearchProviderCall(() => + { + calls++; + expired = true; + return true; + }, CheckDeadline); + UIAutomationService.ExecuteSearchProviderCall(() => + { + calls++; + return true; + }, CheckDeadline); + }); + Assert.Equal(1, calls); + + void CheckDeadline() + { + if (expired) + { + throw new TimeoutException(); + } + } + } }