Skip to content

Conversation

@predatorx7
Copy link
Collaborator

@predatorx7 predatorx7 commented Dec 11, 2025

Update inapp sdk module

Summary by CodeRabbit

  • Bug Fixes

    • Fixed feature flag overrides causing interruptions with certain login pages
    • Fixed feature flag overrides returning incorrect values for non-overridden fields
  • New Features

    • Added support for app theme override
    • Added potentialFailureReasonsLink to themes
  • Chores

    • Updated default feature flag values
    • Version bumped to 0.27.0
    • Added build/setup and in-app SDK publish scripts to streamline releases

✏️ Tip: You can customize this high-level summary in your review settings.

Update inapp sdk module
@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Walkthrough

Adds optional app theme support across the app-info surface: a nullable theme field is added to pigeon/schema types and generated messages, theme JSON parsing is implemented in the reclaim module using a new utility, CHANGELOG/version bumped to 0.27.0, and several release/build scripts added/updated.

Changes

Cohort / File(s) Summary
Release files
CHANGELOG.md, pubspec.yaml
Bumped version to 0.27.0 and added changelog entries (feature flag fixes, theme additions, app theme override support).
Pigeon schema & generated messages
pigeon/schema.dart, lib/src/pigeon/messages.pigeon.dart
Added theme (String?) to ClientReclaimAppInfoOverride (field, constructor, encode/decode, list representation); public API/signature expanded.
Module theme handling
lib/reclaim_verifier_module.dart
Parse appInfo.theme string into a ReclaimAppThemeInfo via fromStringToObject; pass optional theme into AppInfo; log parse failures.
JSON utility
lib/utils/json.dart
New generic fromStringToObject<T>() decoding helper with validation and onInvalidContent error callback; returns T?.
Build & in-app scripts
scripts/build.sh, scripts/inapp/*, scripts/inapp/capacitor.sh, scripts/inapp/flutter.sh, scripts/inapp/inapp.sh
Added new scripts and steps to prepare/pull in-app SDKs, run native setup (rive_native:setup), and prepend changelog entries; several sed-based update steps are present but commented out.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to: error handling and stack trace reporting in fromStringToObject and its caller; consistency between pigeon/schema.dart and generated messages.pigeon.dart; call sites that construct ClientReclaimAppInfoOverride may need updates; build scripts' side effects (cloning, changelog prepend) and safety checks.

Possibly related PRs

Suggested reviewers

  • ChinmayMhatre
  • Karam19
  • kryptocodes

Poem

🐰 Soft bytes hop into the night,
A theme tucked in JSON tight,
Parsers nibble, logs keep score,
Version climbs, we patch the door—
Hooray, the app looks snazzy once more! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add theme to app overrides' directly and concisely summarizes the main change - adding a theme parameter to the ClientReclaimAppInfoOverride class and related theme parsing functionality throughout the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pre-0.27.0

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.

❤️ Share

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

Copy link

@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: 2

🧹 Nitpick comments (1)
lib/utils/json.dart (1)

3-19: Consider capturing actual stack trace for FormatException.

On line 11, StackTrace.current is used when the decoded value isn't a Map. This creates a synthetic stack trace that doesn't represent where the format issue originated. While this is acceptable, you could wrap this check in a try-catch to maintain consistency with the outer error handling pattern.

Optional refactor:

 T? fromStringToObject<T>({
   required String content,
   required T Function(Map<String, dynamic>) fromJson,
   required void Function(Object e, StackTrace s) onInvalidContent,
 }) {
   try {
     final map = json.decode(content);
     if (map is! Map) {
-      onInvalidContent(FormatException('decoded json string wasn\'t a map'), StackTrace.current);
-      return null;
+      throw FormatException('decoded json string wasn\'t a map');
     }
     return fromJson(map as Map<String, dynamic>);
   } catch (e, s) {
     onInvalidContent(e, s);
     return null;
   }
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91a6a9f and a34aaf8.

⛔ Files ignored due to path filters (5)
  • generated/android/src/main/java/org/reclaimprotocol/inapp_sdk/Messages.kt is excluded by !**/generated/**
  • generated/ios/Sources/ReclaimInAppSdk/Messages.h is excluded by !**/generated/**
  • generated/ios/Sources/ReclaimInAppSdk/Messages.m is excluded by !**/generated/**
  • generated/ios/Sources/ReclaimInAppSdk/Messages.swift is excluded by !**/generated/**
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • CHANGELOG.md (1 hunks)
  • lib/reclaim_verifier_module.dart (2 hunks)
  • lib/src/pigeon/messages.pigeon.dart (2 hunks)
  • lib/utils/json.dart (1 hunks)
  • pigeon/schema.dart (1 hunks)
  • pubspec.yaml (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[grammar] ~3-~3: Ensure spelling is correct
Context: ...turning incorrect values for fields not overriden * Update default feature flag values * Add...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🔇 Additional comments (4)
pubspec.yaml (1)

19-19: LGTM!

The version bump to 0.27.0 aligns with the changelog and the new theme override functionality introduced in this PR.

lib/src/pigeon/messages.pigeon.dart (1)

482-512: LGTM!

The generated pigeon code correctly implements the theme field with proper encoding/decoding in _toList() and decode(). The constructor appropriately treats theme as an optional parameter.

lib/reclaim_verifier_module.dart (2)

17-17: LGTM!

The new JSON utility import supports the theme parsing functionality added below.


580-593: LGTM! Theme parsing implementation is sound.

The implementation correctly:

  • Conditionally parses theme only when appInfo.theme is non-null
  • Uses the fromStringToObject utility with proper error handling
  • Logs parse failures without crashing the flow
  • Returns null for the theme field when parsing fails or theme is absent

@@ -1,3 +1,10 @@
## 0.27.0

* Fix feature flag overrides causing interruption with some login pages and returning incorrect values for fields not overriden
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix spelling error.

"overriden" should be "overridden".

Apply this diff:

-* Fix feature flag overrides causing interruption with some login pages and returning incorrect values for fields not overriden
+* Fix feature flag overrides causing interruption with some login pages and returning incorrect values for fields not overridden
📝 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.

Suggested change
* Fix feature flag overrides causing interruption with some login pages and returning incorrect values for fields not overriden
* Fix feature flag overrides causing interruption with some login pages and returning incorrect values for fields not overridden
🧰 Tools
🪛 LanguageTool

[grammar] ~3-~3: Ensure spelling is correct
Context: ...turning incorrect values for fields not overriden * Update default feature flag values * Add...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents
In CHANGELOG.md around line 3, the word "overriden" is misspelled; replace it
with the correct spelling "overridden" so the line reads "* Fix feature flag
overrides causing interruption with some login pages and returning incorrect
values for fields not overridden".

Comment on lines +157 to 168
const ClientReclaimAppInfoOverride({
required this.appName,
required this.appImageUrl,
required this.isRecurring,
required this.theme,
});
final String appName;
final String appImageUrl;
// false
final bool isRecurring;
final String? theme;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix inconsistency: required parameter with nullable type.

The theme parameter is marked as required but has a nullable type String?. This creates confusion about whether the field must be provided. Either make it an optional parameter (remove required) or make the type non-nullable.

Apply this diff to make it optional:

 class ClientReclaimAppInfoOverride {
   const ClientReclaimAppInfoOverride({
     required this.appName,
     required this.appImageUrl,
     required this.isRecurring,
-    required this.theme,
+    this.theme,
   });
   final String appName;
   final String appImageUrl;
   // false
   final bool isRecurring;
   final String? theme;
 }
📝 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.

Suggested change
const ClientReclaimAppInfoOverride({
required this.appName,
required this.appImageUrl,
required this.isRecurring,
required this.theme,
});
final String appName;
final String appImageUrl;
// false
final bool isRecurring;
final String? theme;
}
const ClientReclaimAppInfoOverride({
required this.appName,
required this.appImageUrl,
required this.isRecurring,
this.theme,
});
final String appName;
final String appImageUrl;
// false
final bool isRecurring;
final String? theme;
}
🤖 Prompt for AI Agents
In pigeon/schema.dart around lines 157 to 168, the constructor declares theme as
required but the field is nullable (String?), causing inconsistency; change the
constructor to make theme an optional named parameter (remove the required
keyword for theme) so the nullable field can be omitted when constructing
ClientReclaimAppInfoOverride, leaving the field type as String?.

Copy link

@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: 8

🧹 Nitpick comments (2)
scripts/inapp/flutter.sh (1)

5-5: Make VERSION parsing more robust (and fix SC2155)
Current parsing will misbehave with extra spaces/comments and triggers Shellcheck SC2155.

-export VERSION=$(grep '^version:' pubspec.yaml | sed -e 's/version: //')
+VERSION="$(awk -F': *' '$1=="version"{print $2; exit}' pubspec.yaml)"
+export VERSION
scripts/inapp/capacitor.sh (1)

5-5: Make VERSION parsing more robust (and fix SC2155)

-export VERSION=$(grep '^version:' pubspec.yaml | sed -e 's/version: //')
+VERSION="$(awk -F': *' '$1=="version"{print $2; exit}' pubspec.yaml)"
+export VERSION
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a34aaf8 and 2d99436.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • scripts/build.sh (1 hunks)
  • scripts/inapp/capacitor.sh (1 hunks)
  • scripts/inapp/flutter.sh (1 hunks)
  • scripts/inapp/inapp.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/inapp/flutter.sh

[warning] 5-5: Declare and assign separately to avoid masking return values.

(SC2155)


[warning] 11-11: Declare and assign separately to avoid masking return values.

(SC2155)

scripts/inapp/capacitor.sh

[warning] 5-5: Declare and assign separately to avoid masking return values.

(SC2155)


[warning] 11-11: Declare and assign separately to avoid masking return values.

(SC2155)


flutter clean
flutter pub get
dart run rive_native:setup --verbose --clean --platform android,ios;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check pubspec.yaml for rive and rive_native dependencies
echo "=== Checking pubspec.yaml for rive and rive_native ==="
cat pubspec.yaml

Repository: reclaimprotocol/reclaim-inapp-add-to-app-module

Length of output: 4161


🏁 Script executed:

#!/bin/bash
# Check if rive is mentioned elsewhere in the codebase
echo "=== Searching for 'rive' in codebase ==="
rg -i "rive" --type yaml --type dart

# Check the build.sh script in full context
echo -e "\n=== Full content of scripts/build.sh ==="
cat -n scripts/build.sh

Repository: reclaimprotocol/reclaim-inapp-add-to-app-module

Length of output: 600


Remove dart run rive_native:setup command or add the rive_native dependency to pubspec.yaml.

The command on line 11 references rive_native, but this package is not declared in pubspec.yaml and has no references anywhere in the codebase. The build will fail at this line with a "package not found" error, preventing the Android and iOS builds from running. Either add rive_native as a dependency if it's required, or remove this line if it was added in error.

🤖 Prompt for AI Agents
In scripts/build.sh around line 11, the script runs "dart run rive_native:setup"
but the rive_native package is not declared in pubspec.yaml, causing a
package-not-found build failure; either remove this command from
scripts/build.sh if the package is not needed (delete or comment the line) or
add rive_native to pubspec.yaml (under dependencies or dev_dependencies as
appropriate), run flutter/dart pub get, and update any CI/build scripts to
ensure the dependency is available during the build.

Comment on lines +3 to +19
set -ex;

export VERSION=$(grep '^version:' pubspec.yaml | sed -e 's/version: //')

mkdir -p inapp-sdks;

cd inapp-sdks;

export WORK_DIR="$(pwd)";

export RN_CLONE_DIR="$WORK_DIR/build/capacitor-sdk";

if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
fi
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Don’t leak credentials: set -x + https://user:pass@... will print secrets in logs
Same issue as flutter.sh: with set -x (Line 3), the HTTPS clone (Line 18) will echo credentials into CI logs.

Apply the same mitigation as in scripts/inapp/flutter.sh: avoid URL-embedded creds, or at minimum disable -x when creds env vars are set and quote args.

🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 5-5: Declare and assign separately to avoid masking return values.

(SC2155)


[warning] 11-11: Declare and assign separately to avoid masking return values.

(SC2155)

🤖 Prompt for AI Agents
In scripts/inapp/capacitor.sh around lines 3 to 19, the script enables shell
tracing with set -x and then performs an HTTPS git clone using embedded
credentials which will print secrets to CI logs; to fix it, do not embed
credentials in the clone URL and avoid tracing while secrets are used — either
(a) wrap the HTTPS clone in a set +x ... set -x block so tracing is disabled for
that command and ensure the clone args are properly quoted, or (b) switch to a
credential-safe method (use SSH clone when PACKAGE_CLONE_USER is empty, or
configure git to use a credential helper or GIT_ASKPASS for username/password)
and remove URL-embedded user:pass; implement one of these approaches and ensure
all variables are quoted.

Comment on lines +11 to +22
export WORK_DIR="$(pwd)";

export RN_CLONE_DIR="$WORK_DIR/build/capacitor-sdk";

if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
fi

cd $RN_CLONE_DIR;

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Quote variables on cd/paths

-export WORK_DIR="$(pwd)";
+WORK_DIR="$(pwd)"
+export WORK_DIR
@@
-cd $RN_CLONE_DIR;
+cd "$RN_CLONE_DIR";
📝 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.

Suggested change
export WORK_DIR="$(pwd)";
export RN_CLONE_DIR="$WORK_DIR/build/capacitor-sdk";
if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
fi
cd $RN_CLONE_DIR;
WORK_DIR="$(pwd)"
export WORK_DIR
export RN_CLONE_DIR="$WORK_DIR/build/capacitor-sdk";
if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-capacitor-sdk.git $RN_CLONE_DIR;
fi
cd "$RN_CLONE_DIR";
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 11-11: Declare and assign separately to avoid masking return values.

(SC2155)

🤖 Prompt for AI Agents
In scripts/inapp/capacitor.sh around lines 11 to 22, variable expansions used as
path arguments are unquoted which can break on paths with spaces or special
chars; change RN_CLONE_DIR assignment to export
RN_CLONE_DIR="$WORK_DIR/build/capacitor-sdk" (already mostly correct but ensure
quotes), use the quoted variable when passing destination to git clone: git
clone <repo> "$RN_CLONE_DIR" (both branches), and quote the cd invocation: cd
"$RN_CLONE_DIR"; ensuring all path-like variables are wrapped in double quotes.

Comment on lines +23 to +38
DEFAULT_CHANGELOG="## $VERSION
* Updates inapp module dependency to $VERSION
"

EFFECTIVE_CHANGELOG="${NEXT_CHANGELOG:-$DEFAULT_CHANGELOG}"

# copy current changelog.md
cat CHANGELOG.md > temp;
# enter new changes
echo "$EFFECTIVE_CHANGELOG" > CHANGELOG.md
# append old changelog
cat temp >> CHANGELOG.md;
# remove copy
rm temp;

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use mktemp + trap; handle missing CHANGELOG.md

(Recommend copying the same mktemp/trap prepend logic proposed in scripts/inapp/flutter.sh.)

🤖 Prompt for AI Agents
In scripts/inapp/capacitor.sh around lines 23 to 38, the current temp-file
handling uses a hardcoded "temp" file and blindly reads CHANGELOG.md which can
clobber or fail; replace this with mktemp to create a secure temporary file, add
a trap to remove the temp file on EXIT (and on errors), and check for the
existence of CHANGELOG.md before reading it (create an empty file or skip append
if missing). Specifically: create a temp via mktemp, redirect current
CHANGELOG.md into that temp if it exists, write EFFECTIVE_CHANGELOG into
CHANGELOG.md, append the temp contents back, and ensure trap 'rm -f "$TMP"' is
registered immediately after TMP creation to always clean up.

Comment on lines +3 to +19
set -ex;

export VERSION=$(grep '^version:' pubspec.yaml | sed -e 's/version: //')

mkdir -p inapp-sdks;

cd inapp-sdks;

export WORK_DIR="$(pwd)";

export RN_CLONE_DIR="$WORK_DIR/build/flutter-sdk";

if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
fi
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Don’t leak credentials: set -x + https://user:pass@... will print secrets in logs
With set -x enabled (Line 3) the git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@... command (Line 18) will echo credentials to stdout in CI logs. Also, embedding credentials in URLs commonly ends up in shell history / process lists / logs.

Suggested hardening (disable -x when creds are in play + quote vars):

-set -ex;
+set -euo pipefail
+
+# Avoid leaking secrets in logs when using credentialed HTTPS clone.
+if [[ -n "${PACKAGE_CLONE_USER:-}" || -n "${PACKAGE_CLONE_PASSWD:-}" ]]; then
+  set -e
+else
+  set -ex
+fi
@@
-if [[ -z "$PACKAGE_CLONE_USER" ]]; then
-    git clone [email protected]:reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
+if [[ -z "${PACKAGE_CLONE_USER:-}" ]]; then
+    git clone [email protected]:reclaimprotocol/reclaim-inapp-flutter-sdk.git "$RN_CLONE_DIR";
 else
-    git clone https://$PACKAGE_CLONE_USER:[email protected]/reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
+    git clone "https://${PACKAGE_CLONE_USER}:${PACKAGE_CLONE_PASSWD}@github.com/reclaimprotocol/reclaim-inapp-flutter-sdk.git" "$RN_CLONE_DIR";
 fi

(Strongly preferred: switch to an auth mechanism that doesn’t put creds in the URL, e.g., gh auth git-credential or GIT_ASKPASS.)

🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 5-5: Declare and assign separately to avoid masking return values.

(SC2155)


[warning] 11-11: Declare and assign separately to avoid masking return values.

(SC2155)

🤖 Prompt for AI Agents
In scripts/inapp/flutter.sh around lines 3 to 19, the script enables shell
tracing with set -x and later embeds credentials in the clone URL which will
leak secrets; change the script to avoid printing secrets by removing or
temporarily disabling set -x around the credentialed git clone (e.g., turn off
xtrace before the clone and re-enable after), stop embedding credentials in the
URL by using a safer auth method (prefer SSH clone or use gh auth / git
credential helper / GIT_ASKPASS), and ensure any variables used in URLs or
commands are quoted to avoid word-splitting.

Comment on lines +11 to +22
export WORK_DIR="$(pwd)";

export RN_CLONE_DIR="$WORK_DIR/build/flutter-sdk";

if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
fi

cd $RN_CLONE_DIR;

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Quote variables on cd/paths to avoid breakage with spaces and globbing
Unquoted cd $RN_CLONE_DIR (Line 21) and earlier path usage can fail unexpectedly.

-export WORK_DIR="$(pwd)";
+WORK_DIR="$(pwd)"
+export WORK_DIR
@@
-export RN_CLONE_DIR="$WORK_DIR/build/flutter-sdk";
+RN_CLONE_DIR="$WORK_DIR/build/flutter-sdk"
+export RN_CLONE_DIR
@@
-cd $RN_CLONE_DIR;
+cd "$RN_CLONE_DIR";
📝 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.

Suggested change
export WORK_DIR="$(pwd)";
export RN_CLONE_DIR="$WORK_DIR/build/flutter-sdk";
if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
fi
cd $RN_CLONE_DIR;
WORK_DIR="$(pwd)"
export WORK_DIR
RN_CLONE_DIR="$WORK_DIR/build/flutter-sdk"
export RN_CLONE_DIR
if [[ -z "$PACKAGE_CLONE_USER" ]]; then
git clone [email protected]:reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
else
git clone https://$PACKAGE_CLONE_USER:$PACKAGE_CLONE_PASSWD@github.com/reclaimprotocol/reclaim-inapp-flutter-sdk.git $RN_CLONE_DIR;
fi
cd "$RN_CLONE_DIR";
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 11-11: Declare and assign separately to avoid masking return values.

(SC2155)

🤖 Prompt for AI Agents
In scripts/inapp/flutter.sh around lines 11 to 22, unquoted variable expansions
(e.g., git clone ... $RN_CLONE_DIR and cd $RN_CLONE_DIR) can break when paths
contain spaces or glob characters; update all path usages to use quoted
expansions (e.g., git clone ... "$RN_CLONE_DIR" and cd "$RN_CLONE_DIR") and
ensure any other occurrences of $WORK_DIR/$RN_CLONE_DIR are similarly quoted.

Comment on lines +23 to +38
DEFAULT_CHANGELOG="## $VERSION
* Updates inapp module dependency to $VERSION
"

EFFECTIVE_CHANGELOG="${NEXT_CHANGELOG:-$DEFAULT_CHANGELOG}"

# copy current changelog.md
cat CHANGELOG.md > temp;
# enter new changes
echo "$EFFECTIVE_CHANGELOG" > CHANGELOG.md
# append old changelog
cat temp >> CHANGELOG.md;
# remove copy
rm temp;

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Changelog prepend should use mktemp + trap + handle missing CHANGELOG.md
Current temp filename (Line 31) is non-unique and cat CHANGELOG.md will fail if the file doesn’t exist yet.

-EFFECTIVE_CHANGELOG="${NEXT_CHANGELOG:-$DEFAULT_CHANGELOG}"
+EFFECTIVE_CHANGELOG="${NEXT_CHANGELOG:-$DEFAULT_CHANGELOG}"
@@
-# copy current changelog.md
-cat CHANGELOG.md > temp;
-# enter new changes
-echo "$EFFECTIVE_CHANGELOG" > CHANGELOG.md
-# append old changelog
-cat temp >> CHANGELOG.md;
-# remove copy
-rm temp;
+tmp="$(mktemp)"
+trap 'rm -f "$tmp"' EXIT
+
+if [[ -f CHANGELOG.md ]]; then
+  cat CHANGELOG.md > "$tmp"
+else
+  : > "$tmp"
+fi
+
+printf '%s\n' "$EFFECTIVE_CHANGELOG" > CHANGELOG.md
+cat "$tmp" >> CHANGELOG.md
🤖 Prompt for AI Agents
In scripts/inapp/flutter.sh around lines 23 to 38, the current use of a fixed
"temp" filename is unsafe and will fail if CHANGELOG.md doesn't exist; replace
that logic by creating a unique temporary file with tmp=$(mktemp), register a
trap to remove it on EXIT (trap 'rm -f "$tmp"' EXIT), ensure CHANGELOG.md exists
before reading (e.g. if [ ! -f CHANGELOG.md ]; then : > CHANGELOG.md; fi), then
copy the current changelog into the temp with cat CHANGELOG.md > "$tmp", write
the effective changelog to CHANGELOG.md, append the temp back into CHANGELOG.md,
and rely on the trap to clean up the temp file.

Comment on lines 23 to +28
./scripts/inapp/reactnative.sh;

# publish on npm
./scripts/inapp/capacitor.sh;

./scripts/inapp/flutter.sh;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

New publish steps inherit set -x: ensure capacitor/flutter scripts won’t leak secrets and are executable
Since this orchestrator runs with set -ex (Line 3), make sure the newly added capacitor.sh / flutter.sh don’t echo credentials (they currently do) and that CI marks them executable (otherwise this will fail at runtime).

🤖 Prompt for AI Agents
In scripts/inapp/inapp.sh around lines 23 to 28, the orchestrator runs with set
-ex so calling ./scripts/inapp/capacitor.sh and ./scripts/inapp/flutter.sh will
inherit -x and leak secrets and may fail if not executable; update those scripts
to avoid echoing secrets by surrounding secret operations with set +x / set -x
(or export a flag to disable xtrace) and remove any commands that print
credentials, and ensure both capacitor.sh and flutter.sh are executable (add
chmod +x in CI or git mode change) so the orchestrator can run them without
leaking sensitive data or runtime permission errors.

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