fix(web): route mobile invite downloads to app stores - #7702
Conversation
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
|
@codex review |
|
@builderbot review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@buzz-security-review 16f88e7 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16f88e7984
ℹ️ 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".
github-actions
Bot
added
the
codex-security-review-current
Signed-off-by: Tom Brow <tomb@block.xyz>
16f88e7 to
52c3c10
Compare
github-actions
Bot
removed
the
codex-security-review-current
|
@buzz-security-review 52c3c10 |
|
@builderbot review |
|
@codex review |
github-actions
Bot
added
the
codex-security-review-current
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52c3c104c6
ℹ️ 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".
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@buzz-security-review c2926ac |
|
@codex review |
github-actions
Bot
added
codex-security-review-current
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent reviewed base b36600fc440615e2565f94bffc800e0495180f6b → exact head c2926acaf7b67a2cfb19607df449755404955fb2.
Request changes
P1 — Mobile users can tap the fallback release link before store routing resolves
InvitePage renders the live anchor with BUZZ_RELEASES_URL, and replaces it only after detectBuzzDownloadPlatform() resolves (web/src/features/invite/ui/InvitePage.tsx:56,63-75,275-289). That detector computes the OS synchronously, but still awaits optional high-entropy architecture hints for iOS and Android even though their store destinations do not depend on architecture (web/src/shared/lib/buzz-download.ts:99-120,155-160).
This was reproduced against the production bundle with an Android UA and getHighEntropyValues() delayed by three seconds:
immediate=https://github.com/block/buzz/releases
after100ms=https://github.com/block/buzz/releases
settled=https://play.google.com/store/apps/details?id=xyz.block.buzz.mobile
A user who taps during that interval is sent to GitHub Releases rather than Google Play; a hung implementation can make the wrong destination permanent. The current E2E waits for the eventual href (web/tests/e2e/smoke.spec.ts:359-363), so it does not cover this race.
Author action: return from platform detection immediately for ios and android with architecture unknown, before requesting desktop architecture hints. Add a production-seam regression with delayed or never-settling getHighEntropyValues() proving that the rendered mobile CTA promptly exposes and follows the correct store URL.
P2 — New public exports violate the repository documentation rule
BUZZ_IOS_APP_STORE_URL and BUZZ_ANDROID_PLAY_STORE_URL are new undocumented exports (web/src/shared/lib/buzz-download.ts:2-4), while AGENTS.md:147-150 requires doc comments on new public API.
Author action: add concise doc comments, or keep these constants module-private if they are not intended as public API.
Integrated review evidence
The systems review found the steady-state routing contract sound: iOS/iPadOS is classified before Mac compatibility tokens, Fire OS remains on the generic fallback, Android is classified before Linux tokens, store routes bypass GitHub API/cache work, and the existing buzz://join path plus CTA styling/semantics are unchanged. Its exact-head checks passed: just web-check, just web-build, and just web-e2e-smoke (6/6), with causal mutations confirming coverage of eventual iOS, Android, and Fire OS selection. The apparent disagreement between that steady-state result and the P1 finding is resolved by lifecycle timing: eventual URL selection is correct, but the already-interactive initial anchor is not.
The product/UI lane additionally passed full web E2E (6/6), web check, typecheck, and diff check at the same clean head; its routing mutation failed the relevant smoke assertion and was restored byte-for-byte. At final freshness check, local HEAD and live PR head both remained c2926acaf7b67a2cfb19607df449755404955fb2, and the tree was clean. Clients / Web, aggregate Web, DCO, and security checks were green; unrelated desktop checks remained in progress.
Confidence gaps (not author defects)
- No physical iOS/iPadOS/Android browser tap-through was performed. Playwright validates browser logic, not native store-app handoff. Author action: none. Verification owner: CI/release gate / downstream device validation.
- Apple and Play endpoints were reachable, and the Android package matches
mobile/android/app/build.gradle.kts:154; Apple listing ownership was not independently authenticated beyond its redirect/listing. Author action: none. Verification owner: CI/release gate.
The initial-href race and repository-policy violation are concrete, PR-caused, author-actionable defects. Please fix both before merge.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: b36600fc440615e2565f94bffc800e0495180f6b..c2926acaf7b67a2cfb19607df449755404955fb2 (exact head c2926acaf7b67a2cfb19607df449755404955fb2)
Risk: medium — this changes the user-visible mobile invite destination and relies on asynchronous browser platform detection.
Behavior/contracts traced: invite-page download CTA initialization and update lifecycle; iOS/iPadOS/Android/Fire OS/ChromeOS classification precedence; desktop release fallback and Mac selection; store URL/package identity; rendered anchor semantics; exact-head web checks and smoke coverage.
Blocking findings
-
A fast mobile tap can still open GitHub Releases instead of the platform store.
InvitePagerenders the download anchor withBUZZ_RELEASES_URLinitially and changes it only afterdetectBuzzDownloadPlatform()resolves (web/src/features/invite/ui/InvitePage.tsx:56,63-75,273-290). That detector already knows the OS synchronously, but still awaits optional high-entropy architecture data for iOS and Android (web/src/shared/lib/buzz-download.ts:93-120), even though the mobile destination does not use architecture (web/src/shared/lib/buzz-download.ts:155-160). With an Android navigator whosegetHighEntropyValues()resolves after three seconds, the production bundle exposed GitHub Releases immediately and at 100 ms, changing to Google Play only after settlement. A slow or non-settling implementation therefore leaves the wrong actionable destination visible. The current E2E waits for eventualhref, so it cannot catch this journey failure (web/tests/e2e/smoke.spec.ts:359-363).Author action: return promptly from mobile detection once the OS is known (for example,
ios/androidwith architectureunknown) before requesting desktop architecture hints. Add a production-seam regression with delayed or non-settlinggetHighEntropyValues()proving the rendered mobile CTA promptly receives and follows the correct store destination.Verification owner: author.
-
The new exported URL constants lack the repository-required API documentation.
BUZZ_IOS_APP_STORE_URLandBUZZ_ANDROID_PLAY_STORE_URLare new exports atweb/src/shared/lib/buzz-download.ts:2-4;AGENTS.md:147-150requires doc comments for new public API.Author action: add concise doc comments, or keep the constants module-private and test the documented production behavior without exporting them.
Verification owner: author.
Validation
At clean detached head c2926acaf7b67a2cfb19607df449755404955fb2 with the pinned Hermit toolchain:
pnpm -C web check— PASS.pnpm -C web build— PASS.pnpm -C web test:e2e:smoke— PASS, 6/6.- Independent full web E2E — PASS, 6/6.
- Causal route mutations made the changed smoke test fail for iOS, Android, and Fire OS, then were restored cleanly.
- Live URL probes: Apple’s short URL redirected to the Buzz App Store listing; Google Play returned 200. The Android package matches
mobile/android/app/build.gradle.kts:154. - Authenticated reviewer is
jedwards27; live PR author isbrow.
Manual/native evidence: no real iPhone/iPad/Android browser tap-through was exercised. Playwright proves emulated navigator routing, not store-app handoff. This is a confidence gap owned by reviewer/tooling or the downstream release/device gate, not additional author rework beyond the concrete race regression above.
Residual risk: store listing ownership/availability and physical-device handoff remain externally verified only by URL reachability. Desktop CI jobs were still pending during review; the Web checks were green. A new head expires this verdict and requires delta review.
Signed-off-by: Tom Brow <tomb@block.xyz>
github-actions
Bot
removed
the
codex-security-review-current
|
🤖 Addressed both findings in eaea30e. Mobile detection returns before requesting architecture hints, and the exported store URLs have doc comments. The rendered-link regression fails before the fix with never-settling hints and now verifies the destination after a click. All six web smoke tests, build, and lint pass. |
blockers resolved
|
@buzz-security-review eaea30e |
github-actions
Bot
added
the
codex-security-review-current
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: b36600fc440615e2565f94bffc800e0495180f6b..eaea30e1c2b80c5d44db39ff19a393264d6ae28a (delta from prior reviewed head c2926acaf7b67a2cfb19607df449755404955fb2; exact live head eaea30e1c2b80c5d44db39ff19a393264d6ae28a)
Risk: medium — user-visible mobile invite routing with asynchronous browser platform detection.
Behavior/contracts traced: prompt mobile CTA initialization; iOS/iPadOS/Android/Fire OS/ChromeOS classification; desktop architecture hints and artifact selection; external-link activation/new-page lifecycle; store URL/package identity; exported API policy; exact-head test binding and cleanup.
Findings
No unresolved author-actionable defect remains.
The previous timing blocker is closed: detectBuzzDownloadPlatform() now returns immediately for iOS/Android before optional high-entropy architecture probing (web/src/shared/lib/buzz-download.ts:95-107), while desktop systems retain the existing architecture path (web/src/shared/lib/buzz-download.ts:108-125). This is contract-safe because mobile store resolution depends only on OS (web/src/shared/lib/buzz-download.ts:160-165).
The regression test binds the production journey: it installs a never-settling architecture hint, waits for the rendered destination, intercepts that exact external URL, clicks the real CTA, verifies the newly opened page, and closes each context (web/tests/e2e/smoke.spec.ts:313-329,364-375). Removing the mobile short-circuit made that test time out on the initial iOS row; removing Fire OS precedence made its fallback assertion fail. A separate macOS delayed-entropy probe still selected the ARM64 artifact.
The previous documentation blocker is also closed: both exported store constants now have doc comments (web/src/shared/lib/buzz-download.ts:2-6), satisfying AGENTS.md:147-150.
Author action: none.
Verification owner: none for code findings.
Validation
At clean detached head eaea30e1c2b80c5d44db39ff19a393264d6ae28a with pinned Hermit:
pnpm -C web check/just web-check— PASS.pnpm -C web build/just web-build— PASS.pnpm -C web test:e2e:smoke/just web-e2e-smoke— PASS, 6/6.- Independent full web E2E — PASS, 6/6.
git diff --check— PASS.- Causal mutations for the mobile short-circuit and Fire OS guard failed the changed E2E as expected, then were restored cleanly.
- Live PR head and local head were rechecked at
eaea30e1c2b80c5d44db39ff19a393264d6ae28a; authenticated reviewer isjedwards27, PR author isbrow.
Manual/native evidence: production-bundle Playwright proved prompt href selection and browser popup navigation under representative navigator values. No physical iOS/iPadOS/Android store-app handoff was exercised.
Residual risk: native OS takeover into the store app and authenticated store-console ownership remain unverified. These are confidence gaps, not author defects. Author action: none. Verification owner: CI/release gate / downstream device validation. Unrelated repository jobs may still be pending; the required Web/client checks observed during review were green. A new head expires this approval.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-reviewed the author’s fix delta from c2926acaf7b67a2cfb19607df449755404955fb2 to exact live head eaea30e1c2b80c5d44db39ff19a393264d6ae28a (base b36600fc440615e2565f94bffc800e0495180f6b).
Approved — both prior blockers are closed
-
Mobile CTA timing race:
detectBuzzDownloadPlatform()now returns immediately for iOS/Android before optional high-entropy architecture work (web/src/shared/lib/buzz-download.ts:95-107). Store resolution depends only on OS (:160-165), while desktop systems retain the existing UA baseline and high-entropy architecture path (:108-143). An independent macOS production-bundle probe with delayed ARM64 entropy still selected the_aarch64.dmgafter settlement. -
Public API documentation: both exported store URL constants now have doc comments (
web/src/shared/lib/buzz-download.ts:2-6), satisfyingAGENTS.md:147-150.
Regression quality and validation
The changed E2E installs a never-settling getHighEntropyValues() before page creation, exercises iPhone, iPadOS desktop mode, Android, Fire OS, and ChromeOS, asserts the rendered href, intercepts the exact external destination, clicks the real CTA, verifies the opened page URL, and closes each context (web/tests/e2e/smoke.spec.ts:313-375). This covers prompt mobile routing and both store/fallback activation without page/context leakage.
Independent production-bundle validation with never-settling entropy observed and clicked the correct destinations promptly:
iPhone: href=https://apps.apple.com/app/id6779728271 observed_ms=141 popup=https://apps.apple.com/app/id6779728271
Android: href=https://play.google.com/store/apps/details?id=xyz.block.buzz.mobile observed_ms=126 popup=https://play.google.com/store/apps/details?id=xyz.block.buzz.mobile
Team exact-head evidence:
CI=1 just web-check— PASSCI=1 just web-build— PASSCI=1 just web-e2e-smoke— 6/6 PASSpnpm -C web typecheck— PASS- full PR and fix-delta
git diff --check— PASS - deleting the mobile short-circuit caused the changed E2E to fail/time out on the initial iOS row across retries; deleting Fire OS precedence caused its fallback assertion to fail; both mutations were restored cleanly
- local head and live PR head were rechecked as
eaea30e1c2b80c5d44db39ff19a393264d6ae28a; review tree clean - required Web/client, DCO, and security checks are green; unrelated desktop/integration jobs remain in progress and are owned by CI/release gating
Confidence gaps (not author defects)
- Physical iOS/iPadOS/Android native store-app handoff was not exercised. Browser popup creation and exact URL navigation are proven, not OS takeover. Author action: none. Verification owner: CI/release gate / downstream device validation.
- External listing ownership was not authenticated through store consoles; reachable Buzz listings and the repository Android package binding support the URLs. Author action: none. Verification owner: CI/release gate.
No unresolved author-actionable defect remains at this exact head. Approved.
* origin/main: (75 commits) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) Instrument database pool roles (#7356) ... # Conflicts: # crates/buzz-db/src/runtime/migration.rs
* origin/main: (81 commits) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) ... Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
* origin/main: (75 commits) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) Instrument database pool roles (#7356) ...
* origin/main: (75 commits) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) Instrument database pool roles (#7356) ... Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…p-20260915 * origin/main: (28 commits) fix(mobile): avoid opening empty threads on message tap (block#7756) fix(workflows): make deletion persistent and retryable (block#7735) fix(mobile): preserve thread replies through refresh failures (block#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (block#7758) fix(relay): exclude ephemeral activity from message quota (block#7736) release: push gateway chart 0.3.1 (block#7749) fix(push): label plaintext push gateway service as HTTP (block#7717) Replace personal and internal data in desktop test fixtures (block#7748) Add mobile VISION (block#7710) fix(mobile): keep relay sessions stable during push lease updates (block#7745) fix(desktop): keep managed agent avatars usable across communities (block#7732) fix(mobile): fail open when age checks are unavailable (block#7714) fix(ci): don't run desktop tests for purely mobile client changes (block#7709) fix(mobile): temporarily disable age gating (block#7708) feat(db): expose connection setup metrics (block#7286) Isolate S3 storage metrics from the relay (block#7543) fix(web): route mobile invite downloads to app stores (block#7702) feat(mobile): show build number with version in settings (block#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (block#7685) Add authenticated WebSocket recovery telemetry (block#7546) ... Signed-off-by: Brainy Bumble <0ed7657b57c0e8a9f5288390dd6c8d5d0a3a06abe9b01b9006814f52077d6cdf@buzz.block.builderlab.xyz>
2 participants
Route the invite page’s existing download link to the App Store on iOS/iPadOS and Google Play on Android. Appearance and app-opening behavior stay unchanged.
The browser test checks platform routing using shared URL constants. Web build, lint, and all six smoke tests pass. Rebased onto main to pick up the relay test compilation fix.
Refs #3357. Alternative to #3464 with a smaller implementation and no duplicated store URLs in tests.