Skip to content

fix: keep accessibility failure diagnostics private and fail-closed - #2054

Merged
roboclaw-bot merged 3 commits into
mainfrom
bkudiess-safe-axe-diagnostics
Nov 9, 2026
Merged

roboclaw-bot merged 3 commits into
mainfrom
bkudiess-safe-axe-diagnostics

Conversation

@bkudiess

@bkudiess bkudiess commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Fixes accessibility-test failures that can expose private UI content or crash while reporting incomplete scan metadata.

User Impact

User impact: no shipping UI change. Developers receive bounded diagnostics without UI names, data-derived automation IDs, arbitrary properties or unrelated desktop focus state. Malformed results remain failures, including excluded rules.

Published for review, not an unconditional merge-readiness claim. Current native validation and independent review are recorded below; new-commit CI and the requested Opus pass remain separate gates.

Why This Change Was Made

Keep rule IDs, bounded descriptions and canonical control types with scan-result ordinals. Print at most 20 violations while retaining the total count. Reject incomplete/inconsistent scan output before exclusions or retry. The existing one-time stale-layout retry now requires an automation ID and verifies target process ownership, rather than falling back to names.

Author-side simplification retained the existing helper and distinct data-only regressions; no new framework or production owner.

Evidence

Commit 788b3fb33240728d5c9be9e188f7e80153adb74c has tree 26e5a88b500e62fc6ba70e9ff122c577c312c25a. All 2,175 intended source files were checked byte-for-byte against the retained native validation manifest and the committed Git blobs. Tests ran on the source-identical uncommitted candidate based on 6825288f4123439dcb3b39750fae3d76115e675e; committing did not constitute another test run. Source-manifest SHA256: f5f20ddf9014a8fbbf96c4b9fddbad2213153f487067b51a7f2c91d636b4f1af.

Current proof: 40/40 AxeHelper cases, 33/33 real accessibility cases, and the required build/unit suites passed. The shared test prerequisite corrects standalone-radio keyboard expectations, observes actual hidden content before collapsed navigation, and waits for rendered elapsed-text advancement without increasing the existing deadline. It adds no shipping UI or new framework.

Independent source and rubber-duck implementation/proof-plan reviews by GPT-6 Astra found no P0-P3 blocking findings in the frozen candidate. The actual role was a native OpenClaw subagent; context tier and reasoning effort were not independently attested. The requested Opus 5.5 pass was unavailable in this environment, so no renewed dual-model Hanselman consensus is claimed.

Receipts (Crabbox access may require team sign-in):

Important count distinction: the current hosted diagnostic recorded 683 passed / 7 skipped. Four skipped startup cases lacked the fixture opt-in in that launcher; a separate correctly admitted run passed 4/4. These are two runs, not one 687-pass/3-skip execution.

Historical evidence, not current validation

The earlier PR description reported 15 pure cases on 895ac1ab86505c72c5bc87abc90265c365dd1409 and a source-guard negative control against baseline 122bda71. Those results and the earlier shared-review authentication failure remain historical. The current 40-case helper suite and real native scans below supersede the old current-evidence claims. No newly induced native stale-layout incident is claimed.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Required proof pools

  • windows-winui-interactive: real accessibility scanner integration and native keyboard/rendering prerequisites. The selected accessibility command passed 33/33; its inventory-bound summary has scope=selected-test-execution-only and poolComplete=false.

Retry-policy tests use the real helper with controlled scanner/layout inputs. Real page scans are separate evidence; a newly induced native stale-layout retry incident remains unverified. Other installer, Gateway, MXC and hardware pools are not selected for this test-only change.

Validation

Windows 11 Pro 25H2 x64, .NET SDK 10.0.403, Debug/non-dev identity. All four OPENCLAW_TRAY_* data roots were isolated; OPENCLAW_STATE_DIR was unset only for unit-test processes and restored to task-private state for UI runs. Fresh test-project builds prevented --no-restore no-ops. Source and loaded binaries were verified before and after execution.

Command / selection Actual result
./build.ps1 Passed all default projects and documentation validation
dotnet build ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj Passed
dotnet build ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj Passed
dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore 4,950 passed, 0 failed, 25 ordinary skips
dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore 5,239 passed, 0 failed, 0 skipped
dotnet build ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -c Debug -r win-x64 Passed
UI test filter FullyQualifiedName~AxeHelperTests 40 passed, 0 failed/skipped
UI test filter FullyQualifiedName~ClipboardExpander_SupportsKeyboardSelectionAndCollapse 1 passed, 0 failed/skipped
UI test filter FullyQualifiedName~ActiveRun_AfterAssistantPreamble_KeepsClawAndElapsedTimeVisible 2 passed, 0 failed/skipped
Real Category=Accessibility 33 passed, 0 failed/skipped
Hosted filter Category!=Accessibility&Category!=NativeOnboardingProof&Category!=GatewayFixture 683 passed, 0 failed, 7 skipped; native testhost and outer dotnet exited 0
Separate FullyQualifiedName~StartupWindowUiTests with fixture opt-in 4 passed, 0 failed/skipped
git diff --check Passed

Focused UI commands used dotnet test ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -c Debug -r win-x64 --no-build --no-restore --filter <selection> with fresh invocation-specific TRX directories. Real accessibility used:

pwsh -NoProfile -File ./scripts/run-proof-tests.ps1 -Project tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -Filter Category=Accessibility -ResultName winui-accessibility -RuntimeIdentifier win-x64 -ResultsDirectory ./TestResults/ProofPools/pr2054-shared-accessibility

The hosted diagnostic used --diag, --blame, --blame-hang --blame-hang-timeout 300s --blame-hang-dump-type none. The fixture opt-in omission and separate four-case correction are retained, not hidden. Three native-input/compositor opt-ins remain unexecuted in that selection. No fresh Release or signed-distribution result is claimed. New GitHub CI is not implied by these local results.

Real Behavior Proof

  • Environment tested: authorized Azure Windows 11 Pro 25H2 x64 client, 1600x1200 interactive desktop, isolated app and test profiles.
  • PR head or commit tested: source-identical pre-commit candidate now committed as 788b3fb33240728d5c9be9e188f7e80153adb74c; all 2,175 source blobs match the validation manifest.
  • Exact steps or command run: the selectors and accessibility command above; actual helper failures were exercised through controlled Axe DTO inputs, and native page scans ran in the real app.
  • Evidence after fix: 40 helper cases pass private-field omission, malformed metadata, exclusions, ownership and exactly-one-retry policy; real accessibility selection passes 33/33. Clipboard proof covers Down/Space, forward/reverse Tab, settled collapse and page-reactivation selection retention.
  • Observed result: current source/binary bindings stayed unchanged through execution and export. The current hosted process completed without reproducing the historical crash; its original cause is not established.
  • Screenshot or artifact links verified? Team-access run/session links and retained artifact hashes were verified. No unrelated screenshot is offered as proof of diagnostic privacy.
  • Not verified or blocked: new-commit GitHub CI; requested Opus review; whole-pool completion; a newly induced native stale-layout retry incident; three native-input/compositor opt-ins; new Release/distribution proof.

Retained proof archive SHA256: 6cf300795e42e414be76847e875c8f6e337cd6e58c1d8f0b7ea92a9e6fda73eb (52 independently verified files, eight parsed TRX reports). Raw archives remain in the team work session, not represented as public attachments.

Security Impact

  • New permissions or capabilities? (Yes/No) No.
  • Secrets or tokens handling changed? (Yes/No) No.
  • New or changed network calls? (Yes/No) No.
  • Command or tool execution surface changed? (Yes/No) No.
  • Data access scope changed? (Yes/No) Yes, narrowed in the test helper.
  • If any answer is Yes, explain the risk and mitigation: removes arbitrary UI property logging and unrelated desktop-focus reads. Remaining stale-layout lookup verifies process ownership and no longer falls back to UI names. Static test-authored context and rule descriptions remain bounded diagnostic inputs.

Compatibility and Migration

  • Backward compatible? (Yes/No) Yes for shipping behavior; diagnostic text changes intentionally. Incomplete scan metadata now produces an explicit failure, and stale-layout retry requires an automation ID.
  • Config or environment changes? (Yes/No) No shipped changes.
  • Migration needed? (Yes/No) No.
  • If yes, list the exact upgrade steps: N/A.

Contributor credit

Original PR by @bkudiess. Follow-up fixes and native validation were prepared with Barbara Kudiess; the follow-up commit retains her co-author credit.

Work sessions

Linked for team reference; these sessions aren’t public.

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

Live inline-review-thread lookup before this update returned no threads for this PR. The original checklist remains unclaimed rather than inventing resolved conversations.

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

clawsweeper Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

🦞👀
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.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Nov 7, 2026
@clawsweeper

clawsweeper Bot commented Nov 7, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge.

What this changes

This PR removes private UI content from accessibility failure diagnostics and rejects incomplete scan results before exclusions or retries.

Example: A ClickablePointOnScreen violation contains Name='private conversation' and AutomationId='private dynamic id'.

  • Before: The failure prints those values, arbitrary properties, and desktop focus information.
  • After: The failure reports the rule, canonical control type, and element ordinal without those private values.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) A focused, source-supported repair with meaningful data coverage, but its explicitly required native behavior remains unverified.
Proof confidence 🦪 silver shellfish (2/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Accessibility failure reports can expose private UI content and lose useful assertion output when scan metadata is incomplete.
Reason: The source establishes a concrete diagnostic privacy problem, and collaborator authorship supplies direction for the deliberately stricter reporting contract.

Merge readiness

✅ Ready for maintainer review

Keep this collaborator-authored draft open: current main retains the diagnostic disclosure, and the focused patch addresses it without an identified code defect.

Priority: P2
Reviewed head: 895ac1ab86505c72c5bc87abc90265c365dd1409

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The declared native scan failure and retained retry have no live after-fix evidence; baseline failure is documented only for the global-focus source guard.
Agent review details

How this fits together

The accessibility test helper receives Axe.Windows scan results from Companion's UI Automation tree and produces assertions consumed by test reports and CI diagnostics.

flowchart LR
  A[Companion window] --> B[Axe Windows scanner]
  B --> C[Metadata validation]
  C --> D[Rule exclusions]
  D --> E[Scoped stale layout retry]
  E --> F[Bounded failure report]
Loading

Technical review

Best possible solution:

Keep validation and privacy-safe reporting in AxeHelper, preserving existing exclusions and the single narrowly admitted retry.

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

Current-main source directly prints UI names, IDs, arbitrary properties, and global focus on scan failure; no runtime reproduction was performed during this read-only review.

Is this the best way to solve the issue?

The patch fixes the reporting owner directly, preserves real violations, and uses the dependency's actual count contract without introducing another framework.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 96ea36fe6066.

Provenance checked

  • Real-process accessibility gate and exclusions keeps the original intent (feat: add Accessibility Insights CI tests using Axe.Windows #923: Make native accessibility scans a blocking gate with narrowly scoped framework exclusions.)
  • Live diagnostics and stale-layout rescan changes intended behavior with a stated reason (4712e22: Diagnose composer layout and permit one rescan only when live UIA proves the initial zero-height snapshot stale.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

  • Verified introduction and current-main behavior: The pinned introduction changes only the documented three files; those paths are unchanged between the introduction base and fetched main, which still contains arbitrary-property logging and global-focus reads. (tests/OpenClaw.Tray.UITests/AxeHelper.cs:105, 96ea36fe6066)
  • Fail-closed filtering and bounded diagnostics: Malformed window metadata fails explicitly; incomplete violations survive exclusions and cannot enter the retry; reports omit arbitrary properties and cap printed records at 20. (tests/OpenClaw.Tray.UITests/AxeHelper.cs:105, 895ac1ab8650)
  • Imported dependency contract: The test project imports Axe.Windows 2.4.2, making its DTO contract directly applicable; the tagged assembler assigns Errors and ErrorCount from the same list and obtains RuleInfo from the static rule registry, supporting the count check and use of rule descriptions. (src/Automation/ScanResultsAssembler.cs:33, 1c8709bd1526)
  • Original retry intent: fix(chat): stabilize composer accessibility state #1135 introduced one authoritative rescan for demonstrably stale zero-height snapshots; this PR retains that mechanism while requiring an automation ID and matching process ownership. (tests/OpenClaw.Tray.UITests/AxeHelper.cs:174, 4712e227d110)
  • Discussion, validation, and draft status: The complete body reports successful build, Shared, Tray, and 15 data-only cases, explicitly disclaims native scan/retry proof, and keeps the PR draft; discussion contains only bot status messages and no additional behavioral evidence. (895ac1ab8650)
  • Policy and inspection limits: Read the complete root and docs guidance; tests have no nested AGENTS.md and maintainer notes are absent. Some historical blobs were unavailable locally; available history, blame, and original PR bodies establish the relevant owners and intent. No helper, second reviewer, builds, or tests ran during this review, and the checkout remained clean. (AGENTS.md:5, 895ac1ab8650)

Likely related people:

  • Karen: Raw commit e0a2301 adds tests/OpenClaw.Tray.UITests/AxeHelper.cs:62 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: e0a2301ce2e8; files: tests/OpenClaw.Tray.UITests/AxeHelper.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
Scope Shipping code +0; test helper +88/-91; regressions +161; docs +12 The existing helper shrinks slightly; added coverage protects distinct privacy, malformed-result, exclusion, and output-boundary failures.

Labels

Label changes:

  • add P2: Concrete privacy hardening of developer accessibility diagnostics with no shipping UI change or demonstrated active disclosure.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: Concrete privacy hardening of developer accessibility diagnostics with no shipping UI change or demonstrated active disclosure.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Supply native accessibility failure output and retained-retry evidence from the declared Windows interactive environment, with private data redacted.

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 6, 2026, 8:40 PM ET / November 7, 2026, 01:40 UTC.

bkudiess and others added 2 commits November 6, 2026 21:01
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve keyboard traversal, settled collapse and selection retention while replacing the elapsed-label fixed sleep with observable rendered progress.

Co-authored-by: bkudiess <7658216+bkudiess@users.noreply.github.com>
@roboclaw-bot
roboclaw-bot marked this pull request as ready for review November 8, 2026 23:48
@roboclaw-bot

Copy link
Copy Markdown
Contributor

Current-head CI follow-up

Head 788b3fb33240728d5c9be9e188f7e80153adb74c remains open and ready for review, but landing is blocked by the current UI job failure.

UI job aborted after 198 passed / 6 skipped because the testhost crashed. The last active case was ReasoningPicker_ReflectsInheritedDefaultWithoutOverridingExplicitChoices; this is a location clue, not an identified cause. No individual assertion failure is reported in that aborted TRX. The separate accessibility selection recorded 21 passes / 13 skips, functional UI 19 passes, and Gateway fixture UI 81 passes. These CI counts do not replace the explicitly admitted native results in the PR body.

The same broad hosted-crash symptom has been reproduced during separate local candidates. A bounded native diagnostic narrowed it to immediately before Flyout.ShowAt with a loaded anchor and XamlRoot present, followed by WinRT stowed exception 0xC000027B. The original fatal HRESULT/root cause remains unknown; there is no evidence sufficient to attribute it to the Axe diagnostic privacy changes. A passing isolated class is not being used to clear this gate.

The current 40-case helper proof, source-bound native accessibility result and required build/unit validation remain retained. No assertion was disabled, no crash was relabeled as a passing run, and no rerun-until-green was requested. Investigation is separate from this PR's privacy/fail-closed behavior.

@roboclaw-bot roboclaw-bot added the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Nov 9, 2026
@roboclaw-bot
roboclaw-bot merged commit 636bcbc into main Nov 9, 2026
49 of 51 checks passed
@roboclaw-bot
roboclaw-bot deleted the bkudiess-safe-axe-diagnostics branch November 9, 2026 04:08
@roboclaw-bot roboclaw-bot removed the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants