27240b5 · opencode-android

git.iko.soy/opencode-android

Fix 12 bugs: abort re-light after Stop, Stop-Undo wrong prompt, bulkDelete error data loss, branchFrom orphan sessions, and 8 others across chat/session/repo layers

27240b5iko3 months ago

app/src/main/java/soy/iko/opencode/data/repo/SessionRepository.kt+33−15

compare with master

unifiedsplit

@@ -219,13 +219,23 @@ open class SessionRepository(

219 var seedChanged = false

220 lock.withLock {

221 if (generation == seedGeneration) {

222+ // Prune cache-seeded ghosts here too, not only in the initial

223+ // REST seed. If the SSE stream connected→dropped→reconnected

224+ // before the initial listMessages returned, seedGeneration was

225+ // already >0 by the time the initial seed ran, so its prune was

226+ // skipped (the `seedGeneration == 0` guard below). Without this

227+ // prune, messages deleted server-side while the app was closed

228+ // would survive as ghosts until a future app restart where the

229+ // timing happened not to recur.

230+ val pruned = store.pruneStaleCacheSeeded(cacheSeededIds, fresh)

222 // Merge without pruning: SSE events arriving during the

231 // Merge without pruning: SSE events arriving during the

223 // REST fetch may have added messages not yet indexed by

232 // REST fetch may have added messages not yet indexed by

224 // the REST endpoint. Pruning would silently delete them.

233 // the REST endpoint. Pruning would silently delete them.

225 // Stale messages from deletions during the disconnect

234 // Stale messages from deletions during the disconnect

226 // gap are eventually removed via MessageRemoved events

235 // gap are eventually removed via MessageRemoved events

227 // or on the next app restart.

236 // or on the next app restart.

228− seedChanged = store.seed(fresh, prune = false)

237+ val seeded = store.seed(fresh, prune = false)

238+ seedChanged = pruned || seeded

229 }

239 }

230 }

240 }

231 if (seedChanged) publish()

241 if (seedChanged) publish()

@@ -407,9 +417,10 @@ internal class MessageStore {

407 }

417 }

408

418

409 for (m in initial) {

419 for (m in initial) {

420+ val key = syntheticKeyFor(m)

410− val existing = messages[m.info.id]

421+ val existing = messages[key]

411 if (existing == null) {

422 if (existing == null) {

412− messages[m.info.id] = holderOf(m)

423+ messages[key] = holderOf(m)

413 changed = true

424 changed = true

414 } else {

425 } else {

415 // A part streamed in between subscribe and this initial load may already

426 // A part streamed in between subscribe and this initial load may already

@@ -417,7 +428,6 @@ internal class MessageStore {

417 // overwriting so that newer streamed part isn't discarded: take the

428 // overwriting so that newer streamed part isn't discarded: take the

418 // snapshot's part order, swap in the streamed version where ids overlap,

429 // snapshot's part order, swap in the streamed version where ids overlap,

419 // append any streamed-only parts, and adopt the authoritative REST info.

430 // append any streamed-only parts, and adopt the authoritative REST info.

420− // existing.parts is already keyed by part id (insertion-ordered).

421 val streamedById = existing.parts

431 val streamedById = existing.parts

422 val snapshotIds = m.parts.mapTo(mutableSetOf()) { it.id }

432 val snapshotIds = m.parts.mapTo(mutableSetOf()) { it.id }

423 // Prefer the in-memory version of an overlapping part ONLY if it streamed

433 // Prefer the in-memory version of an overlapping part ONLY if it streamed

@@ -429,25 +439,21 @@ internal class MessageStore {

429 }.toMutableList()

439 }.toMutableList()

430 // Append only parts that streamed in *since the pivot* and are absent from the

440 // Append only parts that streamed in *since the pivot* and are absent from the

431 // snapshot — i.e. genuinely newer than this REST fetch. A part in memory from

441 // snapshot — i.e. genuinely newer than this REST fetch. A part in memory from

432− // before the pivot that the snapshot omits was removed server-side (its

442+ // before the pivot that the snapshot omits was removed server-side; the

433− // MessagePartRemoved missed during a disconnect gap, or it was deleted while the

434− // app was closed and only survives from the cache seed); the authoritative REST

435− // snapshot must be allowed to drop it, else it reappears as a ghost — and, once

443+ // authoritative REST snapshot must be allowed to drop it, else it reappears as

436− // the drain loop persists this merge, gets baked into the on-disk cache forever

444+ // a ghost and gets baked into the on-disk cache forever.

437− // (the message still exists, so pruneStaleCacheSeeded never removes it). This

438− // mirrors the overlapping-part branch above and the info branch below, which both

439− // already gate on the pivot.

440 for (p in existing.parts.values) {

445 for (p in existing.parts.values) {

441 if (p.id !in snapshotIds && p.id in streamedSincePivot) ordered.add(p)

446 if (p.id !in snapshotIds && p.id in streamedSincePivot) ordered.add(p)

442 }

447 }

443 // Adopt the authoritative REST info unless the in-memory info was itself

448 // Adopt the authoritative REST info unless the in-memory info was itself

444 // updated live since the pivot (i.e. during this fetch), in which case it's

449 // updated live since the pivot (i.e. during this fetch), in which case it's

445− // newer than the snapshot. Otherwise the REST snapshot wins, discarding

450+ // newer than the snapshot. Otherwise the REST snapshot wins.

446− // in-memory info that went stale during a disconnect.

447 val info = if (existing.info is UnknownMessage || m.info.id !in messageInfoUpdatedSincePivot) m.info else existing.info

451 val info = if (existing.info is UnknownMessage || m.info.id !in messageInfoUpdatedSincePivot) m.info else existing.info

448 val merged = MessageWithParts(info = info, parts = ordered)

452 val merged = MessageWithParts(info = info, parts = ordered)

449 if (merged != existing.view()) {

453 if (merged != existing.view()) {

454+ // Store under `key` (the synthetic key for an empty-id UnknownMessage),

455+ // not `m.info.id` (which is "" for such a message and would collide).

450− messages[m.info.id] = holderOf(merged)

456+ messages[key] = holderOf(merged)

451 changed = true

457 changed = true

452 }

458 }

453 }

459 }

@@ -489,6 +495,18 @@ internal class MessageStore {

489 return changed

495 return changed

490 }

496 }

491

497

498+ /** Resolve the map key for a seed entry. An UnknownMessage with an empty id (from an

499+ * unrecognized server role with no id field) was stored by handleMessageUpdated under a

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

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

502+ * duplicate under key "". */

503+ private fun syntheticKeyFor(m: MessageWithParts): String =

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

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

506+ } else {

507+ m.info.id

508+ }

509+

492 /** Reorder the message map by message creation time (stable). Ties and entries with no

510 /** Reorder the message map by message creation time (stable). Ties and entries with no

493 * server time yet (e.g. a brand-new message still streaming and not in REST) keep their

511 * server time yet (e.g. a brand-new message still streaming and not in REST) keep their

494 * relative order, so such a message stays last. Preserves each entry's map key so the

512 * relative order, so such a message stays last. Preserves each entry's map key so the

app/src/main/java/soy/iko/opencode/ui/chat/Attachments.kt+6−4

@@ -104,12 +104,14 @@ private fun defaultName(mime: String): String {

104}

105

106/**

107− * Create a temp file in the cache and a shareable content [Uri] for it (via the app's

107+ * Create a temp file in the cache and a shareable content [Uri] for it (via the app's

108− * [FileProvider]) to hand to the camera as the capture target. Returns null if it can't be

108+ * [FileProvider]) to hand to the camera as the capture target. Returns null if it can't be

109− * created. The file is overwritten on each capture; the OS cleans the cache dir.

109+ * created. Uses a fixed filename so each capture overwrites the prior (the OS also evicts

110+ * cache under storage pressure); a unique-per-capture timestamped name would leak a .jpg

111+ * per canceled capture in cacheDir/captures/ with no cleanup path.

110 */

112 */

111fun newCameraCaptureUri(context: Context): Uri? = runCatching {

113fun newCameraCaptureUri(context: Context): Uri? = runCatching {

112 val dir = File(context.cacheDir, "captures").apply { mkdirs() }

114 val dir = File(context.cacheDir, "captures").apply { mkdirs() }

113− val file = File(dir, "capture_${System.currentTimeMillis()}.jpg")

115+ val file = File(dir, "capture.jpg")

114 FileProvider.getUriForFile(context, "${context.packageName}.fileprovider", file)

116 FileProvider.getUriForFile(context, "${context.packageName}.fileprovider", file)

115}.getOrNull()

117}.getOrNull()

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

@@ -989,9 +989,16 @@ fun ChatScreen(

989 val searchMessages: List<MessageWithParts> = if (searchQuery.isEmpty()) {

990 messages

991 } else {

992+ // Capture the messages list via rememberUpdatedState so the latest list is read

993+ // when the debounce completes, WITHOUT re-launching the produceState coroutine on

994+ // every messages emission. Keying produceState on `messages` restarts the debounce

995+ // delay from zero on every streamed token (~20×/sec); if tokens arrive faster than

996+ // the debounce interval the filter never runs and search shows "no matches" for the

997+ // entire run. Keying only on `searchQuery` lets the debounce settle once per query

998+ // while still filtering against the most recent list.

999+ val currentMessages by rememberUpdatedState(messages)

992 val filtered by produceState(

1000 val filtered by produceState(

993 initialValue = emptyList(),

1001 initialValue = emptyList(),

994− messages,

995 searchQuery,

1002 searchQuery,

996 ) {

1003 ) {

997 // Debounce: wait for the user to stop typing before running the filter,

1004 // Debounce: wait for the user to stop typing before running the filter,

@@ -1001,7 +1008,7 @@ fun ChatScreen(

1001 // and scans every part of every message, which is non-trivial work for a

1008 // and scans every part of every message, which is non-trivial work for a

1002 // long conversation.

1009 // long conversation.

1003 value = withContext(Dispatchers.Default) {

1010 value = withContext(Dispatchers.Default) {

1004− messages.filter { m ->

1011+ currentMessages.filter { m ->

1005 m.parts.any { p ->

1012 m.parts.any { p ->

1006 when (p) {

1013 when (p) {

1007 is TextPart -> p.text.contains(searchQuery, ignoreCase = true)

1014 is TextPart -> p.text.contains(searchQuery, ignoreCase = true)

app/src/main/java/soy/iko/opencode/ui/chat/ChatViewModel.kt+62−13

@@ -1108,9 +1108,10 @@ class ChatViewModel(

1108 _optimisticMessages.update { entries -> entries.filterNot { it.failed && it.text.trim() == trimmed } }

1109 _optimisticMessages.update { it + OptimisticEntry(optimisticId, trimmed, System.currentTimeMillis(), failed = false) }

1110 }

1111− // Remember the sent text so an undoable Stop can re-send it. Only text-bearing prompts

1111+ // Remember the sent text so an undoable Stop can re-send it. Overwrite unconditionally:

1112− // are tracked (an image-only prompt has nothing to re-send).

1112+ // an image-only prompt has nothing to re-send, and leaving a prior text prompt tracked

1113+ // would make Stop-Undo silently re-send that unrelated earlier message.

1113− if (trimmed.isNotEmpty()) lastSentPrompt = trimmed

1114+ lastSentPrompt = trimmed.takeIf { it.isNotEmpty() }

1114 viewModelScope.launch {

1115 viewModelScope.launch {

1115 runCatchingCancellable {

1116 runCatchingCancellable {

1116 conn.repository.sendPrompt(

1117 conn.repository.sendPrompt(

@@ -1252,9 +1253,13 @@ class ChatViewModel(

1252 // already true, send() returns false and the draft would be lost forever.

1253 // already true, send() returns false and the draft would be lost forever.

1253 // Attachments aren't re-sent on retry (they were optimistically restored to the

1254 // Attachments aren't re-sent on retry (they were optimistically restored to the

1254 // composer on the original failure, so they'll ride the next manual send).

1255 // composer on the original failure, so they'll ride the next manual send).

1256+ // send() accepts the text synchronously, but its onSuccess/onFailure callbacks run

1257+ // asynchronously. Clearing failedIdempotencyKey here would open a window between this

1258+ // synchronous clear and the async onFailure (which re-stashes the key) during which a

1259+ // manual re-send of the same text gets a fresh UUID — defeating server-side deduplication.

1260+ // Let send()'s own callbacks own the key's lifecycle, as they do for non-retry paths.

1255 if (send(draft, includeAttachments = false, idempotencyKey = key)) {

1261 if (send(draft, includeAttachments = false, idempotencyKey = key)) {

1256 _failedDraft.value = null

1262 _failedDraft.value = null

1257− failedIdempotencyKey = null

1258 }

1263 }

1259 }

1264 }

1260

1265

@@ -1372,6 +1377,12 @@ class ChatViewModel(

1372 runCatchingCancellable { withContext(NonCancellable) { conn.repository.abort(sessionId) } }

1377 runCatchingCancellable { withContext(NonCancellable) { conn.repository.abort(sessionId) } }

1373 .onSuccess {

1378 .onSuccess {

1374 _running.value = false

1379 _running.value = false

1380+ // Close the re-light gate so trailing SSE events in flight between the

1381+ // abort POST's ack and the server's SessionIdle don't re-light the run

1382+ // indicator (and re-arm the FGS / keepScreenOn) after the user hit Stop.

1383+ // send()/runCommand()/continueRun() all open this gate on beginRun();

1384+ // abort() is the one run-ending path that previously missed it.

1385+ runEndedByIdle = true

1375 // Surface an undo opportunity so an accidental Stop is recoverable.

1386 // Surface an undo opportunity so an accidental Stop is recoverable.

1376 // The UI shows a "Stopped — Undo" snackbar; tapping Undo re-sends the

1387 // The UI shows a "Stopped — Undo" snackbar; tapping Undo re-sends the

1377 // last prompt. Only when there's a tracked prompt to undo back to.

1388 // last prompt. Only when there's a tracked prompt to undo back to.

@@ -1543,17 +1554,45 @@ class ChatViewModel(

1543 }

1554 }

1544

1555

1545 /** Remove the pending quote-reply preview and strip the `>`-prefixed blockquote lines from

1556 /** Remove the pending quote-reply preview and strip the `>`-prefixed blockquote lines from

1546− * the draft so canceling the quote card actually removes the quoted text, not just the chip. */

1557+ * the draft so canceling the quote card actually removes the quoted text, not just the chip.

1558+ * Strips by leading `> ` prefix (structural match) rather than by exact full-text replace,

1559+ * so a user who edited any quoted line is still cleaned up — the exact-text replaces would

1560+ * silently no-op on an edit, leaving orphan `>` lines in the draft after the chip vanished. */

1547 fun cancelQuoteReply() {

1561 fun cancelQuoteReply() {

1548 val removed = _pendingQuote.value ?: return

1562 val removed = _pendingQuote.value ?: return

1549 _pendingQuote.value = null

1563 _pendingQuote.value = null

1550− val quotedBlock = removed.trim().lineSequence().joinToString("\n") { "> $it" }

1564+ val quoteLineCount = removed.trim().lineSequence().count()

1551− _draft.value = _draft.value

1565+ // Walk the draft and drop the first contiguous run of `> `-prefixed lines that matches

1552− .replace("\n\n$quotedBlock\n\n", "\n\n", ignoreCase = false)

1566+ // the stashed quote's line count (the block quoteReply inserted). Lines the user edited

1553− .replace("$quotedBlock\n\n", "", ignoreCase = false)

1567+ // still begin with `> ` (quoteReply prefixed every line), so a prefix match finds them

1554− .replace("\n\n$quotedBlock", "", ignoreCase = false)

1568+ // even when their trailing content diverges from the stashed original.

1555− .replace(quotedBlock, "", ignoreCase = false)

1569+ val lines = _draft.value.split("\n").toMutableList()

1556− .trimStart()

1570+ var runStart = -1

1571+ var runLen = 0

1572+ var i = 0

1573+ while (i < lines.size) {

1574+ if (lines[i].startsWith("> ") || lines[i] == ">") {

1575+ if (runStart < 0) runStart = i

1576+ runLen++

1577+ if (runLen == quoteLineCount) {

1578+ // Strip the run plus a single trailing blank line that separated the quote

1579+ // from the reply (quoteReply appends "\n\n" after the block).

1580+ val stripUntil = i + 1

1581+ if (stripUntil < lines.size && lines[stripUntil].isEmpty()) {

1582+ lines.subList(runStart, stripUntil + 1).clear()

1583+ } else {

1584+ lines.subList(runStart, stripUntil).clear()

1585+ }

1586+ break

1587+ }

1588+ i++

1589+ } else {

1590+ runStart = -1

1591+ runLen = 0

1592+ i++

1593+ }

1594+ }

1595+ _draft.value = lines.joinToString("\n").trimStart()

1557 }

1596 }

1558

1597

1559 /** Fork the conversation by creating a brand-new session seeded with [text] as its first

1598 /** Fork the conversation by creating a brand-new session seeded with [text] as its first

@@ -1565,8 +1604,10 @@ class ChatViewModel(

1565 val prompt = text.trim()

1604 val prompt = text.trim()

1566 if (prompt.isEmpty()) return

1605 if (prompt.isEmpty()) return

1567 viewModelScope.launch {

1606 viewModelScope.launch {

1607+ var createdId: String? = null

1568 runCatchingCancellable {

1608 runCatchingCancellable {

1569 val session = conn.repository.createSession(title = null)

1609 val session = conn.repository.createSession(title = null)

1610+ createdId = session.id

1570 conn.repository.sendPrompt(

1611 conn.repository.sendPrompt(

1571 session.id,

1612 session.id,

1572 prompt,

1613 prompt,

@@ -1576,7 +1617,15 @@ class ChatViewModel(

1576 session.id

1617 session.id

1577 }

1618 }

1578 .onSuccess { newId -> _branchEvents.trySend(newId) }

1619 .onSuccess { newId -> _branchEvents.trySend(newId) }

1620+ .onFailure {

1621+ // If the session was created but the prompt failed, delete the orphaned

1622+ // empty session so failed branches don't accumulate on the server.

1623+ val id = createdId

1624+ if (id != null) {

1625+ runCatchingCancellable { conn.repository.deleteSession(id) }

1626+ }

1579− .onFailure { _errorEvents.trySend(ChatError(container.string(R.string.branch_failed))) }

1627+ _errorEvents.trySend(ChatError(container.string(R.string.branch_failed)))

1628+ }

1580 }

1629 }

1581 }

1630 }

1582

1631

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

@@ -66,9 +66,15 @@ class TtsController(context: Context) : RememberObserver {

66 // The chunked text for the currently-speaking utterance, plus the index of the chunk

67 // currently (or about to be) playing. Tracked so pause()/resume() can re-enqueue the

68 // tail without re-splitting the text. Cleared when playback ends or is stopped.

69+ // currentChunk is written from onStart (a binder thread) and read from pause()/resume()

70+ // on the main thread; without @Volatile the main thread could read a stale value and

71+ // resume from the wrong chunk, replaying audio the user already heard. pausedFromChunk

72+ // is derived from currentChunk in pause() and read in resume(), so it carries the same

73+ // visibility requirement. chunks is only mutated on the main thread (start/stop/clear),

74+ // so it doesn't need @Volatile.

69 private var chunks: List<String> = emptyList()

75 private var chunks: List<String> = emptyList()

70− private var currentChunk = 0

76+ @Volatile private var currentChunk = 0

71− private var pausedFromChunk = 0

77+ @Volatile private var pausedFromChunk = 0

73 private val tts: TextToSpeech = TextToSpeech(context) { status ->

79 private val tts: TextToSpeech = TextToSpeech(context) { status ->

74 ready = status == TextToSpeech.SUCCESS

80 ready = status == TextToSpeech.SUCCESS

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

@@ -928,7 +928,17 @@ class SessionListViewModel(private val container: AppContainer) : ViewModel() {

928 },

929 onError = { err ->

930 viewModelScope.launch {

931+ // Restore the session since the delete failed (or there was no connection

932+ // to delete on), matching deleteSession's onError. Without this the row

933+ // stays optimistically hidden forever — a manual refresh is the only way

934+ // to see it again, and with no connection a refresh won't happen. Guard

935+ // against a concurrent refresh having already re-added it so we never

936+ // produce two rows with the same id.

931 pendingDeletes.remove(session.id)

937 pendingDeletes.remove(session.id)

938+ _state.update { s ->

939+ s.copy(sessions = (s.sessions.filterNot { it.id == session.id } + session)

940+ .sortedByMode(s.sortMode, s.sortDescending))

941+ }

932 _transientErrors.tryEmit(container.friendlyError(err))

942 _transientErrors.tryEmit(container.friendlyError(err))

933 }

943 }

934 },

944 },

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

@@ -162,7 +162,16 @@ fun DiagnosticsScreen(onBack: () -> Unit) {

162 // collector withdraws the previously-pending report's scheduled delete when a newer one

163 // supersedes its snackbar, keeping every delete undoable for its whole window.

164 val reportDeleteEvents = remember {

165+ // Buffer capacity matches the snackbar event buffer (NetworkConfig.snackbarEventBufferCapacity).

166+ // With extraBufferCapacity = 1 + DROP_OLDEST, a rapid 3x delete can drop the MIDDLE

167+ // event's snackbar while its scheduleDelete has already fired — silently deleting a

168+ // report with no Undo. A larger buffer lets all pending deletes queue behind the

169+ // collectLatest collector (which cancels the prior snackbar and withdraws its pending

170+ // delete), so every delete stays undoable for its whole window.

171+ MutableSharedFlow<String>(

172+ extraBufferCapacity = NetworkConfig.snackbarEventBufferCapacity,

165− MutableSharedFlow<String>(extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST)

173+ onBufferOverflow = BufferOverflow.DROP_OLDEST,

174+ )

166 }

175 }

167 LaunchedEffect(Unit) {

176 LaunchedEffect(Unit) {

168 var pending: String? = null

177 var pending: String? = null