test(expo): Restore green expo native Maestro flows and enforce them - #9303
Conversation
🦋 Changeset detectedLatest commit: f5c0124 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change gates Android auth and profile views on Clerk initialization and shows progress indicators during loading. It adds runtime-controlled Expo and Clerk Android debug logging. Native configuration failures now emit release warnings and development error details. The Expo template adds conditional OkHttp alignment rules. Maestro execution supports CLI and runner engines. Integration flows now cover platform-specific navigation and input handling. Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoDebug.kt (1)
9-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating
debugLoghere instead of duplicating it per module.
debugLog(tag, message)is defined identically inClerkExpoModule.kt,ClerkAuthViewModule.kt, andClerkUserProfileViewModule.kt, each guarded byclerkExpoDebugEnabled(). Move a singleinternal fun debugLog(tag: String, message: String)into this file and have the three modules call the shared version. This keeps the debug-logging policy defined in one place and avoids future drift if the guard condition changes again.♻️ Proposed consolidation
internal fun clerkExpoDebugEnabled(): Boolean = BuildConfig.DEBUG || Log.isLoggable(CLERK_EXPO_DEBUG_TAG, Log.DEBUG) + +internal fun debugLog(tag: String, message: String) { + if (clerkExpoDebugEnabled()) { + Log.d(tag, message) + } +}Then remove the private
debugLogcopies from the three modules.🤖 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 `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoDebug.kt` around lines 9 - 10, Move the shared debugLog(tag: String, message: String) implementation into ClerkExpoDebug.kt, retaining the clerkExpoDebugEnabled() guard and existing logging behavior. Update ClerkExpoModule, ClerkAuthViewModule, and ClerkUserProfileViewModule to use this internal helper, then remove their duplicated private debugLog definitions.
🤖 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.
Nitpick comments:
In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoDebug.kt`:
- Around line 9-10: Move the shared debugLog(tag: String, message: String)
implementation into ClerkExpoDebug.kt, retaining the clerkExpoDebugEnabled()
guard and existing logging behavior. Update ClerkExpoModule,
ClerkAuthViewModule, and ClerkUserProfileViewModule to use this internal helper,
then remove their duplicated private debugLog definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a7ea04b-4dd7-40f1-b2c9-fe580acb5f07
📒 Files selected for processing (7)
.changeset/expo-native-debug-logging.md.github/workflows/expo-native-build.ymlpackages/expo/android/src/main/java/expo/modules/clerk/ClerkAuthViewModule.ktpackages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoDebug.ktpackages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.ktpackages/expo/android/src/main/java/expo/modules/clerk/ClerkUserProfileViewModule.ktpackages/expo/src/provider/nativeClientSync.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
… embedded flow per platform
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@integration/templates/expo-native/plugins/withClerkOkHttpAlignment.js`:
- Around line 19-23: Update the injection guard in withClerkOkHttpAlignment to
verify that all three required OkHttp coordinates are already present before
skipping GRADLE_BLOCK, or use a unique marker for the complete injected block.
Ensure configurations containing only com.squareup.okhttp3:okhttp:5.4.0 still
receive the logging-interceptor and okhttp-urlconnection rules.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5603267a-a07d-42a2-8355-c906d46e6f66
📒 Files selected for processing (3)
integration/templates/expo-native/app.jsonintegration/templates/expo-native/plugins/withClerkOkHttpAlignment.jsintegration/tests/expo-native/flows/embedded-profile-host-back.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
| module.exports = function withClerkOkHttpAlignment(config) { | ||
| return withAppBuildGradle(config, cfg => { | ||
| if (!cfg.modResults.contents.includes('com.squareup.okhttp3:okhttp:5.4.0')) { | ||
| cfg.modResults.contents += GRADLE_BLOCK; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect all existing OkHttp declarations and force rules in the Expo fixture.
rg -n -C 3 \
'com\.squareup\.okhttp3:(okhttp|logging-interceptor|okhttp-urlconnection)|resolutionStrategy|force' \
integration/templates/expo-nativeRepository: clerk/javascript
Length of output: 1983
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- plugin and related tests ---'
sed -n '1,180p' integration/templates/expo-native/plugins/withClerkOkHttpAlignment.js
rg -n -C 4 \
'withClerkOkHttpAlignment|GRADLE_BLOCK|logging-interceptor|okhttp-urlconnection|resolutionStrategy|okhttp:5\.4\.0' \
integration packages .github 2>/dev/null | head -n 300 || true
printf '%s\n' '--- Expo template Gradle files ---'
find integration/templates/expo-native -type f \( -name '*.gradle' -o -name '*.gradle.kts' -o -name 'app.json' -o -name 'app.config.*' \) -printRepository: clerk/javascript
Length of output: 6057
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("integration/templates/expo-native/plugins/withClerkOkHttpAlignment.js")
text = path.read_text()
marker = "com.squareup.okhttp3:okhttp:5.4.0"
block = [
"com.squareup.okhttp3:okhttp:5.4.0",
"com.squareup.okhttp3:logging-interceptor:5.4.0",
"com.squareup.okhttp3:okhttp-urlconnection:5.0.0-alpha.16",
]
def inject(contents):
return contents + ("""\\nconfigurations.all {
resolutionStrategy {
force 'com.squareup.okhttp3:okhttp:5.4.0'
force 'com.squareup.okhttp3:logging-interceptor:5.4.0'
force 'com.squareup.okhttp3:okhttp-urlconnection:5.0.0-alpha.16'
}
}
""" if marker not in contents else "")
cases = {
"empty": "",
"complete_block": "\\n".join(f"force '{x}'" for x in block),
"core_only": "dependencies { implementation 'com.squareup.okhttp3:okhttp:5.4.0' }",
"core_and_unrelated": "\\n".join([
"implementation 'com.squareup.okhttp3:okhttp:5.4.0'",
"force 'com.squareup.okhttp3:other:1.0.0'",
]),
}
for name, contents in cases.items():
output = inject(contents)
missing = [x for x in block if x not in output]
print(f"{name}: injected={output != contents}, missing_required_coordinates={missing}")
PYRepository: clerk/javascript
Length of output: 615
Check all required OkHttp coordinates before skipping injection. If another plugin adds only com.squareup.okhttp3:okhttp:5.4.0, this condition skips the logging-interceptor and okhttp-urlconnection rules. Use a unique block marker or verify all three coordinates before skipping injection.
🤖 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 `@integration/templates/expo-native/plugins/withClerkOkHttpAlignment.js` around
lines 19 - 23, Update the injection guard in withClerkOkHttpAlignment to verify
that all three required OkHttp coordinates are already present before skipping
GRADLE_BLOCK, or use a unique marker for the complete injected block. Ensure
configurations containing only com.squareup.okhttp3:okhttp:5.4.0 still receive
the logging-interceptor and okhttp-urlconnection rules.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
integration/tests/expo-native/run-flows.sh (1)
18-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject unsupported
MAESTRO_ENGINEvalues.Values such as
CLIor a typo selectmaestro-runner, while Android requiresmaestro. Validatecli|runneronce, fail for other values, and reuse the validated value.🤖 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 `@integration/tests/expo-native/run-flows.sh` around lines 18 - 61, Validate MAESTRO_ENGINE once near the existing engine-selection logic, accepting only cli or runner and exiting with an error for any other value, including case variants such as CLI. Store the validated/defaulted value and reuse it in both the dependency check and run_flow selection instead of repeatedly reading MAESTRO_ENGINE.
🤖 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 `@integration/tests/expo-native/flows/subflows/sign-in-email-password.yaml`:
- Around line 32-55: Escape CLERK_TEST_EMAIL when assigning it to the Maestro
selector expressions used by notVisible and extendedWaitUntil, so regex
metacharacters in the email are matched literally. Keep the original
CLERK_TEST_EMAIL value unchanged for inputText.
---
Outside diff comments:
In `@integration/tests/expo-native/run-flows.sh`:
- Around line 18-61: Validate MAESTRO_ENGINE once near the existing
engine-selection logic, accepting only cli or runner and exiting with an error
for any other value, including case variants such as CLI. Store the
validated/defaulted value and reuse it in both the dependency check and run_flow
selection instead of repeatedly reading MAESTRO_ENGINE.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 503c4483-55ed-4608-9f5d-de597e82f457
📒 Files selected for processing (3)
.github/workflows/expo-native-build.ymlintegration/tests/expo-native/flows/subflows/sign-in-email-password.yamlintegration/tests/expo-native/run-flows.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
| - runFlow: | ||
| when: | ||
| visible: 'Select [Aa]ll' | ||
| notVisible: ${CLERK_TEST_EMAIL} | ||
| commands: | ||
| - tapOn: | ||
| text: 'Select [Aa]ll' | ||
| - eraseText: 100 | ||
| - inputText: ${CLERK_TEST_EMAIL} | ||
| - runFlow: | ||
| when: | ||
| platform: iOS | ||
| commands: | ||
| - longPressOn: | ||
| text: 'Enter your email( or username)?' | ||
| - runFlow: | ||
| when: | ||
| visible: 'Select All' | ||
| commands: | ||
| - tapOn: | ||
| text: 'Select All' | ||
| - eraseText: 100 | ||
| - inputText: ${CLERK_TEST_EMAIL} | ||
| # Wait for the typed value to land before tapping: the tap can race the | ||
| # recomposition that enables the button, and this also catches a mangled | ||
| # identifier inside the retry instead of at the next screen's timeout. | ||
| - extendedWaitUntil: | ||
| visible: ${CLERK_TEST_EMAIL} | ||
| timeout: 5000 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target workflow ---'
cat -n integration/tests/expo-native/flows/subflows/sign-in-email-password.yaml | sed -n '1,110p'
printf '%s\n' '--- email generation and related selectors ---'
rg -n -C 3 'CLERK_TEST_EMAIL|sign-in-email-password|Select All|extendedWaitUntil' integration/tests/expo-nativeRepository: clerk/javascript
Length of output: 29379
🌐 Web query:
Maestro selectors text regular expressions exact text plus sign escape
💡 Result:
In the Maestro automation framework, all text and id selectors are treated as regular expressions by default [1][2]. Because the plus sign (+) is a reserved control character in regular expressions—signifying "one or more" of the preceding element—it must be escaped when you intend to match it as a literal character [1][2]. To match a literal plus sign, you should escape it using a backslash (\) within your regex string [3][4]. For example, to match the literal text "+33", use: - tapOn: "\+33.*" [3] When working with these selectors, keep the following in mind: 1. Full String Matching: Because Maestro treats these selectors as regex, the pattern must match the entire text of the element [2]. To perform a partial match, ensure you include wildcards like .* at the beginning and end of your string as needed (e.g., .*\\+33.*) [2]. 2. YAML and Escape Sequences: If you encounter issues with YAML parsing while using backslashes, you may need to use double backslashes (\\) in your string or use block scalars (e.g., >-) to handle special characters more reliably [3][4]. 3. Alternative: If a specific symbol or character sequence is causing consistent issues with regex matching, a common workaround is to use the wildcard .* to bypass the problematic character, provided the surrounding text is unique enough to identify the element [4].
Citations:
- 1: https://docs.maestro.dev/reference/selectors/core-selectors
- 2: https://github.com/mobile-dev-inc/maestro-docs/blob/main/api-reference/selectors.md
- 3: [Maestro version, 1.31.0] tapOn with regular expression that starts with '+' mobile-dev-inc/Maestro#1479
- 4: Cannot find testID with some characters mobile-dev-inc/Maestro#1211
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- all CLERK_TEST_EMAIL references ---'
rg -n -C 2 'CLERK_TEST_EMAIL|clerk_test' . --glob '!node_modules' --glob '!dist' --glob '!build'
printf '%s\n' '--- Maestro configuration and environment handling ---'
rg -n -C 3 'maestro|--env|test email|TEST_EMAIL' integration/tests/expo-native .github package.json pnpm-workspace.yaml 2>/dev/null || trueRepository: clerk/javascript
Length of output: 38062
🏁 Script executed (no clone):
#!/bin/bash
set -e
python3 - <<'PY'
import re
for email in ("user+clerk_test@example.com", "user+clerk_test@sub.example.com"):
pattern = re.compile(email)
print(email, "fullmatch=", bool(pattern.fullmatch(email)), "search=", bool(pattern.search(email)))
PY
Length of output: 247
Escape CLERK_TEST_EMAIL before using it in Maestro selectors.
The +clerk_test suffix and other regex metacharacters in the email make the raw selector fail. Use an escaped selector value for notVisible and extendedWaitUntil, while retaining CLERK_TEST_EMAIL for inputText.
🤖 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 `@integration/tests/expo-native/flows/subflows/sign-in-email-password.yaml`
around lines 32 - 55, Escape CLERK_TEST_EMAIL when assigning it to the Maestro
selector expressions used by notVisible and extendedWaitUntil, so regex
metacharacters in the email are matched literally. Keep the original
CLERK_TEST_EMAIL value unchanged for inputText.
wobsoriano
changed the title
There was a problem hiding this comment.
clerk-android is built against OkHttp 5, while React Native pins apps to OkHttp 4. That mismatch causes the native instance config fetch to fail at runtime, so sign-in never loads.
This CI fixture forces OkHttp 5, along with the last okhttp-urlconnection alpha that still ships JavaNetCookieJar, to match clerk-android. This is a known ecosystem issue. Expo hit the same problem (expo/expo#44848), and the same workaround is documented on StackOverflow: https://stackoverflow.com/questions/72885577/how-to-implement-okhttp-5-0-0-in-react-native-module
This only affects the CI fixture. Consumer apps are unchanged.
wobsoriano
changed the title
2 participants
Description
Android Maestro e2e had failed on every run since the switch to maestro-runner (#9264) (maestro open source fork). This restores the suite to green on both platforms and removes the burn-in
continue-on-error, so e2e failures now fail the job.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change