chore: move Android+iOS native builds from CI to a local pre-push gate - #608
Conversation
…tead The hosted android-build runner repeatedly hung for 3+ hours on the native C++ builds (react-native-audio-api, op-sqlite) and burned hours per PR. Move it to a LOCAL pre-merge gate: `cd android && ./gradlew assembleDebug assembleRelease` before merging any native/ Android/gradle/deps change (recorded in rules.md). JS-only / docs PRs don't need it.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
📝 WalkthroughWalkthroughThe hosted Android CI job is replaced with local pre-push native build gates. Changed Android and iOS files trigger their respective local builds, while the workflow documents the Android command and exceptions for docs-only and JS-only changes. ChangesNative Build Gates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant PrePushHook
participant NativeBuildTools
Developer->>PrePushHook: push changes
PrePushHook->>PrePushHook: detect Android and iOS native paths
PrePushHook->>NativeBuildTools: run matching local native builds
NativeBuildTools-->>Developer: report build result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 203-207: Revise the comment near the removed Android assemble job
to state specifically that the Android assemble build is not run in CI, while
acknowledging that the test job still installs the NDK and runs
:app:testDebugUnitTest. Keep the local pre-merge assemble commands and scope
guidance unchanged.
- Around line 203-207: Update the Android CI note near the workflow comments to
remove stale references to rules.md and the unverified Gradle command, or
replace them with a real, discoverable pre-merge process and exact
repository-supported gate command. Preserve the stated exceptions only if they
match the documented process.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5faa584f-75c7-4667-a314-63bbeb2d9a02
📒 Files selected for processing (1)
.github/workflows/ci.yml
| # NOTE: the Android build is NOT run in CI — it is a LOCAL pre-merge gate instead (the hosted | ||
| # runner repeatedly hung for 3+ hours on the native C++ builds and burned hours). Run it locally | ||
| # before merging any change that touches native/Android/gradle/deps: | ||
| # cd android && ./gradlew assembleDebug assembleRelease | ||
| # (see rules.md → On-device testing & verification). Docs-only / JS-only PRs don't need it. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Narrow “Android build is NOT run in CI” to the removed assemble job.
The test job still installs the Android NDK and runs :app:testDebugUnitTest at Line [168]. As written, this note may imply that CI performs no Android validation at all.
Proposed wording
- # NOTE: the Android build is NOT run in CI — it is a LOCAL pre-merge gate instead (the hosted
+ # NOTE: the hosted assembleDebug/assembleRelease Android build job is NOT run in CI; Android
+ # unit tests still run in the `test` job above. The full build is a LOCAL pre-merge gate (the hosted📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # NOTE: the Android build is NOT run in CI — it is a LOCAL pre-merge gate instead (the hosted | |
| # runner repeatedly hung for 3+ hours on the native C++ builds and burned hours). Run it locally | |
| # before merging any change that touches native/Android/gradle/deps: | |
| # cd android && ./gradlew assembleDebug assembleRelease | |
| # (see rules.md → On-device testing & verification). Docs-only / JS-only PRs don't need it. | |
| # NOTE: the hosted assembleDebug/assembleRelease Android build job is NOT run in CI; Android | |
| # unit tests still run in the `test` job above. The full build is a LOCAL pre-merge gate (the hosted | |
| # runner repeatedly hung for 3+ hours on the native C++ builds and burned hours). Run it locally | |
| # before merging any change that touches native/Android/gradle/deps: | |
| # cd android && ./gradlew assembleDebug assembleRelease | |
| # (see rules.md → On-device testing & verification). Docs-only / JS-only PRs don't need it. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 203 - 207, Revise the comment near the
removed Android assemble job to state specifically that the Android assemble
build is not run in CI, while acknowledging that the test job still installs the
NDK and runs :app:testDebugUnitTest. Keep the local pre-merge assemble commands
and scope guidance unchanged.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
test -f rules.md
rg -n -F 'cd android && ./gradlew assembleDebug assembleRelease' rules.md
rg -n -i 'On-device testing|docs-only|JS-only|native|Android|Gradle|depend' rules.mdRepository: off-grid-ai/OGAM
Length of output: 154
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Tracked candidate files:\n'
git ls-files | rg '(^|/)rules\.md$|(^|/)ci\.yml$|(^|/)github/workflows/.*\.ya?ml$' || true
printf '\nSearch exact command:\n'
rg -n -F 'cd android && ./gradlew assembleDebug assembleRelease' || true
printf '\nSearch nearby terms:\n'
rg -n -i 'assembleDebug|assembleRelease|On-device testing|on-device testing|docs-only|js-only|JS-only|native|Android|gradle|dependenc' . || trueRepository: off-grid-ai/OGAM
Length of output: 227
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'rules.md present in working tree and tracked files:\n'
test -f rules.md && echo "rules.md exists" || echo "rules.md does not exist"
git ls-tree -r HEAD --name-only | rg '(^|/)rules\.md$|(^|/)ci\.ya?ml$|^\.github/workflows/' || true
printf '\nExact command across repository:\n'
rg -n -F 'cd android && ./gradlew assembleDebug assembleRelease' || true
printf '\nAll ./gradlew/assemble references:\n'
rg -n -E '\./gradlew|assembleDebug|assembleRelease|On-device|docs-only|JS-only|js-only|gradle|dependenc' . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' || true
printf '\ndotgithub workflow relevant sections if present:\n'
if [ -f .github/workflows/ci.yml ]; then
wc -l .github/workflows/ci.yml
sed -n '180,220p' .github/workflows/ci.yml
fiRepository: off-grid-ai/OGAM
Length of output: 427
Remove or fix the stale references.
rules.md is not present, and the referenced cd android && ./gradlew assembleDebug assembleRelease command is not in the repository. Keep the comment only if the exact gate command and exceptions are added to rules.md; otherwise, update it to a real, discoverable pre-merge process.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 203 - 207, Update the Android CI note
near the workflow comments to remove stale references to rules.md and the
unverified Gradle command, or replace them with a real, discoverable pre-merge
process and exact repository-supported gate command. Preserve the stated
exceptions only if they match the documented process.
Replaces the removed CI android-build with a LOCAL pre-push gate, and adds the iOS build too: - Android: `assembleDebug assembleRelease` when .kt/.kts, android/, or package*.json change. - iOS: simulator build (CODE_SIGNING_ALLOWED=NO) when .swift, ios/, or Podfile change. JS-only / docs pushes skip both and stay fast.
alichherawalla
changed the title
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.husky/pre-push:
- Around line 92-108: Ensure the native build gates in .husky/pre-push propagate
failures by enabling fail-fast handling for the hook or explicitly checking the
statuses of the Android Gradle and iOS xcodebuild subshell commands and exiting
nonzero when either fails. Preserve the existing conditional execution based on
PUSHED_ANDROID_NATIVE and PUSHED_IOS_NATIVE.
- Line 46: Update the PUSHED_ANDROID_NATIVE matcher to include all documented
Android gate inputs: native C/C++ source and header files, root Gradle wrapper
files, the gradle/ directory, and Gradle build scripts, while retaining Kotlin,
existing Android, package manifest, and build-directory exclusions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # Native BUILD gate — moved OUT of CI (the hosted android-build runner hung for 3+ hours on the | ||
| # native C++ builds). Run the full native builds LOCALLY on push when native / gradle / pods / deps | ||
| # change. JS-only and docs-only pushes skip these (they stay fast). | ||
| PUSHED_ANDROID_NATIVE=$(printf '%s\n' "$CHANGED_FILES" | grep -E '\.(kt|kts)$|^android/|^package(-lock)?\.json$' | grep -v '/build/' || true) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include all declared Android gate inputs.
This matcher misses native C++ files and Gradle inputs outside android/ (for example root gradlew, gradle/, or *.gradle). Those changes can bypass the replacement Android gate despite the documented native/Gradle scope.
Proposed fix
-PUSHED_ANDROID_NATIVE=$(printf '%s\n' "$CHANGED_FILES" | grep -E '\.(kt|kts)$|^android/|^package(-lock)?\.json$' | grep -v '/build/' || true)
+PUSHED_ANDROID_NATIVE=$(printf '%s\n' "$CHANGED_FILES" | grep -E '\.(kt|kts|c|cc|cpp|cxx|h|hpp)$|^android/|^gradle/|^gradlew$|(^|/)gradle\.properties$|\.gradle$|^package(-lock)?\.json$' | grep -v '/build/' || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PUSHED_ANDROID_NATIVE=$(printf '%s\n' "$CHANGED_FILES" | grep -E '\.(kt|kts)$|^android/|^package(-lock)?\.json$' | grep -v '/build/' || true) | |
| PUSHED_ANDROID_NATIVE=$(printf '%s\n' "$CHANGED_FILES" | grep -E '\.(kt|kts|c|cc|cpp|cxx|h|hpp)$|^android/|^gradle/|^gradlew$|(^|/)gradle\.properties$|\.gradle$|^package(-lock)?\.json$' | grep -v '/build/' || true) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.husky/pre-push at line 46, Update the PUSHED_ANDROID_NATIVE matcher to
include all documented Android gate inputs: native C/C++ source and header
files, root Gradle wrapper files, the gradle/ directory, and Gradle build
scripts, while retaining Kotlin, existing Android, package manifest, and
build-directory exclusions.
| if [ -n "$PUSHED_ANDROID_NATIVE" ]; then | ||
| echo "▶ Android build (assembleDebug + assembleRelease) — local gate, replaces CI android-build..." | ||
| (cd android && ./gradlew assembleDebug assembleRelease) | ||
| fi | ||
|
|
||
| if [ -n "$PUSHED_IOS_NATIVE" ]; then | ||
| echo "▶ iOS build (simulator, no code-signing) — local gate..." | ||
| (cd ios && xcodebuild \ | ||
| -workspace OffgridMobile.xcworkspace \ | ||
| -scheme OffgridMobile \ | ||
| -configuration Debug \ | ||
| -sdk iphonesimulator \ | ||
| -destination 'generic/platform=iOS Simulator' \ | ||
| -derivedDataPath build/prepush-ios \ | ||
| CODE_SIGNING_ALLOWED=NO \ | ||
| build) | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
sed -n '1,115p' .husky/pre-pushRepository: off-grid-ai/OGAM
Length of output: 4068
Make native build failures fail the pre-push hook.
.husky/pre-push does not run with set -e, and each native build runs in a subshell. If ./gradlew assembleDebug assembleRelease or xcodebuild fails, the hook can continue past that checkpoint or return success while the native gate did not block the push. Add explicit failure handling such as set -e at the top of the hook or check/exit on nonzero_status for those build commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.husky/pre-push around lines 92 - 108, Ensure the native build gates in
.husky/pre-push propagate failures by enabling fail-fast handling for the hook
or explicitly checking the statuses of the Android Gradle and iOS xcodebuild
subshell commands and exiting nonzero when either fails. Preserve the existing
conditional execution based on PUSHED_ANDROID_NATIVE and PUSHED_IOS_NATIVE.
1 participant
What
Relocates the native build gate from CI to a local pre-push hook.
android-buildjob from.github/workflows/ci.yml..husky/pre-push, scoped so they only run when relevant files change:./gradlew assembleDebug assembleReleasewhen.kt/.kts,android/, orpackage*.jsonchange.CODE_SIGNING_ALLOWED=NO, no signing prompts) when.swift,ios/, orPodfilechange.Why
The hosted
android-buildrunner repeatedly hung for 3+ hours on the native C++ builds (react-native-audio-api, op-sqlite), blocking merges and burning CI hours for zero signal (it passed whenever it didn't hang). It even blocked a docs-only PR. Building locally on push catches native/gradle/pods breakage before it ever reaches the remote, without the flaky hosted runner.lint,typecheck,test,architecture, SonarCloud and CodeRabbit remain in CI.Summary by CodeRabbit