Repository navigation
fix(desktop): show upgrade guidance (BUZZ-517) - #9033
Conversation
f419873 to
165ddf8
Compare
165ddf8 to
5c4ed04
Compare
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
There was a problem hiding this comment.
🤖 Automated review at e3715a08285457028bc8bc3b73ffa717ddc0ca6c: changes requested for one minimalism issue. Two independent source reviews found no correctness problems in the discovery check, the connection gate or the upgrade card.
Blocking
IMPORTANT — unused upgrade-override settings added by this PR. desktop/src-tauri/src/commands/relay_entry_requirements.rs:39-79 adds configured_upgrade_url, configured_upgrade_instructions and configuration_matches, and :138-157 computes the upgrade_url and upgrade_instructions fields from three new build variables (desktop/src-tauri/build.rs:11-13, BUZZ_BUILD_REQUIRED_AUTH_UPGRADE_*). desktop/src/shared/api/relayEntryRequirements.ts:9-10,119-120 stores both fields, but RelayEntryGate.tsx never reads them. The card always uses the fixed https://builderlab.xyz destination and text. The file is new in this PR, so nothing depends on this code. It adds a second, host- and platform-scoped configuration contract with its own validation and tests, and no product behavior uses it.
Fix: have fetch_relay_entry_requirements return only required. Remove the three helpers and their tests, the build.rs lines, the two snapshot/IPC fields, the matching e2e mock fields (desktop/src/testing/e2eBridge.ts:373-374, desktop/tests/helpers/bridge.ts:281-282), and the README sentence about the "older private-host/platform build overrides". Keep the fixed opener and its failure-path fallback.
Non-blocking
- Fixture provenance.
desktop/src-tauri/tests/fixtures/nip_fi/README.mdpins pre-merge producerd3ae4935band leaves regeneration as a to-do. #8988 is merged; main publishes the same top-levelrequires_nip_fiboolean (crates/buzz-relay/src/nip11.rs:466, effective value fromnip_fi_policy.rs:164), so the fixtures can be re-exported from main and pinned to that SHA. - README to-dos. The new
desktop/README.mdsection mixes behavior documentation with release-readiness steps and an open decision ("Refresh the pinned fixtures … before readiness", "Whether that route is needed remains an owner decision"). Those will go stale once this merges. Keep the behavior description in the README and move the rollout checklist to the release companion.
Checked and sound
- Native discovery rejects userinfo, query and fragment before any request, maps only
ws→httpandwss→https, and uses a dedicated client with redirects disabled and no auth or cookie wiring. One 5 s timeout covers send and the streamed body. The 256 KiB cap is enforced on bothContent-Lengthand accumulated chunks. Only a booleanrequires_nip_fidecides; a missing field means not required, and any other shape is an error. - Only a validated
truethrowsUpgradeRequiredError, andrelayClientSession.ts:601-609stops reconnecting only for that error. Failed or malformed discovery is logged and the socket is dialed as before. Credential-bearing addresses are rejected before that fallback. - Discovery epochs plus
reset()ondisconnect()fence late results from a previous community, and the connection generation is checked before and after discovery and after the dial. The card also requires the snapshot URL to equal the active community URL, whichapply_workspacestores verbatim. - Address edits cancel onboarding only on an actual address change, nulling the transaction refs before clearing state so a pending profile check can't write its completion marker. Retry, navigation and discovery never reset keys or remove saved communities.
- Moving
preconnectintoApp.tsx, keyed on the applied community, lets the check run during the join curtain. The removed shell effect's cancellation flag only suppressed a log line. - The opener receives a literal URL; nothing from the discovery document reaches the UI or the opener.
CI was green at this head. A partial live run at this head passed: the frontend build, all 3 relay-entry-guidance.spec.ts browser journeys, the full desktop JS suite (6,934 helper and 171 jsdom tests), and the 3 native relay_entry_requirements tests. The run was stopped once the blocker was confirmed, so the extra loopback probes of the production native command (redirect, oversized body, 5 s stall, userinfo address) did not run. They'll run on the next head.
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated review at 690a66140661275fe637835a31a69b2cb3463aeb: no blocking findings. This head fixes the issue from the previous review.
Previous blocker fixed. 690a6614 removes the unused build-time upgrade link and message settings: the three helpers, the BUZZ_BUILD_REQUIRED_AUTH_UPGRADE_* build variables, the two response fields, and the matching frontend snapshot fields and test mocks. A search of the whole head tree finds none of them left. fetch_relay_entry_requirements now returns only { required }, and the upgrade card still opens the fixed https://builderlab.xyz, with the same URL as the manual fallback.
Nothing else changed. The commit only deletes code. Two independent source reviews confirmed that the rest of the check is unchanged:
- The discovery client still follows no redirects, sends no credentials or cookies, and keeps its 5-second deadline and 256 KiB body limit.
- A failed check still falls back to the normal connection.
- Only an explicit
requires_nip_fi: truestops reconnecting. - A late answer from one community still can't show the card on another.
Live run, all passing:
- The frontend build, the 3 browser journeys in
relay-entry-guidance.spec.ts, the 3 nativerelay_entry_requirementstests, and the full desktop JS suite all pass. - These probes were run against loopback servers by calling the public native function directly:
- A 302 is rejected: the redirect target received 0 connections.
- 262,181-byte bodies are rejected, both with and without
Content-Length. - A stall in the headers and a stall partway through the body each time out at about 5.003 s.
- An address with credentials in it is rejected, and its listener received 0 connections.
- A plain request returns
required: trueand sends noAuthorizationorCookieheader.
Non-blocking, unchanged from the previous review:
desktop/src-tauri/tests/fixtures/nip_fi/is still named after and pinned to the pre-merge commitd3ae4935bof #8988. That PR is merged now, so the fixtures can be regenerated from main and repinned.- The new section in
desktop/README.mdstill includes release to-dos and an open owner decision ("Whether that route is needed remains an owner decision"). These will go stale after merge. They'd fit better in the release docs, leaving the README with only the behavior description.
|
🤖 Brad Seiler reports that he tested locally:
These are Brad’s manual test results on his running local app, before the requested rebase and fixture/documentation update. |
Read the host requirement before each connection and keep upgrade guidance separate from retryable discovery failures. Preserve community and identity storage. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Keep failed discovery unknown and let normal relay authentication enforce admission. Open the modal only for explicit required policy and cover authenticated reconnect, offline recovery and stale-host results. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Allow packagers to supply text guidance without a download URL. Keep instructions bound to the reviewed community and Apple Silicon macOS. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Keep query and fragment URLs on the existing socket path when native discovery rejects them. Continue to block credentials before fallback dialing. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Avoid colliding with the sibling community query provider so switching removes the old app tree. Assert unique community controls in the upgrade journey. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Require promotion of the maintenance client to the public updater and corrected live discovery before bb-public required admission. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Abandon the previous relay's transaction and fence pending profile checks before continuing with an edited address. Cover name-only edits and late profile completion in the upgrade journey. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Remove release planning and unresolved decisions from the app README. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Export complete discovery responses from main 6b8a72e. Record the corrected Shadow required-host case and reproducible router exporter. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
690a661 to
8caa252
Compare
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated review at 8caa25260b124316f6ad65694d47134a566edeee: approving. The earlier blocker and both non-blocking notes are addressed, and the new commits introduce no problems.
The rebase changed nothing we reviewed. git range-diff shows all 9 earlier commits as identical patches on the new base. That includes 074aca14, the removal of the unused upgrade-override settings that was blocking. The only new work is three commits: the fixture refresh, the README trim and a blank-line fix. None of them touches the discovery reader, the connection gate or the upgrade card. The only code change is the three fixture paths in the native test.
Fixture provenance: fixed. The fixtures are now pinned to 6b8a72e835, which is on main. I ran the exporter from desktop/src-tauri/tests/fixtures/nip_fi/README.md against a detached checkout of that commit. It passed, and all three generated /info bodies are byte-identical to the committed buzz_main_6b8a72e835_*.json files. The relay's NIP-11, policy and router sources haven't changed on main since that commit.
README to-dos: fixed. The release checklist and the open owner decision are gone. The section now only describes behavior.
Non-blocking: desktop/README.md still says "This default applies to … unconfigured public release builds". With the build-time overrides removed, there's nothing to configure, so "default" and "unconfigured" now suggest an override that no longer exists. Saying the card always opens https://builderlab.xyz would be accurate.
CI is green except the Codex security review, which needs a fresh authorization for this range.
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated review at 71689b47087af1a1c7d8763bb2d5efd15f441ccd: approving. The earlier note is addressed, and the new work introduces no problems.
New work: one commit, 749ab5c9, changes the download link from builderlab.xyz to buzz.xyz in three places: the card text, the URL the button opens and the fallback message in RelayEntryGate.tsx. It updates the four matching assertions in relay-entry-guidance.spec.ts and the README to match. The rest is a merge of main that needed no conflict fixes (git show --remerge-diff is empty). The discovery reader, the connection gate and the security code are unchanged, and no builderlab.xyz remains in the changed files.
Earlier note addressed: the README no longer calls the link a "default" for "unconfigured" builds. It now says the card always opens the fixed https://buzz.xyz.
CI is still running on this commit. The Codex security review needs a fresh authorization for this range.
…-media * origin/main: fix(mobile): offer a new identity beside saved keys on unclaimed accounts (#9107) fix(desktop): show upgrade guidance (BUZZ-517) (#9033) fix(mobile): skip the read-state snapshot when nothing needs publishing (#9093) fix(mobile): keep verified names on screen in channels with over 1,024 members (#9111) mobile: reuse channel message rows that did not change (#9115) fix(mobile): back off push bootstrap retries (#9092) fix(mobile): keep reply counts on screen while a reconnect catches up (#9100) fix(mobile): inflate gzip read-state snapshot responses (#9089) perf(relay): cache the community-active check on socket admission (#9109) fix(db): auto-migrate prebuild-guarded indexes on small databases (#8802) docs: mark desktop/ as the deprecated Buzz 0.X app (#8840) Signed-off-by: Brad Seiler <seiler@squareup.com>
…dary * origin/main: fix(mobile): focus the search field when search opens (#9120) perf(mobile): validate each verified profile once, not on every publish (#9101) fix(mobile): offer a new identity beside saved keys on unclaimed accounts (#9107) fix(desktop): show upgrade guidance (BUZZ-517) (#9033) fix(mobile): skip the read-state snapshot when nothing needs publishing (#9093) fix(mobile): keep verified names on screen in channels with over 1,024 members (#9111) mobile: reuse channel message rows that did not change (#9115) fix(mobile): back off push bootstrap retries (#9092) fix(mobile): keep reply counts on screen while a reconnect catches up (#9100) fix(mobile): inflate gzip read-state snapshot responses (#9089) perf(relay): cache the community-active check on socket admission (#9109) fix(db): auto-migrate prebuild-guarded indexes on small databases (#8802) Signed-off-by: Will Pfleger <wpfleger@block.xyz>
Summary
Old Buzz desktop clients now check a community’s public requirement before each connection. An explicit
requires_nip_fi: trueshows an upgrade screen with a fixed https://buzz.xyz download action and manual fallback. Missing or false requirements, and failed discovery, preserve normal connection behavior.Retry, community navigation and address editing preserve keys and saved communities. Changing an address cancels the old onboarding transaction and fences pending profile checks. Native discovery sends no credentials, follows no redirects, and limits requests to five seconds and 256 KiB. Wire fixtures are exported from main
6b8a72e835with reproducible provenance.Related issue
BUZZ-517. Related: #8984 contains mobile parser preparation; #8920 repairs relay rejection messages.
Rollout
There is no client flag. The upgrade screen appears only when the relay advertises
requires_nip_fi: true. Public auto-update delivery requires a desktop release containing this change and promotion to the OSS auto-update feed.In Shadow mode, requiring a community activates the screen; making it optional restores normal connections. Global Enforce requires every community regardless of that setting. Rollback must restore an effective non-required policy. The optional and required outcomes are demonstrated by Brad’s two manual cases below.
Testing
At
71689b4708, Honor’s isolated browser journeys passed 3/3 with zero retries and verified served assets, including the fixedhttps://buzz.xyzopener and manual fallback. McKeon independently built and passed the same three journeys at the previous head8caa25260bwith the repository command:CI=1 pnpm --dir desktop build:e2e, thenCI=1 pnpm --dir desktop exec playwright test relay-entry-guidance.spec.ts --project=smoke. Native IPC was mocked.Brad tested the running pre-rebase app locally: an optional bb-public community in Shadow connected without a message; required bb-block staging showed the upgrade message. These results apply to that running app, not the rebased head.
Before:
After:
Generated with Codex