Polish sidebar update and relay cards#1009
Conversation
956f217 to
7615806
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 234cfc60c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63ca3f1bc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 184d468d46
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolve conflicts with the generic relay reconnect hook, remove the direct WARP command surface, and keep sidebar reconnect success tied to cleared relay state.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 969f37864c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5872906618
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
wpfleger96
left a comment
There was a problem hiding this comment.
tl;dr: Scope this down to one generic relay card. The OSS repo (block/buzz) carries zero Block-internal content, and the BUZZ_BUILD_RELAY_RECONNECT_CMD build seam already does VPN-or-not under the hood transparently — so the VPN-aware UI is redundant, and one button beats three self-diagnosis cards. End state: one Can't reach the relay / Reconnect card for every build. This makes the PR smaller, not bigger.
The sidebar/relay card polish here is great and most of it stays. The one thing we want to change is the VPN-specific card layer. Here's the why first, then the exact what.
Why
1. The OSS repo (block/buzz) must carry zero Block-internal content. No warp-cli, no *.sqprod.co / block.xyz / squareup.com domain lists, no "VPN" product strings. The repo can have a generic, Block-agnostic seam (an env var, a hook point) — but the Block-specific filling lives only in the internal build (squareup/buzz-releases), never in OSS source.
2. That seam already exists and already does the job. The relay_reconnect_hook (relay_reconnect.rs, build-time env var BUZZ_BUILD_RELAY_RECONNECT_CMD) is a pure no-op in OSS and bakes in the warp-cli reconnect only in internal builds. So "reconnect to relay" already reconnects the VPN under the hood on internal builds, transparently, with zero UI involvement. The card never needs to know a VPN exists. The VPN-aware UI layer in this PR is solving a problem the build seam already solved — it's net-additive complexity.
3. Exposing Turn on VPN / Refresh VPN access / Reconnect to relay as three separate cards defeats the design. The whole value of the reconnect feature is that it's one button that does the right thing per build. Three cards pushes transport-layer self-diagnosis back onto the user — they shouldn't have to figure out whether it's their VPN, their access token, or the relay. Internal or not, a connectivity card that asks the user to diagnose the transport is a worse card. One button, one card, all builds.
What to CUT (delete entirely — all Block-internal content)
isBlockRelayUrl()inrelayConnectivityCard.ts— the hardcoded Block domain allowlist.isBlockConnectivityFailure()+shouldRefreshBlockVpnAccess()(same file) — the Block error-classification layer matching on"cloudflare access","vpn",403/401, etc.- The
connect-vpn/refresh-accessvariants ofRelayConnectivityCardVariantand the branching inresolveRelayConnectivityCardVariant(). After the cut the resolver has nothing left to resolve — delete it; the card is unconditionally generic. SidebarBlockVpnOffCompactCard+SidebarBlockAccessRefreshCompactCardinSidebarRelayConnectionCard.tsx(the"Turn on VPN"/"Refresh VPN access"cards).- The 3-way render ternary in
AppSidebar.tsx(~784-820) — collapses to just<SidebarRelayConnectionCard>. - The entire onboarding VPN path in
ProfileStep.tsx: the duplicatedOnboardingRelayCardVariant/OnboardingConnectivityActiontypes, the passthroughresolveOnboardingRelayCardVariant()wrapper, theconnect-vpn/refresh-accessbranches, the two VPN-card imports, and the ~180-lineOnboardingRelayConnectionErrorCard— reduce it to the generic reconnect card. - VPN content in the tests —
relayConnectivityCard.test.mjs,sidebar-relay-card.spec.ts,onboarding.spec.ts— shrink to single-generic-card assertions.
What to SIMPLIFY (fold down to generic)
useReconnectRelay.tstoast:"Reconnect failed — check your VPN or network."->"Reconnect failed — check your network."(drop "VPN"). The two// WARP VPNcode comments in this file are fine — they explain the hook internals, they're not user-facing strings.useSidebarRelayConnectionCard.ts: drop theresolveRelayConnectivityCardVariantimport and thecardVariantit exposes. Keep all the lifecycle (success-tracking, auto-dismiss timer, in-flight guard, visibility gating) — just stop branching on variant.
What to KEEP (no changes — this is the good stuff)
useReconnectRelay.tstimeout hardening:withTimeoutwith the 20s hook cap + 15s preconnect cap, andreconnect()returningboolean. Solid — stops a wedged reconnect from hanging the invoke.sidebar-action-card.tsx— the reusable compact-card primitive. Generic, no Block content.SidebarUpdateCard.tsx,AppSidebar.tsxlayout/footer polish,useSidebarRelayConnectionCard.tscard lifecycle.- The generic
SidebarRelayConnectionCard/SidebarRelayConnectionCompactCard(Can't reach the relay/Reconnect) — this is the one card we want, all builds. - The Poof dismiss animation (
PoofBurstProvider, theglobals.csspoof animation, thepow/assets). We confirmed it's the dismiss animation for these sidebar cards (fires on thebuzz-poof-triggerdismiss button) — it's legit card-dismiss polish, on-topic for this PR. Keep it in.
Net result
The PR collapses to exactly what it was meant to be: beautified sidebar + relay cards, one generic reconnect card for all builds, the reconnect-hook timeout hardening, the poof dismiss polish — and zero Block-internal content in OSS source. A smaller, cleaner diff than what's there now.
Summary
Tests
. ./bin/activate-hermit && pnpm --dir desktop exec biome check .... ./bin/activate-hermit && cargo fmt --manifest-path desktop/src-tauri/Cargo.toml --all -- --check. ./bin/activate-hermit && just desktop-tauri-check. ./bin/activate-hermit && pnpm --dir desktop typecheck. ./bin/activate-hermit && pnpm --dir desktop build. ./bin/activate-hermit && pnpm --dir desktop exec playwright test tests/e2e/relay-connectivity-screenshots.spec.ts --project=smoke. ./bin/activate-hermit && pnpm --dir desktop exec playwright test tests/e2e/sidebar.spec.ts --project=integration. ./bin/activate-hermit && pnpm --dir desktop exec playwright test tests/e2e/onboarding.spec.ts --project=integration -g "failed first profile saves can be skipped"