Conversation
There was a problem hiding this comment.
Pull request overview
Adds a repo-specific contributing guide and an accompanying Claude skill to streamline Freighter Mobile developer environment setup, while simplifying README setup guidance by pointing contributors to the new documentation.
Changes:
- Replace README prerequisite details with a concise list and link to
CONTRIBUTING.md. - Add
CONTRIBUTING.mdwith prerequisites, setup steps, env var guidance, and repo conventions/testing notes. - Add a Claude skill doc at
.claude/skills/freighter-dev-setup/SKILL.mdto automate prerequisite checks and setup.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| README.md | Simplifies prerequisites and redirects env var guidance to CONTRIBUTING.md. |
| CONTRIBUTING.md | New comprehensive contributor/dev-setup guide (prereqs, env vars, commands, conventions, testing). |
| .claude/skills/freighter-dev-setup/SKILL.md | New LLM “skill” instructions intended to automate environment verification and setup steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
README.md
Outdated
| ``` | ||
| - Alternatively, you might prefer to use `npx react-native <command>` for | ||
| running commands without a global installation. | ||
| - **Node.js** >= 22: [nodejs.org](https://nodejs.org/) or `nvm install 22` |
There was a problem hiding this comment.
README now states Node.js >= 22, but the repo currently targets Node 20 in CI (workflows set NODE_VERSION=20) and package.json only requires node: >=20. This mismatch can cause contributors to unnecessarily upgrade or fail to match CI; please align the README requirement with the actual enforced version (or update engines/CI if 22 is truly required).
| - **Node.js** >= 22: [nodejs.org](https://nodejs.org/) or `nvm install 22` | |
| - **Node.js** >= 20: [nodejs.org](https://nodejs.org/) or `nvm install 20` |
CONTRIBUTING.md
Outdated
|
|
||
| | Tool | Version | Install | | ||
| | -------------- | ------------- | ----------------------------------------------------------------- | | ||
| | Node.js | >= 22 | [nodejs.org](https://nodejs.org) or `nvm install 22` | |
There was a problem hiding this comment.
CONTRIBUTING.md requires Node.js >= 22, but the repo’s enforced version is Node 20 (CI workflows set NODE_VERSION=20 and package.json engines is >=20). Please update this prerequisite to match CI/engines (or update CI/engines if you intend to require 22).
| | Node.js | >= 22 | [nodejs.org](https://nodejs.org) or `nvm install 22` | | |
| | Node.js | >= 20 | [nodejs.org](https://nodejs.org) or `nvm install 20` | |
| **Shell environment** — add to `~/.zshrc` or `~/.bashrc`: | ||
|
|
||
| ```bash | ||
| export ANDROID_HOME=$HOME/Library/Android/sdk | ||
| export PATH=$PATH:$ANDROID_HOME/emulator:$ANDROID_HOME/platform-tools | ||
| ``` |
There was a problem hiding this comment.
The suggested ANDROID_HOME value uses the macOS default ($HOME/Library/Android/sdk), but this path is different on Linux/Windows. Since the guide is otherwise cross-platform, consider documenting OS-specific defaults (or referencing ANDROID_HOME from Android Studio’s SDK Manager) to avoid misconfiguration on non-macOS machines.
| java -version 2>&1 | ||
|
|
||
| # nvm (needed for node version management) | ||
| command -v nvm 2>&1 || test -d "$HOME/.nvm" && echo "nvm found" |
There was a problem hiding this comment.
The nvm presence check has incorrect ||/&& chaining: command -v nvm ... || test -d "$HOME/.nvm" && echo "nvm found" will print "nvm found" even when command -v nvm succeeds (and can be confusing). Please rewrite with explicit grouping so the output is accurate (e.g., only echo when nvm is found, otherwise report missing).
| command -v nvm 2>&1 || test -d "$HOME/.nvm" && echo "nvm found" | |
| if command -v nvm >/dev/null 2>&1 || test -d "$HOME/.nvm"; then | |
| echo "nvm found" | |
| else | |
| echo "nvm missing" | |
| fi |
| | Missing tool | Install command | | ||
| | ---------------------- | ------------------------------------------------------------------------------------------------------------------- | ------------------ | ------- | --------------------------------------------------------- | ------------------ | ------- | --------------------------------------------- | | ||
| | Homebrew | `/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"` | | ||
| | nvm | `curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.3/install.sh \| bash` | | ||
| | Node.js 22 | `nvm install 22` | | ||
| | Corepack + Yarn | `corepack enable && corepack prepare yarn@4.10.0 --activate` | | ||
| | Watchman | `brew install watchman` | | ||
| | rbenv + Ruby | `brew install rbenv && rbenv install $(rbenv install -l 2>/dev/null | grep -E '^\s\*3\.' | tail -1 | tr -d ' ') && rbenv global $(rbenv install -l 2>/dev/null | grep -E '^\s\*3\.' | tail -1 | tr -d ' ')` (installs latest stable Ruby 3.x) | | ||
| | Bundler | `gem install bundler` | | ||
| | JDK 17 | `brew install openjdk@17` | | ||
| | Maestro | `brew install mobile-dev-inc/tap/maestro` | | ||
| | Android SDK components | `$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager "platforms;android-36" "build-tools;36.0.0" "ndk;28.2.13676358"` | |
There was a problem hiding this comment.
The auto-installable tools table is malformed: the separator row includes extra columns, and the rbenv + Ruby command row contains unescaped | characters that will break the table layout. Please fix the table to have a consistent number of columns (or switch this section to a simple bullet list) so GitHub renders it correctly.
| | Missing tool | Install command | | |
| | ---------------------- | ------------------------------------------------------------------------------------------------------------------- | ------------------ | ------- | --------------------------------------------------------- | ------------------ | ------- | --------------------------------------------- | | |
| | Homebrew | `/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"` | | |
| | nvm | `curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.3/install.sh \| bash` | | |
| | Node.js 22 | `nvm install 22` | | |
| | Corepack + Yarn | `corepack enable && corepack prepare yarn@4.10.0 --activate` | | |
| | Watchman | `brew install watchman` | | |
| | rbenv + Ruby | `brew install rbenv && rbenv install $(rbenv install -l 2>/dev/null | grep -E '^\s\*3\.' | tail -1 | tr -d ' ') && rbenv global $(rbenv install -l 2>/dev/null | grep -E '^\s\*3\.' | tail -1 | tr -d ' ')` (installs latest stable Ruby 3.x) | | |
| | Bundler | `gem install bundler` | | |
| | JDK 17 | `brew install openjdk@17` | | |
| | Maestro | `brew install mobile-dev-inc/tap/maestro` | | |
| | Android SDK components | `$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager "platforms;android-36" "build-tools;36.0.0" "ndk;28.2.13676358"` | | |
| - **Homebrew**: `/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"` | |
| - **nvm**: `curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.3/install.sh | bash` | |
| - **Node.js 22**: `nvm install 22` | |
| - **Corepack + Yarn**: `corepack enable && corepack prepare yarn@4.10.0 --activate` | |
| - **Watchman**: `brew install watchman` | |
| - **rbenv + Ruby**: `brew install rbenv && rbenv install $(rbenv install -l 2>/dev/null | grep -E '^\s\*3\.' | tail -1 | tr -d ' ') && rbenv global $(rbenv install -l 2>/dev/null | grep -E '^\s\*3\.' | tail -1 | tr -d ' ')` (installs latest stable Ruby 3.x) | |
| - **Bundler**: `gem install bundler` | |
| - **JDK 17**: `brew install openjdk@17` | |
| - **Maestro**: `brew install mobile-dev-inc/tap/maestro` | |
| - **Android SDK components**: `$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager "platforms;android-36" "build-tools;36.0.0" "ndk;28.2.13676358"` |
| # Node.js >= 22 | ||
| node --version 2>&1 || which node | ||
|
|
||
| # Corepack | ||
| corepack --version 2>&1 || which corepack | ||
|
|
||
| # Yarn 4.10.0 | ||
| yarn --version 2>&1 || which yarn | ||
|
|
There was a problem hiding this comment.
This skill pins Node.js 22 (Node.js >= 22 checks, nvm install 22, nvm use 22), but the repo’s CI and package.json currently target Node 20. Please align the skill to the repo’s supported Node version (or update CI/engines if the intent is to move to 22).
| watchman --version 2>&1 || which watchman | ||
|
|
||
| # JDK 17 | ||
| java -version 2>&1 |
There was a problem hiding this comment.
Step 1 says to fall back to which <tool> when version checks fail due to sandbox restrictions, but java -version has no fallback (unlike the other checks). Add a fallback (e.g., which java) to keep the check behavior consistent with the instructions above.
| java -version 2>&1 | |
| java -version 2>&1 || which java |
CONTRIBUTING.md
Outdated
| | `WALLET_KIT_MT_REDIRECT_NATIVE_DEV` | Deep link scheme matching your dev bundle ID | | ||
| | `ANDROID_DEBUG_KEYSTORE_PASSWORD` | Android Studio's default: `android` | | ||
| | `ANDROID_DEBUG_KEYSTORE_ALIAS` | Android Studio's default: `androiddebugkey` | | ||
| | `ANDROID_DEV_KEYSTORE_PASSWORD` | Generate your own keystore with `keytool -genkey -v -keystore dev.keystore -alias dev -keyalg RSA -keysize 2048 -validity 10000`, then use the password you set | |
There was a problem hiding this comment.
The keystore generation command creates dev.keystore, but Android signing config expects the file at android/keystores/dev-release.keystore (see android/app/build.gradle). Please update the instructions to generate the keystore at the expected path/filename (or document the required rename/move) to avoid build failures.
| | `ANDROID_DEV_KEYSTORE_PASSWORD` | Generate your own keystore with `keytool -genkey -v -keystore dev.keystore -alias dev -keyalg RSA -keysize 2048 -validity 10000`, then use the password you set | | |
| | `ANDROID_DEV_KEYSTORE_PASSWORD` | Generate your own keystore with `keytool -genkey -v -keystore android/keystores/dev-release.keystore -alias dev -keyalg RSA -keysize 2048 -validity 10000`, then use the password you set | |
| | `WALLET_KIT_MT_REDIRECT_NATIVE_DEV` | Deep link scheme matching your dev bundle ID | | ||
| | `ANDROID_DEBUG_KEYSTORE_PASSWORD` | `android` (default) | | ||
| | `ANDROID_DEBUG_KEYSTORE_ALIAS` | `androiddebugkey` (default) | | ||
| | `ANDROID_DEV_KEYSTORE_PASSWORD` | Generate: `keytool -genkey -v -keystore dev.keystore -alias dev -keyalg RSA -keysize 2048 -validity 10000` | |
There was a problem hiding this comment.
The keystore generation command produces dev.keystore, but the Gradle config expects android/keystores/dev-release.keystore. Please update the skill to generate the keystore at the path/filename the build actually uses (or add a step to move/rename it), otherwise Android dev builds will still fail even with the env vars set.
| | `ANDROID_DEV_KEYSTORE_PASSWORD` | Generate: `keytool -genkey -v -keystore dev.keystore -alias dev -keyalg RSA -keysize 2048 -validity 10000` | | |
| | `ANDROID_DEV_KEYSTORE_PASSWORD` | Generate: `mkdir -p android/keystores && keytool -genkey -v -keystore android/keystores/dev-release.keystore -alias dev -keyalg RSA -keysize 2048 -validity 10000` | |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Android SDK components | ||
| if [ -n "$ANDROID_HOME" ]; then | ||
| $ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager --list 2>/dev/null | grep -E "platforms;android-36|build-tools;36.0.0|ndk;28.2.13676358" || echo "sdkmanager not available — check Android Studio SDK Manager manually" |
There was a problem hiding this comment.
sdkmanager --list | grep ... || echo "sdkmanager not available" will also print the "sdkmanager not available" message when sdkmanager is available but the required components are missing (since grep exits non-zero). Consider splitting this into (1) an explicit check that sdkmanager exists/is executable, and (2) a separate check/report for each required package so missing Build-Tools/NDK are reported accurately.
| $ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager --list 2>/dev/null | grep -E "platforms;android-36|build-tools;36.0.0|ndk;28.2.13676358" || echo "sdkmanager not available — check Android Studio SDK Manager manually" | |
| sdkmanager_bin="$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager" | |
| if [ -x "$sdkmanager_bin" ]; then | |
| sdkmanager_list="$("$sdkmanager_bin" --list 2>/dev/null)" | |
| if printf '%s\n' "$sdkmanager_list" | grep -Eq "platforms;android-36([[:space:]]|$)"; then | |
| echo "SDK Platform 36: OK" | |
| else | |
| echo "SDK Platform 36: MISSING" | |
| fi | |
| if printf '%s\n' "$sdkmanager_list" | grep -Eq "build-tools;36\.0\.0([[:space:]]|$)"; then | |
| echo "Build-Tools 36.0.0: OK" | |
| else | |
| echo "Build-Tools 36.0.0: MISSING" | |
| fi | |
| if printf '%s\n' "$sdkmanager_list" | grep -Eq "ndk;28\.2\.13676358([[:space:]]|$)"; then | |
| echo "NDK 28.2.13676358: OK" | |
| else | |
| echo "NDK 28.2.13676358: MISSING" | |
| fi | |
| else | |
| echo "sdkmanager not available — check Android Studio SDK Manager manually" | |
| fi |
LLM-QUICK-START.md
Outdated
| `brew install rbenv && rbenv install 3.4.5 && rbenv global 3.4.5` (macOS) or | ||
| use system package manager on Linux |
There was a problem hiding this comment.
The guide recommends installing Ruby 3.4.5 via rbenv, but this repo’s CI and Gemfile.lock are currently pinned to Ruby 3.1.4. To avoid contributors hitting gem resolution / native extension differences vs CI, consider recommending the same Ruby version as CI (or explicitly calling out that 3.1.4 is the validated version and newer Rubies may work but are not guaranteed).
| `brew install rbenv && rbenv install 3.4.5 && rbenv global 3.4.5` (macOS) or | |
| use system package manager on Linux | |
| `brew install rbenv && rbenv install 3.1.4 && rbenv global 3.1.4` (macOS) or | |
| use system package manager on Linux. `3.1.4` is the validated version for this | |
| repo; newer Rubies may work but are not guaranteed. |
LLM-QUICK-START.md
Outdated
| nvm use # Use version from .nvmrc (or nvm install 20 if not set) | ||
| bundle install # Ruby deps (Fastlane, CocoaPods — no separate pod install needed) | ||
| yarn install # Node deps (postinstall handles Husky, polyfills, pods) |
There was a problem hiding this comment.
This step references using .nvmrc, but the repository doesn’t include a .nvmrc file (so nvm use won’t select a project version). Either add a .nvmrc to the repo, or change the instructions to use an explicit version (nvm install/use 20) or reference package.json#engines.node.
| nvm use # Use version from .nvmrc (or nvm install 20 if not set) | |
| bundle install # Ruby deps (Fastlane, CocoaPods — no separate pod install needed) | |
| yarn install # Node deps (postinstall handles Husky, polyfills, pods) | |
| nvm install 20 && nvm use 20 # Use explicit Node.js version required by this repo | |
| bundle install # Ruby deps (Fastlane, CocoaPods — no separate pod install needed) | |
| yarn install # Node deps (postinstall handles Husky, polyfills, pods) |
CONTRIBUTING.md
Outdated
| | Variable | Notes | | ||
| | --------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `IS_E2E_TEST` | Set to `true` when running Maestro | | ||
| | `E2E_TEST_RECOVERY_PHRASE` | Generate a new wallet via any Stellar wallet and copy the 12/24-word phrase (used by `CreateWallet`/`ImportWallet` flows) | | ||
| | `E2E_TEST_FUNDED_RECOVERY_PHRASE` | Recovery phrase for an account **funded on mainnet** with real XLM. Required by `SendClassicTokenMainnet` and `SwapClassicTokenMainnet` flows which execute real transactions on mainnet. Fund the account before running these tests. | | ||
|
|
||
| See `.env.example` for the full list with inline comments. |
There was a problem hiding this comment.
The E2E section instructs using a recovery phrase for an account “funded on mainnet with real XLM”. Existing E2E docs caution against using phrases that hold real funds; this doc should align by recommending a dedicated low-balance test wallet (never a personal wallet) and linking to the E2E local setup doc for safe handling guidance.
| | Variable | Notes | | |
| | --------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | |
| | `IS_E2E_TEST` | Set to `true` when running Maestro | | |
| | `E2E_TEST_RECOVERY_PHRASE` | Generate a new wallet via any Stellar wallet and copy the 12/24-word phrase (used by `CreateWallet`/`ImportWallet` flows) | | |
| | `E2E_TEST_FUNDED_RECOVERY_PHRASE` | Recovery phrase for an account **funded on mainnet** with real XLM. Required by `SendClassicTokenMainnet` and `SwapClassicTokenMainnet` flows which execute real transactions on mainnet. Fund the account before running these tests. | | |
| See `.env.example` for the full list with inline comments. | |
| | Variable | Notes | | |
| | --------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | |
| | `IS_E2E_TEST` | Set to `true` when running Maestro | | |
| | `E2E_TEST_RECOVERY_PHRASE` | Generate a new wallet via any Stellar wallet and copy the 12/24-word phrase (used by `CreateWallet`/`ImportWallet` flows) | | |
| | `E2E_TEST_FUNDED_RECOVERY_PHRASE` | Recovery phrase for a **dedicated low-balance mainnet test wallet only**. Required by `SendClassicTokenMainnet` and `SwapClassicTokenMainnet` flows which execute real transactions on mainnet. **Never use a personal wallet or any wallet holding meaningful funds.** | | |
| See `.env.example` for the full list with inline comments, and see the [E2E local setup guide](./docs/e2e-local-setup.md) for safe handling guidance for recovery phrases and funded test wallets. |
| - See [CONTRIBUTING.md](CONTRIBUTING.md#environment-variables) for how to | ||
| obtain each variable (public endpoints, Reown dashboard, keystore | ||
| generation) |
There was a problem hiding this comment.
This new pointer to CONTRIBUTING is helpful, but the README’s example .env block above still shows PROD variables/placeholders, while CONTRIBUTING’s “Required” section focuses on DEV variables for local development. Consider adjusting the README example to match the recommended local-dev variables (or removing the example block entirely) to avoid sending newcomers to fill in the wrong set first.
Align README .env example to use DEV variables matching CONTRIBUTING.md, instead of PROD placeholders that confuse newcomers.
Use a more human-readable filename and update references in CONTRIBUTING.md.
Skill Eval:
freighter-dev-setup(Mobile) — Benchmark ReportDate: 2026-04-08
Iterations: 4
Repo: stellar/freighter-mobile
Model: Claude Opus 4.6
Prompt: "I just cloned freighter-mobile and want to set up my dev environment. Check what I need and help me get started."
Quantitative (n=4)
With Skill
Without Skill (baseline)
Delta
The skill uses more resources because it actually runs 74 tool calls on average checking each prerequisite, inspecting Android SDK directories, reading .env values, and verifying versions. The baseline reads docs and produces a static guide — faster but finds zero actual issues.
Qualitative (consistent across all 4 iterations)
Assertion Results (aggregated across 4 iterations)
The skill's 88% pass rate (vs 21% baseline) is limited only by JDK version checks being sandbox-blocked in 2 runs — the skill correctly fell back to flagging it for manual verification.
Analysis
88% vs 21% pass rate across 4 iterations. The skill reliably finds real issues; the baseline never does.
Watchman detected missing in all 4 runs. This is a real gap on this machine that no baseline run found. A contributor following the baseline would hit build failures.
Android SDK version mismatches found in 2/4 runs (Build-Tools 36.1.0 installed but 36.0.0 required; NDK 29.0 installed but 28.2 required). The variance (2/4 vs 4/4) is due to sandbox intermittently blocking directory listing. The other 2 runs still flagged these as needing verification.
Token cost is justified. The skill uses 24% more tokens on average, but the extra cost buys:
Duration variance (195-259s) correlates with how many SDK directory checks succeeded before sandbox interference. The skill handles this gracefully with fallback methods.
The baseline produces documentation, not diagnosis. Every baseline run output a comprehensive setup guide, but none checked whether the contributor's machine actually had the tools. The skill's value is the difference between "here's what you need" and "here's what's missing on your machine."