add v1 of freighter mobile best practices skill#810
add v1 of freighter mobile best practices skill#810leofelix077 wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a “freighter mobile best practices” documentation skill and supporting context files to help AI agents and contributors navigate Freighter Mobile’s architecture, tooling, and development conventions.
Changes:
- Introduces
llms.txtandCLAUDE.mdas entry-point context/reference docs for the repo. - Adds
docs/skills/freighter-mobile-best-practices/with a skill definition and focused reference guides (architecture, code style, security, WalletConnect, testing, etc.).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| llms.txt | High-level repo index for docs, dev, testing, and key concepts |
| CLAUDE.md | Consolidated AI agent / contributor context (tooling, structure, conventions) |
| docs/skills/freighter-mobile-best-practices/SKILL.md | Skill definition + reference index for best-practices topics |
| docs/skills/freighter-mobile-best-practices/references/architecture.md | Architecture + layering + duck/store patterns reference |
| docs/skills/freighter-mobile-best-practices/references/anti-patterns.md | Common mistakes/anti-pattern guidance |
| docs/skills/freighter-mobile-best-practices/references/code-style.md | Formatting, ESLint/Prettier rules, naming conventions |
| docs/skills/freighter-mobile-best-practices/references/dependencies.md | Dependency management + native dependency workflow |
| docs/skills/freighter-mobile-best-practices/references/error-handling.md | Error normalization + store async patterns + WC error responses |
| docs/skills/freighter-mobile-best-practices/references/git-workflow.md | Branching/commit/PR/release process guidance |
| docs/skills/freighter-mobile-best-practices/references/i18n.md | i18n framework usage + key structure + lint enforcement notes |
| docs/skills/freighter-mobile-best-practices/references/navigation.md | Navigator hierarchy + typing + deep links conventions |
| docs/skills/freighter-mobile-best-practices/references/performance.md | Performance rules/checklist and optimization guidance |
| docs/skills/freighter-mobile-best-practices/references/security.md | Storage tiers + auth/security-sensitive areas overview |
| docs/skills/freighter-mobile-best-practices/references/styling.md | NativeWind/SDS/bottom-sheet/modal styling guidance |
| docs/skills/freighter-mobile-best-practices/references/testing.md | Jest + Maestro structure, commands, and e2e guidance |
| docs/skills/freighter-mobile-best-practices/references/walletconnect.md | WalletConnect architecture, request handling, and validations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
llms.txt
Outdated
| - 434 TypeScript files, ~73,500 LOC | ||
| - Navigation: React Navigation with nested navigators | ||
| - dApp integration: WalletConnect v2 (4 RPC methods) | ||
| - Testing: Jest (unit) + Maestro (e2e, 6 test flows) |
There was a problem hiding this comment.
The doc hardcodes "Maestro (e2e, 6 test flows)" and a TS file/LOC count. In the repo there are currently 10 YAML flows under e2e/flows/ (including debug/shared helpers), so this number is already inaccurate; consider updating the count or avoiding hardcoded numbers that will drift.
| - 434 TypeScript files, ~73,500 LOC | |
| - Navigation: React Navigation with nested navigators | |
| - dApp integration: WalletConnect v2 (4 RPC methods) | |
| - Testing: Jest (unit) + Maestro (e2e, 6 test flows) | |
| - TypeScript codebase with React Native screens, state, and platform integrations | |
| - Navigation: React Navigation with nested navigators | |
| - dApp integration: WalletConnect v2 (4 RPC methods) | |
| - Testing: Jest (unit) + Maestro (e2e) |
| ## Reference Index | ||
|
|
||
| | Concern | File | When to Read | | ||
| | -------------------- | ---------------------------- | --------------------------------------------------- | | ||
| | Code Style | references/code-style.md | Writing or reviewing any code | | ||
| | Architecture | references/architecture.md | Adding features, understanding the codebase | |
There was a problem hiding this comment.
The markdown table syntax here uses a leading || which GitHub renders as an extra empty column. Replace || ... | ... | rows with standard | ... | ... | table rows so the Reference Index renders correctly.
| ## Supported RPC Methods | ||
|
|
||
| | Method | Purpose | | ||
| | -------------------------- | -------------------------------------------- | | ||
| | `stellar_signXDR` | Sign a transaction XDR (does not submit) | | ||
| | `stellar_signAndSubmitXDR` | Sign and submit a transaction to the network | | ||
| | `stellar_signMessage` | Sign an arbitrary text message | | ||
| | `stellar_signAuthEntry` | Sign a Soroban authorization entry | | ||
|
|
There was a problem hiding this comment.
Table rows start with || (double pipe), which will render with an unintended empty first column in GitHub markdown. Use a single leading | for each row so the table displays as intended.
| ### Available Test Flows (7 total) | ||
|
|
||
| 1. `CreateWallet` — Full wallet creation flow | ||
| 2. `ImportWallet` — Import via recovery phrase | ||
| 3. `ImportFundedWallet` — Import a pre-funded wallet | ||
| 4. `SendClassicTokenMainnet` — Send a classic Stellar token on mainnet | ||
| 5. `SwapClassicTokenMainnet` — Swap tokens on mainnet | ||
| 6. `SignMessageMockDapp` — Sign a message via WalletConnect mock dApp | ||
| 7. `SignAuthEntryMockDapp` — Sign a Soroban auth entry via WalletConnect mock | ||
| dApp |
There was a problem hiding this comment.
This section says there are "7 total" Maestro flows, but e2e/flows/ currently contains 10 YAML flows (onboarding/transactions/walletconnect/debug/shared, with SwitchToTestnet duplicated). Update the count/list or avoid hardcoding totals so the doc stays accurate.
| | Tier | Implementation | Use For | | ||
| | ---------------------- | --------------------------------------------------------- | ---------------------------------------------------- | | ||
| | `secureDataStorage` | `react-native-keychain` (iOS Keychain / Android Keystore) | Private keys, seed phrases, encrypted credentials | | ||
| | `biometricDataStorage` | Biometric-gated keychain access | Passwords that require biometric confirmation | | ||
| | `dataStorage` | AsyncStorage | Non-sensitive settings, preferences, cached UI state | | ||
|
|
There was a problem hiding this comment.
Markdown table rows here use || which creates an extra empty column when rendered on GitHub. Switch to standard |-prefixed rows for these tables (same for other tables in this doc).
| - `BalancesList.tsx` — missing windowSize, maxToRenderPerBatch | ||
| - `CollectiblesGrid.tsx` — missing keyExtractor, windowSize, maxToRenderPerBatch |
There was a problem hiding this comment.
This section references BalancesList.tsx and CollectiblesGrid.tsx as missing FlatList optimizations, but those filenames don't exist anywhere under src/ in the current repo. Either update these to the correct component filenames or remove the specific callouts to avoid misleading readers.
| - `BalancesList.tsx` — missing windowSize, maxToRenderPerBatch | |
| - `CollectiblesGrid.tsx` — missing keyExtractor, windowSize, maxToRenderPerBatch | |
| - Some FlatList usages in the current codebase are still missing required | |
| performance props and should be updated to include stable `keyExtractor`, | |
| `windowSize`, `maxToRenderPerBatch`, and related virtualization settings as | |
| applicable. |
| # Performance -- Freighter Mobile | ||
|
|
||
| Based on analysis of 436 React Native files. Current performance score: | ||
| **6.8/10**. | ||
|
|
There was a problem hiding this comment.
This doc starts with "Based on analysis of 436 React Native files" while llms.txt claims 434 TypeScript files; these hardcoded metrics will drift and are already inconsistent within the PR. Consider removing precise counts/scores or generating them automatically so the docs remain trustworthy.
|
|
||
| | Category | Convention | Evidence | | ||
| | ----------------- | --------------------------------------------------- | --------------------------- | | ||
| | Screen components | `XxxScreen` suffix always | 40+ screens, 100% adherence | | ||
| | Hooks | `useXxx` always | 57 hooks, 100% adherence | | ||
| | Zustand stores | `useXxxStore` | 24 ducks, 100% adherence | | ||
| | Store actions | verb + noun (`fetchBalances`, `clearData`) | 95%+ | | ||
| | Constants | SCREAMING_SNAKE_CASE in `config/constants.ts` | 95%+ | | ||
| | Types/Interfaces | PascalCase with `Props`, `State`, `Config` suffixes | 500+ types, 100% | | ||
| | Enum values | Mixed PascalCase/SCREAMING_SNAKE per enum | Context-dependent | |
There was a problem hiding this comment.
The tables in this doc use || at the start of rows (e.g., the naming deep dive table), which GitHub will render with an extra empty column. Use a single leading | per row for proper markdown table formatting.
| ## Dual Bundle IDs | ||
|
|
||
| The app maintains separate identities for development and production: | ||
|
|
||
| | Concern | Dev | Prod | | ||
| | ---------------- | -------------------------- | ----------------------------- | | ||
| | Bundle ID | `org.stellar.freighterdev` | `org.stellar.freighterwallet` | | ||
| | Signing | Dev certificates | Prod certificates | | ||
| | Push tokens | Separate | Separate | | ||
| | Deep link scheme | `freighterdev://` | `freighterwallet://` | | ||
| | Keychain entries | Isolated | Isolated | | ||
|
|
There was a problem hiding this comment.
The dual bundle ID table rows start with ||, which renders incorrectly in GitHub markdown (extra empty first column). Use standard |-prefixed rows.
| ## Running Tests | ||
|
|
||
| | Command | Purpose | | ||
| | ------------------------------ | -------------------------------------- | | ||
| | `yarn test` | Run Jest suite once | | ||
| | `yarn test:watch` | Run Jest in watch mode | | ||
| | `yarn test:e2e:ios <flow>` | Run a Maestro flow on iOS Simulator | | ||
| | `yarn test:e2e:android <flow>` | Run a Maestro flow on Android Emulator | | ||
|
|
||
| ## CI Pipelines | ||
|
|
||
| | Workflow | Trigger | What It Does | | ||
| | ----------------- | ------------------- | ------------------------------------------ | | ||
| | `test.yml` | Push / PR | Runs the Jest test suite | | ||
| | `ios-e2e.yml` | Configured triggers | Runs Maestro iOS tests on macOS runner | | ||
| | `android-e2e.yml` | Configured triggers | Runs Maestro Android tests on Linux runner | |
There was a problem hiding this comment.
The command/workflow tables here use || at the start of each row, which renders as an extra empty column in GitHub markdown. Use standard table rows starting with a single | so these tables format correctly.
Rewrite AGENTS.md as the single AI agent entry point: - Add glossary section with domain-specific terminology - Add documentation link index (replaces llms.txt) - Remove sections that duplicate best-practices reference files (code style details, branch conventions, PR instructions) - Keep unique context: repo map, architecture orientation (ducks/nav/WC), security alert list, known complexity/gotchas, pre-submission checklist - Delete llms.txt (content absorbed into AGENTS.md) - Delete CLAUDE.md (content absorbed into AGENTS.md)
Code reviewFound 1 issue:
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Skill Eval:
freighter-mobile-best-practices— Benchmark ReportDate: 2026-04-09
Iterations: 1
Repo: stellar/freighter-mobile
Model: Claude Opus 4.6
Summary
on further passes both get close to 100%, as Claude picks up automatically on the rules and the entry points
Assertion Results (aggregated across 3 iterations)
architecture-screen
architecture-zustand
code-style-hook
code-style-naming
err-handling-retry
err-handling-zustand
i18n-settings
nav-typed-routes
performance-flatlist
performance-selectors
security-storage
security-walletconnect
styling-card
testing-zustand