[larroy]

Summary

• Keep the element a successful UIA poll observed instead of looking it up again with ! (fixes the NullReferenceException at WorkspaceWindowProofTests.cs:304); popup elements are re-resolved at the point of use, not held across later waits.
• Route sibling popup lookups (WorkspaceAgent:*, WorkspaceOwnerProfile) through WaitForPopupAsync.
• Reconnect continuity test: add a ValuePattern draft read-back (TryGetCurrentPattern), env-overridable ReconnectSendTimeout, and derive the test deadline from it (ReconnectTestBudget).
• docs/CI_FAILURES.md: new entries; the draft-loss entry is worded as diagnostics added, not a fix (root cause unproven).

Test-only change; no production code touched.

Required proof pools

none: test infrastructure only.

Validation

• dotnet build tests/OpenClaw.Tray.UITests -p:Platform=x64: 0 errors.
• Not run: ./build.ps1, shared/tray tests, fixture suites.

Real behavior proof

Not verified / blocked: requires a CI rerun of the affected fixture and Workspace proof suites. CI run links and the "107 of 108" count in the doc are unverified.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[larroy]

Hanselman adversarial dual-model review

• Date: 2026-11-11
• Head: 49f6a8e6471bd1ff807e5cdb64e80c04a8045184 ("test: stop re-looking up UI Automation elements after a successful wait")
• Base: a6ff6a164b20835bd5d77200c9134578dd7ce0c6 (HEAD~1)
• Live-head re-check after both reviews completed: unchanged, so the verdict is current.

Scope

File	Change
tests/OpenClaw.Tray.UITests/WorkspaceWindowProofTests.cs	Adds FindPopupForPid, WaitForPopupAsync, WaitForSearchBoxAsync, WaitForFocusedSearchBoxAsync; ExpandOwnerGatewayMenuAsync now returns the status row; replaces the "wait, then look the element up again" shape at the popup and settings-search call sites.
tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.Continuity.cs	Keeps the element the composer poll observed, adds a ValuePattern read-back for the draft, adds ReconnectSendTimeout, renames the Send wait message.
tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.cs	Adds ReconnectSendTimeoutVariable, ReconnectSendTimeout (30 s default, env-overridable), SendActionName.
docs/CI_FAILURES.md	Two new failure entries plus an NU1301 entry marked fixed.

Reviewed as a test-infrastructure change for a WinUI 3 app driven through System.Windows.Automation. No source or production code is touched.

Reviewers

Reviewer	Model ID	Context	Reasoning effort
1	inference-nvidia/aws/anthropic/bedrock-claude-opus-5-5	1M (provider default; the omp CLI has no long_context flag)	high
2	inference-nvidia/azure/openai/gpt-6.1-sol	1.1M (provider default)	high

Both ran as isolated non-interactive omp sessions on the same patch and the same prompt, and both returned complete severity-tagged reviews. Context sizes are what omp models list reports; neither model was given an extended-context flag.

Limits recorded:

• The Sol transcript ends with a self-disclaimer that per-call model selection was disabled. That is the reviewer describing its own nested sandbox, not this run: the launch command passed --model inference-nvidia/azure/openai/gpt-6.1-sol explicitly to each session.
• Run links, the 13812745182 screenshot artifact, and the "107 of 108" count in docs/CI_FAILURES.md were not verified by either reviewer or by the coordinator ([INFERENCE]).
• No runtime proof was produced. This is a source review of test code; it does not substitute for a CI rerun of the affected fixture suites.

Verdict

Both models found no CRITICAL, no HIGH, and no security issue. The core fix holds: the element a successful poll observed is the element used, and transient IsOffscreen/not-ready states are retried inside the wait's own budget instead of consuming a zero-length second lookup. Three MEDIUM-class problems agree across models; the rest are single-model findings.

Both models agree: HIGH consensus

Issue	Opus 5.5	GPT-6.1 Sol	Fix confidence
The new draft read-back cannot prove the draft survived, yet the doc calls the failure "Mitigated". Opus: SetValue targets the element captured by the earlier wait, so a reconnect re-render sends the text to a replaced control, and both suspected root causes still surface as one message. Sol: the read-back can pass and a later render can clear the draft, after which the opaque Send wait returns.	MEDIUM	MEDIUM	80%
FindPopupForPid returns the first on-screen match and never considers later candidates, so a closing or stale element can shadow the live one. Worst case is Collapse then Expand. The old FirstOrDefault had the same window-order assumption; the patch simply holds the chosen element longer.	LOW	MEDIUM	70%
Elements from a successful poll are now retained across further waits and a 350 ms capture sleep, then invoked: gatewayStatusOpen, notificationsFlyout, sendAction, researchEntry, newAgentEntry, ownerMenu. A rebuild in that window throws ElementNotAvailableException where the old code re-looked-up.	LOW	MEDIUM	85%

Only one model flagged: LOW consensus

Issue	Opus 5.5	GPT-6.1 Sol	Fix confidence
Sibling popup lookups still use !: WorkspaceAgent:main, WorkspaceAgent:fallback, WorkspaceOwnerProfile (WorkspaceWindowProofTests.cs:99,101,107,111,175). The doc says that re-lookup shape "was removed from the assistant dropdown, agent dialog and account menu lookups". Same failure class as the line-304 bug, now documented as fixed.	MEDIUM	not flagged	95%
FindPopupForPid swallows ElementNotAvailableException, so "gone" proofs pass on a single failed read: SearchBox() is null (:331,346,351,363), FindPopup(...) is null (:176,423,427,789,956,1001). Base let that error escape and failed loudly.	MEDIUM	not flagged	70%
GetCurrentPattern(ValuePattern.Pattern) inside the new draft wait throws InvalidOperationException when the pattern is absent, and WaitUiAsync only catches ElementNotAvailableException, so the 30 s budget and the new message are bypassed. The same call existed at the same element before; the new one sits inside a retry loop.	not flagged	MEDIUM	90%
WaitForFocusedSearchBoxAsync applies its control-type and focus checks after FindPopupForPid already picked a candidate, so an unfocused older control can be re-selected on every poll. Base bound one element and then waited on its focus, which failed fast instead of timing out.	adjacent only (L3)	MEDIUM	70%
The ReconnectSendTimeout comment's "inside the test's own 150 s body deadline" claim is false at defaults: the post-selection waits sum to 20+20+20+20+30+30+20 = 160 s, so body.WaitAsync can throw a generic TimeoutException before either new message.	LOW	not flagged (Sol only confirmed the deadline exists)	90%
Removing Assert.Equal(ControlType.Edit, …) and adding no failureMessage to WaitForPopupAsync degrades diagnostics to the generic "Native navigation did not reach the expected state."	LOW	not flagged	95%
Two WaitForPopupAsync(pid, …) call sites are stricter than base: they now require !IsOffscreen where the old code accepted any match.	LOW	not flagged	90%
WaitForSearchBoxAsync and WaitForFocusedSearchBoxAsync duplicate WaitForPopupAsync. A single helper with an optional Func<AutomationElement, bool>? ready would stop the three from drifting.	LOW	not flagged	80%
Doc entry 1 names WaitForSearchBoxAsync for the line-304 path, but that path now calls WaitForFocusedSearchBoxAsync.	LOW	not flagged	99%

Checks that produced no finding (both models)

• return ready! is safe. WaitUntilAsync (WorkspaceWindowProofTests.cs:1155-1165) evaluates the predicate once more after the loop and only returns when that call is true, and that call is what assigns ready. No timeout path can return a non-matching element.
• The doc's cause for the line-304 crash is backed by the code: AutomationElement.Current is a struct, so input.Current.ControlType can only throw NullReferenceException when input itself is null, meaning the second SearchBox() lookup returned null.
• Other doc facts confirmed by Opus: 243f9cff is improve(chat): align native conversation and tool activity with web #2093 and adds ./scripts/Initialize-ReactorPackages.ps1 to the fixture restore; CanSend requires a non-empty draft; the composer is a controlled TextBox(text, vm.SetDraft, …); the fixture default wait is 20 s; the fixture forces OPENCLAW_LANGUAGE=en-US; base Continuity.cs:83 is the Send wait.
• The patch contains no em dashes.
• Checkbox state follows the file's own rule: fixes without a CI rerun stay unchecked; NU1301 is checked with its PR linked.

Coordinator verification

Claim	How verified
Live head equals the reviewed head	git rev-parse HEAD after both reviews: 49f6a8e6…, working tree clean for the reviewed paths
ready! cannot return a non-match	Traced WaitUntilAsync at WorkspaceWindowProofTests.cs:1155-1165
WaitUiAsync catches only ElementNotAvailableException	Traced GatewayFixtureUiTests.cs:2007-2018
150 s body deadline exists	GatewayFixtureUiTests.cs:1654
Post-selection wait budget exceeds 150 s at defaults	Arithmetic over the waits in SelectedAssistantAndConversationSurviveTransientRosterLossButNotAuthoritativeRemoval with the 20 s default wait and two 30 s ReconnectSendTimeout waits: 160 s
Remaining ! sibling lookups and "gone" proof sites	Grepped the working tree at the cited lines
Run IDs, screenshot artifact, "107 of 108"	Not verified by anyone - [INFERENCE]

Disputed / rejected

• "This is not a dual-model verdict" (Sol's closing line): rejected. The session was launched with an explicit per-session --model, and the output is a complete severity-tagged review of the same patch both models received.
• Sol's finding 3 is only half new: the per-window FindFirst blind spot existed at base. Retained as a real but pre-existing risk with reduced fix confidence rather than dropped.

Suggested fix order

1. HIGH consensus: do not hold a polled element across another wait or capture; invoke inside the successful poll (gatewayStatusOpen, notificationsFlyout). Cheap, removes the largest new failure mode.
2. HIGH consensus: make the draft proof discriminating or stop claiming mitigation. Record the composer runtime ID before SetValue, re-check the draft during the Send probe, and reword the doc to "diagnostics added".
3. LOW consensus but verified: fix the doc overclaim about removal from assistant-dropdown and account-menu lookups, or wire those siblings through WaitForPopupAsync.
4. LOW consensus but verified: correct the 150 s comment, or derive testTimeout from ReconnectSendTimeout.
5. LOW consensus: three-state popup presence so "gone" proofs do not pass on a read error; TryGetCurrentPattern in the draft predicate; restore failure messages; merge the three wait helpers.

[clawsweeper]

Codex review: needs changes before merge.

What this changes

The PR retains successfully polled UI Automation elements and adds reconnect draft diagnostics and configurable test deadlines.

Example: The Dark-theme Settings proof opens SettingsSearchBox.

• Before: A successful wait discards its element, and a second lookup can return null before dereferencing it.
• After: The wait returns the edit control it observed with keyboard focus.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	Useful, focused test repair, but an absence-assertion regression remains and validation currently establishes compilation only.
Proof confidence	🦪 silver shellfish (2/6)	This maintainer-authored, test-only PR is outside the ordinary contributor proof gate; its reported compilation does not establish native suite execution.
Patch quality	🦪 silver shellfish (2/6)	1 actionable review finding remain.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Timing races in native UI tests obscure real regressions with null dereferences and poorly distinguished reconnect timeouts.
Reason: The maintainer-authored direction improves existing native coverage without changing product behavior; the draft-loss root cause is explicitly left unresolved.

Merge readiness

⛔ Needs changes before merge - 2 items remain

Keep open: this maintainer-authored PR contains useful popup-wait repairs and reconnect diagnostics beyond the Settings crash fix already on main.

Priority: P2
Reviewed head: 4f5c387db46b3234a57722796354bfcdd7c8d2ca

Before merge

• Keep failed popup inspections distinct from confirmed absence (P2) - If FindFirst or IsOffscreen throws ElementNotAvailableException during a popup rebuild, this catch suppresses the failed observation and the helper can return null. Existing dismissal checks such as SearchBox() is null at lines 346, 351, and 363 then succeed without establishing that the popup closed, potentially advancing keyboard input while replacement UI remains open. Retry failed observations at the wait boundary, with an error remaining false for both presence and absence predicates.
• Complete next step - Resolve the reported merge conflict while preserving main's readyInput repair and existing assertions.

Findings

• [P2] Keep failed popup inspections distinct from confirmed absence — tests/OpenClaw.Tray.UITests/WorkspaceWindowProofTests.cs:1291-1293

Tests

• Missing end-to-end proof: Affected native Workspace and reconnect suites have not been shown executing on this head, and no base-fail/head-pass result or required full build and Shared/Tray closeout is supplied.

Agent review details

How this fits together

The Windows UI test harness observes Companion controls through UI Automation, invokes native actions, and checks navigation and synthetic Gateway outcomes.

flowchart LR
  A[UI proof tests] --> B[Bounded UIA polling]
  B --> C[Companion native controls]
  C --> D[Native actions]
  D --> E[Synthetic Gateway]
  E --> F[Outcome assertions]

Loading

Technical review

Best possible solution:

Retain main's search fix, reuse bounded waits for sibling controls, and keep failed UIA probes distinct from confirmed absence.

Do we have a high-confidence way to reproduce the issue?

Source confirms the wait-then-relookup race at sibling popup sites; main already repairs the original Settings dereference, and no native reproduction was run during this review.

Is this the best way to solve the issue?

Returning the observed element is a focused repair using existing test owners; transient inspection failures must remain unsuccessful observations rather than evidence of dismissal.

Full review comments:

• [P2] Keep failed popup inspections distinct from confirmed absence — tests/OpenClaw.Tray.UITests/WorkspaceWindowProofTests.cs:1291-1293
  If FindFirst or IsOffscreen throws ElementNotAvailableException during a popup rebuild, this catch suppresses the failed observation and the helper can return null. Existing dismissal checks such as SearchBox() is null at lines 346, 351, and 363 then succeed without establishing that the popup closed, potentially advancing keyboard input while replacement UI remains open. Retry failed observations at the wait boundary, with an error remaining false for both presence and absence predicates.
  Confidence: 0.87
  Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 018a207a839f.

Provenance checked

• Workspace popup and Settings search assertions keeps the original intent (3a58bf3: The native Workspace work established keyboard, navigation, and popup proof; later shared-search and account-menu changes extended those assertions.)
• Reconnect continuity scenario keeps the original intent (ffc54d6: The earlier repair preserves the selected assistant and conversation across roster loss and proves an ordinary send after reconnect.)
• Transient popup inspection errors changes intended behavior with a stated reason (unknown: The base lookup propagates UIA read failures; the PR explicitly changes rebuilding controls to return null, but that result also feeds existing absence assertions.)

Testing

Proof path: none.

Security

None.

Evidence

Acceptance criteria:

• ./build.ps1
• dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
• dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore
• dotnet test ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj --filter FullyQualifiedName~WorkspaceWindowProofTests
• ./scripts/test-gateway-fixture.ps1 -AppPath ./src/OpenClaw.Tray.WinUI/bin/Debug/net10.0-windows10.0.22621.0/win-x64/OpenClaw.Tray.WinUI.exe -NativeInputProof

What I checked:

• Introduced scope: The pinned introduced diff changes only three UI test files and CI failure documentation; no production code changes. (4f5c387db46b)
• Partial overlap with main: Main already retains readyInput at the original Settings crash site, carried by feat(automations): native Automations page with web parity #1893 (feat(automations): native Automations page with web parity); sibling popup and reconnect changes remain distinct. (tests/OpenClaw.Tray.UITests/WorkspaceWindowProofTests.cs:302, 018a207a839f)
• Absence assertion regression: FindPopupForPid suppresses ElementNotAvailableException and returns null, while existing dismissal waits interpret null as confirmed absence. (tests/OpenClaw.Tray.UITests/WorkspaceWindowProofTests.cs:1291, 4f5c387db46b)
• Discussion and current head: The author's earlier review discussion concerns 49f6a8e; the current head addresses retained popup lifetimes, sibling lookups, pattern availability, and the body deadline. That discussion was inspected as historical evidence; no second reviewer was run. (4f5c387db46b)
• Merge and validation state: GitHub reports mergeable=false and mergeable_state=dirty for the reviewed head; its only reported check is a successful dispatch, and the body reports a UI-test build but no affected-suite execution. (4f5c387db46b)
• Policy and inspection limits: Read the complete root and docs AGENTS.md files; no additional test-subtree instructions or maintainer-notes directory were found. The checkout remained clean. WinApp was unavailable, and no builds, tests, or target runtime code were executed under this read-only review. (AGENTS.md, 4f5c387db46b)

Likely related people:

• Pedro Larroy: Raw commit b6c849e adds tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.cs:25 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: b6c849e8ed44; files: tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.cs)
• Paul Campbell: Raw commit ffc54d6 adds tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.Continuity.cs:47 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: ffc54d60cf1e; files: tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.Continuity.cs)
• Karen: Raw commit 3a58bf3 adds tests/OpenClaw.Tray.UITests/WorkspaceWindowProofTests.cs:216 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: 3a58bf34902f; files: tests/OpenClaw.Tray.UITests/WorkspaceWindowProofTests.cs)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Review metrics

Metric	Value	Why it matters
Change footprint	Production +0/-0; tests +144/-72; docs +15/-0	Growth stays within existing test owners and supports popup readiness and reconnect diagnosis without adding a production abstraction.

Labels

Label changes:

• add P2: This improves CI reliability and failure diagnosis with no direct production behavior change.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

• P2: This improves CI reliability and failure diagnosis with no direct production behavior change.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed November 11, 2026, 4:00 PM ET / 21:00 UTC.

[roboclaw-bot]

Refreshing this branch with current main (3d5f142). I will narrowly reconcile the Workspace UI Automation test conflict, retain current-main readiness/fixture assertions and the existing successful-poll lookup repair, and preserve main’s deletion of the retired CI-failure backlog. No production changes, force push, PR state/body changes, or timeout inflation are planned. I will run available checks and report native Windows validation blockers explicitly before handing the branch back.

[roboclaw-bot]

Refreshed this original branch with a non-force merge commit:

• Previous head: d5b878b5631af87c6ec403d8f601d72cbc6bfe29
• Main: 3d5f14270dd6bd8b260a723ff2e73a68ea858ca8
• New head: 90b4f132f4c98ac2e428a6102f1c50d2194fa92a

The Settings-search conflict now keeps main's enabled invocation and the PR's successful focused UIA observation. Independent review also identified two assertion regressions in the existing patch, now repaired: unavailable popup inspection is not treated as confirmed absence, and both Ctrl+F paths still require keyboard focus. Added three polling regression cases. Reconnect read-back, send assertions, and timeout values are unchanged. The retired CI backlog remains absent; the net diff is three test files, with no production changes.

Source validation: independent GPT-6.1 Sol P0-P3 review of the final tree found no remaining actionable findings. Both diff-whitespace checks and 14/14 temporary static contract checks passed. This is source evidence, not native execution.

Native validation blocked: 0 tests executed. This Linux host has no PowerShell, .NET SDK, WinApp, or Windows desktop. No installation or remote allocation was attempted.

Exact validation attempts

• ./build.ps1: exit 126; invoking it through pwsh -NoProfile -File ./build.ps1: exit 127, missing pwsh.
• pwsh -NoProfile -File ./scripts/setup-dev.ps1 -CheckOnly: exit 127.
• dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore: exit 127, 0 tests.
• dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore: exit 127, 0 tests.
• dotnet test ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj --filter FullyQualifiedName~WorkspaceWindowProofTests: exit 127, 0 tests. The narrower FullyQualifiedName~WorkspaceWindowProofTests.WaitUntil_ filter was also blocked.
• pwsh -NoProfile -File ./scripts/test-gateway-fixture.ps1 -AppPath ./src/OpenClaw.Tray.WinUI/bin/Debug/net10.0-windows10.0.22621.0/win-x64/OpenClaw.Tray.WinUI.exe -NativeInputProof: exit 127.

GitHub confirms the new head and no merge conflict with the captured main. Build and Test and CodeQL are still in progress on this head. Compilation, native suites, and base-fail/head-pass proof remain unverified. This is a branch-refresh handoff, not a merge-readiness claim. PR state/body/attribution are unchanged; removing the temporary actively-landing label.