From b70bbf366e61d442808c31ce1ddf943f83691df3 Mon Sep 17 00:00:00 2001 From: Mohamed Ibrahim Date: Wed, 1 Jul 2026 16:37:17 +0100 Subject: [PATCH 1/2] fix(android): isolate logger locale so it can't corrupt the host app Opening the Sharingan logger from a host that uses per-app locales (e.g. AppCompat set to Arabic/RTL) rendered the logger in an RTL frame and, worse, permanently corrupted the host: after closing the logger the host was stuck in English text with RTL layout (issue #38). Root cause: SharinganActivity is a plain ComponentActivity that neither pins its own locale nor guards the process-global one. On API 33+ the framework applied the host's per-app locale (ar -> RTL) to the logger, and the logger's config handling flipped the process-global Locale/LocaleList default to English, which leaked back to the host. The logger is a locale-neutral surface: always English + LTR, and it must never read or mutate the host's locale. Two-layer fix: - Activity: attachBaseContext pins an en/LTR Configuration via the non-mutating createConfigurationContext (never resources.updateConfiguration). - Compose: the stateless SharinganScreenContent forces LocalLayoutDirection = Ltr around its whole body, so the frame is LTR on every platform (including iOS) and in Studio previews. The global-locale guard was needed: an instrumented test proved that createConfigurationContext(en) resets LocaleList.getDefault() to English. SharinganActivity now snapshots the host's LocaleList in attachBaseContext and restores it (right after, and again in onDestroy), leaving the host's locale untouched. Verified red->green on an API 34 emulator with a new instrumented test that pins the process to Arabic, opens/closes the logger, and asserts the logger is LTR while open and every process-global JVM locale default is still Arabic after. The test is @SdkSuppress(minSdkVersion = 33) since it drives the API 33+ LocaleManager. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sharingan/sample/LoggerLocaleLeakTest.kt | 107 ++++++++++++++++++ .../kotlin/dev/sharingan/SharinganActivity.kt | 31 +++++ .../dev/sharingan/ui/SharinganScreen.kt | 62 +++++----- 3 files changed, 173 insertions(+), 27 deletions(-) create mode 100644 sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt diff --git a/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt b/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt new file mode 100644 index 0000000..22cf322 --- /dev/null +++ b/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt @@ -0,0 +1,107 @@ +package dev.sharingan.sample + +import android.app.LocaleManager +import android.content.Context +import android.os.LocaleList +import android.view.View +import androidx.test.core.app.ActivityScenario +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SdkSuppress +import androidx.test.platform.app.InstrumentationRegistry +import dev.sharingan.SharinganActivity +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.util.Locale + +/** + * Regression for issue #38: opening the Sharingan logger from a host app that + * uses per-app locales (Arabic/RTL) must not corrupt the host. The logger is a + * locale-neutral surface — English + LTR while open — and must leave every + * process-global locale knob exactly as the host set it. + * + * ``` + * ./gradlew :sample:composeApp:connectedDebugAndroidTest + * ``` + * + * We stand in for the AppCompat host by driving the framework per-app locale + * (`LocaleManager`, what `AppCompatDelegate.setApplicationLocales` calls under + * the hood on API 33+) directly in the process, since a test-APK host Activity + * can't be launched into the app-under-test's process and the leak we assert is + * process-global anyway. Requires API 33+ (the emulators CI runs on). + */ +@RunWith(AndroidJUnit4::class) +@SdkSuppress(minSdkVersion = 33) // LocaleManager is API 33+; skip (don't crash) below. +class LoggerLocaleLeakTest { + + private val instrumentation = InstrumentationRegistry.getInstrumentation() + private val context: Context get() = instrumentation.targetContext + private val localeManager get() = context.getSystemService(LocaleManager::class.java) + + private fun setAppLocale(tags: String) = instrumentation.runOnMainSync { + localeManager.applicationLocales = LocaleList.forLanguageTags(tags) + } + + @Before + fun pinHostToArabic() { + setAppLocale("ar") + // LocaleManager applies asynchronously; wait until the host is Arabic. + val deadline = System.currentTimeMillis() + 5_000 + while (localeManager.applicationLocales.toLanguageTags() != "ar" && + System.currentTimeMillis() < deadline + ) { + Thread.sleep(50) + } + assertEquals( + "precondition: host must be pinned to Arabic", + "ar", + localeManager.applicationLocales.toLanguageTags(), + ) + // The above only confirms system-server state. The process-local JVM + // default is what SharinganActivity snapshots/restores, and it updates + // separately — wait for it too, so we don't snapshot a stale value. + val jvmDeadline = System.currentTimeMillis() + 2_000 + while (LocaleList.getDefault()[0].language != "ar" && + System.currentTimeMillis() < jvmDeadline + ) { + Thread.sleep(50) + } + assertEquals( + "precondition: process default locale must be Arabic before launch", + "ar", + LocaleList.getDefault()[0].language, + ) + } + + @After + fun clearAppLocale() = setAppLocale("") + + @Test + fun `Given_Arabic_host_When_logger_opens_and_closes_Then_stays_LTR_and_host_locale_untouched`() { + // Open the logger, assert it renders LTR, then close it. + ActivityScenario.launch(SharinganActivity::class.java).use { logger -> + logger.onActivity { activity -> + assertEquals( + "logger frame must be LTR while open", + View.LAYOUT_DIRECTION_LTR, + activity.resources.configuration.layoutDirection, + ) + } + } + + // Sanity: system-server per-app locale is untouched. This lives in + // system_server, not our process, so the process-local fix can't corrupt + // it — it passes regardless of the bug and just guards the test setup. + assertEquals( + "system per-app locale changed unexpectedly (test setup sanity)", + "ar", + localeManager.applicationLocales.toLanguageTags(), + ) + // The REAL leak assertions: the process-global JVM defaults the buggy + // activity flipped to English must be back to the host's Arabic. + assertEquals("Locale.getDefault() leaked", "ar", Locale.getDefault().language) + assertEquals("LocaleList.getDefault() leaked", "ar", LocaleList.getDefault()[0].language) + } +} diff --git a/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt b/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt index d7c6d42..3e9a506 100644 --- a/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt +++ b/sharingan/src/androidMain/kotlin/dev/sharingan/SharinganActivity.kt @@ -1,17 +1,43 @@ package dev.sharingan +import android.content.Context +import android.content.res.Configuration import android.os.Bundle +import android.os.LocaleList import androidx.activity.ComponentActivity import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge import dev.sharingan.ui.SharinganScreen +import java.util.Locale /** * Hosts the Sharingan log browser. Launched by tapping the capture * notification or by calling [show]; apps never need to declare it — * it ships in the library manifest. + * + * The logger is a locale-neutral surface: always English + LTR regardless of + * the host's per-app locale, and it never leaks its locale back into the host + * (issue #38). [attachBaseContext] pins an English/LTR configuration for this + * activity via the non-mutating [Context.createConfigurationContext]; that call + * has the side effect of resetting the process-global [LocaleList] default to + * English, so we snapshot the host's default and restore it — both right after, + * and again in [onDestroy] — leaving the host's locale untouched. */ public class SharinganActivity : ComponentActivity() { + + private var hostLocales: LocaleList? = null + + override fun attachBaseContext(newBase: Context) { + hostLocales = LocaleList.getDefault() + val config = Configuration(newBase.resources.configuration).apply { + setLocale(Locale.ENGLISH) + setLayoutDirection(Locale.ENGLISH) + } + // Non-mutating: createConfigurationContext, never resources.updateConfiguration. + super.attachBaseContext(newBase.createConfigurationContext(config)) + hostLocales?.let(LocaleList::setDefault) + } + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) enableEdgeToEdge() @@ -19,4 +45,9 @@ public class SharinganActivity : ComponentActivity() { SharinganScreen() } } + + override fun onDestroy() { + hostLocales?.let(LocaleList::setDefault) + super.onDestroy() + } } diff --git a/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt b/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt index 2151534..a581884 100644 --- a/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt +++ b/sharingan/src/commonMain/kotlin/dev/sharingan/ui/SharinganScreen.kt @@ -9,6 +9,7 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.safeDrawing import androidx.compose.material3.Scaffold import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue @@ -18,6 +19,8 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalLayoutDirection +import androidx.compose.ui.unit.LayoutDirection import dev.sharingan.HttpEvent import dev.sharingan.Sharingan import dev.sharingan.SharinganEvent @@ -137,34 +140,39 @@ internal fun SharinganScreenContent( ) { val colors = LocalSharinganColors.current PlatformBackHandler(enabled = selectedEvent != null, onBack = onBack) - Scaffold( - modifier = modifier, - containerColor = colors.bg, - contentWindowInsets = WindowInsets.safeDrawing, - ) { innerPadding -> - Box( - Modifier - .padding(innerPadding) - .consumeWindowInsets(innerPadding) - .fillMaxSize(), - ) { - if (selectedEvent != null) { - DetailScreenContent(event = selectedEvent, onBack = onBack, onShare = onShareSingle) - } else { - HomeScreenContent( - state = homeState, - onSelectProtocol = onSelectProtocol, - onQueryChange = onQueryChange, - onChipChange = onChipChange, - onToggleRecording = onToggleRecording, - onOpenEvent = onOpenEvent, - onShareAll = onShareAll, - ) + // The logger is a locale-neutral surface — always LTR, on every platform + // (including iOS) and in Studio previews, regardless of the host's layout + // direction (issue #38). + CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Ltr) { + Scaffold( + modifier = modifier, + containerColor = colors.bg, + contentWindowInsets = WindowInsets.safeDrawing, + ) { innerPadding -> + Box( + Modifier + .padding(innerPadding) + .consumeWindowInsets(innerPadding) + .fillMaxSize(), + ) { + if (selectedEvent != null) { + DetailScreenContent(event = selectedEvent, onBack = onBack, onShare = onShareSingle) + } else { + HomeScreenContent( + state = homeState, + onSelectProtocol = onSelectProtocol, + onQueryChange = onQueryChange, + onChipChange = onChipChange, + onToggleRecording = onToggleRecording, + onOpenEvent = onOpenEvent, + onShareAll = onShareAll, + ) + } + SharinganToast(toastMessage, Modifier.align(Alignment.BottomCenter)) } - SharinganToast(toastMessage, Modifier.align(Alignment.BottomCenter)) } - } - if (shareState != null) { - ShareSheet(state = shareState, onAction = onShareAction, onDismiss = onShareDismiss) + if (shareState != null) { + ShareSheet(state = shareState, onAction = onShareAction, onDismiss = onShareDismiss) + } } } From 79026e34936f4e531a6cd7e82ee8727e1a02be1e Mon Sep 17 00:00:00 2001 From: Mohamed Ibrahim Date: Thu, 2 Jul 2026 14:56:33 +0100 Subject: [PATCH 2/2] ci: gate PRs on the instrumented suites with an API 34 emulator job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The locale-leak regression (LoggerLocaleLeakTest) and the other connected suites (:sharingan UI tests, CaptureNotificationE2eTest) previously never ran in CI — build.yml only covered JVM and iOS unit tests, so the issue-#38 fix was only verified by manual emulator runs. Add an `android` job (reactivecircus/android-emulator-runner, API 34, KVM-accelerated ubuntu) that runs both modules' connectedDebugAndroidTest and uploads reports on failure, and correct the LoggerLocaleLeakTest KDoc that wrongly claimed CI already ran emulators. Co-Authored-By: Claude Fable 5 --- .github/workflows/build.yml | 54 ++++++++++++++++++- .../sharingan/sample/LoggerLocaleLeakTest.kt | 3 +- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1cd7cf7..2477449 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -9,7 +9,9 @@ name: Build & Test # Neither builds the code or runs tests on a PR, so this workflow is the actual # correctness gate. A PR that introduces a failing unit test is blocked here: # - a broken common/JVM test fails the `jvm` job's testDebugUnitTest step; -# - a broken iOS test fails the `ios` job's iosSimulatorArm64Test step. +# - a broken iOS test fails the `ios` job's iosSimulatorArm64Test step; +# - a broken instrumented/UI test fails the `android` job's connected +# test step, which runs the device suites on an API 34 emulator. # Docs/site-only changes skip this gate (paths-ignore): they can't affect the # Kotlin/Gradle build or tests, so there's no point spending ~25 min of macOS # runner time linking the iOS frameworks for a README or CSS edit. Any PR that @@ -106,3 +108,53 @@ jobs: # skip) — a deviceId follow-up, not a hidden gap. - name: Run iOS simulator unit tests run: ./gradlew :sharingan:iosSimulatorArm64Test :sharingan-noop:iosSimulatorArm64Test + + # Android instrumented (connected) tests on a Linux-hosted emulator. This is + # the only job that exercises the device suites — :sharingan's Compose UI + # tests and the sample app's notification E2E + locale-leak regression + # (issue #38, LoggerLocaleLeakTest) — so device-only regressions are caught + # at PR time instead of relying on manual emulator runs. API 34 satisfies + # the test's @SdkSuppress(minSdkVersion = 33) gate (LocaleManager is 33+); + # anything lower would silently skip the locale regression. + android: + runs-on: ubuntu-latest + steps: + # Grant the runner access to /dev/kvm — the emulator needs hardware + # acceleration or it fails to boot within the action's timeout. + - name: Enable KVM group perms + run: | + echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules + sudo udevadm control --reload-rules + sudo udevadm trigger --name-match=kvm + + - uses: actions/checkout@v6 + + - name: Set up JDK 17 + uses: actions/setup-java@v5 + with: + distribution: temurin + java-version: "17" + + - name: Set up Gradle + uses: gradle/actions/setup-gradle@v4 + + # Boots the emulator and runs every connected suite in one go. A failing + # instrumented test (e.g. the logger leaking its locale into the host) + # blocks the PR here. + - name: Run instrumented tests (API 34 emulator) + uses: reactivecircus/android-emulator-runner@v2 + with: + api-level: 34 + arch: x86_64 + script: ./gradlew :sharingan:connectedDebugAndroidTest :sample:composeApp:connectedDebugAndroidTest + + # Connected-test HTML/XML reports are only useful on red runs; keep CI + # storage lean by uploading them just on failure. + - name: Upload instrumented test reports + if: failure() + uses: actions/upload-artifact@v4 + with: + name: connected-test-reports + path: | + **/build/reports/androidTests/connected/ + **/build/outputs/androidTest-results/connected/ diff --git a/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt b/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt index 22cf322..2b1b988 100644 --- a/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt +++ b/sample/composeApp/src/androidInstrumentedTest/kotlin/dev/sharingan/sample/LoggerLocaleLeakTest.kt @@ -30,7 +30,8 @@ import java.util.Locale * (`LocaleManager`, what `AppCompatDelegate.setApplicationLocales` calls under * the hood on API 33+) directly in the process, since a test-APK host Activity * can't be launched into the app-under-test's process and the leak we assert is - * process-global anyway. Requires API 33+ (the emulators CI runs on). + * process-global anyway. Requires API 33+; CI's `android` job (build.yml) runs + * the connected suites on an API 34 emulator, so this regression gates PRs. */ @RunWith(AndroidJUnit4::class) @SdkSuppress(minSdkVersion = 33) // LocaleManager is API 33+; skip (don't crash) below.