chore: point dev to builds.yml#27036
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
weitingsun
force-pushed
the
wsun/point-dev-env-to-builds.yml
branch
from
e460c17 to
84ace64
Compare
tommasini
changed the title
| @@ -244,6 +244,12 @@ class AppInformation extends PureComponent { | |||
| <Text style={styles.branchInfo}> | |||
| {`Remote Feature Flag Distribution: ${getFeatureFlagAppDistribution()}`} | |||
| </Text> | |||
| <Text style={styles.branchInfo}> | |||
| {`Rewards API URL: ${process.env.REWARDS_API_URL ?? '—'}`} | |||
There was a problem hiding this comment.
DO we want to keep the api url at AppInformation?
There was a problem hiding this comment.
I think it'd be good for awhile to test - this is listed in Builds.yml so it should not be a secret
There was a problem hiding this comment.
happy to remove it though if you don't think it's needed
| remapEnvVariable() { | ||
| local old_var_name="$1" | ||
| local new_var_name="$2" | ||
| if [ -z "${!old_var_name}" ]; then | ||
| echo "Error: $old_var_name does not exist in the environment." | ||
| return 1 | ||
| return 0 |
There was a problem hiding this comment.
return 0 is a silent failure risk on Bitrise. If a secret is ever misconfigured there, the build will pass and ship broken instead of failing fast
The proper fix would have been to distinguish Bitrise from local, e.g. using a BITRISE_IO env var (which Bitrise sets automatically):
if [ -z "${GITHUB_ACTIONS:-}" ]; then
if [ -n "${BITRISE_IO:-}" ]; then
# On Bitrise: missing source var IS an error
remapMainProdEnvVariables # still uses return 1
fi
# local: skip remap entirely — .js.env already has canonical names
fi
Or maybe cleaner
# Only Bitrise needs legacy remap
if [ -z "${GITHUB_ACTIONS:-}" ] && [ "${BUILDS_ENABLED_WITH_GH_ACTIONS_TEMPORARY:-false}" != "true" ]; then
# Bitrise: _DEV/_QA/_PROD vars MUST exist — keep return 1
remapMainProdEnvVariables
...
fi
remove appinformation display add back environment remove add spacing back use BUILDS_ENABLED_WITH_GH_ACTIONS_TEMPORARY flag address comment
weitingsun
force-pushed
the
wsun/point-dev-env-to-builds.yml
branch
from
fca7b53 to
d7c572f
Compare
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
No available E2E tag specifically covers the App Information screen, and the modified areas do not overlap with flows validated by SmokeAccounts, SmokeConfirmations, SmokeWalletPlatform, or any other tag. Therefore, running Detox E2E suites is not required for safe validation of this PR. Given the strictly informational UI scope and isolated build script change, the overall risk is low. Performance Test Selection: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| if ! loadBuildConfig "$METAMASK_BUILD_TYPE" "$METAMASK_ENVIRONMENT"; then | ||
| # Non-GHA: source .js.env early so BUILDS_ENABLED_WITH_GH_ACTIONS_TEMPORARY is set for the gate (local can opt in) | ||
| if [ -z "${GITHUB_ACTIONS:-}" ] && [ -e "$JS_ENV_FILE" ]; then | ||
| source "$JS_ENV_FILE" |
There was a problem hiding this comment.
Re-sourcing .js.env overwrites command-line build arguments
High Severity
The new source "$JS_ENV_FILE" calls at lines 1007 and 1024 unconditionally overwrite METAMASK_BUILD_TYPE and METAMASK_ENVIRONMENT, which were already correctly set from command-line arguments (MODE/ENVIRONMENT) at lines 34–35. Since .js.env.example exports both variables unconditionally (e.g., export METAMASK_BUILD_TYPE="main"), any developer passing a different build type or environment via CLI (e.g., ./scripts/build.sh ios flask production) will have those values silently replaced by .js.env defaults. This causes loadBuildConfig and the legacy remap to operate on the wrong build configuration.
Additional Locations (1)
There was a problem hiding this comment.
it's fine for .js.env to override these values
|
metamaskbot
added
the
release-7.70.0
4 participants
Description
Start to use builds.yml locally.
If we have
BUILDS_ENABLED_WITH_GH_ACTIONS_TEMPORARYset to true locally in .js.env then we will use the envs in Builds.yml and .js.env.Here are the steps:
This way - when we retire BUILDS_ENABLED_WITH_GH_ACTIONS_TEMPORARY, all the local builds will use builds.yml and .js.env
Changelog
CHANGELOG entry: Added ability for dev build to use builds.yml as source of truth
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Using Pre build
Using local build (yarn start:ios)
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Touches the build pipeline and environment variable precedence, which can break local/CI builds if the gating or sourcing order is wrong. Runtime app changes are limited to extra debug info on the settings screen.
Overview
Build configuration changes:
scripts/build.shnow conditionally loadsbuilds.ymlwhenBUILDS_ENABLED_WITH_GH_ACTIONS_TEMPORARY=true(for GitHub Actions and local opt-in), sources.js.envearly to read the flag, then sources it again afterward so local values overridebuilds.yml. Legacy Bitrise env remapping is kept for non-GHA runs, and missing*_DEV/_QA/_PRODsecrets only hard-fail on the legacy path.Debug visibility: The hidden environment info section in
AppInformationnow also displaysREWARDS_API_URLandMM_PORTFOLIO_URL(or—if unset).Written by Cursor Bugbot for commit 54fe502. This will update automatically on new commits. Configure here.