Skip to content
This repository was archived by the owner on May 1, 2026. It is now read-only.

fix: include Cordova plugin files in Android cloud-build upload bundle#577

Merged
riderx merged 2 commits into
mainfrom
fix/cordova-plugin-bundle-upload
Apr 7, 2026
Merged

fix: include Cordova plugin files in Android cloud-build upload bundle#577
riderx merged 2 commits into
mainfrom
fix/cordova-plugin-bundle-upload

Conversation

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

@WcaleNieWolny WcaleNieWolny commented Apr 7, 2026

Summary

  • Capgo cloud builds for Android failed when the project used a Cordova plugin (e.g. onesignal-cordova-plugin) with errors like Could not read script '.../node_modules/<plugin>/...gradle' as it does not exist.
  • Root cause: extractNativeDependencies in src/build/request.ts only parsed capacitor.settings.gradle, which lists Capacitor plugins. Cordova plugins are wired by cap sync into android/capacitor-cordova-android-plugins/build.gradle via apply from: "../../node_modules/<plugin>/<file>.gradle" and were never detected, so their files were stripped from the upload zip.
  • Fix: parse capacitor-cordova-android-plugins/build.gradle for apply from references, track them on a new cordovaPackages set in NativeDependencies, and include the entire package contents in the bundle. Cordova plugins don't follow Capacitor's <pkg>/android/ convention — supporting gradle scripts live at the package root, native sources under src/android/, etc., so the simplest correct behavior is to ship the whole package.
  • The recursion check in addDirectoryToZip is updated to walk into directories leading to either Capacitor or Cordova packages.

Test plan

  • Existing test:build-zip-filter cases still pass.
  • New regression test in test/test-build-zip-filter.mjs reproduces the onesignal-cordova-plugin scenario:
    • capacitor.settings.gradle only contains @capacitor/android
    • capacitor-cordova-android-plugins/build.gradle references the Cordova plugin via apply from
    • Asserts the produced zip contains build-extras-onesignal.gradle, plugin.xml, src/android/OneSignal.java, package.json, and the generated capacitor-cordova-android-plugins/build.gradle itself.
  • `npx tsc --noEmit` clean.
  • `npm run build` succeeds.
  • Real cloud build of a tutorial app using `onesignal-cordova-plugin` succeeds end-to-end (to verify in follow-up).

Summary by CodeRabbit

  • New Features
    • Improved Android native dependency detection to automatically discover and include Cordova plugins in build archives for better Cordova compatibility.
  • Bug Fixes
    • Ensure plugin files are included while excluding bundled transitive dependencies nested inside a plugin to avoid bloated archives.
  • Tests
    • Added end-to-end tests verifying Cordova plugin discovery, correct inclusion of plugin assets, and exclusion of nested transitive deps.

The Capgo CLI's extractNativeDependencies only parsed
capacitor.settings.gradle, which lists Capacitor plugins. Cordova
plugins are wired by `cap sync` into
android/capacitor-cordova-android-plugins/build.gradle via
`apply from: "../../node_modules/<plugin>/<file>.gradle"` lines and were
never detected, so their files were stripped from the upload zip.
Cloud builds then failed with errors like:

  Could not read script '.../node_modules/onesignal-cordova-plugin/
  build-extras-onesignal.gradle' as it does not exist.

This change parses that file for `apply from` references, tracks
Cordova plugins in a new cordovaPackages set on NativeDependencies,
and includes the entire package contents in the bundle (Cordova plugins
do not follow Capacitor's <pkg>/android/ convention — gradle scripts
live at the root, native sources under src/android/, etc.).

A regression test reproducing the onesignal-cordova-plugin scenario is
added to test/test-build-zip-filter.mjs.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

Added Cordova plugin detection to Android native dependency extraction: parse capacitor-cordova-android-plugins/build.gradle to collect Cordova plugin package roots, include those package directories in the build zip, and exclude their nested node_modules transitive deps.

Changes

Cohort / File(s) Summary
Core packaging logic
src/build/request.ts
Added cordovaPackages: Set<string> to native dependency tracking. Parse capacitor-cordova-android-plugins/build.gradle for apply from: ... node_modules/<pkg>/... lines (with pnpm path normalization and scoped package handling). Updated shouldIncludeFile to include full Cordova plugin package roots while excluding their nested node_modules. Adjusted addDirectoryToZip recursion/prefix logic to consider both Capacitor packages and Cordova cordovaPackages.
Tests
test/test-build-zip-filter.mjs
Extended test nativeDeps fixture with cordovaPackages: new Set(). Added end-to-end test that creates a mock Android project referencing a Cordova plugin via capacitor-cordova-android-plugins/build.gradle, validates plugin files and native sources are present in the produced zip, and asserts nested transitive node_modules under the plugin are excluded.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I sniffed the gradle trails at dawn,
Found cordova roots where paths were drawn,
Packed plugin hearts and left their crumbs,
Hopped home singing build-time hums,
Zip snug — I’ve done my bunny job! 🎶

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding support for including Cordova plugin files in the Android cloud-build upload bundle, which directly addresses the core problem and solution described in the PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cordova-plugin-bundle-upload

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/build/request.ts`:
- Around line 809-820: The cordova package match currently includes nested
node_modules (node_modules/<plugin>/node_modules/...), causing bundled deps to
be pulled into build.zip; modify the check in the android cordovaPackages loop
(using variables cordovaPrefix and normalizedPath and related to
addDirectoryToZip) to reject paths that are inside the plugin's nested
node_modules directory—i.e., after verifying normalizedPath
startsWith(cordovaPrefix), compute the remaining subpath and ensure it does NOT
start with "node_modules/" (or equal "node_modules"), only then return true so
only the plugin's own files (not its bundled dependencies) are included.
🪄 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

Run ID: 586dd9d7-473a-4106-9bfd-4ffd0cb82115

📥 Commits

Reviewing files that changed from the base of the PR and between 136db48 and 0c208b4.

📒 Files selected for processing (2)
  • src/build/request.ts
  • test/test-build-zip-filter.mjs

Comment thread src/build/request.ts Outdated
The previous Cordova plugin inclusion in shouldIncludeFile matched
anything under node_modules/<plugin>/, which would also pull the
plugin's bundled transitive dependencies (node_modules/<plugin>/
node_modules/...) into the upload zip. Reject those subpaths so only
the plugin's own files are bundled.

Extended the Cordova regression test to also create a bundled
transitive dep and assert it is not in the produced zip.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 7, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
14.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/build/request.ts (1)

764-764: Consider moving the regex to module scope to avoid recompilation.

The apply from regex is compiled on every call to extractNativeDependencies. Moving it to module scope would avoid repeated compilation, though the performance impact is minimal since this function is called infrequently.

♻️ Optional refactor to module scope
+// Regex for parsing Cordova plugin apply-from references (module-level for reuse)
+const CORDOVA_APPLY_FROM_REGEX = /apply\s+from\s*:\s*["'](?:\.\.\/)+node_modules\/([^"']+)["']/g
+
 async function extractNativeDependencies(

Then at line 764:

-      const applyFromMatches = cordovaContent.matchAll(/apply\s+from\s*:\s*["'](?:\.\.\/)+node_modules\/([^"']+)["']/g)
+      const applyFromMatches = cordovaContent.matchAll(CORDOVA_APPLY_FROM_REGEX)

Note: Since matchAll with a global regex returns an iterator and doesn't mutate the regex, this is safe for reuse.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/build/request.ts` at line 764, Move the inline regex in
extractNativeDependencies to module scope to avoid recompiling it on each call:
create a top-level constant (e.g., APPLY_FROM_REGEX) with the pattern
/apply\s+from\s*:\s*["'](?:\.\.\/)+node_modules\/([^"']+)["']/g and replace the
inline regex usage in extractNativeDependencies (the
cordovaContent.matchAll(...) call) with
cordovaContent.matchAll(APPLY_FROM_REGEX); it’s safe to reuse because matchAll
with a global regex returns an iterator and does not mutate the regex.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/build/request.ts`:
- Line 764: Move the inline regex in extractNativeDependencies to module scope
to avoid recompiling it on each call: create a top-level constant (e.g.,
APPLY_FROM_REGEX) with the pattern
/apply\s+from\s*:\s*["'](?:\.\.\/)+node_modules\/([^"']+)["']/g and replace the
inline regex usage in extractNativeDependencies (the
cordovaContent.matchAll(...) call) with
cordovaContent.matchAll(APPLY_FROM_REGEX); it’s safe to reuse because matchAll
with a global regex returns an iterator and does not mutate the regex.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 13087ede-51da-4336-9c4f-a8f1b6056566

📥 Commits

Reviewing files that changed from the base of the PR and between 0c208b4 and 53a4cce.

📒 Files selected for processing (2)
  • src/build/request.ts
  • test/test-build-zip-filter.mjs

@riderx riderx merged commit 0f755c1 into main Apr 7, 2026
18 of 19 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants