Skip to content

[Repo Assist] A11y: expose tool-output body in automation subtree for CompactSummary#608

Draft
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/fix-issue-590-a11y-tool-output-automation-4696eacb641b3e62
Draft

[Repo Assist] A11y: expose tool-output body in automation subtree for CompactSummary#608
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/fix-issue-590-a11y-tool-output-automation-4696eacb641b3e62

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Closes #590

Fixes the accessibility regression where the tool-output body content was rendered in a Border element that sat outside the header Button's automation subtree in CompactSummary mode. Screen readers and automation tools could navigate to the header button but could not discover the expanded body content.

Root Cause

In CompactSummary style, the tool-burst UI builds a headerButton (a Button element that toggles collapse/expand) and separately appends a body Border as a sibling. Because the body was a sibling of the button rather than a child, it was invisible to the button's AutomationName and was not reachable via UI Automation's control tree when the button was the interaction target.

Fix

  • Set an explicit AutomationName on the headerButton reflecting the current expanded/collapsed state (e.g., "Expand tool steps: <summary>" / "Collapse tool steps: <summary>").
  • Set an explicit AutomationName on the body Border ("Tool steps for: <summary>") so it is independently accessible even when it sits outside the button's subtree.

These two names give screen readers unambiguous labels for both the toggle target and the expandable content region.

Test Status

  • Build: Infrastructure failure — GitVersion MSBuild task fails to write GitHub Actions env-file (set_env_*) in this runner environment. This is a pre-existing infrastructure issue unrelated to this change (no C# compiler errors).
  • Tray tests: 870 passed / 0 failed (verified on the commit in this branch before master diverged).

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

… body

In the CompactSummary tool-burst style, the header is a standalone Button
and the expanded body (step rows) is a sibling element outside the Button's
automation subtree. This meant Narrator only announced the button's visual
content (a FlexRow with text nodes that may not compose cleanly) without
context about the task state.

Changes:
- Build a headerAutomationName string: "<summary>, <N step(s)>, <status>,
  expanded/collapsed" and pass it to .AutomationName() on headerButton.
  Narrator now announces the full card state when the button is focused.
- Add .AutomationName("Tool steps for: <summary>") on the body Border so
  scan-mode navigation gives the body region a clear label linking it back
  to the task the header describes.

Fixes #590

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

clawsweeper Bot commented May 31, 2026

Codex review: found issues before merge. Reviewed May 30, 2026, 9:44 PM ET / 01:44 UTC.

Summary
The PR adds explicit AutomationName values to the CompactSummary tool-output header Button and expanded body Border in the WinUI tray chat timeline.

Reproducibility: no. high-confidence runtime reproduction was established here. Source inspection confirms the body is a sibling outside the header Button subtree, but no current-main or PR-head Narrator/UIA proof is attached.

Review metrics: 2 noteworthy metrics.

  • Automation names added: 2 added. Both additions alter the UI Automation surface of the tray tool-output card and need runtime accessibility proof.
  • Changed surface: 1 C# file, 15 additions, 1 deletion. The patch is small but concentrated in a user-facing screen-reader path.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted Narrator or UI Automation tree proof showing the header and expanded step rows are discoverable after the fix.
  • [P1] Rework or prove the body-region naming so the wrapper does not hide descendant step rows.
  • [P1] Run the AGENTS.md-required build, shared tests, and tray tests on the current PR head.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix Narrator/UIA tree, recording, terminal output, log, or artifact proof is attached; add redacted proof to the PR body for re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun automatically. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A real Windows UIA/Narrator capture would materially help verify this accessibility change. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: capture Windows UIA/Narrator proof for an expanded CompactSummary tool card, including header focus and per-step row navigation.

Risk before merge

  • [P1] Naming the expanded body Border may make the wrapper reachable while hiding the per-step row descendants from UI Automation, which would preserve the accessibility failure in a different form.
  • [P1] No after-fix Narrator/UIA tree proof is attached, so the real Windows accessibility behavior is unverified.
  • [P1] The PR body does not show the AGENTS.md-required full build, shared tests, and tray tests passing on the current PR head.

Maintainer options:

  1. Preserve Row Discoverability Before Merge (recommended)
    Adjust the body accessibility approach or prove with a UIA tree that the named body region still exposes every expanded step row.
  2. Accept With Explicit A11y Proof
    Maintainers can accept the current structure only after independent Windows accessibility proof shows it fixes the issue without hiding nested content.
  3. Pause Until Human-Tested
    If no real UIA/Narrator proof is available, keep the linked issue open and pause this draft rather than merging an unverified accessibility change.

Next step before merge

  • [P2] Needs maintainer review for the UIA container-naming risk and real Windows accessibility proof; this is not a safe automated repair lane from the current evidence.

Security
Cleared: The diff only changes WinUI accessibility naming in one C# file and introduces no concrete security or supply-chain concern.

Review findings

  • [P2] Keep expanded tool rows visible to UI Automation — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:2091
Review details

Best possible solution:

Expose the expanded body in a way that preserves per-step UIA descendants, then land only with current full validation and redacted Narrator/UIA tree proof showing the header and rows are discoverable.

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

No high-confidence runtime reproduction was established here. Source inspection confirms the body is a sibling outside the header Button subtree, but no current-main or PR-head Narrator/UIA proof is attached.

Is this the best way to solve the issue?

Unclear as written. The header button label is plausible, but naming the body wrapper conflicts with existing local UIA guidance unless the PR proves descendant step rows remain visible.

Full review comments:

  • [P2] Keep expanded tool rows visible to UI Automation — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:2091
    This names the Border that wraps VStack(8, stepRows.ToArray()). The current timeline already documents that naming a container can make UIA treat it as a leaf and hide nested tool content; applying the same pattern to the expanded body can make the body label reachable while still hiding the actual per-step rows. Use a non-leaf relationship/labeling approach or attach UIA tree proof that the descendants remain navigable.
    Confidence: 0.83

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against eb06fba21c44.

Label changes

Label justifications:

  • P2: This is a normal-priority accessibility bug fix with limited blast radius in the tray chat timeline.
  • merge-risk: 🚨 other: Merging could leave expanded tool rows hidden from screen readers if the named body wrapper becomes a UIA leaf.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix Narrator/UIA tree, recording, terminal output, log, or artifact proof is attached; add redacted proof to the PR body for re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun automatically. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • PR diff changes UIA names on the CompactSummary card: The PR head adds a computed AutomationName to the header button and adds AutomationName("Tool steps for: ...") to the Border wrapping the expanded step rows. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:2091, 50d5f856d1d6)
  • Current code warns named containers can become UIA leaves: The current timeline deliberately avoids AutomationName on a parent when nested tool UI is present because UIA can treat the named container as a leaf and hide nested content from screen readers. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:1324, eb06fba21c44)
  • FunctionalUI AutomationName maps directly to WinUI AutomationProperties.Name: The helper stores the modifier and later calls AutomationProperties.SetName on the rendered FrameworkElement, so naming the Border changes the actual UIA element name. (src/OpenClawTray.FunctionalUI/FunctionalUI.cs:1607, eb06fba21c44)
  • Current CompactSummary body is a sibling of the header button: On current master the expanded body is appended as a sibling Border containing VStack(8, stepRows.ToArray()), which matches the related accessibility concern but also means the body wrapper owns the descendant rows. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:2075, eb06fba21c44)
  • Validation and proof are not complete in the PR body: The PR body reports a build infrastructure failure and tray tests from an earlier branch state, with no attached after-fix Narrator, UIA tree, recording, terminal output, log, or artifact proof. (50d5f856d1d6)
  • Feature history points to recent chat timeline owners: Git history shows recent work in this exact tool-call timeline area from the native chat UI fixes and tool-call visibility toggle commits. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, bf21e7b216aa)

Likely related people:

  • RBrid: Authored the native chat UI fixes in commit bf21e7b, which changed CompactSummary and nestable tool-burst behavior in the same timeline file and is referenced by the related accessibility issue. (role: recent area contributor; confidence: high; commits: bf21e7b216aa; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs)
  • Scott Hanselman: Authored commit 50837d7 for the production tool-call visibility toggle in the same chat timeline surface and is also a co-author on the native chat UI fix history. (role: recent adjacent contributor; confidence: medium; commits: 50837d79ecc2, bf21e7b216aa; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels May 31, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

github-actions Bot commented Jun 4, 2026

🤖 This is a note from Repo Assist.

PR #650 (Fix tool output accessibility labels) by @ranjeshj was submitted for the same issue (#590) and includes a regression test. It is currently marked as ready for maintainer review and appears to be a more complete fix.

Suggesting this PR (#608) be closed in favour of #650 to avoid confusion. Happy to defer to the maintainer's judgement on which approach to take.

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. repo-assist status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A11y: tool-output body is outside the header Button's automation subtree

0 participants