d518ef7 · opencode-android

git.iko.soy/opencode-android

Fix 12 bugs: ServerProfile rememberSaveable crash, permission countdown reset, swallowed CancellationException, draft flush drops, password visibility leak, crash export overwrite, GoToLine truncation, RemoteImage retry no-op, AppLock grace inconsistency, toast TOCTOU, CrashLogger I/O on crash thread, FGS instance race

commitd518ef7a5735dbd5c1ebfe559cf79804da5842f8 authoriko <mail@iko.soy> dateMon 6 Jul 2026, 03:31 (3 months ago) parent56dd3f3

13 files changed+149−42

compare with master

unifiedsplit

app/src/main/java/soy/iko/opencode/data/repo/CrashLogger.kt+12−1

@@ -68,6 +68,11 @@ class CrashLogger private constructor(private val appContext: Context) {

6868

6969 fun refresh() {

7070 val generation = refreshGeneration.incrementAndGet()

71+ // Trim over-capacity reports here (off the crashing thread, on the IO dispatcher) so

72+ // the UncaughtExceptionHandler path does only the single writeText it must to land the

73+ // report before the OS kills the process. Trimming on refresh keeps the cap enforced

74+ // across normal app launches (install calls refresh, and every delete/clear calls it).

75+ trimOldReports()

7176 val scanned = crashDir.listFiles { f -> f.isFile && f.name.endsWith(".txt") }

7277 ?.sortedByDescending { it.lastModified() }

7378 ?.map { f ->

@@ -231,8 +236,14 @@ class CrashLogger private constructor(private val appContext: Context) {

231236 // top-level message above is futile — Ktor/OkHttp embed the full request URL

232237 // in those messages, which may contain auth or internal paths. Scrubbing the

233238 // assembled text guarantees no URL survives anywhere in the stored report.

239+ //

240+ // NOTE: trimOldReports() is intentionally NOT called here. writeReport runs on the

241+ // crashing thread inside the UncaughtExceptionHandler, where the OS gives ~5s before

242+ // SIGKILL; trimOldReports does listFiles + sortedByDescending + N deletes that could

243+ // exceed that window on a slow device with many old reports, losing THIS report. The

244+ // trim is driven by refresh() instead (IO dispatcher, off the crashing thread), so the

245+ // crash path does only the single writeText it must to preserve the report.

234246 file.writeText(scrubUrls(sw.toString()))

235− trimOldReports()

236247 }

237248

238249 /** Keep only the [MAX_REPORTS] most recent crash reports, deleting older ones. Runs inside

app/src/main/java/soy/iko/opencode/data/repo/DraftStore.kt+26−8

@@ -150,12 +150,13 @@ open class DraftStore private constructor(

150150 if (text.isBlank()) remove(key) else putString(key, text)

151151 }.apply()

152152 }.onFailure { Log.w("DraftStore", "Failed to persist $key", it) }

153+ Unit

153154 }

154155 // FIX: always submit to the single-thread flushExecutor (never apply directly) so

155156 // writes persist in submission order. Previously the post-init fast path applied

156157 // synchronously on the caller's thread, which could land BEFORE an earlier still-queued

157158 // write, persisting stale text. The task resolves the prefs lazy and calls apply().

158− runCatching { flushExecutor.execute { write() } }

159+ submitFlush(write, key)

159160 }

160161

161162 /** Update the in-memory draft immediately without persisting to disk.

@@ -202,13 +203,14 @@ open class DraftStore private constructor(

202203 // Submit the (combined draft + follow-up) removal to the same single-thread

203204 // flushExecutor the other writers use, so it can't race ahead of an earlier queued

204205 // write and resurrect stale text. Mirrors flushDraft's submission pattern.

205− runCatching {

206− flushExecutor.execute {

206+ submitFlush(

207+ {

207208 runCatching {

208209 prefs.edit().remove(sessionId).remove(FOLLOWUP_PREFIX + sessionId).apply()

209210 }.onFailure { Log.w("DraftStore", "Failed to remove draft for $sessionId", it) }

210− }

211− }

211+ },

212+ "remove($sessionId)",

213+ )

212214 }

213215

214216 /**

@@ -238,15 +240,31 @@ open class DraftStore private constructor(

238240 // still-queued write completed, clobbering the draft with stale text. The executor task

239241 // resolves the prefs lazy and uses apply() (async) exactly as before; the write may be

240242 // lost only if the process exits before the queued task runs (as previously).

241− runCatching {

242− flushExecutor.execute {

243+ submitFlush(

244+ {

243245 runCatching {

244246 prefs.edit().apply {

245247 if (text.isBlank()) remove(sessionId) else putString(sessionId, text)

246248 }.apply()

247249 }.onFailure { Log.w("DraftStore", "Failed to flush draft for $sessionId", it) }

248− }

250+ },

251+ "flushDraft($sessionId)",

252+ )

253+ }

254+

255+ /** Submit a flush task to the single-thread executor, surfacing a post-shutdown submit as a

256+ * logged warning instead of silently swallowing the RejectedExecutionException. Previously

257+ * the bare `runCatching { flushExecutor.execute { ... } }` swallowed RejectedExecutionException

258+ * after [shutdown], so the in-memory _drafts was already updated but the disk write never

259+ * landed with no signal — a draft silently lost. Logging makes the failure observable so

260+ * ordering bugs (shutdown before all VM onCleared flushes) are diagnosable. */

261+ private fun submitFlush(task: () -> Unit, label: String) {

262+ if (flushExecutor.isShutdown) {

263+ Log.w("DraftStore", "Dropping flush '$label': executor is shut down")

264+ return

249265 }

266+ runCatching { flushExecutor.execute { task() } }

267+ .onFailure { Log.w("DraftStore", "Failed to submit flush '$label'", it) }

250268 }

251269

252270 private companion object {

app/src/main/java/soy/iko/opencode/data/repo/RecentSessionsStore.kt+6−1

@@ -9,6 +9,7 @@ import kotlinx.serialization.Serializable

99import kotlinx.serialization.decodeFromString

1010import kotlinx.serialization.encodeToString

1111import kotlinx.serialization.json.Json

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

1213import java.io.File

1314

1415/** A minimal session summary persisted for the home-screen widget and launcher shortcuts.

@@ -55,7 +56,11 @@ object RecentSessionsStore {

5556 /** Persist [sessions] (capped to [MAX]); a no-op empty list writes an empty file. */

5657 suspend fun write(context: Context, sessions: List<RecentSession>) = withContext(Dispatchers.IO) {

5758 writeMutex.withLock {

58− runCatching {

59+ // runCatchingCancellable (not plain runCatching): this runs inside a coroutine, and

60+ // plain runCatching swallows CancellationException — if the caller is cancelled

61+ // mid-write, cancellation wouldn't propagate and the write could complete (or

62+ // partially complete) on a dying scope. Matches the AGENTS.md convention.

63+ runCatchingCancellable {

5964 val target = file(context)

6065 val encoded = json.encodeToString(sessions.take(MAX))

6166 val tmp = File(target.parentFile, "$FILE.tmp")

app/src/main/java/soy/iko/opencode/notification/RunForegroundService.kt+6

@@ -118,6 +118,12 @@ class RunForegroundService : Service() {

118118 * service isn't alive yet, so the first progress update still lands. */

119119 @SuppressLint("MissingPermission")

120120 private fun updateProgress(progress: String?) {

121+ // Guard against the companion start() → instance race: start() reads @Volatile instance

122+ // on a background dispatcher and calls updateProgress, but onDestroy (on the main thread)

123+ // can null instance and call stopForeground between the read and this notify(). isInForeground

124+ // is cleared in onDestroy, so checking it here covers the stopForeground-already-called case

125+ // and avoids touching NotificationManager during teardown.

126+ if (!isInForeground) return

121127 lastProgress = progress

122128 val notification = buildNotification(lastTitle, lastSessionId, lastProfileId, progress)

123129 runCatching {

app/src/main/java/soy/iko/opencode/ui/AppLockGate.kt+9−7

@@ -138,13 +138,15 @@ fun AppLockGate(enabled: Boolean, reLockDelaySeconds: Int = 0, content: @Composa

138138 // common reason users disable biometric locks. A killed process resets unlocked anyway, so the

139139 // grace period only governs the in-memory re-lock decision across a surviving background stint.

140140 //

141− // rememberSaveable (not plain remember): a config change NOT in the manifest's configChanges

142− // (e.g. a locale change, which recreates the Activity) would otherwise reset this to 0L, and

143− // the new DisposableEffect observer gets ON_START delivered synchronously — the grace check

144− // sees lastStopTimeMs == 0 and re-locks even though the user unlocked 2s ago and only changed

145− // the system language. The stop timestamp is not security-sensitive (it's a wall-clock value),

146− // so persisting it through the Bundle is safe and preserves the grace period across recreation.

147− val lastStopTimeMs = rememberSaveable { longArrayOf(0L) }

141+ // Plain remember (not rememberSaveable), matching `unlocked` above: the grace check gates on

142+ // `unlocked`, which is non-saveable, so persisting only the stop timestamp through the Bundle

143+ // is inconsistent — on a config change NOT in the manifest's configChanges (e.g. a locale

144+ // change) `unlocked` resets to false while `lastStopTimeMs` would survive, and the grace

145+ // check `if (unlocked && ...)` short-circuits to false anyway, re-prompting the user despite

146+ // having unlocked seconds ago. Resetting both together on recreation is consistent: the new

147+ // DisposableEffect observer gets ON_START synchronously, sees lastStopTimeMs == 0, and prompts

148+ // cleanly (a recreation mid-grace re-prompts, which is the safe default for an app lock).

149+ val lastStopTimeMs = remember { longArrayOf(0L) }

148150 val reLockDelayMs = reLockDelaySeconds * 1000L

149151

150152 val title = stringResource(R.string.app_lock_prompt_title)

app/src/main/java/soy/iko/opencode/ui/chat/PermissionDialog.kt+11−1

@@ -27,6 +27,7 @@ import androidx.compose.runtime.Composable

2727import androidx.compose.runtime.LaunchedEffect

2828import androidx.compose.runtime.mutableStateOf

2929import androidx.compose.runtime.remember

30+import androidx.compose.runtime.saveable.rememberSaveable

3031import androidx.compose.runtime.getValue

3132import androidx.compose.runtime.setValue

3233import androidx.compose.ui.Alignment

@@ -78,7 +79,16 @@ fun PermissionDialog(

7879 var responded by remember(permission.id) { mutableStateOf(false) }

7980 // Elapsed ms since the dialog opened for this permission, used to surface a "still

8081 // waiting" reminder after the threshold. Caps at the auto-reject deadline.

81− var elapsedMs by remember(permission.id) { mutableStateOf(0L) }

82+ //

83+ // rememberSaveable (not plain remember): the countdown progress must survive an

84+ // Activity recreation (a config change NOT in the manifest's configChanges, e.g. a

85+ // system locale change) so the auto-reject deadline doesn't reset to 0 — otherwise a

86+ // permission the user walked away from would never auto-reject after a locale change,

87+ // leaving the run hanging open indefinitely and skipping the "auto-rejected"

88+ // notification. Long is Bundle-savable. Keyed on permission.id so a fresh prompt

89+ // resets the timer. (Unlike `responded`, restoring elapsed progress is safe: it only

90+ // advances the countdown, it doesn't lock out the buttons.)

91+ var elapsedMs by rememberSaveable(permission.id) { mutableStateOf(0L) }

8292 val respond: (PermissionResponse) -> Unit = { response ->

8393 if (!responded) {

8494 responded = true

app/src/main/java/soy/iko/opencode/ui/components/MarkdownText.kt+11−2

@@ -847,9 +847,18 @@ internal fun showCopyToast(context: Context, message: String) {

847847 showToast(context, message)

848848}

849849

850+// Self-suppressing toast: canceling the previous toast before showing the next so two rapid

851+// copy/share actions don't stack two toasts. Synchronized on a dedicated lock (lastToast is

852+// nullable, so it can't be the monitor itself) to close the TOCTOU window between cancel and

853+// assign — showToast is called from main-thread coroutine resumes and the unconfined save/share

854+// flows, so without the lock a second caller could read lastToast between the cancel and the

855+// assignment, leaving a toast uncancelled.

856+private val toastLock = Any()

850857private var lastToast: Toast? = null

851858

852859internal fun showToast(context: Context, message: String) {

853− lastToast?.cancel()

854− lastToast = Toast.makeText(context.applicationContext, message, Toast.LENGTH_SHORT).also { it.show() }

860+ synchronized(toastLock) {

861+ lastToast?.cancel()

862+ lastToast = Toast.makeText(context.applicationContext, message, Toast.LENGTH_SHORT).also { it.show() }

863+ }

855864}

app/src/main/java/soy/iko/opencode/ui/components/RemoteImage.kt+24−11

@@ -173,17 +173,23 @@ fun RemoteImage(part: FilePart, ctx: ImageLoadContext, modifier: Modifier = Modi

173173 // leak off-origin), a malformed data URI, or a source with no path — we show the broken-image

174174 // fallback instead of an empty gap (PartView already committed to RemoteImage, so there's no

175175 // FileChip to fall through to).

176− val model = produceState<Any?>(initialValue = ImageResolving, part.source, part.url, ctx.baseUrl) {

176+ //

177+ // Bumping retryKey re-runs resolveModel (and re-builds the ImageRequest below) so a failed

178+ // load can be re-issued by tapping the error state. Without retryKey in these keys, a null

179+ // model (e.g. a cross-origin URL blocked at resolve time) would stay null forever and the

180+ // retry button would be a no-op that re-renders the broken-image icon — see the ImageRetry

181+ // callback at the SubcomposeAsyncImage error slot.

182+ var retryKey by remember(part.source, part.url, ctx.baseUrl) { mutableIntStateOf(0) }

183+ val model = produceState<Any?>(initialValue = ImageResolving, part.source, part.url, ctx.baseUrl, retryKey) {

177184 value = withContext(Dispatchers.Default) { part.resolveModel(ctx) }

178185 }.value

179186 if (model === ImageResolving) return

180187 if (model == null) {

181− ImageStatusBox { BrokenImageIcon() }

188+ ImageStatusBox {

189+ BrokenImageIcon(onRetry = { retryKey++ })

190+ }

182191 return

183192 }

184− // Bumping retryKey re-builds the ImageRequest, so a failed load can be re-issued by tapping

185− // the error state instead of being a permanent broken-image icon.

186− var retryKey by remember(part.source, part.url, ctx.baseUrl) { mutableIntStateOf(0) }

187193 // Fullscreen zoomable viewer state: opened by tapping the inline image.

188194 var showFullscreen by remember { mutableStateOf(false) }

189195 val request = remember(part.source, part.url, ctx.baseUrl, ctx.basicAuthHeader, model, retryKey) {

@@ -501,12 +507,19 @@ private fun ImageStatusBox(content: @Composable () -> Unit) {

501507}

502508

503509@Composable

504−private fun BrokenImageIcon() {

505− Icon(

506− Icons.Filled.BrokenImage,

507− contentDescription = stringResource(R.string.image_failed),

508− tint = MaterialTheme.colorScheme.onSurfaceVariant,

509− )

510+private fun BrokenImageIcon(onRetry: (() -> Unit)? = null) {

511+ if (onRetry == null) {

512+ Icon(

513+ Icons.Filled.BrokenImage,

514+ contentDescription = stringResource(R.string.image_failed),

515+ tint = MaterialTheme.colorScheme.onSurfaceVariant,

516+ )

517+ } else {

518+ // Reuse the retry affordance so a null-model (e.g. a cross-origin URL blocked at resolve

519+ // time) is retryable just like a failed network fetch — tapping re-runs resolveModel

520+ // (retryKey is a produceState key) which may succeed if the block was transient.

521+ ImageRetry(onRetry = onRetry)

522+ }

510523}

511524

512525/** Decode the image behind [request] via Coil and write it to the system MediaStore

app/src/main/java/soy/iko/opencode/ui/file/FileViewScreen.kt+15−4

@@ -253,10 +253,19 @@ fun FileViewScreen(

253253 matchIndices.getOrNull(matchPos)?.let { idx -> runCatchingCancellable { listState.animateScrollToItem(idx) } }

254254 }

255255 // "Go to line" dialog state. Clears the jump highlight and scrolls the LazyColumn to the

256− // requested line (clamped to the rendered range) when the user confirms.

256+ // requested line (clamped to the file's true line count) when the user confirms.

257257 var showGoToLine by rememberSaveable(path) { mutableStateOf(false) }

258258 fun goToLine(n: Int) {

259− val target = (n - 1).coerceIn(0, (lines.size - 1).coerceAtLeast(0))

259+ // Clamp to the true file line count (allLines), not the rendered subset (lines), so a

260+ // number beyond the current render cap is still accepted. If the target is beyond the

261+ // rendered range, expand renderLimit first so the LazyColumn actually has that item to

262+ // scroll to — otherwise animateScrollToItem would clamp to the last rendered line and

263+ // the user would land nowhere near their requested line.

264+ val clamped = n.coerceIn(1, allLines.size)

265+ if (clamped - 1 >= lines.size) {

266+ renderLimit = (clamped + MAX_RENDERED_LINES - 1).coerceAtMost(allLines.size)

267+ }

268+ val target = (clamped - 1).coerceIn(0, lines.size - 1)

260269 highlightLine.value = null

261270 scope.launch { runCatchingCancellable { listState.animateScrollToItem(target) } }

262271 }

@@ -462,7 +471,7 @@ fun FileViewScreen(

462471

463472 GoToLineLauncher(

464473 visible = showGoToLine,

465− maxLine = lines.size,

474+ maxLine = allLines.size,

466475 onConfirm = { n -> showGoToLine = false; goToLine(n) },

467476 onDismiss = { showGoToLine = false },

468477 )

@@ -488,7 +497,9 @@ private fun GoToLineDialog(maxLine: Int, onConfirm: (Int) -> Unit, onDismiss: ()

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

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

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

491− var text by remember { mutableStateOf("") }

500+ // Keyed on maxLine so a reload that shrinks the file (e.g. the user edited it externally)

501+ // resets the field instead of keeping a now-out-of-range number that would show as invalid.

502+ var text by remember(maxLine) { mutableStateOf("") }

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

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

494505 AlertDialog(

app/src/main/java/soy/iko/opencode/ui/server/ServerEditScreen.kt+5−1

@@ -74,7 +74,11 @@ fun ServerEditScreen(

7474 viewModel(factory = vmFactory { ServerEditViewModel(container) })

7575 val state by vm.state.collectAsStateWithLifecycle()

7676 var showDiscardConfirm by rememberSaveable { mutableStateOf(false) }

77− var passwordVisible by rememberSaveable { mutableStateOf(false) }

77+ // Keyed on state.id so the password-reveal flag resets when a different profile is loaded

78+ // into the same back-stack entry. Without the key, revealing server A's password and then

79+ // editing server B (which reuses this entry via nav back-and-forth) would show B's password

80+ // in cleartext without the user tapping the eye icon — a minor credential info-leak.

81+ var passwordVisible by rememberSaveable(state.id) { mutableStateOf(false) }

7882

7983 fun safeExit() {

8084 when {

app/src/main/java/soy/iko/opencode/ui/server/ServerSettingsScreen.kt+5−1

@@ -83,7 +83,11 @@ fun ServerSettingsScreen(

8383 viewModel(factory = vmFactory { ServerSettingsViewModel(container, profileId) })

8484 val state by vm.state.collectAsStateWithLifecycle()

8585 var showDiscardConfirm by rememberSaveable { mutableStateOf(false) }

86− var passwordVisible by rememberSaveable { mutableStateOf(false) }

86+ // Keyed on profileId so the password-reveal flag resets when a different profile is loaded

87+ // into the same back-stack entry. Without the key, revealing server A's password and then

88+ // viewing server B (which reuses this entry via nav back-and-forth) would show B's password

89+ // in cleartext without the user tapping the eye icon — a minor credential info-leak.

90+ var passwordVisible by rememberSaveable(profileId) { mutableStateOf(false) }

8791

8892 fun safeExit() {

8993 when {

app/src/main/java/soy/iko/opencode/ui/session/SessionListScreen.kt+13−4

@@ -1907,7 +1907,16 @@ private fun ServerSwitcherMenu(

19071907 // Switching servers kills the SSE stream and any in-flight run (same hazard as disconnecting),

19081908 // so confirm first when a run is active. Kept inside the switcher so the call site stays a

19091909 // plain onSelect and SessionListScreen's complexity doesn't grow.

1910− var pendingSwitch by rememberSaveable { mutableStateOf<ServerProfile?>(null) }

1910+ //

1911+ // Persist only the profile id (a String) instead of the whole ServerProfile: ServerProfile is

1912+ // @Immutable but neither Parcelable nor java.io.Serializable (only kotlinx.serialization's

1913+ // @Serializable, which Compose's Bundle autoSaver can't use). rememberSaveable on a non-savable

1914+ // type throws IllegalStateException on a config change NOT in the manifest's configChanges

1915+ // (e.g. a system locale change) — a hard crash mid-confirm. The id survives in the Bundle and

1916+ // is re-resolved against the current profiles list; if the profile vanished mid-switch the

1917+ // confirm dialog simply doesn't re-show, which is the safe fallback.

1918+ var pendingSwitchId by rememberSaveable { mutableStateOf<String?>(null) }

1919+ val pendingSwitch = pendingSwitchId?.let { id -> profiles.firstOrNull { it.id == id } }

19111920 Column {

19121921 Box {

19131922 Row(

@@ -1966,7 +1975,7 @@ private fun ServerSwitcherMenu(

19661975 onClick = {

19671976 when {

19681977 isActiveProfile -> onDismiss()

1969− anyRunActive -> pendingSwitch = profile

1978+ anyRunActive -> pendingSwitchId = profile.id

19701979 // Don't dismiss here: keep the switcher open so its per-row spinner

19711980 // (driven by switchingId) stays visible during the switch. The

19721981 // LaunchedEffect in SessionListScreen dismisses it once it resolves.

@@ -1991,8 +2000,8 @@ private fun ServerSwitcherMenu(

19912000 }

19922001 SwitchServerConfirmDialog(

19932002 target = pendingSwitch,

1994− onSwitch = { profile -> pendingSwitch = null; onSelect(profile) },

1995− onDismiss = { pendingSwitch = null },

2003+ onSwitch = { profile -> pendingSwitchId = null; onSelect(profile) },

2004+ onDismiss = { pendingSwitchId = null },

19962005 )

19972006}

19982007

app/src/main/java/soy/iko/opencode/ui/settings/DiagnosticsScreen.kt+6−1

@@ -236,7 +236,12 @@ fun DiagnosticsScreen(onBack: () -> Unit) {

236236 if (reports.isNotEmpty()) {

237237 IconButton(onClick = {

238238 runCatching {

239− exportAllLauncher.launch("opencode-crash-reports.txt")

239+ // Timestamped export name so successive exports don't overwrite

240+ // one another when the user exports more than once (e.g. after new

241+ // crashes accumulate). Mirrors SettingsScreen.backupFilename().

242+ val now = java.text.SimpleDateFormat("yyyyMMdd-HHmmss", java.util.Locale.US)

243+ .format(java.util.Date())

244+ exportAllLauncher.launch("opencode-crash-reports-$now.txt")

240245 }

241246 }) {

242247 Icon(Icons.Filled.IosShare, contentDescription = stringResource(R.string.export_all))