From 7223c60a97ca097dbd578b49446fe4819c666a75 Mon Sep 17 00:00:00 2001 From: AlexDev_ <56083016+alexdev03@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:24:41 +0200 Subject: [PATCH 1/3] fix(notification): defer shutdown until foreground promotion --- .../notification/ChatNotificationService.kt | 35 ++++-- .../ForegroundServiceLifecycle.kt | 72 +++++++++++ .../ChatNotificationServiceTest.kt | 12 ++ .../ForegroundServiceLifecycleTest.kt | 113 ++++++++++++++++++ 4 files changed, 223 insertions(+), 9 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycle.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycleTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt b/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt index 0d3d1503..5bfd9fda 100644 --- a/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt +++ b/app/src/main/java/com/m57/hermescontrol/notification/ChatNotificationService.kt @@ -66,6 +66,7 @@ class ChatNotificationService : Service() { internal const val PENDING_NOTIFICATION_ID = 2 private val isAppInForeground = AtomicBoolean(false) + internal val lifecycle = ForegroundServiceLifecycle() fun setAppForeground(foreground: Boolean) { isAppInForeground.set(foreground) @@ -80,7 +81,6 @@ class ChatNotificationService : Service() { override fun onCreate() { super.onCreate() createNotificationChannels() - startForeground(NOTIFICATION_ID, buildForegroundNotification(getString(R.string.notif_waiting_replies))) startEventCollection() } @@ -89,6 +89,7 @@ class ChatNotificationService : Service() { serviceScope.launch { HermesWsClient.events.collect { event -> if (!isAppInForeground.get()) { + val generation = lifecycle.generation launch { delay(500) if (!isAppInForeground.get()) { @@ -111,8 +112,8 @@ class ChatNotificationService : Service() { // HermesWsClient's own collector, so the // service is not restarted on the next // ON_STOP (issue #794). - stopForeground(STOP_FOREGROUND_REMOVE) - stopSelf() + // A delayed completion must not retire a newer turn/start. + if (!HermesWsClient.pendingReply) lifecycle.complete(generation) } is WsEvent.ClarifyRequest -> { @@ -260,11 +261,23 @@ class ChatNotificationService : Service() { intent: Intent?, flags: Int, startId: Int, - ): Int = START_NOT_STICKY + ): Int { + lifecycle.onStart( + owner = this, + promote = { + startForeground(NOTIFICATION_ID, buildForegroundNotification(getString(R.string.notif_waiting_replies))) + }, + // Do not remove foreground status before Android accepts retirement: + // a newer start may already be queued, but not delivered to us yet. + stopLatest = { stopSelfResult(startId) }, + ) + return START_NOT_STICKY + } override fun onBind(intent: Intent?): IBinder? = null override fun onDestroy() { + lifecycle.onDestroyed(this) eventCollector?.cancel() serviceScope.cancel() super.onDestroy() @@ -283,15 +296,19 @@ object NotificationHelper { // when nothing is in flight. if (!HermesWsClient.pendingReply) return val intent = Intent(context, ChatNotificationService::class.java) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - context.startForegroundService(intent) - } else { - context.startService(intent) + ChatNotificationService.lifecycle.start { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + context.startForegroundService(intent) + } else { + context.startService(intent) + } } } fun stop(context: Context) { - context.stopService(Intent(context, ChatNotificationService::class.java)) + // Rotation can resume the activity before the queued service is created. + // Direct stopService here crashes Android while foreground promotion is owed. + ChatNotificationService.lifecycle.stop() } fun setAppForeground( diff --git a/app/src/main/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycle.kt b/app/src/main/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycle.kt new file mode 100644 index 00000000..8409750b --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycle.kt @@ -0,0 +1,72 @@ +package com.m57.hermescontrol.notification + +/** + * Orders foreground-service requests and retirement (rotation/start-stop regression). + * A stop before creation is deferred until promotion. Android's stopSelfResult(startId) + * protects a delivered start from tearing down a newer start still queued by the system. + */ +internal class ForegroundServiceLifecycle { + private var requested = false + private var owner: Any? = null + private var stopCurrent: (() -> Boolean)? = null + + var generation = 0L + @Synchronized get + private set + + @Synchronized + fun start(request: () -> Unit) { + val wasRequested = requested + val previousGeneration = generation + generation++ + requested = true + try { + request() + } catch (error: Throwable) { + requested = wasRequested + generation = previousGeneration + throw error + } + } + + @Synchronized + fun stop() { + requested = false + retire() + } + + @Synchronized + fun onStart( + owner: Any, + promote: () -> Unit, + stopLatest: () -> Boolean, + ) { + // Fulfil each startForegroundService request before allowing any retirement. + promote() + this.owner = owner + stopCurrent = stopLatest + if (!requested) retire() + } + + @Synchronized + fun complete(expectedGeneration: Long) { + if (generation != expectedGeneration) return + requested = false + retire() + } + + @Synchronized + fun onDestroyed(owner: Any) { + if (this.owner === owner) { + this.owner = null + stopCurrent = null + } + } + + private fun retire() { + if (stopCurrent?.invoke() == true) { + owner = null + stopCurrent = null + } + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt b/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt index 7d406b81..4d4fca1c 100644 --- a/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt @@ -1,5 +1,8 @@ package com.m57.hermescontrol.notification +import android.content.Context +import io.mockk.mockk +import io.mockk.verify import org.junit.Assert.assertEquals import org.junit.Test @@ -17,6 +20,15 @@ import org.junit.Test * or through the public `setAppForeground` API. */ class ChatNotificationServiceTest { + @Test + fun `stop never tears down a service awaiting foreground promotion`() { + val context = mockk(relaxed = true) + + NotificationHelper.stop(context) + + verify(exactly = 0) { context.stopService(any()) } + } + @Test fun `setAppForeground true sets the flag`() { val field = diff --git a/app/src/test/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycleTest.kt b/app/src/test/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycleTest.kt new file mode 100644 index 00000000..70d01548 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/notification/ForegroundServiceLifecycleTest.kt @@ -0,0 +1,113 @@ +package com.m57.hermescontrol.notification + +import org.junit.Assert.assertEquals +import org.junit.Test + +class ForegroundServiceLifecycleTest { + @Test + fun `stop before creation waits for promotion`() { + val lifecycle = ForegroundServiceLifecycle() + val calls = mutableListOf() + lifecycle.start { calls += "request" } + lifecycle.stop() + assertEquals(listOf("request"), calls) + lifecycle.onStart(Any(), { calls += "promote" }) { + calls += "stop" + true + } + assertEquals(listOf("request", "promote", "stop"), calls) + } + + @Test + fun `every delivered start promotes an existing instance`() { + val lifecycle = ForegroundServiceLifecycle() + val owner = Any() + var promotions = 0 + lifecycle.start {} + repeat(2) { lifecycle.onStart(owner, { promotions++ }) { error("Unexpected stop") } } + assertEquals(2, promotions) + } + + @Test + fun `a queued newer start cannot be stopped with an older start id`() { + val lifecycle = ForegroundServiceLifecycle() + val calls = mutableListOf() + val owner = Any() + lifecycle.start {} + lifecycle.onStart(owner, { calls += "promote1" }) { + calls += "stop1-rejected" + false // Android stopSelfResult rejects an ID older than its latest queued start. + } + lifecycle.start {} + lifecycle.stop() + lifecycle.onStart(owner, { calls += "promote2" }) { + calls += "stop2" + true + } + assertEquals(listOf("promote1", "stop1-rejected", "promote2", "stop2"), calls) + } + + @Test + fun `delayed completion cannot retire a newer request`() { + val lifecycle = ForegroundServiceLifecycle() + var stops = 0 + lifecycle.start {} + lifecycle.onStart(Any(), {}) { + stops++ + true + } + val oldGeneration = lifecycle.generation + lifecycle.start {} + lifecycle.complete(oldGeneration) + assertEquals(0, stops) + lifecycle.complete(lifecycle.generation) + assertEquals(1, stops) + } + + @Test + fun `stop followed by a new start before creation keeps service alive`() { + val lifecycle = ForegroundServiceLifecycle() + lifecycle.start {} + lifecycle.stop() + lifecycle.start {} + lifecycle.onStart(Any(), {}) { error("New request must survive") } + } + + @Test + fun `destroying an old instance does not detach its replacement`() { + val lifecycle = ForegroundServiceLifecycle() + val old = Any() + var stops = 0 + lifecycle.start {} + lifecycle.onStart(old, {}) { true } + lifecycle.onStart(Any(), {}) { + stops++ + true + } + lifecycle.onDestroyed(old) + lifecycle.stop() + assertEquals(1, stops) + } + + @Test + fun `idle stop does not launch a service`() { + ForegroundServiceLifecycle().stop() + } + + @Test + fun `rejected start preserves completion of the existing request`() { + val lifecycle = ForegroundServiceLifecycle() + var stops = 0 + lifecycle.start {} + lifecycle.onStart(Any(), {}) { + stops++ + true + } + val generation = lifecycle.generation + val failure = IllegalStateException("Start rejected") + val result = runCatching { lifecycle.start { throw failure } } + assertEquals(failure, result.exceptionOrNull()) + lifecycle.complete(generation) + assertEquals(1, stops) + } +} From 487d8d1feb7c9785a0c196fc3aad8038fc42594a Mon Sep 17 00:00:00 2001 From: AlexDev_ <56083016+alexdev03@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:24:50 +0200 Subject: [PATCH 2/3] fix(lifecycle): preserve connection across activity recreation --- .../com/m57/hermescontrol/MainActivity.kt | 11 ++- .../MainActivityLifecycleTest.kt | 75 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/com/m57/hermescontrol/MainActivityLifecycleTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/MainActivity.kt b/app/src/main/java/com/m57/hermescontrol/MainActivity.kt index 1c73dc76..794a8ee2 100644 --- a/app/src/main/java/com/m57/hermescontrol/MainActivity.kt +++ b/app/src/main/java/com/m57/hermescontrol/MainActivity.kt @@ -89,11 +89,20 @@ class MainActivity : ComponentActivity() { override fun onPause() { ExternalActivityLifecycleGuard.onHostPaused() - NotificationHelper.setAppForeground(this, false) + // Prepare the FGS while still eligible to start it. A pause alone does + // not mean background: rotation also pauses and recreates this activity. NotificationHelper.start(this) super.onPause() } + override fun onStop() { + // isChangingConfigurations is reliable here, not during onPause. + // Keep the shared socket alive across recreation, but still release an + // idle connection when the user really leaves the app or locks the phone. + if (!isChangingConfigurations) NotificationHelper.setAppForeground(this, false) + super.onStop() + } + private fun consumeNotificationIntent(intent: Intent?) { // Issue #832: MainActivity is exported (launcher requirement) — only // honor intents stamped with our own notification action. diff --git a/app/src/test/java/com/m57/hermescontrol/MainActivityLifecycleTest.kt b/app/src/test/java/com/m57/hermescontrol/MainActivityLifecycleTest.kt new file mode 100644 index 00000000..12fbc1ad --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/MainActivityLifecycleTest.kt @@ -0,0 +1,75 @@ +package com.m57.hermescontrol + +import com.m57.hermescontrol.notification.NotificationHelper +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkAll +import io.mockk.verify +import org.junit.After +import org.junit.Test +import java.lang.reflect.InvocationTargetException + +class MainActivityLifecycleTest { + @After + fun tearDown() { + unmockkAll() + ExternalActivityLifecycleGuard.resetForTest() + } + + @Test + fun `pause prepares notifications without declaring the app backgrounded`() { + mockkObject(NotificationHelper) + every { NotificationHelper.start(any()) } returns Unit + every { NotificationHelper.setAppForeground(any(), any()) } returns Unit + val activity = mockk(relaxed = true) + every { activity invoke "onPause" withArguments emptyList() } answers { callOriginal() } + + invokeLifecycle(activity, "onPause") + + verify(exactly = 1) { NotificationHelper.start(activity) } + verify(exactly = 0) { NotificationHelper.setAppForeground(activity, false) } + } + + @Test + fun `configuration stop keeps the shared connection foregrounded`() { + mockkObject(NotificationHelper) + every { NotificationHelper.setAppForeground(any(), any()) } returns Unit + val activity = mockk(relaxed = true) + every { activity.isChangingConfigurations } returns true + every { activity invoke "onStop" withArguments emptyList() } answers { callOriginal() } + + invokeLifecycle(activity, "onStop") + + verify(exactly = 0) { NotificationHelper.setAppForeground(activity, false) } + } + + @Test + fun `real background stop still enables idle socket cleanup`() { + mockkObject(NotificationHelper) + every { NotificationHelper.setAppForeground(any(), any()) } returns Unit + val activity = mockk(relaxed = true) + every { activity.isChangingConfigurations } returns false + every { activity invoke "onStop" withArguments emptyList() } answers { callOriginal() } + + invokeLifecycle(activity, "onStop") + + verify(exactly = 1) { NotificationHelper.setAppForeground(activity, false) } + } + + private fun invokeLifecycle( + activity: MainActivity, + method: String, + ) { + try { + MainActivity::class.java + .getDeclaredMethod(method) + .apply { isAccessible = true } + .invoke(activity) + } catch (error: InvocationTargetException) { + // Execute real app code; only the trailing Android framework stub is unavailable on the JVM. + val expected = "Method $method in android.app.Activity not mocked." + if (error.targetException.message?.startsWith(expected) != true) throw error + } + } +} From f7318325c71b4c5b1610033d55f84efc37100ba3 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Sun, 6 Sep 2026 18:26:53 +0300 Subject: [PATCH 3/3] ci(labeler): achieve 100% path coverage and add notification/assistant labels --- .github/labeler.yml | 49 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/.github/labeler.yml b/.github/labeler.yml index d15a39ab..4553fba3 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -7,15 +7,18 @@ documentation: - changed-files: - any-glob-to-any-file: - "docs/**" - - "README.md" - - "CONTRIBUTING.md" - - "AGENTS.md" + - "fastlane/**" + - "*.md" + - "**/*.md" + - "LICENSE" # ── CI / workflows ─────────────────────────────────────────────── ci: - changed-files: - any-glob-to-any-file: - ".github/**" + - ".weblate.json" + - ".envrc" # ── tests ───────────────────────────────────────────────────────── testing: @@ -24,6 +27,8 @@ testing: - "app/src/test/**" - "app/src/androidTest/**" - "**/*Test.kt" + - "**/*Benchmark.kt" + - "**/fakes/**" # ── UI / UX ─────────────────────────────────────────────────────── ux: @@ -32,15 +37,22 @@ ux: - "app/src/main/**/ui/**" - "app/src/main/**/theme/**" - "app/src/main/res/**" + - "app/src/main/java/com/m57/hermescontrol/MainActivity.kt" + - "app/src/main/java/com/m57/hermescontrol/Navigation*.kt" + - "app/src/main/java/com/m57/hermescontrol/ScreenRegistry.kt" + - "app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt" + - "app/src/main/java/com/m57/hermescontrol/ExternalActivityLifecycleGuard.kt" + - "app/src/main/java/com/m57/hermescontrol/util/**" -# ── networking (WS, REST, remote) ───────────────────────────────── +# ── networking (WS, REST, remote, updates) ──────────────────────── networking: - changed-files: - any-glob-to-any-file: - "app/src/main/**/data/ws/**" - "app/src/main/**/data/remote/**" + - "app/src/main/**/data/update/**" -# ── data layer (Room, models, local) ────────────────────────────── +# ── data layer (Room, models, local, config) ────────────────────── data-layer: - changed-files: - any-glob-to-any-file: @@ -55,14 +67,35 @@ security: - changed-files: - any-glob-to-any-file: - "app/src/main/**/authlogin/**" + - "app/src/main/**/pairing/**" + - "app/src/main/**/keys/**" + - "app/src/main/**/data/local/AuthManager*" - "app/src/main/**/data/remote/Auth*" - "app/src/main/**/data/remote/*Token*" + - "app/src/main/**/data/remote/EncryptedCookieStore*" + - "app/src/main/**/data/remote/PersistentCookieJar*" + - "app/src/main/res/xml/network_security_config.xml" -# ── build system (Gradle, Nix, wrapper) ────────────────────────── +# ── notification (foreground & push services) ───────────────────── +notification: + - changed-files: + - any-glob-to-any-file: + - "app/src/main/**/notification/**" + - "app/src/main/res/drawable/ic_notification.xml" + +# ── assistant (digital assistant & voice interaction) ───────────── +assistant: + - changed-files: + - any-glob-to-any-file: + - "app/src/main/**/assistant/**" + - "app/src/main/res/xml/voice_interaction_service.xml" + +# ── build system (Gradle, Nix, wrapper, manifests) ──────────────── build: - changed-files: - any-glob-to-any-file: - "*.gradle.kts" + - "**/*.gradle.kts" - "gradle/**" - "gradle.properties" - "gradlew" @@ -70,3 +103,7 @@ build: - "flake.nix" - "flake.lock" - ".editorconfig" + - ".gitignore" + - "**/.gitignore" + - "app/proguard-rules.pro" + - "app/src/main/AndroidManifest.xml"