fix(setup): avoid duplicate failure message - #1187
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
RomneyDa
added
the
status: 🚢 actively landing
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. Reviewed August 20, 2026, 9:02 PM ET / August 21, 2026, 01:02 UTC. ClawSweeper reviewWhat this changesThis PR hides the completion-page subtitle on setup failure so details appear only in the red error card, while restoring the subtitle on success and adding a regression contract test. Merge readinessKeep open. This is a narrow, correct fix for an existing setup failure presentation defect; it awaits the member-owned PR's required Windows CI and landing decision. Priority: P3 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherThe setup pipeline sends success or failure results to the WinUI completion page. That page renders the final user-facing status, error details, help link, and recovery actions. flowchart LR
A[Setup pipeline result] --> B[Completion navigation]
B --> C{Succeeded?}
C -->|Yes| D[Success summary and subtitle]
C -->|No| E[Failure heading]
E --> F[Red error card]
F --> G[Help, log, and fallback actions]
Decision needed
Why: The patch is correct by source inspection, but the remaining decision is the acceptance threshold for a member-owned WinUI change whose local Windows proof was unavailable. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land after the required Windows checks pass, and capture an isolated setup-failure screen showing a single visible error message when a Windows validation host is available. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: the current-main failure path reached CompletePage with an error message that the base branch rendered in both the subtitle and ErrorText. A live Windows reproduction was not supplied. Is this the best way to solve the issue? Yes. Collapsing only the redundant subtitle preserves the specific error, help link, log link, fallback action, and normal success subtitle without adding a parallel presentation path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d3ed6c78bf14. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
1 participant
Summary
This is independent of #1178. The duplicate rendering already exists on
main, and #1178 does not change the failure-state logic.Validation
git diff --check(passed).agents/skills/autoreview/scripts/autoreview --mode local(passed, no accepted/actionable findings)pwsh -NoProfile -File ./build.ps1(blocked:pwshis not installed on this macOS controller)dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore(blocked:dotnetis not installed)dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore(blocked:dotnetis not installed)prlctlis not installed)Repository Windows CI is required to provide the mandatory build and test validation.
Real behavior proof
Not verified / blocked. This controller has no native Windows validation backend, so the current-head onboarding failure UI could not be launched for a screenshot. The source change collapses
SubtitleTextin the failure branch and leavesErrorText.Text = errorMessagein the red error card. The success branch explicitly restores subtitle visibility.Rubber-duck review
The structured Codex autoreview passed with no accepted/actionable findings. It confirmed the patch removes the duplicate failure subtitle while retaining the error card and restoring success-state subtitle visibility.