3efea16 · opencode-android

git.iko.soy/opencode-android

Fix swipe MARK_READ toggle, warm-start invalid-link toast, unshareSession 404 race, connection close cancellation, and UnknownMessage pivot-key leak

3efea16iko3 months ago

app/src/main/java/soy/iko/opencode/MainActivity.kt+34−4

compare with master

unifiedsplit

@@ -53,7 +53,15 @@ class MainActivity : FragmentActivity() {

53 private var openFileHandled = false

54 // Set when a deep link was present but failed validation (malformed id/path). Surfaced

55 // as a toast from onResume so the user knows the link was rejected, not silently ignored.

56+ // On a warm start (onNewIntent while already resumed) the toast is shown immediately from

57+ // handleIntent, since onResume is NOT re-called on an already-resumed activity — without

58+ // that the toast would stay pending until the next pause/resume cycle (e.g. user

59+ // backgrounds and returns), making the rejected link look silently ignored.

56 private var showInvalidLinkToast = false

60 private var showInvalidLinkToast = false

61+ // True between onResume and onPause. Used by handleIntent to decide whether to fire the

62+ // invalid-link toast immediately (warm start) or defer it to onResume (cold start, where

63+ // a toast from onCreate could flash before the first frame is drawn).

64+ private var isResumed = false

57 // Cold-start prompts (crash report, notification rationale) rendered as Compose M3

65 // Cold-start prompts (crash report, notification rationale) rendered as Compose M3

58 // dialogs. Hoisted as Activity-level mutableState so the non-composable trigger logic

66 // dialogs. Hoisted as Activity-level mutableState so the non-composable trigger logic

59 // (onCreate/onResume) can flip them and the Compose tree (inside OpencodeTheme) reads

67 // (onCreate/onResume) can flip them and the Compose tree (inside OpencodeTheme) reads

@@ -244,19 +252,34 @@ class MainActivity : FragmentActivity() {

244

252

245 override fun onResume() {

253 override fun onResume() {

246 super.onResume()

254 super.onResume()

255+ isResumed = true

247 // Re-check on resume: an Activity recreated by a config change not in configChanges

256 // Re-check on resume: an Activity recreated by a config change not in configChanges

248 // (e.g. locale) has savedInstanceState != null, so onCreate skipped the request. This

257 // (e.g. locale) has savedInstanceState != null, so onCreate skipped the request. This

249 // ensures the user is prompted once per Activity instance when still ungranted rather

258 // ensures the user is prompted once per Activity instance when still ungranted rather

250 // than silently losing all notifications.

259 // than silently losing all notifications.

251 maybeRequestNotificationPermission()

260 maybeRequestNotificationPermission()

252 // Surface a rejected deep link once, so a malformed opencode:// link doesn't appear

261 // Surface a rejected deep link once, so a malformed opencode:// link doesn't appear

253− // to be silently ignored. Cleared after firing so a rotation doesn't re-toast.

262+ // to be silently ignored. Cleared after firing so a rotation doesn't re-toast. This

263+ // covers the cold-start path (handleIntent ran from onCreate, before the activity was

264+ // resumed, so it deferred the toast here). Warm-start rejections fire immediately from

265+ // handleIntent and never set this flag.

254 if (showInvalidLinkToast) {

266 if (showInvalidLinkToast) {

255 showInvalidLinkToast = false

267 showInvalidLinkToast = false

256− android.widget.Toast.makeText(this, R.string.deep_link_invalid, android.widget.Toast.LENGTH_SHORT).show()

268+ showInvalidLinkToastNow()

257 }

269 }

258 }

270 }

259

271

272+ override fun onPause() {

273+ super.onPause()

274+ isResumed = false

275+ }

276+

277+ /** Fire the invalid-deep-link toast. Extracted so the cold-start (onResume) and warm-start

278+ * (handleIntent) paths share a single implementation. */

279+ private fun showInvalidLinkToastNow() {

280+ android.widget.Toast.makeText(this, R.string.deep_link_invalid, android.widget.Toast.LENGTH_SHORT).show()

281+ }

282+

260 /** Capture text shared from another app so it can be prefilled into a session draft,

283 /** Capture text shared from another app so it can be prefilled into a session draft,

261 * and session ids from notification taps / deep links so we can open them. */

284 * and session ids from notification taps / deep links so we can open them. */

262 private fun handleIntent(intent: Intent?) {

285 private fun handleIntent(intent: Intent?) {

@@ -333,7 +356,11 @@ class MainActivity : FragmentActivity() {

333 // The link carried a session host but no valid id (or a blank/traversal

356 // The link carried a session host but no valid id (or a blank/traversal

334 // attempt). Surface a toast so the user knows the link was rejected,

357 // attempt). Surface a toast so the user knows the link was rejected,

335 // rather than silently no-op'ing (which reads as "the app did nothing").

358 // rather than silently no-op'ing (which reads as "the app did nothing").

359+ // On a warm start (already resumed) fire immediately; on a cold start

360+ // defer to onResume so the toast doesn't race the first frame.

336− if (!seg.isNullOrBlank()) showInvalidLinkToast = true

361+ if (!seg.isNullOrBlank()) {

362+ if (isResumed) showInvalidLinkToastNow() else showInvalidLinkToast = true

363+ }

337 // Mark the link handled even on rejection, so a process-death-and-restore

364 // Mark the link handled even on rejection, so a process-death-and-restore

338 // (which re-delivers the same intent) doesn't re-fire the toast on every

365 // (which re-delivers the same intent) doesn't re-fire the toast on every

339 // restore — the rejection is final, not a transient state to retry.

366 // restore — the rejection is final, not a transient state to retry.

@@ -348,7 +375,10 @@ class MainActivity : FragmentActivity() {

348 if (!openFileHandled) {

375 if (!openFileHandled) {

349 rawPath?.takeIf { isValidFilePath(it) }

376 rawPath?.takeIf { isValidFilePath(it) }

350 ?.let { container.requestOpenFile(it); openFileHandled = true } ?: run {

377 ?.let { container.requestOpenFile(it); openFileHandled = true } ?: run {

351− if (!rawPath.isNullOrBlank()) showInvalidLinkToast = true

378+ if (!rawPath.isNullOrBlank()) {

379+ // Same warm-vs-cold logic as the session branch above.

380+ if (isResumed) showInvalidLinkToastNow() else showInvalidLinkToast = true

381+ }

352 // Mark handled on rejection too, mirroring the session branch —

382 // Mark handled on rejection too, mirroring the session branch —

353 // otherwise a process-death restore re-fires the invalid-link toast.

383 // otherwise a process-death restore re-fires the invalid-link toast.

354 openFileHandled = true

384 openFileHandled = true

app/src/main/java/soy/iko/opencode/data/network/OpencodeApiClient.kt+12−2

@@ -245,13 +245,23 @@ open class OpencodeApiClient private constructor(

245 /** Revoke the session's public share link. Returns the session with share cleared.

246 * Like [deleteSession], a retried DELETE (first reached the server but the response was

247 * lost) hits an already-unshared session and gets 404 — revoking a non-shared session is

248− * effectively success, so swallow 404 rather than surfacing a spurious failure. */

248+ * effectively success, so swallow 404 rather than surfacing a spurious failure.

249+ * The fallback `GET` re-reads the current session state so the caller sees the cleared

250+ * share. If the session itself was deleted in the window between the DELETE-404 and the

251+ * GET (race with another client / a concurrent deleteSession), the GET also 404s —

252+ * swallow that too and surface a synthesized gone-session rather than an unexpected

253+ * ClientRequestException, mirroring deleteSession's contract. */

249 open suspend fun unshareSession(sessionId: String): Session {

254 open suspend fun unshareSession(sessionId: String): Session {

250 return try {

255 return try {

251 withRetry { client!!.delete("session/${encode(sessionId)}/share").body() }

256 withRetry { client!!.delete("session/${encode(sessionId)}/share").body() }

252 } catch (t: ClientRequestException) {

257 } catch (t: ClientRequestException) {

253 if (t.response.status.value != 404) throw t

258 if (t.response.status.value != 404) throw t

259+ try {

254− withRetry { client!!.get("session/${encode(sessionId)}").body() }

260+ withRetry { client!!.get("session/${encode(sessionId)}").body<Session>() }

261+ } catch (t2: ClientRequestException) {

262+ if (t2.response.status.value != 404) throw t2

263+ Session(id = sessionId, title = null, share = null)

264+ }

255 }

265 }

256 }

266 }

257

267

app/src/main/java/soy/iko/opencode/data/repo/SessionRepository.kt+13−1

@@ -634,7 +634,19 @@ internal class MessageStore {

634 eventSession == null && !messages.containsKey(id) -> return false

635 }

636 val removed = messages.remove(id) ?: return false

637+ // Derive the same key handleMessageUpdated would have stored this holder under. For an

638+ // UnknownMessage with an empty id, that's the synthetic "unknown-c<created>" /

639+ // "unknown-h<hash>" key (not the raw empty id) — removing the raw id here would leave

637− messageInfoUpdatedSincePivot.remove(id)

640+ // the synthetic entry stranded in messageInfoUpdatedSincePivot forever (no correctness

641+ // impact today — seed() only consults it when the holder exists — but unbounded growth

642+ // across many add/remove cycles in a long-lived session). Mirrors evictOldMessages,

643+ // which already uses the map key (oldestKey) for its pivot cleanup.

644+ val pivotKey = if (id.isEmpty() && removed.info is UnknownMessage) {

645+ removed.info.time?.created?.let { "unknown-c$it" } ?: "unknown-h${removed.info.hashCode()}"

646+ } else {

647+ id

648+ }

649+ messageInfoUpdatedSincePivot.remove(pivotKey)

638 removed.parts.keys.forEach { streamedSincePivot.remove(it) }

650 removed.parts.keys.forEach { streamedSincePivot.remove(it) }

639 return true

651 return true

640 }

652 }

app/src/main/java/soy/iko/opencode/di/OpencodeConnection.kt+6−1

@@ -10,6 +10,7 @@ import kotlinx.coroutines.CoroutineScope

10import kotlinx.coroutines.Dispatchers

11import kotlinx.coroutines.SupervisorJob

12import kotlinx.coroutines.cancel

13+import soy.iko.opencode.util.runCatchingCancellable

14/**

15/**

15 * An active connection to one opencode server. Bundles the Ktor client, REST API,

16 * An active connection to one opencode server. Bundles the Ktor client, REST API,

@@ -42,6 +43,10 @@ open class OpencodeConnection(

42 open suspend fun close() {

43 open suspend fun close() {

43 scope.cancel()

44 scope.cancel()

44 scopeJob.join()

45 scopeJob.join()

46+ // runCatchingCancellable (not plain runCatching) per the repo convention: close() is a

47+ // suspend function, and plain runCatching swallows CancellationException — if the

48+ // caller's scope is cancelled mid-close, the cancellation must propagate rather than

49+ // be eaten as a Result.failure (which would leave the client half-closed).

45− runCatching { client.close() }

50+ runCatchingCancellable { client.close() }

46 }

51 }

47}

52}

app/src/main/java/soy/iko/opencode/ui/session/SessionListScreen.kt+10−1

@@ -544,6 +544,7 @@ fun SessionListScreen(

544 onPin = { vm.togglePin(it) },

545 onArchive = { vm.toggleArchive(it) },

546 onMarkUnread = { vm.markUnread(it) },

547+ onMarkRead = { vm.markRead(it) },

547 onToggleMute = { container.toggleSessionMute(it) },

548 onToggleMute = { container.toggleSessionMute(it) },

548 onFilterByDirectory = vm::setDirectoryFilter,

549 onFilterByDirectory = vm::setDirectoryFilter,

549 onClearDirectoryFilter = { vm.setDirectoryFilter(null) },

550 onClearDirectoryFilter = { vm.setDirectoryFilter(null) },

@@ -666,6 +667,7 @@ private fun androidx.compose.foundation.layout.BoxScope.SessionListBody(

666 onPin: (Session) -> Unit,

667 onPin: (Session) -> Unit,

667 onArchive: (Session) -> Unit,

668 onArchive: (Session) -> Unit,

668 onMarkUnread: (String) -> Unit,

669 onMarkUnread: (String) -> Unit,

670+ onMarkRead: (String) -> Unit,

669 onToggleMute: (String) -> Unit,

671 onToggleMute: (String) -> Unit,

670 onFilterByDirectory: (String) -> Unit,

672 onFilterByDirectory: (String) -> Unit,

671 onClearDirectoryFilter: () -> Unit,

673 onClearDirectoryFilter: () -> Unit,

@@ -981,11 +983,18 @@ private fun androidx.compose.foundation.layout.BoxScope.SessionListBody(

981 runCatching { SwipeAction.valueOf(swipeRightAction) }.getOrDefault(SwipeAction.ARCHIVE)

983 runCatching { SwipeAction.valueOf(swipeRightAction) }.getOrDefault(SwipeAction.ARCHIVE)

982 }

984 }

983 // Apply a swipe action to the session. NONE is a no-op.

985 // Apply a swipe action to the session. NONE is a no-op.

986+ // MARK_READ toggles the unread badge: a read session becomes unread

987+ // (re-badged for revisit) and an already-unread session is cleared,

988+ // mirroring the "Mark read / unread" label exposed in Settings.

989+ val currentUnreadCount = unread[session.id] ?: 0

984 val applySwipe: (SwipeAction) -> Unit = { action ->

990 val applySwipe: (SwipeAction) -> Unit = { action ->

985 when (action) {

991 when (action) {

986 SwipeAction.DELETE -> onDelete(session.id)

992 SwipeAction.DELETE -> onDelete(session.id)

987 SwipeAction.ARCHIVE -> onArchive(session)

993 SwipeAction.ARCHIVE -> onArchive(session)

988− SwipeAction.MARK_READ -> onMarkUnread(session.id)

994+ SwipeAction.MARK_READ -> {

995+ if (currentUnreadCount > 0) onMarkRead(session.id)

996+ else onMarkUnread(session.id)

997+ }

989 SwipeAction.NONE -> {}

998 SwipeAction.NONE -> {}

990 }

999 }

991 }

1000 }

app/src/main/java/soy/iko/opencode/ui/session/SessionListViewModel.kt+7

@@ -909,6 +909,13 @@ class SessionListViewModel(private val container: AppContainer) : ViewModel() {

909 container.markSessionUnread(id)

910 }

911

912+ /** Mark a single session as read (clears its unread badge). The read-direction half of the

913+ * swipe "MARK_READ" toggle; the overflow menu only offers "Mark as unread" (gated on

914+ * unreadCount == 0), so this is only reachable via the swipe gesture today. */

915+ fun markRead(id: String) {

916+ container.markSessionRead(id)

917+ }

918+

912 /**

919 /**

913 * Permanently delete the given sessions. Unlike a single swipe-delete this is NOT undoable

920 * Permanently delete the given sessions. Unlike a single swipe-delete this is NOT undoable

914 * (the batch undo would need to cancel N deferred deletes); the caller confirms via a dialog

921 * (the batch undo would need to cancel N deferred deletes); the caller confirms via a dialog