Skip to content

BOM_1.5.1#236

Merged
jakubuid merged 7 commits intomasterfrom
develop
Nov 19, 2025
Merged

BOM_1.5.1#236
jakubuid merged 7 commits intomasterfrom
develop

Conversation

@jakubuid
Copy link
Collaborator

No description provided.

@claude
Copy link

claude bot commented Nov 19, 2025

Claude finished @jakubuid's task —— View job


PR Review Complete

  • Read and analyze changed files
  • Check for code quality and security issues
  • Scan for external domain URLs
  • Post review findings

Issues Found

Issue 1: Critical dependency configuration conflict in core module

buildSrc/src/main/kotlin/Versions.kt:8-16, core/android/build.gradle.kts:101-109

Changed yttrium dependency from compileOnly to implementation in core/android/build.gradle.kts:101. This creates transitive dependency exposure for all modules consuming android-core. If yttrium was intentionally scoped as compileOnly before, this change makes it part of the published artifact's runtime dependencies.

Recommendation: Verify if yttrium should be exposed transitively to consumers of android-core, or if it should remain as compileOnly with consumers explicitly declaring it when needed. If transitive exposure is intended, ensure release notes document this API surface change.


Issue 2: Inconsistent JNI library conflict resolution across sample apps

sample/dapp/build.gradle.kts:70-72, sample/modal/build.gradle.kts:70-72, sample/wallet/build.gradle.kts:76-81

Added pickFirsts for libuniffi_yttrium.so to all sample apps, but only sample/wallet includes libuniffi_yttrium_utils.so (lines 79-81). This suggests:

  • Either dapp/modal apps also need yttrium_utils natives (missing)
  • Or wallet's exclusion of yttrium in dependencies (lines 168-176) isn't working correctly

Recommendation: Audit which native libraries each app actually needs. If dapp/modal don't use yttrium_utils features, document why. If they do, add the pickFirsts. The asymmetry suggests incomplete dependency configuration.


Issue 3: Version resolution override applied globally without scope validation

sample/wallet/build.gradle.kts:90-96

Added configurations.all { resolutionStrategy.eachDependency { ... } } that force-overrides ALL yttrium dependency references to kotlin-utils-0.9.113 across ALL configurations (test, androidTest, implementation, etc.). This:

  • Overrides even transitive yttrium dependencies from other modules
  • May cause version mismatches if core-android expects 0.9.91 but gets 0.9.113
  • Bypasses Gradle's normal conflict resolution

Recommendation: Scope this to only the configurations that need kotlin-utils branch (e.g., configurations.matching { it.name.startsWith("implementation") }), or better yet, declare the specific version in dependencies and let Gradle resolve conflicts naturally. Document why this override is necessary.


Issue 4: Duplicate conditional checks in Signer.kt

sample/wallet/src/main/kotlin/com/reown/sample/wallet/domain/signer/Signer.kt:162-163, 206-210

Lines 162-163 and 206-210 contain identical fallback conditions for Eth chain. Lines 165-166 and 209-210 duplicate Cosmos chain fallbacks. This is dead code - the second instances at 206-210 are unreachable since identical conditions already matched earlier.

Recommendation: Remove duplicate conditions at lines 206-210. Suggests incomplete refactoring or merge conflict resolution.


Issue 5: AndroidX Navigation major version bump without migration validation

gradle/libs.versions.toml:53

Bumped androidxNavigation from 2.8.0 → 2.9.6. Navigation 2.9.x introduced type-safe navigation breaking changes. Without corresponding code changes visible in this PR, sample apps using navigation may have:

  • Deprecated API usage
  • Runtime crashes if type-safe APIs are expected
  • Serialization issues with arguments

Recommendation: Verify all navigation code in sample apps is compatible with 2.9.x APIs. Check for deprecation warnings. Consider testing navigation flows thoroughly, especially argument passing and deep links.


🔒 External Domain URL Detected (Non-blocking)

URL: https://appkit-lab.reown.com
Files: sample/dapp/build.gradle.kts:33,38,44, sample/modal/build.gradle.kts:32,37,43, sample/wallet/build.gradle.kts:35,40,46

Found multiple references to appkit-lab.reown.com. Verified this is an approved reown.com subdomain per custom instructions.


@sonarqubecloud
Copy link

@jakubuid jakubuid merged commit 3b1c0ab into master Nov 19, 2025
21 of 22 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 19, 2025
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.

1 participant