9f5d1ba · opencode-android

git.iko.soy/opencode-android

on master help sign in

Fix wrong-server notification routing, MCP add-dialog data loss, ProGuard keep scope, and stale UI state bugs

9f5d1baiko3 months ago

app/proguard-rules.pro+5−1

compare with master

unifiedsplit

@@ -14,7 +14,11 @@

14 *** Companion;

15}

16# Keep the serializable model classes (reflection-free serializer lookup needs the class).

17+# Covers ALL @Serializable classes in the app — not just data.model — because several

18+# classes used for persistent storage live elsewhere (OutboxMessage/PersistedAttachment in

19+# data/repo, StoredProfile in ProfileStore, Backup* in BackupManager, RecentSession, …)

20+# and R8 stripping/renaming their fields would break on-disk deserialization in release.

17−-keep @kotlinx.serialization.Serializable class soy.iko.opencode.data.model.** { *; }

21+-keep @kotlinx.serialization.Serializable class soy.iko.opencode.** { *; }

19# --- Ktor / OkHttp (HTTP client + SSE) ---

23# --- Ktor / OkHttp (HTTP client + SSE) ---

20# Don't warn about missing optional dependencies; keep only the classes that are

24# Don't warn about missing optional dependencies; keep only the classes that are

app/src/main/java/soy/iko/opencode/di/AppContainer.kt+11−6

@@ -1012,15 +1012,18 @@ open class AppContainer private constructor(

1012 }

1013

1014 /** Abort the active run for [sessionId] from a notification Stop action. Routes to the

1015− * originating profile (or the active connection) so the abort reaches the server that

1015+ * originating profile only — sending the abort to a different server would 404 (silently

1016+ * swallowed) and leave the original run stranded. A profile mismatch reports failure so the

1017+ * notification stays up and the user can switch back, mirroring

1018+ * [respondToPermissionFromNotification]'s routing guard. Runs on the app scope so it

1016− * owns the session. Runs on the app scope so it survives the BroadcastReceiver's lifetime. */

1019+ * survives the BroadcastReceiver's lifetime. */

1017 open fun abortRunFromNotification(sessionId: String, originProfileId: String?, onDone: (Boolean) -> Unit) {

1020 open fun abortRunFromNotification(sessionId: String, originProfileId: String?, onDone: (Boolean) -> Unit) {

1018 appScope.launch {

1021 appScope.launch {

1019 var success = false

1022 var success = false

1020 try {

1023 try {

1021 val conn = activeConnection.value?.takeIf {

1024 val conn = activeConnection.value?.takeIf {

1022 originProfileId == null || it.profile.id == originProfileId

1025 originProfileId == null || it.profile.id == originProfileId

1023− } ?: activeConnection.value

1026+ }

1024 if (conn != null) {

1027 if (conn != null) {

1025 success = runCatchingCancellable { conn.repository.abort(sessionId) }.isSuccess

1028 success = runCatchingCancellable { conn.repository.abort(sessionId) }.isSuccess

1026 }

1029 }

@@ -1034,8 +1037,10 @@ open class AppContainer private constructor(

1034 * Retry the last user prompt for [sessionId] from an error notification's "Retry last"

1037 * Retry the last user prompt for [sessionId] from an error notification's "Retry last"

1035 * action. Fetches the session's messages, re-sends the most recent user prompt, and

1038 * action. Fetches the session's messages, re-sends the most recent user prompt, and

1036 * cancels the error notification on success. Routes to the originating profile's

1039 * cancels the error notification on success. Routes to the originating profile's

1037− * connection (falling back to the active one) so the send reaches the server that owns

1040+ * connection only — sending the retry to a different server would 404 or start a fresh

1041+ * run on an unrelated session id. A profile mismatch reports failure so the notification

1042+ * stays up and the user can switch back, mirroring [respondToPermissionFromNotification]'s

1038− * the session. A no-op (reported as false) when there's no connection or no prior prompt.

1043+ * routing guard. A no-op (reported as false) when there's no connection or no prior prompt.

1039 */

1044 */

1040 open fun retryLastFromNotification(sessionId: String, originProfileId: String?, onDone: (Boolean) -> Unit) {

1045 open fun retryLastFromNotification(sessionId: String, originProfileId: String?, onDone: (Boolean) -> Unit) {

1041 appScope.launch {

1046 appScope.launch {

@@ -1043,7 +1048,7 @@ open class AppContainer private constructor(

1043 try {

1048 try {

1044 val conn = activeConnection.value?.takeIf {

1049 val conn = activeConnection.value?.takeIf {

1045 originProfileId == null || it.profile.id == originProfileId

1050 originProfileId == null || it.profile.id == originProfileId

1046− } ?: activeConnection.value

1051+ }

1047 if (conn != null) {

1052 if (conn != null) {

1048 success = runCatchingCancellable {

1053 success = runCatchingCancellable {

1049 val messages = conn.api.listMessages(sessionId)

1054 val messages = conn.api.listMessages(sessionId)

app/src/main/java/soy/iko/opencode/notification/RunForegroundService.kt+13−1

@@ -6,6 +6,7 @@ import android.app.Service

6import android.content.Context

7import android.content.Intent

8import android.content.pm.ServiceInfo

9+import android.content.res.Configuration

9import android.os.Build

10import android.os.Build

10import android.os.IBinder

11import android.os.IBinder

11import android.util.Log

12import android.util.Log

@@ -166,6 +167,15 @@ class RunForegroundService : Service() {

166

167

167 override fun onBind(intent: Intent?): IBinder? = null

168 override fun onBind(intent: Intent?): IBinder? = null

168

169

170+ // Reset the brand-color cache when the configuration (notably uiMode/night mode) changes

171+ // so the notification icon tint follows the app theme instead of staying pinned to whatever

172+ // the first build resolved. The service isn't recreated on a config change (it's a service,

173+ // not an Activity), so without this the cached color is stale until onDestroy.

174+ override fun onConfigurationChanged(newConfig: Configuration) {

175+ super.onConfigurationChanged(newConfig)

176+ brandColorCache = 0

177+ }

178+

169 companion object {

179 companion object {

170 private const val TAG = "RunForegroundService"

180 private const val TAG = "RunForegroundService"

171 private const val NOTIF_ID = 1

181 private const val NOTIF_ID = 1

@@ -176,7 +186,9 @@ class RunForegroundService : Service() {

176 @Volatile private var runStartMillis: Long = 0L

186 @Volatile private var runStartMillis: Long = 0L

177 // Brand accent for the foreground notifications' small-icon tint circle. Resolved

187 // Brand accent for the foreground notifications' small-icon tint circle. Resolved

178 // from resources (R.color.notif_brand) so it follows the app theme (light/dark)

188 // from resources (R.color.notif_brand) so it follows the app theme (light/dark)

179− // instead of a hardcoded constant. Lazily computed on first use from the app context.

189+ // instead of a hardcoded constant. Lazily computed on first use from the app context

190+ // and reset in onConfigurationChanged so a theme toggle mid-run doesn't pin the tint

191+ // to the old theme for the rest of the run.

180 private var brandColorCache: Int = 0

192 private var brandColorCache: Int = 0

181 private fun brandColor(context: Context): Int {

193 private fun brandColor(context: Context): Int {

182 if (brandColorCache == 0) {

194 if (brandColorCache == 0) {

app/src/main/java/soy/iko/opencode/notification/SessionNotifications.kt+19−5

@@ -35,7 +35,14 @@ object SessionNotifications {

36 private const val NOTIF_ID_PREFIX = 4000

37 private const val TAG = "SessionNotifications"

38− private const val UNREAD_BADGE_ID = NOTIF_ID_PREFIX + 9999

38+ // Fixed notification IDs use NEGATIVE space (mirroring the SUMMARY_*_ID constants below)

39+ // so they can never collide with the hash-derived per-session IDs returned by notifId(),

40+ // which are always positive (NOTIF_ID_PREFIX + (hash % (0x7FFFFFFF - NOTIF_ID_PREFIX)),

41+ // range [4000, 2147483646]). A collision would let a session-specific notification

42+ // overwrite or be canceled by the fixed one (~1 in 2 billion per session, but a real

43+ // ID-space flaw). Negative IDs are valid for NotificationManager.cancel/notify.

44+ private const val UNREAD_BADGE_ID = -10

45+ private const val CONNECTION_LOST_ID = -11

40 // Distinct namespaces so a session can have a completion, a permission, and an error

47 // Distinct namespaces so a session can have a completion, a permission, and an error

41 // notification outstanding at once without their ids colliding.

48 // notification outstanding at once without their ids colliding.

@@ -395,7 +402,7 @@ object SessionNotifications {

395 @SuppressLint("MissingPermission")

402 @SuppressLint("MissingPermission")

396 fun postConnectionLost(context: Context, profileLabel: String, profileId: String?) {

403 fun postConnectionLost(context: Context, profileLabel: String, profileId: String?) {

397 if (!canPost(context)) return

404 if (!canPost(context)) return

398− val notifId = NOTIF_ID_PREFIX + 7000

405+ val notifId = CONNECTION_LOST_ID

399 // Tap reopens the app at the server list so the user can reconnect / fix credentials.

406 // Tap reopens the app at the server list so the user can reconnect / fix credentials.

400 val openIntent = Intent(context, MainActivity::class.java).apply {

407 val openIntent = Intent(context, MainActivity::class.java).apply {

401 flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP

408 flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP

@@ -421,7 +428,7 @@ object SessionNotifications {

421

428

422 /** Cancel a previously-posted [postConnectionLost] notification (e.g. on reconnect). */

429 /** Cancel a previously-posted [postConnectionLost] notification (e.g. on reconnect). */

423 fun cancelConnectionLost(context: Context) {

430 fun cancelConnectionLost(context: Context) {

424− NotificationManagerCompat.from(context).cancel(NOTIF_ID_PREFIX + 7000)

431+ NotificationManagerCompat.from(context).cancel(CONNECTION_LOST_ID)

425 }

432 }

426

433

427 /** FLAG_MUTABLE where required (Android 12+) so RemoteInput results can be injected. */

434 /** FLAG_MUTABLE where required (Android 12+) so RemoteInput results can be injected. */

@@ -463,9 +470,16 @@ object SessionNotifications {

463 .onFailure { Log.w(TAG, "Failed to post unread badge notification", it) }

470 .onFailure { Log.w(TAG, "Failed to post unread badge notification", it) }

464 }

471 }

465

472

466− /** Cancel a session's permission notification (on reply, or when the user opens it). */

473+ /** Cancel a session's permission notification (on reply, or when the user opens it). Also

474+ * cancels the auto-rejected-permission notification (posted under notifId + 1) so opening

475+ * the session by any route other than tapping that notification clears it too — without

476+ * this a stale "auto-rejected" heads-up lingers in the shade (setAutoCancel only dismisses

477+ * on a direct tap). */

467 fun cancelPermission(context: Context, sessionId: String) {

478 fun cancelPermission(context: Context, sessionId: String) {

468− NotificationManagerCompat.from(context).cancel(notifId(NS_PERMISSION, sessionId))

479+ val nm = NotificationManagerCompat.from(context)

480+ val baseId = notifId(NS_PERMISSION, sessionId)

481+ nm.cancel(baseId)

482+ nm.cancel(baseId + 1) // postPermissionAutoRejected uses baseId + 1

469 maybeCancelSummary(context, GROUP_PERMISSION, SUMMARY_PERMISSION_ID)

483 maybeCancelSummary(context, GROUP_PERMISSION, SUMMARY_PERMISSION_ID)

470 }

484 }

471

485

app/src/main/java/soy/iko/opencode/ui/file/FileViewScreen.kt+5−1

@@ -445,7 +445,11 @@ private fun GoToLineLauncher(

445

446@Composable

447private fun GoToLineDialog(maxLine: Int, onConfirm: (Int) -> Unit, onDismiss: () -> Unit) {

448+ // Use plain remember (not rememberSaveable) so each open starts with an empty field.

449+ // The dialog is conditionally composed (gated by `visible` in GoToLineLauncher); with

450+ // rememberSaveable the typed text would be restored from the saved-state registry on

451+ // re-open, pre-filling whatever the user typed — and dismissed without — last time.

448− var text by rememberSaveable { mutableStateOf("") }

452+ var text by remember { mutableStateOf("") }

449 val parsed = text.trim().toIntOrNull()

453 val parsed = text.trim().toIntOrNull()

450 val valid = parsed != null && parsed in 1..maxLine

454 val valid = parsed != null && parsed in 1..maxLine

451 AlertDialog(

455 AlertDialog(

app/src/main/java/soy/iko/opencode/ui/mcp/McpScreen.kt+12−5

@@ -84,6 +84,8 @@ fun McpScreen(container: AppContainer, onBack: () -> Unit) {

84 val addFailedMsg = stringResource(R.string.mcp_add_failed)

85 val notConnectedMsg = stringResource(R.string.not_connected)

87+ var showAddDialog by rememberSaveable { mutableStateOf(false) }

88+

87 LaunchedEffect(Unit) {

89 LaunchedEffect(Unit) {

88 vm.messages.collect { msg ->

90 vm.messages.collect { msg ->

89 val text = when (msg) {

91 val text = when (msg) {

@@ -91,12 +93,16 @@ fun McpScreen(container: AppContainer, onBack: () -> Unit) {

91 McpMessage.ADD_FAILED -> addFailedMsg

93 McpMessage.ADD_FAILED -> addFailedMsg

92 McpMessage.NOT_CONNECTED -> notConnectedMsg

94 McpMessage.NOT_CONNECTED -> notConnectedMsg

93 }

95 }

96+ // Dismiss the Add dialog only on success (the reload makes the new server appear).

97+ // On failure keep it open with adding=false so the user can adjust and retry without

98+ // re-typing their input — the dialog's own spinner/disabled-state machinery was

99+ // designed for this; dismissing synchronously after the (async) POST would lose the

100+ // entered name/command/URL/env on every transient failure.

101+ if (msg == McpMessage.ADDED) showAddDialog = false

94 snackbar.showSnackbar(text)

102 snackbar.showSnackbar(text)

95 }

103 }

96 }

104 }

105

98− var showAddDialog by rememberSaveable { mutableStateOf(false) }

99−

100 Scaffold(

106 Scaffold(

101 topBar = {

107 topBar = {

102 AppTopBar(

108 AppTopBar(

@@ -199,9 +205,10 @@ fun McpScreen(container: AppContainer, onBack: () -> Unit) {

199 onDismiss = { if (!adding) showAddDialog = false },

205 onDismiss = { if (!adding) showAddDialog = false },

200 onAdd = { name, kind, target, env ->

206 onAdd = { name, kind, target, env ->

201 vm.addServer(name, kind, target, env)

207 vm.addServer(name, kind, target, env)

202− // Close immediately; the snackbar reports the outcome and a successful add

208+ // Don't dismiss synchronously: addServer launches a coroutine, so the POST is

203− // triggers a reload so the new server appears in the list.

209+ // still in flight. The dialog stays open showing the spinner (adding=true);

204− showAddDialog = false

210+ // the messages collector above dismisses on ADDED, or re-enables the fields

211+ // on ADD_FAILED so the user can retry without re-typing.

205 },

212 },

206 )

213 )

207 }

214 }