4459e94 · opencode-android

git.iko.soy/opencode-android

Fix 9 bugs: backup cross-version import, mDNS marker leak, idempotency reuse, follow-up duplicate, unknown message coalesce, file refresh blank, markdown fence leak, provider default selection, code text extraction

4459e94iko3 months ago

app/src/main/java/soy/iko/opencode/data/model/Provider.kt+5−1

compare with master

unifiedsplit

@@ -73,7 +73,11 @@ fun ProvidersResponse.toOptions(): List<ModelOption> =

74/** The default option to preselect: the server's default for some provider, else the first model. */

75fun ProvidersResponse.defaultOption(options: List<ModelOption> = toOptions()): ModelOption? {

76+ // Try each configured provider default in turn and take the first whose model actually

77+ // survives in `options` (the list may be filtered). Checking only the first `default` entry

78+ // would mispreselect an arbitrary first option whenever that one provider's default model

79+ // isn't present, ignoring the other configured defaults.

76− default.entries.firstOrNull()?.let { (providerID, modelID) ->

80+ for ((providerID, modelID) in default) {

77 options.firstOrNull { it.providerID == providerID && it.modelID == modelID }?.let { return it }

81 options.firstOrNull { it.providerID == providerID && it.modelID == modelID }?.let { return it }

78 }

82 }

79 return options.firstOrNull()

83 return options.firstOrNull()

app/src/main/java/soy/iko/opencode/data/network/NsdDiscovery.kt+13−4

@@ -71,10 +71,14 @@ open class NsdDiscovery(context: Context?) {

71 // timeout, one stuck resolve would hang this single serialized resolver forever,

72 // so every later-discovered service queued behind it is never processed and the

73 // emitted list silently stops updating. Abandon a stuck resolve and keep draining.

74− val resolved = withTimeoutOrNull(RESOLVE_TIMEOUT_MS) { resolveService(nsd, info) } ?: continue

74+ val resolved = withTimeoutOrNull(RESOLVE_TIMEOUT_MS) { resolveService(nsd, info) }

75− // The service was reported lost while queued behind other resolves: don't

75+ // Consume any "lost while queued" marker for this name REGARDLESS of the resolve

76− // resurrect it. Clear the marker so the name is free for a future re-appearance.

76+ // outcome, and skip resurrecting a service reported lost. If the resolve failed or

77+ // timed out (e.g. the service really is gone), an early `?: continue` on the line

78+ // above would skip this and leak the marker — which would then wrongly suppress the

79+ // SAME name's next successful resolution when the server restarts and re-announces

80+ // (onServiceLost won't fire again to clear it). Also drop a null resolve outright.

77− if (lostPending.remove(discoveredName)) continue

81+ if (lostPending.remove(discoveredName) || resolved == null) continue

78 val name = resolved.serviceName.orEmpty()

82 val name = resolved.serviceName.orEmpty()

79 // Keep only opencode's own advertisements, not every _http._tcp service

83 // Keep only opencode's own advertisements, not every _http._tcp service

80 // (printers, routers, other web servers) on the network.

84 // (printers, routers, other web servers) on the network.

@@ -82,6 +86,11 @@ open class NsdDiscovery(context: Context?) {

82 @Suppress("DEPRECATION")

86 @Suppress("DEPRECATION")

83 val host = resolved.host?.hostAddress ?: continue

87 val host = resolved.host?.hostAddress ?: continue

84 found[discoveredName] = DiscoveredServer(name = name, host = host, port = resolved.port)

88 found[discoveredName] = DiscoveredServer(name = name, host = host, port = resolved.port)

89+ // Close the check-then-insert race: an onServiceLost that fired in the non-suspending

90+ // window since the marker check above couldn't remove this name from `found` (nothing

91+ // was inserted yet), so it set a lostPending marker instead. Re-check and evict now,

92+ // else the just-departed service lingers as a phantom onServiceLost won't clear.

93+ if (lostPending.remove(discoveredName)) found.remove(discoveredName)

85 trySend(found.values.sortedBy { it.name })

94 trySend(found.values.sortedBy { it.name })

86 }

95 }

87 }

96 }

app/src/main/java/soy/iko/opencode/data/repo/BackupManager.kt+11−6

@@ -22,12 +22,17 @@ data class BackupServer(

22/** App preferences in a backup. */

23@Serializable

24data class BackupSettings(

25+ // Every field is defaulted (to SettingsStore's own defaults) so a backup written by an older

26+ // app version — one whose settings block predates a field added later — still decodes. Without

27+ // defaults a missing field throws MissingFieldException in import() BEFORE the server-restore

28+ // loop, aborting the ENTIRE restore (servers/pins/archives included) despite the best-effort

29+ // contract. ignoreUnknownKeys only tolerates EXTRA keys, not missing required ones.

25− val themeMode: String,

30+ val themeMode: String = ThemeMode.SYSTEM.name,

26− val dynamicColor: Boolean,

31+ val dynamicColor: Boolean = true,

27− val sendOnEnter: Boolean,

32+ val sendOnEnter: Boolean = true,

28− val appLock: Boolean,

33+ val appLock: Boolean = false,

29− val chatTextScale: Float,

34+ val chatTextScale: Float = SettingsStore.DEFAULT_CHAT_TEXT_SCALE,

30− val codeWrap: Boolean,

35+ val codeWrap: Boolean = false,

31)

36)

33/** The full backup document. [version] guards against future format changes. */

38/** The full backup document. [version] guards against future format changes. */

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

@@ -387,9 +387,6 @@ internal class MessageStore {

387 messageInfoUpdatedSincePivot.clear()

388 }

389

390− /** Monotonic counter for synthetic UnknownMessage keys, avoiding nanoTime collisions. */

391− private val unknownCounter = java.util.concurrent.atomic.AtomicLong(0)

392−

393 /** Maximum number of messages to keep in memory; oldest are evicted when exceeded. */

390 /** Maximum number of messages to keep in memory; oldest are evicted when exceeded. */

394 internal val maxMessages = NetworkConfig.maxInMemoryMessages

391 internal val maxMessages = NetworkConfig.maxInMemoryMessages

395

392

@@ -535,10 +532,13 @@ internal class MessageStore {

535 val info = event.properties.info

532 val info = event.properties.info

536 if (info.sessionID != sessionId) return false

533 if (info.sessionID != sessionId) return false

537 // An UnknownMessage with an empty id (e.g. from an unrecognized server role with no id

534 // An UnknownMessage with an empty id (e.g. from an unrecognized server role with no id

538− // field) would collide with other such messages in the map. Generate a unique synthetic

535+ // field) would collide with other such messages in the map. Derive a synthetic key from

536+ // the message's own content (it carries no id, only sessionID/time) so each distinct

539− // key so each unknown message gets its own entry instead of overwriting others.

537+ // unknown message gets its own entry, while a byte-identical re-send (a redundant event or

538+ // a post-reconnect re-seed) coalesces via the existing==info guard below instead of piling

539+ // up duplicate holders — which a fresh per-event counter caused (existing was always null).

540 val key = if (info.id.isEmpty() && info is UnknownMessage) {

541− "unknown-${unknownCounter.incrementAndGet()}"

541+ "unknown-${info.hashCode()}"

542 } else {

543 info.id

544 }

app/src/main/java/soy/iko/opencode/ui/chat/ChatViewModel.kt+27−4

@@ -775,11 +775,21 @@ class ChatViewModel(

775 // A new run is starting: reopen the re-light gate so this run's streamed parts keep the

776 // indicator lit (and a later SessionIdle re-closes it).

777 runEndedByIdle = false

778+ // A plain manual Send of the auto-restored failed draft must reuse the stashed key too

779+ // (retryFailed passes it explicitly; the composer's Send does not), so a lost-response

780+ // re-send stays deduplicated server-side instead of starting a duplicate agent run.

781+ // Capture the match BEFORE clearing _failedDraft just below.

782+ val reuseKey = if (idempotencyKey == null && trimmed == _failedDraft.value?.trim()) {

783+ failedIdempotencyKey

784+ } else {

785+ null

786+ }

778 _failedDraft.value = null

787 _failedDraft.value = null

779 // Stable idempotency key for THIS online attempt: a brand-new message gets a fresh key,

788 // Stable idempotency key for THIS online attempt: a brand-new message gets a fresh key,

789+ // while retryFailed() (or a manual re-send of the restored failed draft) re-submits with the

780− // while retryFailed() re-submits with the key it stashed on the original failure — so a

790+ // key it stashed on the original failure — so a retry after a lost response is deduplicated

781− // retry after a lost response is deduplicated server-side (no duplicate agent run).

791+ // server-side (no duplicate agent run).

782− val key = idempotencyKey ?: java.util.UUID.randomUUID().toString()

792+ val key = idempotencyKey ?: reuseKey ?: java.util.UUID.randomUUID().toString()

783 // Clear the staged attachments optimistically so the composer empties immediately;

793 // Clear the staged attachments optimistically so the composer empties immediately;

784 // restore them if the send fails (mirrors the draft handling below).

794 // restore them if the send fails (mirrors the draft handling below).

785 if (attachments.isNotEmpty()) _attachments.value = emptyList()

795 if (attachments.isNotEmpty()) _attachments.value = emptyList()

@@ -822,6 +832,9 @@ class ChatViewModel(

822 _running.value = false

832 _running.value = false

823 }.onSuccess {

833 }.onSuccess {

824 suppressDraftPersist.set(false)

834 suppressDraftPersist.set(false)

835+ // This attempt reached the server, so the stashed retry key is spent — drop it so a

836+ // later unrelated send can't reuse it (retryFailed clears it on success too).

837+ failedIdempotencyKey = null

825 // Send succeeded — the attachments rode along, so clear their persisted copy

838 // Send succeeded — the attachments rode along, so clear their persisted copy

826 // (or keep any the user staged after the optimistic clear).

839 // (or keep any the user staged after the optimistic clear).

827 persistAttachments()

840 persistAttachments()

@@ -946,7 +959,17 @@ class ChatViewModel(

946 // The text now lives in the queued chip, so clear the input field just as a

959 // The text now lives in the queued chip, so clear the input field just as a

947 // normal send would. No-op for the cancel case (blank text), which must leave

960 // normal send would. No-op for the cancel case (blank text), which must leave

948 // whatever the user has since typed untouched.

961 // whatever the user has since typed untouched.

949− if (trimmed.isNotEmpty()) _draft.value = ""

962+ if (trimmed.isNotEmpty()) {

963+ _draft.value = ""

964+ // Persist the cleared draft IMMEDIATELY, not just in memory: the follow-up was already

965+ // persisted synchronously above, so if the process is killed before the debounced

966+ // draft-persist fires, the same text would remain under BOTH the draft and follow-up

967+ // keys — and cold-start recovery would restore it to the composer AND re-queue it (a

968+ // duplicate). Mirror send()'s lastPersistedDraft update so the drafts observer treats

969+ // this store change as our own echo and doesn't write the cleared value back.

970+ lastPersistedDraft = ""

971+ container.draftStore.flushDraft(sessionId, "")

972+ }

950 }

973 }

951

974

952 /** Set or clear the queued follow-up and mirror it to persistence, so a follow-up

975 /** Set or clear the queued follow-up and mirror it to persistence, so a follow-up

app/src/main/java/soy/iko/opencode/ui/components/MarkdownText.kt+8−4

@@ -274,10 +274,14 @@ internal fun extractFenceLanguage(raw: String, isFenced: Boolean): String? {

274 */

275internal fun extractCodeText(raw: String, isFenced: Boolean): String {

276 if (!isFenced) return raw.trimIndent()

277− val lines = raw.lines()

278− val body = lines.drop(1).toMutableList()

277+ val body = raw.lines().drop(1).toMutableList()

278+ // Strip the closing fence. getTextInNode can include a trailing newline, making the true last

279+ // line "" — so locate the last NON-blank line and drop from there, rather than only checking

280+ // body.last() (which would miss the fence in that case and leak the closing ```/~~~ into the

281+ // displayed/copied code).

282+ val lastNonBlank = body.indexOfLast { it.isNotBlank() }

279− if (body.isNotEmpty() && (body.last().startsWith("```") || body.last().startsWith("~~~"))) {

283+ if (lastNonBlank >= 0 && (body[lastNonBlank].startsWith("```") || body[lastNonBlank].startsWith("~~~"))) {

280− body.removeAt(body.lastIndex)

284+ while (body.size > lastNonBlank) body.removeAt(body.lastIndex)

281 }

285 }

282 return body.joinToString("\n").trimEnd()

286 return body.joinToString("\n").trimEnd()

283}

287}

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

@@ -147,7 +147,11 @@ class FileBrowserViewModel(private val container: AppContainer) : ViewModel() {

147 textResults = emptyList(),

148 symbolResults = emptyList(),

149 searching = false,

150− loading = true,

150+ // Keep the current listing visible during a same-directory refresh (pull-to-refresh)

151+ // instead of blanking it behind a full-screen spinner; only show the spinner when

152+ // there's nothing to display yet or we're navigating to a DIFFERENT path (where the

153+ // existing entries belong to the old directory and shouldn't linger).

154+ loading = it.entries.isEmpty() || it.path != path,

151 error = null,

155 error = null,

152 ) }

156 ) }

153 openJob?.cancel()

157 openJob?.cancel()