Skip to content

fix: make ledger app gate actually reliable#11502

Merged
NeOMakinG merged 5 commits intodevelopfrom
fix_flaky_ledger_app_gate
Dec 24, 2025
Merged

fix: make ledger app gate actually reliable#11502
NeOMakinG merged 5 commits intodevelopfrom
fix_flaky_ledger_app_gate

Conversation

@gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 19, 2025

Description

tl;dr of this device is we stop spewing the device with concurrent USB connections, which results in transport errors, and more and more spew going in, which results in transport errors...

By doing things sequentially the right way (i.e wait for open app to complete, then and only then poll for app open status) we ensure that we don't end up in a state where the device is getting spewed with requests and unresponsive, and this whole flow is finally great.

Issue (if applicable)

closes #11492

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Medium - this being borked could bork Ledger

Testing

  • Test regular app gate i.e no app open, see app gate, accept, should work
  • Test regular app gate i.e no app open, see app gate, accept but this time around after say 10-15s, should work
  • Test with app already opened, should be no app gate (previously, you could see a flash of app gate at times)
  • Try this back and forth with various apps, many times over (which, with the current flow, could result in the device being more and more spewed, which would make it less reliable the more times you had app gates)

Engineering

  • ^

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)
  • ^

Screenshots (if applicable)

https://jam.dev/c/bda65b98-4c17-4da6-b204-a27e04efceb7

Summary by CodeRabbit

  • Bug Fixes
    • Fixed race condition that could cause instability when opening the Ledger app
    • Improved Ledger device connection reliability and reduced flakiness during event listener registration

✏️ Tip: You can customize this high-level summary in your review settings.

gomesalexandre and others added 2 commits December 19, 2025 20:20
The update-icon.svg import was causing a MIME type error in dev build:
"Expected a JavaScript-or-Wasm module script but the server responded
with a MIME type of 'image/svg+xml'."

Adding ?url tells Vite to import the SVG as a static asset URL instead
of trying to process it as a module script.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@gomesalexandre gomesalexandre requested a review from a team as a code owner December 19, 2025 18:35
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 19, 2025

📝 Walkthrough

Walkthrough

Fixes a race condition in the Ledger app open flow where polling for app readiness previously competed with USB requests, causing flakiness. Updates ledgerAppGate.ts to delay polling until after openApp resolves, and modifies AppContext.tsx to register Ledger listeners only on mount instead of every render, preventing re-registration.

Changes

Cohort / File(s) Change Summary
Ledger app open flow synchronization
packages/chain-adapters/src/utils/ledgerAppGate.ts
Reworked polling strategy to avoid racing with USB requests by deferring startPolling until after openApp resolves; normalized promise and non-promise return values from wallet.openApp using Promise.resolve; polling remains at 1-second interval.
Event listener registration optimization
src/context/AppProvider/AppContext.tsx
Added empty dependency array to useEffect registering Ledger event listeners, converting from every-render registration to mount-only; added explanatory comment and eslint-disable-next-line for exhaustive-deps to lock in handler references.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify the polling deferral strategy correctly eliminates the race condition between USB requests and app readiness checks
  • Confirm Promise.resolve normalization handles both promise and non-promise return types from wallet.openApp
  • Validate that the empty dependency array in AppContext prevents handler re-registration flakiness without breaking listener lifecycle
  • Ensure polling cleanup and error handling paths remain intact

Possibly related PRs

Suggested labels

capy

Suggested reviewers

  • NeOMakinG
  • kaladinlight

Poem

🐰 A race it was, USB and poll,
Now sequenced right, they reach their goal,
Event listeners locked in place,
No more flakiness to embrace! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: making the Ledger app gate mechanism reliable by fixing flakiness.
Linked Issues check ✅ Passed The PR directly addresses issue #11492 by reworking the Ledger app-open flow to prevent concurrent USB requests that cause device unresponsiveness and flakiness during account loading and address verification.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the Ledger app-gate reliability issue: ledgerAppGate.ts reworks the polling flow and AppContext.tsx fixes event listener registration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_flaky_ledger_app_gate

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c734873 and 52d00ab.

📒 Files selected for processing (1)
  • packages/chain-adapters/src/utils/ledgerAppGate.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always run yarn lint --fix and yarn type-check after making changes
Avoid let variable assignments - prefer const with inline IIFE switch statements or extract to functions for conditional logic

Files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern: userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables in useMemo and callbacks in useCallback where possible
For static JSX icon elements (e.g., <TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode using useColorModeValue hook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook: useTranslate() from react-polyglot
Use useFeatureFlag('FlagName') hook to access feature flag values in components
Prefer type over interface for type definitions
Use strict typing - avoid any
Use Nominal types for domain identifiers (e.g., WalletId, AccountId)
Import types from @shapeshiftoss/caip for chain/account/asset IDs
Use useAppSelector for Redux state
Use useAppDispatch for Redux actions
Memoize expensive computations with useMemo
Memoize callbacks with useCallback

**/*.{ts,tsx}: Use Result<T, E> pattern for error handling in swappers and APIs; ALWAYS use Ok() and Err() from @sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from @shapeshiftoss/errors with meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...

Files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)

**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Use handle prefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names with is, has, can, should prefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names like data, item, obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names like fn, func, or callback

Files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11170
File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19
Timestamp: 2025-11-25T21:43:10.838Z
Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10879
File: src/context/WalletProvider/WalletConnectV2/components/WalletConnectDirectRow.tsx:64-81
Timestamp: 2025-10-22T22:11:22.918Z
Learning: In early WalletConnect POC/features behind a flag, gomesalexandre prioritizes connection correctness/stability over UX polish; minimal safety guards (like preventing concurrent connects) are preferred, while visuals will be wired later by reallybeard.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/StructuredMessage.tsx:0-0
Timestamp: 2025-09-13T16:45:17.166Z
Learning: gomesalexandre appreciates safety-focused suggestions for UI rendering in WalletConnect components, specifically defensive programming approaches that prevent null/undefined values from displaying as literal "null"/"undefined" strings in the user interface.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx:40-140
Timestamp: 2025-09-12T12:04:24.562Z
Learning: gomesalexandre prefers to defer error handling improvements in new/preparatory code until real-world error cases are encountered, rather than implementing speculative error handling without concrete failure scenarios to test against. This aligns with keeping PR scope focused and evidence-based development practices.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerConnectionState.ts:1-1
Timestamp: 2025-09-24T20:44:29.333Z
Learning: gomesalexandre prefers not to touch complex connection state logic in useLedgerConnectionState when the current implementation is working, even when potential stale closure issues are identified, demonstrating his cautious approach to modifying intricate state management code within PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-11-19T16:59:50.569Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-10-01T07:42:40.195Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10596
File: src/components/Layout/Header/NavBar/WalletConnectedMenu.tsx:77-99
Timestamp: 2025-10-01T07:42:40.195Z
Learning: In WalletConnectedMenu.tsx's handleReconnectWallet handler, gomesalexandre prefers throwing an error for unsupported wallet types in the default case rather than gracefully handling with a fallback. His reasoning: "if we have a problem here, we have bigger problems" - only supported wallets (KeepKey, Ledger, MetaMask, Coinbase, Phantom) should reach the reconnect flow when disconnected/locked, so encountering an unsupported type indicates a larger architectural issue that should be surfaced explicitly rather than masked with graceful degradation.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-12-03T23:16:28.342Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11261
File: src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx:117-172
Timestamp: 2025-12-03T23:16:28.342Z
Learning: In TRON transaction confirmation polling (e.g., approval flows in useAllowanceApproval.tsx), gomesalexandre is comfortable with optimistic completion when polling times out after the configured duration (e.g., 60 seconds). He considers the timeout a "paranoia" safety net for unlikely scenarios, expecting normal transactions to complete much faster. He prefers to defer more sophisticated timeout/failure handling as a separate follow-up concern rather than expanding PR scope.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-09-10T15:35:46.223Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-09-13T16:45:17.166Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/StructuredMessage.tsx:0-0
Timestamp: 2025-09-13T16:45:17.166Z
Learning: gomesalexandre appreciates safety-focused suggestions for UI rendering in WalletConnect components, specifically defensive programming approaches that prevent null/undefined values from displaying as literal "null"/"undefined" strings in the user interface.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-09-12T13:44:17.019Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:0-0
Timestamp: 2025-09-12T13:44:17.019Z
Learning: gomesalexandre prefers letting chain adapter errors throw naturally in useSimulateEvmTransaction rather than adding explicit error handling for missing adapters, consistent with his fail-fast approach and dismissal of defensive validation as "stale" in WalletConnect transaction simulation flows.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-09-12T10:44:46.723Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/SendTransactionContent.tsx:0-0
Timestamp: 2025-09-12T10:44:46.723Z
Learning: gomesalexandre dismissed a clipboard error handling suggestion in PR #10461 for SendTransactionContent.tsx, demonstrating that the current navigator.clipboard.writeText implementation works as expected and preferring to keep it simple without additional try/catch error handling.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-10-22T22:11:22.918Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10879
File: src/context/WalletProvider/WalletConnectV2/components/WalletConnectDirectRow.tsx:64-81
Timestamp: 2025-10-22T22:11:22.918Z
Learning: In early WalletConnect POC/features behind a flag, gomesalexandre prioritizes connection correctness/stability over UX polish; minimal safety guards (like preventing concurrent connects) are preferred, while visuals will be wired later by reallybeard.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
📚 Learning: 2025-11-03T05:46:24.190Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10985
File: packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeRate/getPortalsTradeRate.tsx:130-141
Timestamp: 2025-11-03T05:46:24.190Z
Learning: In packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeRate/getPortalsTradeRate.tsx, gomesalexandre prefers "let it crash" approach when Portals returns zero outputAmount or 100% buffer causing division by zero, rather than adding defensive guards to surface zero quotes. He wants to fail fast with exceptions that bubble up to the try/catch wrapper, surfacing upstream data issues rather than masking them with defensive programming.

Applied to files:

  • packages/chain-adapters/src/utils/ledgerAppGate.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install and Cache
🔇 Additional comments (1)
packages/chain-adapters/src/utils/ledgerAppGate.ts (1)

105-121: Excellent refactoring to eliminate concurrent USB requests.

The solution correctly enforces sequential flow by:

  1. Defining startPolling() to encapsulate the polling logic
  2. Using Promise.resolve() to normalize both promise and non-promise return values from wallet.openApp()
  3. Calling startPolling() only after openApp settles (via both .then() and .catch())

This ensures the device is no longer flooded with overlapping requests, directly addressing the flakiness described in issue #11492. The error handling is appropriate—swallowing openApp errors and proceeding to poll makes sense because the user might manually open the app, and the polling is the definitive check for app-open status.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a0648c1 and c734873.

📒 Files selected for processing (3)
  • packages/chain-adapters/src/utils/ledgerAppGate.ts (1 hunks)
  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx (1 hunks)
  • src/context/AppProvider/AppContext.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always run yarn lint --fix and yarn type-check after making changes
Avoid let variable assignments - prefer const with inline IIFE switch statements or extract to functions for conditional logic

Files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
  • src/context/AppProvider/AppContext.tsx
  • packages/chain-adapters/src/utils/ledgerAppGate.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern: userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables in useMemo and callbacks in useCallback where possible
For static JSX icon elements (e.g., <TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode using useColorModeValue hook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook: useTranslate() from react-polyglot
Use useFeatureFlag('FlagName') hook to access feature flag values in components
Prefer type over interface for type definitions
Use strict typing - avoid any
Use Nominal types for domain identifiers (e.g., WalletId, AccountId)
Import types from @shapeshiftoss/caip for chain/account/asset IDs
Use useAppSelector for Redux state
Use useAppDispatch for Redux actions
Memoize expensive computations with useMemo
Memoize callbacks with useCallback

**/*.{ts,tsx}: Use Result<T, E> pattern for error handling in swappers and APIs; ALWAYS use Ok() and Err() from @sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from @shapeshiftoss/errors with meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...

Files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
  • src/context/AppProvider/AppContext.tsx
  • packages/chain-adapters/src/utils/ledgerAppGate.ts
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{tsx,jsx}: ALWAYS wrap React components in error boundaries and provide user-friendly fallback components with error logging
ALWAYS use useErrorToast hook for displaying errors with translated error messages and handle different error types appropriately

Use PascalCase for React component names and match the component name to the file name

Files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
  • src/context/AppProvider/AppContext.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)

**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Use handle prefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names with is, has, can, should prefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names like data, item, obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names like fn, func, or callback

Files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
  • src/context/AppProvider/AppContext.tsx
  • packages/chain-adapters/src/utils/ledgerAppGate.ts
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

**/*.{jsx,tsx}: ALWAYS use useMemo for expensive computations, object/array creations, and filtered data
ALWAYS use useMemo for derived values and computed properties
ALWAYS use useMemo for conditional values and simple transformations
ALWAYS use useCallback for event handlers and functions passed as props
ALWAYS use useCallback for any function that could be passed as a prop or dependency
ALWAYS include all dependencies in useEffect, useMemo, useCallback dependency arrays
NEVER use // eslint-disable-next-line react-hooks/exhaustive-deps unless absolutely necessary, and ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components; NEVER use default exports for components
KEEP component files under 200 lines when possible; BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly in async operations
ALWAYS provide user-friendly error messages in error handling
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components using React.lazy for code splitting
ALWAYS use Suspense wrapper for lazy loaded components
USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places
Wrap components receiving props with memo for performance optimization

Files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
  • src/context/AppProvider/AppContext.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

Ensure TypeScript types are explicit and proper; avoid use of any type

Files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
  • src/context/AppProvider/AppContext.tsx
🧠 Learnings (19)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11170
File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19
Timestamp: 2025-11-25T21:43:10.838Z
Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10879
File: src/context/WalletProvider/WalletConnectV2/components/WalletConnectDirectRow.tsx:64-81
Timestamp: 2025-10-22T22:11:22.918Z
Learning: In early WalletConnect POC/features behind a flag, gomesalexandre prioritizes connection correctness/stability over UX polish; minimal safety guards (like preventing concurrent connects) are preferred, while visuals will be wired later by reallybeard.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerConnectionState.ts:1-1
Timestamp: 2025-09-24T20:44:29.333Z
Learning: gomesalexandre prefers not to touch complex connection state logic in useLedgerConnectionState when the current implementation is working, even when potential stale closure issues are identified, demonstrating his cautious approach to modifying intricate state management code within PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx:40-140
Timestamp: 2025-09-12T12:04:24.562Z
Learning: gomesalexandre prefers to defer error handling improvements in new/preparatory code until real-world error cases are encountered, rather than implementing speculative error handling without concrete failure scenarios to test against. This aligns with keeping PR scope focused and evidence-based development practices.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:69-74
Timestamp: 2025-09-10T15:35:36.547Z
Learning: gomesalexandre dismissed alt text accessibility suggestion with "meh" in PR #10458 for EIP155SignTypedDataConfirmation.tsx Image component, consistent with team pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/StructuredMessage.tsx:0-0
Timestamp: 2025-09-13T16:45:17.166Z
Learning: gomesalexandre appreciates safety-focused suggestions for UI rendering in WalletConnect components, specifically defensive programming approaches that prevent null/undefined values from displaying as literal "null"/"undefined" strings in the user interface.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-08-22T12:59:01.702Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:37-53
Timestamp: 2025-08-22T12:59:01.702Z
Learning: In RewardDistributionActionCard component (src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx), NeOMakinG confirmed that runeAsset is expected to always be defined when the component renders, so defensive guards against undefined runeAsset are not needed.

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/pages/Explore/ExploreCategory.tsx:231-238
Timestamp: 2025-08-08T14:59:40.422Z
Learning: In src/pages/Explore/ExploreCategory.tsx, for the PageHeader filter trigger, NeOMakinG considers changing a clickable Chakra Icon to IconButton too nitpicky for this PR and prefers to keep the current Icon-based trigger; such minor a11y/UI nitpicks should be deferred to a follow-up if needed.

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-08-25T12:58:39.547Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx:69-70
Timestamp: 2025-08-25T12:58:39.547Z
Learning: In RewardDistributionNotification component (src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx), NeOMakinG confirmed that action is expected to always be defined when the component renders, so defensive guards against undefined action are not needed.

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-08-22T12:58:36.070Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx:33-55
Timestamp: 2025-08-22T12:58:36.070Z
Learning: In RewardDistributionNotification component (src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx), NeOMakinG confirmed that runeAsset is expected to always be defined when the component renders, so defensive guards against undefined runeAsset are not needed.

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Use Chakra UI components and conventions

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-08-25T12:59:14.469Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RfoxInitiatedActionCard.tsx:37-46
Timestamp: 2025-08-25T12:59:14.469Z
Learning: NeOMakinG prefers to maintain consistency with existing notification date formatting patterns in the codebase, even when a more theoretically consistent approach (like using dayjs.format()) might be available. This was specifically noted in RfoxInitiatedActionCard.tsx where toLocaleString() was kept to match other recent notification implementations.

Applied to files:

  • src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Avoid useEffect where practical - use it only when necessary and following best practices

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : ALWAYS include all dependencies in `useEffect`, `useMemo`, `useCallback` dependency arrays

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-19T16:59:50.569Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : EXTRACT complex logic into custom hooks

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-17T10:56:25.026Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11006
File: src/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.ts:36-36
Timestamp: 2025-11-17T10:56:25.026Z
Learning: In PR #11006 (React Compiler PoC), gomesalexandre declined adding explanatory comments for 'use no memo' directives throughout the codebase, dismissing the suggestion with "meh". The opt-outs are intentionally kept minimal without inline documentation explaining the rationale for each component/hook.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : NEVER use `// eslint-disable-next-line react-hooks/exhaustive-deps` unless absolutely necessary, and ALWAYS explain why dependencies are excluded if using eslint disable

Applied to files:

  • src/context/AppProvider/AppContext.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install and Cache
🔇 Additional comments (2)
src/components/Layout/Header/ActionCenter/components/AppUpdateActionCard.tsx (1)

9-9: LGTM! SVG import fix resolves MIME type error.

The ?url suffix correctly imports the SVG as a URL string for use in the Image component's src prop (line 34), fixing the dev build MIME type error mentioned in the commit message.

src/context/AppProvider/AppContext.tsx (1)

102-107: Excellent fix for Ledger listener flakiness.

The empty dependency array with thorough explanation correctly prevents re-registration of event listeners that was causing the flakiness described in issue #11492. The captured closeModal/openModal functions are stable references from ModalProvider, making this approach safe and addressing the root cause of the issue.

Copy link
Collaborator

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NeOMakinG NeOMakinG enabled auto-merge (squash) December 24, 2025 10:02
@NeOMakinG NeOMakinG merged commit bfcc0df into develop Dec 24, 2025
4 checks passed
@NeOMakinG NeOMakinG deleted the fix_flaky_ledger_app_gate branch December 24, 2025 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some flakiness with Open app prompt for apps on Ledger

2 participants