db959e1 · opencode-android

git.iko.soy/opencode-android

Fix StepFinishPart pinning run indicator, stale deep-link on deleted profile, null-session removal leak, and image auth divergence

db959e1iko3 months ago

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

compare with master

unifiedsplit

@@ -13,6 +13,7 @@ import soy.iko.opencode.data.model.ModelRef

13import soy.iko.opencode.data.model.Part

14import soy.iko.opencode.data.model.SessionError

15import soy.iko.opencode.data.model.SessionIdle

16+import soy.iko.opencode.data.model.StepFinishPart

16import soy.iko.opencode.data.model.UnknownMessage

17import soy.iko.opencode.data.model.UnknownMessage

17import soy.iko.opencode.data.network.EventStreamClient

18import soy.iko.opencode.data.network.EventStreamClient

18import soy.iko.opencode.data.network.NetworkConfig

19import soy.iko.opencode.data.network.NetworkConfig

@@ -329,7 +330,17 @@ open class SessionRepository(

329 // The session id rides on the part for message.part.updated; properties.sessionID

330 // The session id rides on the part for message.part.updated; properties.sessionID

330 // is null on the wire. Mirror reduce()'s `part.sessionID ?: properties.sessionID`

331 // is null on the wire. Mirror reduce()'s `part.sessionID ?: properties.sessionID`

331 // so a run that's only streaming parts still re-lights the indicator on reconnect.

332 // so a run that's only streaming parts still re-lights the indicator on reconnect.

333+ // A step-finish part is the trailing completion marker for a run (it carries the

334+ // final cost/token totals) and arrives after the last stream delta. Treating it as

335+ // live activity would re-light the running indicator after a reconnect when the run

336+ // already finished during the outage — with no following SessionIdle to clear it,

337+ // pinning the spinner (and the foreground service) indefinitely. Mirrors

338+ // AppContainer.isLiveRunActivity's exclusion.

339+ is MessagePartUpdated -> {

340+ val part = event.properties.part

341+ part !is StepFinishPart &&

332− is MessagePartUpdated -> (event.properties.part.sessionID ?: event.properties.sessionID) == sessionId

342+ (part.sessionID ?: event.properties.sessionID) == sessionId

343+ }

333 is MessageUpdated -> {

344 is MessageUpdated -> {

334 val info = event.properties.info

345 val info = event.properties.info

335 info.sessionID == sessionId && info is AssistantMessage && !info.isComplete && info.error == null

346 info.sessionID == sessionId && info is AssistantMessage && !info.isComplete && info.error == null

@@ -404,11 +415,15 @@ internal class MessageStore {

404

415

405 fun seed(initial: List<MessageWithParts>, prune: Boolean = false, clearPivot: Boolean = true): Boolean {

416 fun seed(initial: List<MessageWithParts>, prune: Boolean = false, clearPivot: Boolean = true): Boolean {

406 var changed = false

417 var changed = false

407− // On re-seed (after SSE reconnect), remove messages that are no longer in the

418+ // seed() does NOT prune by default. Pruning is intentionally NOT wired into the

408− // server snapshot (e.g. deleted during the disconnection gap). This keeps the

419+ // reconnect re-seed (the only path that would want it): the live SSE stream — via

409− // in-memory state in sync with the authoritative REST snapshot rather than

420+ // MessageRemoved events — drives deletions, and pruneStaleCacheSeeded() handles

410− // accumulating stale messages forever. Skipped on the initial seed to avoid

421+ // ghosts from the on-disk cache on restart. Pruning on re-seed would race with

411− // racing with just-arrived SSE events whose messages may not yet be in REST.

422+ // just-arrived SSE events whose messages may not yet be indexed by REST, dropping

423+ // live messages. The `prune = true` branch below is retained as a tested capability

424+ // for callers that can guarantee no concurrent SSE (e.g. a future explicit

425+ // "discard and reload from REST" entry point); the current production re-seed

426+ // callsite passes `prune = false` for the race reason above.

412 if (prune) {

427 if (prune) {

413 // Build the retain-set from the synthetic key (matching the map's keying for

428 // Build the retain-set from the synthetic key (matching the map's keying for

414 // UnknownMessages with empty ids) so an entry stored under "unknown-c<created>"

429 // UnknownMessages with empty ids) so an entry stored under "unknown-c<created>"

@@ -596,20 +611,28 @@ internal class MessageStore {

596 return true

611 return true

597 }

612 }

598

613

599− /** Returns false if [eventSession] is non-null and doesn't match [sessionId]. */

600− private fun matchesSession(eventSession: String?, sessionId: String): Boolean =

601− eventSession == null || eventSession == sessionId

602−

603 private fun handlePartRemoved(sessionId: String, event: MessagePartRemoved): Boolean {

614 private fun handlePartRemoved(sessionId: String, event: MessagePartRemoved): Boolean {

604 val messageId = event.properties.messageID ?: return false

615 val messageId = event.properties.messageID ?: return false

605 val partId = event.properties.partID ?: return false

616 val partId = event.properties.partID ?: return false

606− if (!matchesSession(event.properties.sessionID, sessionId)) return false

617+ val eventSession = event.properties.sessionID

618+ when {

619+ eventSession != null && eventSession != sessionId -> return false

620+ // Null session id: only act if this store already holds the parent message, so a

621+ // null-session part removal can't drop a part from an unrelated session's store.

622+ eventSession == null && !messages.containsKey(messageId) -> return false

623+ }

607 return removePart(messageId, partId)

624 return removePart(messageId, partId)

608 }

625 }

609

626

610 private fun handleMessageRemoved(sessionId: String, event: MessageRemoved): Boolean {

627 private fun handleMessageRemoved(sessionId: String, event: MessageRemoved): Boolean {

611 val id = event.properties.messageID ?: return false

628 val id = event.properties.messageID ?: return false

612− if (!matchesSession(event.properties.sessionID, sessionId)) return false

629+ val eventSession = event.properties.sessionID

630+ when {

631+ eventSession != null && eventSession != sessionId -> return false

632+ // Null session id: only act if this store already holds the message (message ids

633+ // are globally unique, so this attributes the removal to the owning session only).

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

635+ }

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

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

614 messageInfoUpdatedSincePivot.remove(id)

637 messageInfoUpdatedSincePivot.remove(id)

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

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

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

@@ -285,7 +285,17 @@ fun OpencodeApp(container: AppContainer) {

285 // whichever is active. The effect re-runs when connection changes (keyed on it),

286 // so on the second run the profile matches and the open proceeds.

287 if (pending.profileId != null && connection?.profile?.id != pending.profileId) {

288+ // If the originating profile no longer exists (e.g. deleted after the

289+ // notification was queued), connectByProfileId returns false: the active

290+ // connection is unchanged and none of this effect's keys change, so it

291+ // would never re-run — the request would sit in _pendingOpenSession

292+ // forever and re-trigger on every later connection change. Consume it so

293+ // the stale request is dropped, mirroring the invalid-deep-link path in

294+ // MainActivity.

288− if (!container.connectByProfileId(pending.profileId)) return@LaunchedEffect

295+ if (!container.connectByProfileId(pending.profileId)) {

296+ container.consumePendingOpenSession()

297+ return@LaunchedEffect

298+ }

289 return@LaunchedEffect // re-run on the new connection

299 return@LaunchedEffect // re-run on the new connection

290 }

300 }

291 // In two-pane mode the detail pane (TwoPaneSessionChat, hosted on the SESSIONS

301 // In two-pane mode the detail pane (TwoPaneSessionChat, hosted on the SESSIONS

app/src/main/java/soy/iko/opencode/ui/components/RemoteImage.kt+6−2

@@ -82,8 +82,12 @@ data class ImageLoadContext(

83/** Build an [ImageLoadContext] from a resolved connection profile (password included). */

84fun ServerProfile.toImageContext(): ImageLoadContext {

85+ // Mirror HttpClientFactory's auth condition (hasAuth, i.e. a non-blank username): a

86+ // profile configured with a username but an empty password is still authenticated by the

87+ // REST/SSE channel (password defaults to the empty string there), so image loads must

88+ // attach the same Basic header or they'd 401 while every other request succeeds.

85− val auth = if (hasAuth && !password.isNullOrEmpty()) {

89+ val auth = if (hasAuth) {

86− val raw = "$username:$password".toByteArray()

90+ val raw = "$username:${password.orEmpty()}".toByteArray()

87 "Basic " + Base64.encodeToString(raw, Base64.NO_WRAP)

91 "Basic " + Base64.encodeToString(raw, Base64.NO_WRAP)

88 } else {

92 } else {

89 null

93 null