e21470f · opencode-android

git.iko.soy/opencode-android

Fix continueRun/regenerate/ping-null/go-to-line bugs and harden cache writes

commite21470fa16e217d52eafac6458cb4c9d5dae4362 authoriko <mail@iko.soy> dateMon 6 Jul 2026, 20:59 (3 months ago) parent3efea16

11 files changed+90−25

compare with master

unifiedsplit

app/src/main/java/soy/iko/opencode/data/repo/AttachmentDraftStore.kt+5−2

@@ -87,8 +87,11 @@ open class AttachmentDraftStore private constructor(

8787 val tmp = File(file.parentFile, file.name + ".tmp")

8888 tmp.writeText(encoded)

8989 if (!tmp.renameTo(file)) {

90− file.writeText(encoded)

91− tmp.delete()

90+ try {

91+ file.writeText(encoded)

92+ } finally {

93+ tmp.delete()

94+ }

9295 }

9396 }

9497 }.onFailure { Log.w("AttachmentDraftStore", "Failed to persist attachments for $sessionId", it) }

app/src/main/java/soy/iko/opencode/data/repo/CrashLogger.kt+8

@@ -48,7 +48,15 @@ class CrashLogger private constructor(private val appContext: Context) {

4848 private val _reports = MutableStateFlow<List<CrashReport>>(emptyList())

4949 val reports: StateFlow<List<CrashReport>> = _reports.asStateFlow()

5050

51+ @Volatile private var installed = false

52+

5153 fun install() {

54+ // Guard against double-install: a second call would capture this handler as `previous`

55+ // and chain it to itself, double-writing crash reports and double-invoking the original

56+ // handler on every crash. The singleton get() mitigates this in production, but install()

57+ // itself must be idempotent (tests, a misconfigured init) so the chain can't form.

58+ if (installed) return

59+ installed = true

5260 val previous = Thread.getDefaultUncaughtExceptionHandler()

5361 Thread.setDefaultUncaughtExceptionHandler { thread, throwable ->

5462 runCatching { writeReport(thread, throwable) }

app/src/main/java/soy/iko/opencode/data/repo/MessageCacheStore.kt+7−2

@@ -100,8 +100,13 @@ open class MessageCacheStore private constructor(

100100 val tmp = File(file.parentFile, file.name + ".tmp")

101101 tmp.writeText(encoded)

102102 if (!tmp.renameTo(file)) {

103− file.writeText(encoded)

104− tmp.delete()

103+ // try/finally so a throw from file.writeText (disk full, permission) still

104+ // cleans up tmp — otherwise *.tmp files accumulate across failed writes.

105+ try {

106+ file.writeText(encoded)

107+ } finally {

108+ tmp.delete()

109+ }

105110 }

106111 }

107112 }.onFailure { Log.w("MessageCacheStore", "Failed to cache messages for $sessionId", it) }

app/src/main/java/soy/iko/opencode/data/repo/OutboxStore.kt+5−2

@@ -158,8 +158,11 @@ open class OutboxStore private constructor(

158158 val tmp = File(f.parentFile, f.name + ".tmp")

159159 tmp.writeText(encoded)

160160 if (!tmp.renameTo(f)) {

161− f.writeText(encoded)

162− tmp.delete()

161+ try {

162+ f.writeText(encoded)

163+ } finally {

164+ tmp.delete()

165+ }

163166 }

164167 }

165168 }.onFailure { Log.w("OutboxStore", "Failed to persist outbox", it) }

app/src/main/java/soy/iko/opencode/data/repo/RecentSessionsStore.kt+13−9

@@ -68,18 +68,22 @@ object RecentSessionsStore {

6868 // Atomic replace on POSIX filesystems (app's internal storage); the reader

6969 // never observes a torn file. renameTo can fail when the target exists on

7070 // some OEM filesystems — delete the target first so the rename is a pure

71− // create, and only if that still fails fall back to a copy-and-delete that

72− // preserves atomicity from the reader's perspective (the reader sees either

73− // the old complete file or the new complete file, never a half-written one,

74− // because the copy writes to the temp and then atomically moves it).

71+ // create. If that still fails, copy to a SECOND temp file and atomically

72+ // rename THAT to target — writing tmp.copyTo(target) directly would expose a

73+ // half-written target to a concurrent reader (read takes no mutex), breaking

74+ // the documented invariant. The second-rename leaves target either old or new,

75+ // never torn.

7576 if (!tmp.renameTo(target)) {

7677 target.delete()

7778 if (!tmp.renameTo(target)) {

78− // Last-resort: copy bytes then delete temp. The target is briefly

79− // absent during the copy, but a concurrent reader sees an empty

80− // list (the runCatching on the read side handles a missing file),

81− // never a torn file — preserving the documented invariant.

82− tmp.copyTo(target, overwrite = true)

79+ val tmp2 = File(target.parentFile, "$FILE.tmp2")

80+ tmp2.writeText(encoded)

81+ if (!tmp2.renameTo(target)) {

82+ // Truly last resort: some OEM filesystems reject even a create-rename.

83+ // Only here do we accept a non-atomic overwrite, with a final cleanup.

84+ target.delete()

85+ tmp2.renameTo(target)

86+ }

8387 tmp.delete()

8488 }

8589 }

app/src/main/java/soy/iko/opencode/data/repo/SessionRepository.kt+10−3

@@ -102,7 +102,10 @@ open class SessionRepository(

102102

103103 fun schedule(snapshot: List<MessageWithParts>) {

104104 val cache = messageCache ?: return

105− if (snapshot.isEmpty()) return

105+ // Don't early-return on an empty snapshot: MessageCacheStore.save deletes the file

106+ // for an empty list, which is exactly what we want when a conversation becomes empty

107+ // via MessageRemoved events. Skipping here would leave stale messages cached on disk

108+ // until an explicit remove() on session deletion.

106109 latest = snapshot

107110 val now = System.currentTimeMillis()

108111 pending?.cancel()

@@ -179,7 +182,11 @@ open class SessionRepository(

179182 if (cached.isNotEmpty()) {

180183 val changed = lock.withLock {

181184 if (seedGeneration == 0) {

182− cacheSeededIds = cached.mapTo(mutableSetOf()) { it.info.id }

185+ // Store the synthetic key for UnknownMessages (which have empty ids) so

186+ // pruneStaleCacheSeeded's messages.remove(id) hits the actual map key

187+ // the store used ("unknown-c<created>" / "unknown-h<hash>") instead of

188+ // the empty id that never matches and leaves ghost entries un-pruned.

189+ cacheSeededIds = cached.mapTo(mutableSetOf()) { store.syntheticKeyFor(it) }

183190 store.seed(cached, clearPivot = false)

184191 } else false

185192 }

@@ -522,7 +529,7 @@ internal class MessageStore {

522529 * synthetic key ("unknown-c<created>" or "unknown-h<hash>"); derive the same key here so

523530 * a REST seed merges onto the existing holder instead of missing it and inserting a

524531 * duplicate under key "". */

525− private fun syntheticKeyFor(m: MessageWithParts): String =

532+ internal fun syntheticKeyFor(m: MessageWithParts): String =

526533 if (m.info.id.isEmpty() && m.info is UnknownMessage) {

527534 m.info.time?.created?.let { "unknown-c$it" } ?: "unknown-h${m.info.hashCode()}"

528535 } else {

app/src/main/java/soy/iko/opencode/ui/chat/ChatScreen.kt+2−2

@@ -1486,14 +1486,14 @@ fun ChatScreen(

14861486 // once the stream finishes). This is invisible in practice: agent

14871487 // label is set on the first part, quote text grows with text parts

14881488 // but is only consumed on a swipe (user-initiated, post-stream).

1489− val agentLabel = remember(messageId, message.parts) {

1489+ val agentLabel = remember(messageId) {

14901490 message.parts

14911491 .filterIsInstance<soy.iko.opencode.data.model.AgentPart>()

14921492 .firstOrNull { it.name.isNotBlank() }?.name

14931493 }

14941494 // Text used to drive swipe-to-reply (and pre-fill the quote). Empty for

14951495 // image/code-only messages, which then opt out of the swipe gesture.

1496− val quoteText = remember(messageId, message.parts) {

1496+ val quoteText = remember(messageId) {

14971497 message.parts

14981498 .filterIsInstance<TextPart>()

14991499 .joinToString("\n\n") { it.text }

app/src/main/java/soy/iko/opencode/ui/chat/ChatViewModel.kt+20−2

@@ -1597,7 +1597,16 @@ class ChatViewModel(

15971597 ?: return

15981598 viewModelScope.launch {

15991599 runCatchingCancellable { conn.api.revert(sessionId, assistantMessageId) }

1600− .onSuccess { send(precedingUserText, includeAttachments = false) }

1600+ .onSuccess {

1601+ // send() can return false if the beginRun() CAS lost to a concurrent run

1602+ // or the message had to be enqueued offline and that failed. The revert has

1603+ // already hidden the messages, so a silent false would leave the user with a

1604+ // reverted conversation and no reply and no error — surface it instead.

1605+ val sent = send(precedingUserText, includeAttachments = false)

1606+ if (!sent) {

1607+ _errorEvents.trySend(ChatError(container.string(R.string.error_generic)))

1608+ }

1609+ }

16011610 .onFailure { _errorEvents.trySend(ChatError(container.friendlyError(it))) }

16021611 }

16031612 }

@@ -1618,8 +1627,17 @@ class ChatViewModel(

16181627 // foreground service off for the whole run.

16191628 runEndedByIdle = false

16201629 viewModelScope.launch {

1630+ // Route through the repository (not conn.api directly) so the prompt inherits

1631+ // withRetry (transient-failure backoff) and the user's selected model/agent — a

1632+ // direct api.sendPrompt would neither retry nor carry the picker selection, and

1633+ // would mint a fresh idempotency key on every retry attempt instead of sharing one.

16211634 runCatchingCancellable {

1622− conn.api.sendPrompt(sessionId, text = "continue")

1635+ conn.repository.sendPrompt(

1636+ sessionId,

1637+ text = "continue",

1638+ model = _selectedModel.value?.ref,

1639+ agent = _selectedAgent.value,

1640+ )

16231641 }.onFailure {

16241642 _running.value = false

16251643 _errorEvents.trySend(ChatError(container.friendlyError(it)))

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

@@ -265,7 +265,12 @@ fun FileViewScreen(

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

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

267267 }

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

268+ // When the render limit was just expanded, `lines` still reflects the OLD take()

269+ // (the remember(rawText, renderLimit) hasn't recomputed yet), so clamping against

270+ // lines.size here would scroll to the last currently-rendered line instead of the

271+ // requested one. Compute the target against allLines (the true line count) so the

272+ // scroll lands on the right item once recomposition expands the LazyColumn.

273+ val target = (clamped - 1).coerceIn(0, allLines.size - 1)

269274 highlightLine.value = null

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

271276 }

app/src/main/java/soy/iko/opencode/ui/server/ServerEditViewModel.kt+8−1

@@ -335,7 +335,14 @@ class ServerEditViewModel(

335335 // that accepts the connect but is half-up (wrong path, unresponsive /info) would

336336 // navigate to the session list as if everything succeeded, leaving the user

337337 // stranded on an empty screen with no error.

338− val pingOk = runCatchingCancellable { container.activeConnection.value?.api?.ping() }.isSuccess

338+ // A null activeConnection must NOT count as a successful ping: `null?.api?.ping()`

339+ // evaluates to null without throwing, so runCatchingCancellable would wrap it as

340+ // Result.success(null) and .isSuccess would be true — masking a connection that

341+ // dropped between connect() and the ping and navigating the user to an empty

342+ // session list with no error.

343+ val pingOk = container.activeConnection.value?.let {

344+ runCatchingCancellable { it.api.ping() }.isSuccess

345+ } ?: false

339346 if (!pingOk) {

340347 throw java.io.IOException(container.string(R.string.error_ping_failed))

341348 }

app/src/main/java/soy/iko/opencode/ui/server/ServerSettingsViewModel.kt+6−1

@@ -185,7 +185,12 @@ class ServerSettingsViewModel(

185185 // takes effect immediately. A non-active profile just persists.

186186 if (container.activeConnection.value?.profile?.id == saved.id) {

187187 container.connect(saved)

188− val pingOk = runCatchingCancellable { container.activeConnection.value?.api?.ping() }.isSuccess

188+ // A null activeConnection must NOT count as a successful ping — see

189+ // ServerEditViewModel for the reasoning (null?.api?.ping() yields null, which

190+ // runCatchingCancellable wraps as success and masks a dropped connection).

191+ val pingOk = container.activeConnection.value?.let {

192+ runCatchingCancellable { it.api.ping() }.isSuccess

193+ } ?: false

189194 if (!pingOk) {

190195 throw java.io.IOException(container.string(R.string.error_ping_failed))

191196 }