Skip to content

ci: check the supported-minimum Expo build and fix docs - #91

Merged
tolgahan-arikan merged 11 commits into
masterfrom
docs-consumer-fixes
Oct 2, 2026
Merged

tolgahan-arikan merged 11 commits into
masterfrom
docs-consumer-fixes

Conversation

@tolgahan-arikan

@tolgahan-arikan tolgahan-arikan commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Consumer and contributor documentation fixes, plus a supported-minimum Expo fixture in the Expo compatibility check. A Kotlin SDK change that breaks the React Native SDK's minimum (Expo SDK 56 / React Native 0.85) now fails here, before a release. No source, build, or version changes to the SDK itself.

Changes

Expo compatibility: current and supported minimum

  • New fixture compatibility-tests/expo-min/.
  • tools/check-expo-compatibility.sh
    • Takes current or minimum to build one fixture, or builds both when run without an argument.
    • The freshness check (locked Expo must equal npm latest) applies only to the current fixture. The minimum is never freshness-checked.
    • The minimum prebuild pins expo-template-bare-minimum@56.0.37. Every expo 56.x from 56.0.15 on ships the SDK 57 native template, so a plain expo prebuild would silently generate an Expo 57 project.
  • Config plugin moved to compatibility-tests/shared/with-local-kotlin-sdk.js and used by both fixtures. It loads expo/config-plugins from the fixture being prebuilt.
  • CI (android-ci.yml)
    • The Expo check moves out of build into an expo-compat matrix job, current and minimum, that runs in parallel with build on standard runners.
    • The fixture builds no longer pass --refresh-dependencies, which came from fix(build): restore stable Expo compatibility #87. Gradle reads mavenLocal() directly and never caches it, so the freshly published SDK is used without it. This machine's Gradle cache holds only the Maven Central releases of the SDK, never the -expo-compat builds. The flag only forced every remote dependency to be re-checked.
    • Before this change, building both fixtures one after the other took the Expo step from about 6 to 10 minutes.
    • The workflow now limits GITHUB_TOKEN to contents: read. Only the build job also gets checks: write, which mikepenz/action-junit-report needs to publish the unit test report.
  • Claude PR review (.github/workflows/claude-review.yml): both the automatic review and @claude replies now use Claude Sonnet 5.5 (claude-sonnet-5-5) instead of Opus 4.8.
  • Dependabot: the minimum fixture gets patch updates only. Major and minor updates for expo, react, and react-native are ignored, and patches are grouped.

Docs

  • README
    • The Sign Typed Data example was missing import kotlinx.serialization.json.put. It fails to compile without it (Argument type mismatch: String vs JsonElement).
    • Documents the android.permission.INTERNET requirement, since the SDK's manifest declares no permissions.
    • Adds a "Packages" map showing which package each public type lives in.
    • Mentions the trails-actions module and CI's ./gradlew --build-cache verify plus tools/check-expo-compatibility.sh.
  • CONTRIBUTING, TESTING, PR template: the "CI-equivalent" command now matches android-ci.yml, and passing current or minimum builds a single Expo fixture locally. Also covers the Node 24 prerequisite, the generateApiDocs/checkApiDocs/public API baseline workflow, and what the Expo compatibility check covers.
  • AGENTS.md: CI description, repository layout, and development commands updated to match the build files and workflows. New Maintenance Matrix with two rows: raising the React Native SDK's minimum updates compatibility-tests/expo-min, and Kotlin toolchain or artifact metadata changes must pass both Expo fixtures.

Testing

  • ./gradlew ktlintCheck passes (run by verify)
  • ./gradlew :oms-wallet-kotlin-sdk:testDebugUnitTest passes (run by verify)
  • ./gradlew --build-cache verify passes (BUILD SUCCESSFUL in 1m 39s)
  • Instrumented tests: not applicable

Also:

  • tools/check-expo-compatibility.sh passes locally in each mode, without --refresh-dependencies and with a warm cache.
    • minimum: 98s, with the fixture build passing in 91s.
    • current: 91s, with the fixture build passing in 89s.
    • With --refresh-dependencies, the fixture builds took 136s and 142s.
  • The minimum build resolves com.facebook.react:react-android -> 0.85.3 and the local io.github.0xsequence:oms-wallet-kotlin-sdk:0.3.1-expo-compat from Maven Local.
  • dependabot.yml and android-ci.yml validate against their JSON schemas.
  • The corrected README Sign Typed Data snippet compiles against the published 0.3.1.

Related

Part of one documentation pass across the OMS Wallet SDKs and docs site:

Add missing put import to the typed-data snippet, INTERNET permission note, package map, trails-actions and CI check notes, a 0.3.1 migration entry, and correct CI, API-docs, and layout descriptions for contributors.
Move with-local-kotlin-sdk.js to compatibility-tests/shared/ and resolve
expo/config-plugins from the fixture being prebuilt, so each fixture uses
its own Expo version.
Mirror the React Native SDK's supported minimum (expo 56.0.23,
react-native 0.85.3, react 19.2.3) so Kotlin SDK changes that break it
fail here. The check publishes to Maven Local once, then builds the
current-stable fixture (still freshness-checked) and the minimum fixture
(never freshness-checked), prebuilding the latter with the pinned SDK 56
template expo-template-bare-minimum@56.0.37 because expo 56.0.15+ bundles
the SDK 57 template. CI caches npm for both lockfiles.
@tolgahan-arikan tolgahan-arikan changed the title docs: fix consumer and contributor docs docs, ci: fix docs and check the supported-minimum Expo build Oct 1, 2026
@tolgahan-arikan tolgahan-arikan changed the title docs, ci: fix docs and check the supported-minimum Expo build ci: check the supported-minimum Expo build and fix docs Oct 1, 2026
Comment thread .github/workflows/android-ci.yml Fixed
@tolgahan-arikan
tolgahan-arikan marked this pull request as ready for review October 1, 2026 12:32
@tolgahan-arikan
tolgahan-arikan requested a review from a team October 1, 2026 12:32
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-01T12:35:15.569334Z 7823efe Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Claude finished @tolgahan-arikan's task in 2m 13s —— View job


PR Review: CI supported-minimum Expo build + docs fixes

  • Reviewed Expo compatibility script & CI changes
  • Reviewed docs (README, AGENTS, CONTRIBUTING, TESTING)
  • Reviewed config plugin, fixtures, dependabot
  • Verified README Packages map against source

Verdict: Looks good. This is a docs + CI + Expo-fixture change with no SDK source, build, or version edits — nothing touches wallet, signing, keystore, or session code, so there's no security-sensitive surface and no public API change. I verified the claims I could check locally.

What I verified ✅

  • README Sign Typed Data import fix is correct. The snippet calls put("name", "name") with String values, which needs the put(key, String) overload from kotlinx.serialization.json.put. Without the added import it wouldn't resolve.
  • README "Packages" map is accurate. Spot-checked declaring packages against source:
    • IndexerClient → …omswallet.indexer ✓
    • parseUnits/formatUnits → …omswallet.utils (OMSWalletUnits.kt) ✓
    • WalletImportPrivateKey, WalletKeyOrigin, SmartSessionGrant, AccessGrant → …omswallet.models ✓
    • CompleteAuthResult, PendingWalletSelection, OidcRedirectAuthResult → …omswallet.wallet ✓
  • Config plugin sharing is sound. Moving to compatibility-tests/shared/with-local-kotlin-sdk.js and resolving expo/config-plugins via require.resolve(..., { paths: [projectRoot] }) is exactly right — each fixture loads its own Expo version's plugin API, which is what makes a single shared plugin work across SDK 56 and current-stable. Both app.json files reference ../shared/with-local-kotlin-sdk consistently, and the old per-fixture copy was removed.
  • Script argument handling (current|minimum|all) and the freshness gate correctly apply the npm latest check only to the current fixture. The --template expo-template-bare-minimum@56.0.37 pin for the minimum matches the documented SDK-57-template-bundling issue.
  • Dependabot expo-min config (ignore major/minor, group patches) matches the stated "patch-only" intent.

Nits (non-blocking)

  1. android-ci.yml — no permissions block (the CodeQL comment on line 72). The new expo-compat job — and the existing build job — don't restrict GITHUB_TOKEN. Adding a top-level permissions: { contents: read } is good hardening. It's pre-existing on build too, so not introduced by this PR, and I can't edit workflow files from here anyway — just flagging for a maintainer. Fix this →
  2. Local "Full CI check" docs run both fixtures. README/CONTRIBUTING/TESTING tell contributors to run bare tools/check-expo-compatibility.sh, which builds both fixtures (~10 min, Node 24, network, downloads the SDK 56 template). Prerequisites are documented, but it might help to mention that current / minimum can be passed to build just one locally.
  3. expo-min/README.md link to the RN SDK's expo-min fixture only resolves once oms-wallet-react-native-sdk#123 merges — already called out in the PR body. Worth a follow-up to confirm it doesn't 404 after merge.

No test-coverage concerns: there's no behavior change to cover, and the fixture is the test. ktlint/KDoc are N/A — no Kotlin changed.

The one thing I can't verify locally is that the two expo-compat matrix jobs go green (network/Node 24/emulator-free assemble). Worth a glance at the CI run before merge.
· docs-consumer-fixes

@tolgahan-arikan
tolgahan-arikan merged commit d0ee311 into master Oct 2, 2026
8 checks passed
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.

3 participants