Wait for workspace chrome UIA children instead of fixed sleeps by coneilen · Pull Request #440 · scgopi/GraphCode

coneilen · 2026-09-24T01:57:01Z

Summary

The live gate's workspace chrome block asserted on UIA children after fixed Start-Sleep -Milliseconds 150 calls, and waited only 10s for that chrome to mount. Both budgets proved too short on a loaded CI runner.

This widens the mount budget to 20s and replaces the two fixed sleeps with bounded polls that wait for the exact control each following assertion already requires.

No assertion is weakened. All 378 Require predicates in the file are byte-identical to main (verified by diffing the extracted predicate lines). This changes only how long the gate is willing to wait, not what it demands. If a control genuinely never appears, every assertion still fails exactly as before.

Why this is a real defect, not test tuning

Two CI runs of the identical commit 912c6ee (PR #439, zero code change between them) failed at two different assertions inside this one block — proving non-determinism rather than a product regression:

Run 107451794210 → workspace right panel did not expose expand control after collapse (the fixed-sleep site)
Run 107455723154 → workspace chrome omitted the toolbar identity child (the 10s mount-budget site)

One site per run, one run each. In the first run, the eight-child mount loop converged and assertions through the non-empty-bounds check all passed, so the chrome was correct — only the post-invoke repaint had not landed within 150ms.

This is the sleep-before-assert class tracked in #438.

Changes

Tools/windows/uia-live-gate.ps1 (+31/-11, no product code):

Workspace chrome mount loop: 100 → 200 attempts (10s → 20s). Break condition unchanged — all eight children must still appear.
Collapse invoke: fixed 150ms sleep → bounded 3s poll for Expand loop panel, the control the next assertion requires.
Expand invoke: fixed 150ms sleep → bounded 3s poll for New Tab, the control the following assertion requires.

Polls break on success, so the common case stays fast.

Evidence

RED: two CI runs of identical commit 912c6ee with zero code change -> job 107451794210 failed workspace right panel did not expose expand control after collapse at the fixed-sleep site, job 107455723154 failed workspace chrome omitted the toolbar identity child at the 10s mount-budget site; two different assertions in one block proves the flake is in the waits, not the product

GREEN: this branch's windows-shell job runs the same gate through the same workspace chrome block -> the block is expected to pass, with the CI result on this PR as the evidence, since the narrow/loaded runner is the only environment that reproduces the timing

REGRESSION: [System.Management.Automation.Language.Parser]::ParseFile on the edited gate -> PARSE OK, and extracting every Require line from main and from this branch and comparing -> 378 vs 378, all byte-identical, confirming no predicate was weakened

The live gate's workspace chrome block asserted on UIA children after fixed 150ms sleeps and inside a 10s mount budget. Both proved too short on a loaded CI runner: two runs of the same commit failed at two different assertions in this block, one at each site. Widen the mount budget to 20s and replace the two fixed sleeps with bounded polls that wait for the exact control each following assertion already requires. Every Require predicate in the file is byte-identical -- this changes only how long the gate is willing to wait, not what it demands. Refs #438 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>

The default Find-FragmentByIdWithRetry budget (20 attempts / 3s) was exhausted on a loaded CI runner immediately after the dynamic project/loop invocation, right before the workspace chrome children that #440 just widened from a 10s to a 20s settle window for the same event. Re-fetching the graph fragment itself is the first thing that has to succeed in that sequence, so give it comparable headroom (80 attempts / 12s) at this call site only -- the other six Find-FragmentByIdWithRetry call sites, owned by different flows, keep the default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>

…#441) PR #440 widened the workspace chrome mount loop, but its break condition tests only that the eight children are non-null. The assertion immediately afterwards tests something stronger -- that four of them have non-empty BoundingRectangle -- so the loop can exit the instant the children appear in the UIA tree, before the loop panel has laid them out. Wait for the layout to settle on exactly the condition being asserted. All 378 Require predicates remain byte-identical to main. Refs #438 Signed-off-by: Colin Neilens <coneilen@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

The default Find-FragmentByIdWithRetry budget (20 attempts / 3s) was exhausted on a loaded CI runner immediately after the dynamic project/loop invocation, right before the workspace chrome children that #440 just widened from a 10s to a 20s settle window for the same event. Re-fetching the graph fragment itself is the first thing that has to succeed in that sequence, so give it comparable headroom (80 attempts / 12s) at this call site only -- the other six Find-FragmentByIdWithRetry call sites, owned by different flows, keep the default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>

…andles (#439) * Validate folder lanes, attention rail, connector handles, and zoom controls live Fixes three real accessibility bugs found while extending the live UIA gate against the now-unblocked Windows shell toolchain: - App.zig's .overview mouse-click switch arm was missing syncAccessibility(), so a lane's Open/Worktrees click had no observable effect through the accessibility tree even though the underlying surface changed. - App.zig's .cycle_attention action handler was missing the same syncAccessibility() call. - AccessibilityProvider.cpp exposed a bogus BoundingRectangle for the native graph UIA element; added canvas_bounds/setCanvasBounds so the live gate can locate real screen coordinates for synthesized input. Extends Tools/windows/uia-live-gate.ps1 with genuine synthesized-input assertions for: - Folder lanes/bands: real WM_LBUTTONDOWN/UP at the lane's Open and Worktrees hit-test rects, verifying routing to the project canvas and scoped worktree inspection. - Canvas attention rail: a real click at the rail's full-width band, verifying SelectionItemPattern selection moves to the NEEDS YOU card. - Connector handles: synthesized hover (WM_MOUSEMOVE) confirmed via a live screen-pixel check, plus a full drag-to-connect (WM_LBUTTONDOWN/MOUSEMOVE/LBUTTONUP) confirmed via the resulting native edge dialog's locked From/To fields. - Zoom controls: live bounds/InvokePattern resolution plus actual invokes of zoom-in, actual-size, zoom-out, and fit-canvas against the running executable. Also hardens Get-DirectChildren/Find-FragmentById against transient UIA tree-walk races near native HWND-hosted view teardown (ElementNotAvailableException/COMException retries, null-root guard), adds Find-FragmentByIdWithRetry, and captures shell stderr via -RedirectStandardError with diagnostic output on unexpected exit. Adds a focused GraphCanvas.zig unit test proving overviewLaneBounds/overviewCardBounds/overviewLaneActionAt correctly stack and independently address multiple open-folder lanes. Updates investigation/ui-parity-matrix.md for the seven owned rows: Folder lanes/bands, Canvas attention rail, and Connector handles move to Validated with genuine live evidence. Cross-project global graph, Notebook grid, Pan and anchored zoom stay Partial with sharpened text on exactly what remains missing (multi-lane live fixture, live pixel-scan for the grid, and native WM_POINTER/WM_GESTURE pinch zoom which is entirely unimplemented). Zoom controls moves to Validated. Discovered but did not fix (outside GraphCanvas.zig ownership and ambiguous fleet ownership): a reproducible panic in TerminalSurface.zig:883 (index out of bounds in readAttachOutput) reached via Worktrees inspection -> Quick Chats -> New Chat -> zoom actions; reported for follow-up rather than risking an unreviewed fix in a file outside scope. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Maximize shell window before wide-window overview lane geometry check CI runners can launch the shell narrower than a local desktop, tripping the Folder lanes/bands geometry precondition (Width -gt 808) that the lane-position formula assumes. Add a MaximizeWindow wrapper around the existing ShowWindow P/Invoke and call it before the check, polling up to 3s for the resize to be reflected in the graph element's BoundingRectangle, rather than weakening or removing the threshold. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Force an explicit window rect instead of maximizing for lane geometry MaximizeWindow only grows the shell to whatever work area the runner's virtual desktop reports, which can still be narrower than the wide-window lane-geometry formula assumes on CI (some runners have no interactive Explorer desktop at all). Replace it with SetWindowPos-based ResizeWindow to an explicit 1400x900 rect, which is not clamped to monitor bounds. Also re-resolve \ via Find-FragmentByIdWithRetry on each poll iteration instead of trusting a BoundingRectangle read against a handle captured before the resize, matching every other \ acquisition in this file, and surface the observed width in the failure message so a repeat failure is self-describing instead of requiring another blind CI round trip. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Iteratively grow requested window rect using measured client width A fixed 1400x900 SetWindowPos request still produced a graph width of exactly 808 in the last CI run, well short of what 1400 minus the known chrome/sidebar overhead should have yielded. The likely cause is a DPI awareness mismatch between the PowerShell caller (not per-monitor-DPI- aware) and the shell (per-monitor aware), which lets Windows rescale cross-process window-sizing coordinates -- so a fixed requested size is not reliably a fixed achieved size. Rather than guess the runner's scale factor, measure the real achieved client width via a new ClientWidth (GetClientRect) helper after each resize attempt, and grow the requested rect proportionally to the observed shortfall, up to 6 attempts and a sane cap. The failure message now reports the final requested size plus both the UIA graph width and the raw client width, so a repeat failure is diagnosable without another blind CI round trip. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Compute overview lane geometry analytically instead of forcing window width CI logged the real cause of the '-gt 808' failure: a fixed-size and then a growing SetWindowPos request both landed at the same ~1028px client width no matter how large a rect was requested (confirmed via the new ClientWidth diagnostic, observed graph width stuck at exactly 808 for requests up to 6000x2687). The OS clamps window growth to the monitor's actual work area, so no resize request can make a small-desktop CI runner's client area wider than that area actually is - the previous fix was chasing an environment constraint that no in-process resize can satisfy. GraphCanvas.overviewLaneBounds' lane width is max(760, width - 48): a floor, not a two-regime formula that only produces a correct button position once some threshold is crossed. The floor branch is just as real a code path as the non-floor branch, so this replaces the window-widening attempt with an analytic computation of lane.right from the graph element's live BoundingRectangle, matching the Zig formula exactly for whichever branch actually applies. CanvasState is reset to its identity transform first (via the already-invokable 'actual-size' canvas action) so leftover zoom/pan from an earlier gate step can't shift the computed screen coordinates. This works at any window size and needs no resize at all, so the now-unused ResizeWindow/SetWindowPos and ClientWidth helpers are removed along with it (re-verified: 32 externs/45 publics, zero duplicates, isolated Add-Type compiles clean). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Widen graph-fragment retry after dynamic project/loop invocation The default Find-FragmentByIdWithRetry budget (20 attempts / 3s) was exhausted on a loaded CI runner immediately after the dynamic project/loop invocation, right before the workspace chrome children that #440 just widened from a 10s to a 20s settle window for the same event. Re-fetching the graph fragment itself is the first thing that has to succeed in that sequence, so give it comparable headroom (80 attempts / 12s) at this call site only -- the other six Find-FragmentByIdWithRetry call sites, owned by different flows, keep the default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Retrigger CI after apparent runner hang on windows-shell Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Retrigger CI to distinguish transient load from a real regression Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Stop swallowing ElementNotAvailableException in Get-DirectChildren be1497d added a bounded retry to Get-DirectChildren that caught ElementNotAvailableException and, after a couple of 150ms sleeps, silently returned an empty child list instead of letting the exception propagate. It also added a speculative COMException retry alongside it. Neither had any RED/GREEN evidence behind it - both were added defensively in the same commit that introduced the new canvas assertions, with no observed failure driving either. That swallow makes every downstream '-eq 0' / '-not' assertion built on Get-DirectChildren vacuous whenever the parent is transiently or permanently unavailable. Assert-FragmentLinks is the clearest case: 'Require (\.Count -eq 0)' passes trivially on an empty result, so a dead-parent read reports 'no unexpected children' while verifying nothing. It also explains three different outcomes observed across otherwise byte-identical CI runs of this branch: a 60-minute hard timeout, and two faster runs that failed at two different downstream assertions. The retried/swallowed ENA sits inside Find-FragmentById's BFS, which is called once per node of a whole-window tree walk, itself wrapped in Find-FragmentByIdWithRetry's up-to-80-attempt outer retry at the dynamic-graph call site. A brief ENA storm coinciding with the outer retry multiplies into tens of minutes of sleeping instead of surfacing the real, fast failure that main's unguarded version would have thrown immediately. Revert Get-DirectChildren to main's plain, unguarded tree walk: a dead or unavailable parent throws immediately, matching main's original behavior. Callers that need to tolerate a genuinely remounting parent already re-resolve the parent on every attempt (Wait-ForGraphChildren), which is the correct way to absorb a transient ENA without hiding a real failure. Require-predicate count is unchanged (414 branch / 380 main) since this only removes retry/catch control flow and adds no assertions and drops none. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Restore a narrow COMException retry in Get-DirectChildren The prior commit dropped both the ElementNotAvailableException catch and the COMException catch from Get-DirectChildren, reasoning that neither had RED/GREEN evidence behind it. CI on that commit reproduced a real, reproducible failure: 'Exception calling GetFirstChild ... Catastrophic failure (0x8000FFFF (E_UNEXPECTED))', thrown seconds after the freshly launched shell's UI Automation provider registered, before any assertion ran (windows-shell run 36027303446, ~13s into the live gate). That is exactly the transient COM race the original comment described ('Could not open the process token' / E_UNEXPECTED right after a new native HWND has appeared) and is now backed by an actual observed failure rather than speculation. Restore only the bounded COMException retry (4 attempts, 150ms apart, then rethrow) to absorb it. The ElementNotAvailableException catch stays removed: unlike COMException, which always either recovers within budget or rethrows, the ENA catch returned a masking @() after its attempts were exhausted, which is what made downstream '-eq 0'/'-not' assertions vacuous. The COMException path added back here never does that - it always surfaces a real failure to the caller, so it cannot hide a genuine defect the way the ENA swallow did. Require-predicate count unchanged (414 branch / 380 main); parse clean; exactly one Get-DirectChildren definition (no duplication left over from editing). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Absorb the shell UIA provider startup race once, not per call The previous commit restored a narrow, bounded COMException retry inside Get-DirectChildren (4 attempts, ~600ms) after CI reproduced a real 'Catastrophic failure (0x8000FFFF (E_UNEXPECTED))' from GetFirstChild seconds after the freshly launched shell's UI Automation provider registered. A second CI run on that exact commit reproduced the same class of failure again (this time 'Unrecognized error.', still a COMException, still ~11s into the gate, still before any assertion ran) - the 600ms budget was not enough to outlast the provider's startup window on that run. Rather than growing that per-call budget further, which is paid on every one of the ~35 Find-FragmentById call sites for the rest of the run, absorb the race once: immediately after graphcode-root first responds to WM_GETOBJECT, poll a raw tree walk against it with a longer budget (up to 30 attempts, 250ms apart, ~7.5s) before any real assertion begins. Once this settles, the per-call COMException retry stays as a bounded safety net for the rest of the run rather than the primary defense against a race it was never sized for. This adds one deliberate new assertion - 'shell UI Automation provider did not settle after graphcode-root appeared' - which fails loudly if the provider never stabilizes, rather than silently proceeding. Require count is 415 (up from 414) for that single intentional addition; no existing assertion changed. Parse clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Widen and instrument the UIA provider settle wait CI on the prior commit reproduced the same startup-window failure again, this time as 'Unrecognized error.' rather than the earlier 'Catastrophic failure (E_UNEXPECTED)' - two different messages for what looks like the same provider-registration race, ~11s into the gate both times, before any assertion ran. Two changes in response: 1. Widen the settle budget from 30 attempts (~7.5s) to 60 (~15s). The first budget was sized as a guess; it was not enough on the second CI run. 2. Catch broadly instead of narrowing to COMException, and log the concrete exception type, HRESULT, and message on every retry attempt (UIA_PROVIDER_SETTLE_RETRY). Two different error messages already surfaced for the same apparent race, so assuming a fixed exception type would just move the guesswork rather than remove it. If this still fails, the failure message and CI log will say exactly what was thrown instead of leaving us to infer it again. Still polls the same precondition (can a raw tree walk on the fresh root succeed at all?), never an assertion - same rule as Wait-ForGraphChildren and Wait-ForDesktopElement. Require count unchanged at 415 (the settle guard's failure message gained detail; it did not become a new predicate or change its pass/fail condition). Parse clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Wait for workspace card children after dynamic project/loop invocation This call site (mine: I added the -maxAttempts 80 widening on Find-FragmentByIdWithRetry) re-fetched the 'graph' fragment after a dynamic project/loop invocation, then read its 'canvas-card-' children with a single, unwaited Get-DirectChildren call immediately afterward. That is the same remount race Wait-ForGraphChildren exists for elsewhere in this file: graph can reappear before its children have (re)mounted on it, and a single-shot read right after resolving graph can observe an empty or partial set - exactly what produced 'loop invocation did not transition to the selected workspace loop' on CI (dfc37d5, gate line 2051 at the time). Replace the separate Find-FragmentByIdWithRetry + single-shot Get-DirectChildren with one Wait-ForGraphChildren call. It re-resolves graph on every attempt, so it covers both the graph-refetch race this site was already budgeted for and the children-mount race that was missing a wait entirely. -maxAttempts 150 (~15s) preserves the extra headroom the original 80-attempt/12s graph-refetch budget was sized for. Per the established rule for this helper: it waits on the presence precondition only (both cards exist), never on the full assertion. The existing Require covering count, ordering, names, and IsSelected is byte-identical to before - only the read leading into it changed. Require count unchanged at 415 (same two predicates: the missing-graph guard and the transition assertion, now sourced from the wait's result instead of a fresh unwaited read). Parse clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Instrument Get-DirectChildren's COM retry to disambiguate the next failure CI on c290a39 threw a raw, uncaught exception ('Unrecognized error.', via GetFirstChild) about 17s into the gate, with zero UIA_PROVIDER_SETTLE_RETRY lines - meaning the one-time provider-settle wait succeeded on its first attempt this run, so this is a separate, later occurrence of a similar transient COM race, not the startup race that wait absorbs. The failure surfaced with no Require message at all, meaning it escaped uncaught all the way to the top of the script. It landed in Get-DirectChildren's narrow COMException retry (8243eed), but that catch block never logged anything, so there is no way to tell from this evidence alone whether it: (a) genuinely retried up to the 4-attempt/450ms budget as a real COMException and then exhausted it, or (b) was a differently-typed exception that never matched the typed catch at all and threw on the very first occurrence. Both would produce an identical-looking raw uncaught exception, so guessing between 'widen the budget' and 'broaden the caught type' would be exactly the kind of unverified assumption this file's history has repeatedly punished. Replace the typed catch with a bare catch that checks the exception type explicitly and logs attempt/type/hresult/ message on every occurrence (mirroring UIA_PROVIDER_SETTLE_RETRY's pattern) before deciding whether to retry. The retry POLICY is unchanged: only a genuine COMException is retried, capped at the same 4 attempts; anything else, or an exhausted COMException, still throws immediately with the original exception preserved. This is diagnostic-only - no Require added or removed (predicate count unchanged at 415), one Get-DirectChildren definition, parse clean. Next failing run's log will show definitively which case this is, so the actual fix (if any) can be targeted instead of guessed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Fix silently-disabled COM retry; make its budget wall-clock, not attempts 9d27d32's bare catch checked '$_.Exception -is [COMException]' directly, which is always False on a bare catch: a direct .NET method call that throws is unwrapped by a *typed* catch clause, but a bare catch instead receives it wrapped in MethodInvocationException, with the real exception in .InnerException. Verified locally against a compiled method that throws a genuine COMException: catch [COMException] -> matches (unwraps to inner) bare catch, \.Exception -is [COMException] -> False bare catch, \.Exception.InnerException -> the real COMException So 9d27d32 threw on attempt 1 every time - the retry was silently disabled - while still logging 'retried=False', which reads exactly like 'never matched the typed catch' (hypothesis b) when it was really hypothesis (a): the retry regressed, not the exception type. Fixed by checking both \.Exception and \.Exception.InnerException for COMException; verified with a local repro (3 calls, first 2 throw a compiled COMException, retry now succeeds after 2 attempts instead of throwing on the first). Separately: switch the budget from a fixed attempt count (4) to a 5-second wall-clock deadline. This call runs inside every tree walk across ~35 call sites; a fixed attempt count multiplied across that many sites is exactly the arithmetic that produced the 60-minute CI hang earlier on this branch (be1497d). A duration cap keeps the worst-case cost per call bounded regardless of how many sites hit it, and gives the same call more headroom to recover from a COM stall that outlasts a few hundred milliseconds mid-run (as opposed to only at startup, which the existing one-time settle wait already covers). Diagnostic logging kept and extended with elapsed-ms; retry policy is otherwise the same shape (only a genuine COMException is retried, anything else or an exhausted budget still throws immediately, no fall-through to a return that could mask a failure). Predicate count unchanged at 415, one Get-DirectChildren definition, parse clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Also retry genuine ElementNotAvailableException in Get-DirectChildren 473b64b's own CI run surfaced the next piece of concrete evidence: the 'Windows port validation' (windows-spikes) job hit 'UIA_GETCHILDREN_RETRY attempt=1 elapsedMs=27 type=System.Management.Automation.MethodInvocationException hresult=0x80040201 retried=False ... Unrecognized error.' and then threw uncaught, while the sibling windows-shell job on the identical commit passed clean with zero retry lines - consistent with an environmental, load-dependent race (that run's resource snapshots show several concurrent pwsh/conhost sessions live during a 'windows-shell:large- paste' phase). hresult=0x80040201 is UIA_E_ELEMENTNOTAVAILABLE. Verified directly by loading UIAutomationTypes.dll and constructing the real exception: ElementNotAvailableException's HResult is exactly 0x80040201, and it derives from SystemException, not COMException - so neither the original typed COMException catch nor 473b64b's fixed COMException check could ever have retried it. This is a real, transient ElementNotAvailableException occurring mid-walk under load, not a type- check bug this time. This is deliberately NOT a reinstatement of the be1497d swallow that was removed in 91abf5c: that bug caught ENA and returned @() after exhausting its budget, making every '-eq 0'/'-not' assertion built on a dead parent pass vacuously - it recovered by lying. This still always rethrows the original exception on exhaustion or on any other exception type; it only widens which transient, recoverable exception types get the same bounded, wall-clock-limited retry already given to COMException before that unconditional throw. Verified locally: constructing a real ElementNotAvailableException and running it through the updated retry-eligibility check now retries and recovers, instead of throwing immediately. Also added innerType to the diagnostic log line, since the outer type is always the same MethodInvocationException wrapper for any bare-catch failure on a direct .NET method call and was not by itself informative. Predicate count unchanged at 415, one Get-DirectChildren definition, parse clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Re-resolve graphcode-root after modal teardown instead of retrying a stale reference The update-offer dialog's Later action rebuilds the shell's fragment tree. \ captured before that teardown can become a permanently dead reference - not a transient blip Get-DirectChildren's bounded COM/ENA retry can recover from. CI evidence: a walk that failed mid-enumeration with 'Catastrophic failure (E_UNEXPECTED)' on GetNextSibling, then failed every subsequent attempt with ElementNotAvailableException on GetFirstChild for the rest of a 5s retry budget - the same event seen at two stages, not two unrelated glitches. Add Wait-ForRootReconnect: re-acquire \ via FromHandle on the shell's main window handle (checking AutomationId, exactly like the initial acquisition loop), then prove a raw tree walk of it succeeds before returning it as live. Waits on that precondition only; does not touch any caller assertion. On exhaustion it fails with HasExited and the last exception observed, so a genuine product crash is distinguishable from a gate-side reconnection failure. Convert only the one call site after the Later-dismissal teardown. Two other modal teardowns (~1265, ~3124) have the identical latent exposure but are noted, not converted, per explicit scope for this fix. Predicate count: 415 -> 416 (one new Require inside the helper, on reconnect exhaustion). Parse-clean. This file is not exercised by Tools/windows/Tests/WindowsShell.Tests.ps1 (Zig-only suite); no Zig source changed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Wait-ForRootReconnect: verify both RawView and ControlView walkers before trusting the reconnected root CI on 36ab812 showed the first version of this helper isn't sufficient: it verified reconnection with only the raw-view walker, and the very next statement at this call site (status lookup via ControlView) then failed with ElementNotAvailableException for the entire 5s Get-DirectChildren retry budget, never recovering - immediately downstream of a reconnect that had just returned successfully. RawView and ControlView are separate client-side views of the same element and can settle at different times after a remount; proving one is walkable does not prove the other is. This call site immediately exercises both (status via ControlView, root-children assertions via both views), so Wait-ForRootReconnect now takes an array of walkers and requires every one of them to complete a GetFirstChild call on the candidate before trusting it as reconnected. Added UIA_ROOT_RECONNECT_RETRY diagnostic logging on each failed attempt, matching the existing UIA_PROVIDER_SETTLE_RETRY / UIA_GETCHILDREN_RETRY pattern in this file, in case this needs disambiguating again. Verified the control-flow in isolation with fakes (no real UIA needed): recovers once every walker eventually succeeds, exhausts its budget and rethrows with the last exception when the element stays permanently dead, and surfaces a genuine process exit with a distinct message rather than looping on it. Predicate count unchanged (416): this changes only what is proven before the existing Require statements run, not any assertion itself. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Rewrite Wait-ForRootReconnect comment to observation-only; add handle logging as (a) vs (b) insurance CI on 94014a1 (the both-walker verification fix) landed a second data point that the prior comment's stated mechanism does not fit. That comment asserted 'RawView and ControlView ... can settle at different times after a remount' as the explanation. But this run showed zero UIA_ROOT_RECONNECT_RETRY lines - both views were proven walkable on the very first attempt - and the very next Get-DirectChildren call still failed 5ms later, immediately, and stayed dead for the full 5s budget. That does not fit 'the two views settle at different times' (both had just been proven walkable); it fits equally well with the element dying in the window between the check and its use. Rewrote the comment to state what was observed at each of the two prior CI failures without asserting which explanation is correct. Both stay live hypotheses; the fix (require every view the caller uses to be walkable before trusting the reconnect) is defensible under either one, so it did not need to change. Added logging to distinguish them empirically without another diagnostic round-trip: Wait-ForRootReconnect now logs .MainWindowHandle on successful reconnect (UIA_ROOT_RECONNECT_OK), and Get-DirectChildren's existing UIA_GETCHILDREN_RETRY line now logs the same handle. If a future failure shows the handle changed between a preceding reconnect and the failing read, that proves the time-of-check/time-of-use explanation outright. Predicate count unchanged (416): comment and diagnostic-only, no assertion added, removed, or altered. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> * Make handle diagnostic deterministic; add call-site and window-enumeration diagnostics Get-DirectChildren's UIA_GETCHILDREN_RETRY log now calls $process.Refresh() before reading MainWindowHandle. This file has 18 separate $process.Refresh() call sites, so without an explicit Refresh() here the logged handle depended on whichever unrelated site last refreshed it rather than on state at this call. CI evidence on d98f2ca (handle=0 on 33 retries vs handle=262398 at the preceding reconnect) shows the read was NOT vacuous/echoing a cached value as an earlier commit message on this branch claimed - that framing was wrong and is corrected here. Refresh() still earns its keep: it makes the read deterministic instead of dependent on unrelated code paths, and documents the $process vs $settingsProcess two-shell-process ambiguity for future diagnostics logged during the Product Settings fixture phase. Also adds two purely diagnostic capabilities requested to localize a still-open failure (MainWindowHandle enumerating to 0 for a full retry budget ~11s into the gate, cause not yet established): - A call-site marker on every UIA_GETCHILDREN_RETRY line, built from Get-PSCallStack, identifying which of Get-DirectChildren's ~35 call sites is failing. Verified in isolation that the marker resolves to the correct caller chain. - At retry exhaustion, before the existing unconditional rethrow, log $process.HasExited (+ exit code if exited) and an enumeration of the process's current top-level windows (handle, class, visibility, title) via a new GraphCodeUiaGateState.DescribeTopLevelWindows helper built on the class's existing EnumWindows/GetWindowThreadProcessId/GetClassName/ IsWindowVisible P/Invoke primitives (no new P/Invoke declarations). Isolated-compiled the embedded C# block with the file's exact -ReferencedAssemblies and confirmed it compiles; functionally exercised DescribeTopLevelWindows against a live process (explorer.exe) and confirmed it correctly distinguishes visible/hidden windows and empty titles. This separates four otherwise-indistinguishable failure modes: crashed, window-destroyed, window-hidden, and handle-churn. Both additions are diagnostic-only: no assertion changed, no behavior change on any passing path. Require predicate count unchanged at 416. Signed-off-by: Colin Neilens <coneilen@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Report shell crashes with stderr from UIA tree-walk exhaustion When Get-DirectChildren exhausts its bounded retry and the shell has exited, dump the redirected shell stderr between explicit markers before rethrowing the original UI Automation exception. Log ExitCode as "unavailable" when Process.ExitCode cannot be read; do not coerce a null value to int, which manufactured exit 0 during local reproduction. Also make the immediate post-project/loop invocation liveness assertion report the exit code when available and dump the same stderr if the crash has already completed by that point. The check remains at its existing location and timing; this adds no waits and changes no passing behavior. The real pinned ReleaseSafe and Debug local gate reproductions confirmed the path: the shell panic is preserved in stderr, exitCode is truthfully reported as unavailable locally, and the final thrown error remains the original ElementNotAvailableException rather than a diagnostic wrapper. Require predicate count remains 416. Signed-off-by: Colin Neilens <coneilen@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Preserve terminal cell buffers when replacing a live surface Workspace.openNode created a replacement in a free Surface slot, destroyed the target, copied the replacement over it, and reset the donor slot to its default value. Every slot receives a cell_count-sized terminal cell buffer at Workspace.init, so that assignment both leaked the destroyed target's buffer and left the donor with cells=&.{}. A later project rebind could reuse that donor slot, move its empty cell slice into the active slot, and panic when terminal output reached putCodepoint: index out of bounds: index 0, len 0 TerminalSurface.zig:1921 slot.cells[...] Swap the destroyed target and live replacement Surface values instead. This moves the complete live native/session/attach state to the requested index while preserving both slots' allocator-owned cell buffers. Callbacks continue to resolve the moved native surface by pointer, and input callbacks compute its current slot index at callback time. The regression test invokes the same private helper as openNode, uses std.testing.allocator-backed cell buffers, verifies live state and both distinct allocations survive, feeds printable output through the production parser on the recycled donor, then repeats replacement/reuse. With the original assignment/reset it failed expected 4800/found 0 and reported the target allocation leaked; with the swap all 21 focused tests pass. Signed-off-by: Colin Neilens <coneilen@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden terminal replacement test cleanup Register allocator cleanup before either test buffer allocation so a failure allocating the second buffer still releases the first. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com> --------- Signed-off-by: Colin Neilens <coneilen@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

coneilen merged commit b1965a6 into main Sep 24, 2026
10 checks passed

coneilen deleted the fix-workspace-chrome-uia-waits branch September 24, 2026 02:25

coneilen mentioned this pull request Sep 24, 2026

Wait for workspace detail children to lay out before asserting bounds #441

Merged

coneilen mentioned this pull request Sep 24, 2026

Validate folder lanes, zoom controls, attention rail, and connector handles #439

Merged

coneilen mentioned this pull request Sep 24, 2026

Re-resolve the graph and poll preconditions across canvas assertions #444

Merged

coneilen mentioned this pull request Sep 24, 2026

Windows shell validation does not run on pushes to main #445

Open

Navigation Menu

Wait for workspace chrome UIA children instead of fixed sleeps - #440

Wait for workspace chrome UIA children instead of fixed sleeps#440
coneilen merged 1 commit into
mainfrom
fix-workspace-chrome-uia-waits

coneilen commented Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

coneilen commented Sep 24, 2026

Summary

Why this is a real defect, not test tuning

Changes

Evidence

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant