Dedupe connection error bars into a single thin top banner#865
Open
bkudiess wants to merge 1 commit into
Open
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 5:23 PM ET / 21:23 UTC. Summary Reproducibility: yes. at source/proof level: current main still defines the in-page AuthErrorBar and separately publishes connection:authentication-failed, and the inspected before screenshot shows both bars for one auth failure. I did not run the WinUI app in this read-only review. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused top-banner consolidation once required checks finish green and maintainers accept the intended connection-notification priority behavior. Do we have a high-confidence way to reproduce the issue? Yes, at source/proof level: current main still defines the in-page AuthErrorBar and separately publishes connection:authentication-failed, and the inspected before screenshot shows both bars for one auth failure. I did not run the WinUI app in this read-only review. Is this the best way to solve the issue? Yes: consolidating onto the existing snapshot-driven connection:issue banner and making actionable connection notifications win is the narrowest maintainable path I found. The added tests cover the priority cases that caused Open Connection to degrade to Show more. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f51a8618ed7b. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
proof: sufficient
A connection/auth failure rendered two error bars: the global top-window InfoBar plus an in-page "Connection Error" InfoBar on the Connection page. The auth pipeline also published two global notifications for one failure, which forced the banner action to degrade to "Show more". Remove the in-page AuthErrorBar and consolidate to one top banner that: - carries the gateway/node error states the in-page bar covered - always routes the user to the Connection page via an "Open Connection" action (connection notifications are prioritized as the visible banner, preferring an actionable one over an action-less transient) - renders as a single thin line: bold headline + " - detail", with the action as a right-aligned hyperlink (Windows-aligned InfoBar) Other changes: - Stop publishing the duplicate connection:authentication-failed banner; the snapshot-driven connection:issue notification is the single source. - Re-home transient errors: WSL host-action failures use the inline card status (or a top notification when the card is hidden); connect/switch failures publish via ShowTransientConnectionError on the same banner id. - Remove now-dead resw strings (ConnectionPage_AuthGuidance*, ConnectionPage_ConnectFailed) across all five locales. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkudiess
force-pushed
the
bkudiess-dedupe-connection-info-bars
branch
from
19ad06d to
bd37f32
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A connection/auth failure rendered two error bars: the global top-window InfoBar and an in-page "Connection Error" InfoBar on the Connection page. The auth pipeline also published two global notifications for a single failure, which forced the banner's action to degrade to "Show more" instead of routing to Connection.
This consolidates everything into one thin top banner and removes the in-page bar, with no loss of states/messages.
What changed
AuthErrorBar(XAML + all code paths) and its now-dead resw strings (ConnectionPage_AuthGuidance*,ConnectionPage_ConnectFailed) across all 5 locales.connection:authentication-failedbanner; the snapshot-drivenconnection:issuenotification is the single source.ShowTransientConnectionErroron the same banner id (cannot double-bank).Validation
./build.ps1— all projects buildScreenshots
Before: two bars (top "Gateway connection failed" + in-page "Connection Error").
After: one thin banner —
⊗ Gateway connection failed — Transport error … Open Connection ✕— with the Connection page showing only its own recovery card.