Repository navigation
Clarify Orca Mobile pairing connection paths - #9425
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMobile pairing now stores a preferred connection path, defaults to automatic relay pairing, and gates QR generation based on authentication. The mobile page and settings pane synchronize persisted modes, invalidate stale QR requests, and pass generation state through pairing controls. Connection settings use accessible Orca Relay and Local network options with authentication-aware panels. Pairing setup layout, installation links, relay availability content, search keywords, tests, and localized strings were updated. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/src/components/settings/MobilePairingConnectionOptions.tsx (2)
160-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid the arbitrary
text-[10px]font size. Prefer a documented size token fromsrc/renderer/src/assets/main.cssinstead of an inline pixel value.As per coding guidelines: "do not invent documented colors, font sizes, or shadow tiers".
Source: Coding guidelines
55-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse proper radio-group keyboard handling here. Each option is independently tabbable and only handles Space/Enter, so arrow-key roving focus is missing; if this stays custom, add the full radiogroup interaction pattern.
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8becffcd-c1eb-4b08-b679-d283f0cbe4c0
📒 Files selected for processing (18)
src/renderer/src/components/mobile/MobilePage.test.tsxsrc/renderer/src/components/mobile/MobilePage.tsxsrc/renderer/src/components/settings/MobilePairingConnectionOptions.test.tsxsrc/renderer/src/components/settings/MobilePairingConnectionOptions.tsxsrc/renderer/src/components/settings/MobilePairingSetupSection.test.tsxsrc/renderer/src/components/settings/MobilePairingSetupSection.tsxsrc/renderer/src/components/settings/MobilePane.tsxsrc/renderer/src/components/settings/MobileSettingsPane.tsxsrc/renderer/src/components/settings/mobile-settings-search.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/shared/constants.tssrc/shared/mobile-pairing-connection-mode.test.tssrc/shared/mobile-pairing-connection-mode.tssrc/shared/types.ts
Applies the code-review findings for the reworked pairing UI: - Clear a displayed Relay QR on sign-out in both MobilePage and MobilePane (the rework dropped the wasSignedInRef watcher, leaving a stale Relay QR next to a "sign in required" prompt). Anywhere stays selected; the QR re-mints as local-only. [#1, #3] - Extract useMobilePairingConnectionMode so both panes resolve the saved preference identically instead of duplicating the state + resync effect. [#6] - Delete ~24 i18n keys the rework left unreferenced; sync locale catalogs to parity. [#4] - Give the connection-path radiogroup roving tabindex + arrow-key nav and document why it diverges from SettingsSegmentedControl. [#7] - Add MobilePane.test.tsx (previously untested safety logic) and a MobilePage saved-local-only restore test. [#2, #5] Verified: 22 targeted tests pass, typecheck + oxlint clean, localization catalog/coverage and max-lines ratchet green.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/mobile/MobilePage.tsx (1)
253-255: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear
pairQrDataUrlalongsidepairingUrlto prevent flashing stale QR codes.When invalidating the pairing request due to a mode change,
pairingUrlis cleared butpairQrDataUrlis left behind. This can cause the old QR image to remain visible while the new request is loading, leading to a race condition where the user might scan a stale token.For consistency with the other invalidation paths (e.g.,
startPairingFlowand the signed-out effect), clear both states together.🐛 Proposed fix
const shouldRegenerate = hasGeneratedRef.current || pairLoading hasGeneratedRef.current = false + setPairQrDataUrl(null) setPairingUrl(null)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7ef5d01e-75e1-47ad-a016-9d2f2c58ab03
📒 Files selected for processing (11)
src/renderer/src/components/mobile/MobilePage.test.tsxsrc/renderer/src/components/mobile/MobilePage.tsxsrc/renderer/src/components/mobile/use-mobile-pairing-connection-mode.tssrc/renderer/src/components/settings/MobilePairingConnectionOptions.tsxsrc/renderer/src/components/settings/MobilePane.test.tsxsrc/renderer/src/components/settings/MobilePane.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/src/components/settings/MobilePane.tsx
- src/renderer/src/components/settings/MobilePairingConnectionOptions.tsx
Address accepted findings from the mobile pairing UI rework: - MobilePane: add a request-generation epoch so a late getPairingQR response can't paint a stale Relay QR after sign-out, a mode switch, or an address change; arm rotation when discarding a pending mint. - MobilePage: on sign-in, upgrade a signed-out local-only fallback QR to Relay (invalidate + rotate-regenerate); handle null->connected too. - Extract useMobilePairingQrInvalidation so both sign-in/out edges and cross-window preference syncs invalidate/re-mint the QR consistently. - MobilePane: clear + rotate the QR when the selected address changes (manual pick or refresh-driven) so it can't encode the old endpoint. - MobilePairingConnectionOptions: guard the Sign in CTA on configured; show an Unavailable panel on unconfigured builds instead of dead CTA. - Drop the duplicate sign-in helper from MobilePairingSetupSection. - Remove dead i18n keys (title, recommended, signInToGenerate) and add relayUnavailable across all locale catalogs. - Add tests: deferred sign-out/mode-switch races, sign-in upgrade, cross-window sync, unconfigured build, arrow-key radiogroup. TODO left for the relay-label-honesty finding: getPairingQR does not expose the actually-encoded mode when an automatic offer degrades to local-only, so the mismatch can't be surfaced without a new return field.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/src/components/mobile/MobilePage.test.tsx (1)
169-190: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest doesn't catch the intermediate stale-QR state during the sign-in upgrade.
This test only asserts the end state after
getPairingQRresolves withrotate: true; it never checks the QR value in the window between the sign-in edge firing andregenerateresolving. That gap is exactly whereuse-mobile-pairing-qr-invalidation.tscurrently leavespairQrDataUrlstale (see the companion comment on that file). Consider asserting the QR is cleared/loading immediately after the auth-state flip, before the mockedgetPairingQRpromise resolves, to guard against the regression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6b98807d-f6f8-43d7-bbaf-31d99454c08a
📒 Files selected for processing (14)
src/renderer/src/components/mobile/MobilePage.test.tsxsrc/renderer/src/components/mobile/MobilePage.tsxsrc/renderer/src/components/mobile/use-mobile-pairing-qr-invalidation.tssrc/renderer/src/components/settings/MobilePairingConnectionOptions.test.tsxsrc/renderer/src/components/settings/MobilePairingConnectionOptions.tsxsrc/renderer/src/components/settings/MobilePairingSetupSection.test.tsxsrc/renderer/src/components/settings/MobilePairingSetupSection.tsxsrc/renderer/src/components/settings/MobilePane.test.tsxsrc/renderer/src/components/settings/MobilePane.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.json
💤 Files with no reviewable changes (1)
- src/renderer/src/components/settings/MobilePairingSetupSection.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- src/renderer/src/components/settings/MobilePairingSetupSection.test.tsx
- src/renderer/src/i18n/locales/es.json
- src/renderer/src/components/settings/MobilePairingConnectionOptions.test.tsx
- src/renderer/src/components/settings/MobilePane.tsx
- src/renderer/src/components/settings/MobilePairingConnectionOptions.tsx
- src/renderer/src/i18n/locales/ko.json
- src/renderer/src/components/mobile/MobilePage.tsx
- src/renderer/src/i18n/locales/zh.json
- MobilePane: invalidatePairing now clears loading so a superseded mid-flight generate can't wedge Generate disabled forever - MobilePage: stop auto-minting a local-only QR under the Relay label when signed out with Anywhere; gate Step 2 auto-generate and the Generate button on a shared canMintMobilePairingOffer helper, align with Settings, clear QR + loading on sign-out, mint Relay on sign-in - qr-invalidation: clear pairQrDataUrl (and loading) on every invalidation path so a stale QR can't stay scannable during rotation - Strengthen MobilePane/MobilePage tests for the aligned behavior and stuck-loading coverage - Translate relayUnavailable in es/ja/ko/zh Co-authored-by: Orca <help@stably.ai>
Replace silent degradation of Anywhere mode to local-only QR with explicit refusal. Add canMintMobilePairingOffer guard across all mint paths (address change, network invalidation, connection mode switch). This ensures the UI honestly encodes the selected pairing path. Improve keyboard focus visibility on the path selector by adding a persistent focus ring.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 98f16b89-0d75-4146-afa2-684d9afb94e8
📒 Files selected for processing (16)
src/renderer/src/components/mobile/MobileHero.test.tsxsrc/renderer/src/components/mobile/MobileHero.tsxsrc/renderer/src/components/mobile/MobilePage.test.tsxsrc/renderer/src/components/mobile/MobilePage.tsxsrc/renderer/src/components/mobile/MobilePageContent.tsxsrc/renderer/src/components/mobile/use-mobile-pairing-generation.tssrc/renderer/src/components/mobile/use-mobile-pairing-qr-invalidation.tssrc/renderer/src/components/settings/MobilePairingConnectionOptions.tsxsrc/renderer/src/components/settings/MobilePane.test.tsxsrc/renderer/src/components/settings/MobilePane.tsxsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/shared/mobile-pairing-connection-mode.test.tssrc/shared/mobile-pairing-connection-mode.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/shared/mobile-pairing-connection-mode.test.ts
- src/renderer/src/components/mobile/use-mobile-pairing-qr-invalidation.ts
- src/renderer/src/i18n/locales/es.json
- src/renderer/src/components/settings/MobilePairingConnectionOptions.tsx
- src/renderer/src/i18n/locales/zh.json
- src/renderer/src/i18n/locales/ja.json
- src/renderer/src/components/mobile/MobilePage.test.tsx
…confusing-relay-ui
Contributor
Author
|
@coderabbitai[bot] resolved |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
When Relay provisioning fails during automatic (Anywhere) pairing offer creation, the offer silently degrades to local-only. This confuses users who selected Anywhere expecting cellular capability. Add connectionMode field to the pairing offer result to expose what the offer actually encodes. The UI now flags degradation when the offered mode mismatches the user's selection. Also move credential rotation logic to the main process: rotate when requested mode differs from the pending token's encoded mode. This ensures QR codes displayed under an old policy can't pair under a new one, and windows reminting after preference sync converge on one token. Rename MobileRelayBetaAvailability → MobileRelayBetaNotice.
Assert that the pairing QR and URL remain hidden while the Relay mint is pending, preventing confusing intermediate states when signing in unlocks Relay.
8 of 10 tasks
6 tasks
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
* Restructure mobile pairing setup into a clear stepped layout * Rework mobile pairing UI: radio-style path selector with sign-in gate fo * fix(mobile): resolve mobile-pairing review findings Applies the code-review findings for the reworked pairing UI: - Clear a displayed Relay QR on sign-out in both MobilePage and MobilePane (the rework dropped the wasSignedInRef watcher, leaving a stale Relay QR next to a "sign in required" prompt). Anywhere stays selected; the QR re-mints as local-only. [stablyai#1, stablyai#3] - Extract useMobilePairingConnectionMode so both panes resolve the saved preference identically instead of duplicating the state + resync effect. [stablyai#6] - Delete ~24 i18n keys the rework left unreferenced; sync locale catalogs to parity. [stablyai#4] - Give the connection-path radiogroup roving tabindex + arrow-key nav and document why it diverges from SettingsSegmentedControl. [stablyai#7] - Add MobilePane.test.tsx (previously untested safety logic) and a MobilePage saved-local-only restore test. [stablyai#2, stablyai#5] Verified: 22 targeted tests pass, typecheck + oxlint clean, localization catalog/coverage and max-lines ratchet green. * fix(mobile): resolve adversarial-review findings for pairing UI Address accepted findings from the mobile pairing UI rework: - MobilePane: add a request-generation epoch so a late getPairingQR response can't paint a stale Relay QR after sign-out, a mode switch, or an address change; arm rotation when discarding a pending mint. - MobilePage: on sign-in, upgrade a signed-out local-only fallback QR to Relay (invalidate + rotate-regenerate); handle null->connected too. - Extract useMobilePairingQrInvalidation so both sign-in/out edges and cross-window preference syncs invalidate/re-mint the QR consistently. - MobilePane: clear + rotate the QR when the selected address changes (manual pick or refresh-driven) so it can't encode the old endpoint. - MobilePairingConnectionOptions: guard the Sign in CTA on configured; show an Unavailable panel on unconfigured builds instead of dead CTA. - Drop the duplicate sign-in helper from MobilePairingSetupSection. - Remove dead i18n keys (title, recommended, signInToGenerate) and add relayUnavailable across all locale catalogs. - Add tests: deferred sign-out/mode-switch races, sign-in upgrade, cross-window sync, unconfigured build, arrow-key radiogroup. TODO left for the relay-label-honesty finding: getPairingQR does not expose the actually-encoded mode when an automatic offer degrades to local-only, so the mismatch can't be surfaced without a new return field. * fix(mobile): resolve relay-pairing deep-review regressions - MobilePane: invalidatePairing now clears loading so a superseded mid-flight generate can't wedge Generate disabled forever - MobilePage: stop auto-minting a local-only QR under the Relay label when signed out with Anywhere; gate Step 2 auto-generate and the Generate button on a shared canMintMobilePairingOffer helper, align with Settings, clear QR + loading on sign-out, mint Relay on sign-in - qr-invalidation: clear pairQrDataUrl (and loading) on every invalidation path so a stale QR can't stay scannable during rotation - Strengthen MobilePane/MobilePage tests for the aligned behavior and stuck-loading coverage - Translate relayUnavailable in es/ja/ko/zh Co-authored-by: Orca <help@stably.ai> * fix(mobile): refuse to mint QR when signed-out with Anywhere selected Replace silent degradation of Anywhere mode to local-only QR with explicit refusal. Add canMintMobilePairingOffer guard across all mint paths (address change, network invalidation, connection mode switch). This ensures the UI honestly encodes the selected pairing path. Improve keyboard focus visibility on the path selector by adding a persistent focus ring. * fix(mobile): detect and flag Relay provisioning degradation When Relay provisioning fails during automatic (Anywhere) pairing offer creation, the offer silently degrades to local-only. This confuses users who selected Anywhere expecting cellular capability. Add connectionMode field to the pairing offer result to expose what the offer actually encodes. The UI now flags degradation when the offered mode mismatches the user's selection. Also move credential rotation logic to the main process: rotate when requested mode differs from the pending token's encoded mode. This ensures QR codes displayed under an old policy can't pair under a new one, and windows reminting after preference sync converge on one token. Rename MobileRelayBetaAvailability → MobileRelayBetaNotice. * test(mobile): verify pairing codes don't flash during Relay mint Assert that the pairing QR and URL remain hidden while the Relay mint is pending, preventing confusing intermediate states when signing in unlocks Relay. --------- Co-authored-by: Orca <help@stably.ai>
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
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
Testing