Skip to content

fix(rn_cli_wallet): pin Android NDK version to stop flaky E2E builds#564

Draft
ignaciosantise wants to merge 3 commits into
mainfrom
fix/flaky-android-ci
Draft

fix(rn_cli_wallet): pin Android NDK version to stop flaky E2E builds#564
ignaciosantise wants to merge 3 commits into
mainfrom
fix/flaky-android-ci

Conversation

@ignaciosantise

Copy link
Copy Markdown
Collaborator

Problem

The Android E2E job intermittently failed during the Gradle build with a corrupted NDK download, passing only on rerun:

java.util.zip.ZipException: Archive is not a ZIP archive
"Install NDK (Side by side) 27.1.12297006" failed.
> Failed to apply plugin 'com.facebook.react.rootproject'.
   > com.android.builder.sdk.InstallFailedException

Root cause: the action already pre-installed the NDK (to avoid RN's flaky on-the-fly fetch), but its hardcoded 27.0.12077973 had drifted from the 27.1.12297006 that Expo SDK 56 now wants. So it seeded the wrong NDK and Gradle re-fetched the right one at build time — the exact download that intermittently corrupts.

Fix

Make app.json the single source of truth for the NDK version:

  • plugins/withAndroidNdkVersion.js (new config plugin) injects ext.ndkVersion into the generated android/build.gradle, before apply plugin: "expo-root-project" — so Expo's setIfNotExist("ndkVersion") default becomes a no-op and our pin wins. app/build.gradle keeps reading ndkVersion rootProject.ext.ndkVersion unchanged.
  • app.json holds the version as the plugin arg.
  • .github/actions/walletkit-build-and-maestro reads that same app.json entry to pre-install the exact NDK via sdkmanager, now wrapped in a 3× retry so a corrupt archive self-heals instead of failing the run.
  • AGENTS.md documents the upgrade-time check (re-verify the pin on every RN/Expo bump).

Now the native build and the CI pre-install both derive from one line in app.json — no drift, no node_modules grepping.

Verification

  • expo prebuild --platform android --cleanandroid/build.gradle gets ext.ndkVersion = "27.1.12297006" immediately above apply plugin: "expo-root-project".
  • Transform is idempotent and throws loudly if the anchor line disappears (e.g. an Expo restructure).
  • CI read resolves 27.1.12297006 from app.json.
  • Real proof is a green Android E2E run on this branch.

🤖 Generated with Claude Code

The Android E2E job intermittently failed with a corrupted NDK download
(ndk;27.1.12297006 -> "Archive is not a ZIP archive" ->
InstallFailedException), passing on rerun. The action already pre-installed
the NDK to avoid the on-the-fly fetch, but its hardcoded 27.0.12077973 had
drifted from the 27.1.12297006 the Expo SDK 56 build now wants — so it seeded
the wrong NDK and Gradle re-fetched the right one at build time (the flaky path).

Make app.json the single source of truth via a new withAndroidNdkVersion config
plugin: it injects ext.ndkVersion into the generated android/build.gradle (before
expo-root-project applies, so Expo's setIfNotExist default is a no-op), and the
E2E action reads the same app.json entry to pre-install that exact NDK (now with
a retry so a corrupt archive self-heals). Documents the upgrade-time check in
AGENTS.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
deposit-app-demo Error Error Jul 8, 2026 5:57pm
pos-demo Ready Ready Preview, Comment Jul 8, 2026 5:57pm

Request Review

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Claude finished @ignaciosantise's task in 2m 35s —— View job


Code Review

  • Gather context
  • Read changed files
  • Run breaking changes subagent — no issues found
  • Run data classification subagent — no issues found
  • Post review findings

Found 1 issue(s)

Issue 1: Kotlin DSL build.gradle silently skips NDK injection

ID: withAndroidNdkVersion-kotlin-dsl-silent-skip-a3f1
File: wallets/rn_cli_wallet/plugins/withAndroidNdkVersion.js:48
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: The plugin guards NDK injection with if (cfg.modResults.language === 'groovy') but returns cfg unchanged — no warning, no error — when the language is anything else (e.g. Kotlin DSL .kts)
  • Risk: If Expo switches the generated build.gradle to Kotlin DSL on a future SDK bump, the plugin silently no-ops: the NDK pin is never injected, the CI pre-install seeds a version Gradle ignores, and the corrupt-download flake silently returns
  • Impact: Flaky E2E builds return without any indication that the plugin was skipped — exactly the scenario this PR is designed to prevent
  • Trigger: Any Expo SDK upgrade that moves to Kotlin DSL for project-level build.gradle

Recommendation: Throw on unsupported language so the failure is immediate and loud:

return withProjectBuildGradle(config, cfg => {
  if (cfg.modResults.language !== 'groovy') {
    throw new Error(
      `withAndroidNdkVersion: expected groovy build.gradle, got '${cfg.modResults.language}'. ` +
      'Update the plugin for Kotlin DSL support.'
    );
  }
  cfg.modResults.contents = setNdkVersion(cfg.modResults.contents, ndkVersion);
  return cfg;
});

Fix this →

Comment thread wallets/rn_cli_wallet/plugins/withAndroidNdkVersion.js Outdated
Address PR review: the plugin silently no-op'd when the generated root
build.gradle wasn't groovy. If a future Expo SDK emits a Kotlin DSL
(build.gradle.kts) root project, that skip would leave the NDK unpinned and
quietly reintroduce the corrupt-download flake this plugin exists to prevent.
Throw loudly instead so the failure is immediate and obvious.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Pins the Android NDK version for wallets/rn_cli_wallet so the Expo/Gradle build and the CI pre-install both use the same version, preventing flaky CI failures caused by intermittent corrupted NDK downloads during on-the-fly installation.

Changes:

  • Added an Expo config plugin to inject ext.ndkVersion into the generated android/build.gradle before expo-root-project applies its default.
  • Pinned the NDK version in wallets/rn_cli_wallet/app.json as the single source of truth.
  • Updated the walletkit E2E composite action to read that pinned version from app.json and pre-install it via sdkmanager (with retries), plus documented upgrade guidance in AGENTS.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
wallets/rn_cli_wallet/plugins/withAndroidNdkVersion.js New config plugin that injects ext.ndkVersion into the generated root build.gradle before Expo’s default is applied.
wallets/rn_cli_wallet/app.json Adds the plugin entry and pins the NDK version (27.1.12297006).
wallets/rn_cli_wallet/AGENTS.md Documents the NDK pinning approach and upgrade-time verification steps.
.github/actions/walletkit-build-and-maestro/action.yml Reads the pinned NDK version from app.json and pre-installs via sdkmanager with retries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/actions/walletkit-build-and-maestro/action.yml
Address PR review (Copilot): the retry loop disabled pipefail but left
errexit on, so a non-zero `yes | sdkmanager` aborted the step before
`status=$?` ran — attempts 2/3 never executed. Capture the result via an
`if` (exempt from set -e) so failures are caught and retried as intended.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants