8b41e1c · opencode-android

git.iko.soy/opencode-android

Fix 18 bugs: reconnect cache wipe on failed re-seed, file deep-link leading slash, foreground notification stale fields/thread-safety, unrevert/revert retry hazards, nullable BusEvent info props + consumers, search back-handler, go-to-line recomposition race, and 10 smaller correctness fixes (wrap-toggle keying, ErrorKind redirect branch, SHA-256 Stop request code, runCatchingCancellable in probe finally, CrashLogger undo race, evicted-message late parts, DraftStore shutdown drain, RecentSessionsStore atomic fallback)

8b41e1ciko3 months ago

app/src/main/java/soy/iko/opencode/MainActivity.kt+12−3

compare with master

unifiedsplit

@@ -369,9 +369,18 @@ class MainActivity : FragmentActivity() {

369 }

370 }

371 "file" -> {

372− // Reconstruct the path from the path segments after "file", preserving internal

372+ // Reconstruct the path from the URI. `file` is the URI *host* (authority), so

373− // slashes. Decoded + validated against traversal/control characters.

373+ // data.path is the path AFTER the authority — e.g. `opencode://file/foo/bar.txt`

374+ // yields data.path = "/foo/bar.txt", which begins with a leading slash that the

375+ // server's file endpoints don't expect (the in-app browser uses server-returned

376+ // FileNode.path values with no leading slash, e.g. "foo/bar.txt"). Strip the

377+ // single leading slash, then percent-decode and validate against traversal/control

378+ // characters. (Using pathSegments would also work but over-decodes `+` and other

379+ // characters that aren't percent-encoded in real workspace paths.)

374− val rawPath = data.path?.removePrefix("/file/")?.let { android.net.Uri.decode(it) }

380+ val rawPath = data.path?.let { p ->

381+ val stripped = if (p.startsWith("/")) p.substring(1) else p

382+ android.net.Uri.decode(stripped)

383+ }

375 if (!openFileHandled) {

384 if (!openFileHandled) {

376 rawPath?.takeIf { isValidFilePath(it) }

385 rawPath?.takeIf { isValidFilePath(it) }

377 ?.let { container.requestOpenFile(it); openFileHandled = true } ?: run {

386 ?.let { container.requestOpenFile(it); openFileHandled = true } ?: run {

app/src/main/java/soy/iko/opencode/data/model/BusEvent.kt+2−2

@@ -22,7 +22,7 @@ data object ServerConnected : BusEvent

22@SerialName("message.updated")

23data class MessageUpdated(val properties: Props) : BusEvent {

24 @Serializable

25− data class Props(val info: MessageInfo)

25+ data class Props(val info: MessageInfo? = null)

26}

28@Serializable

@@ -78,7 +78,7 @@ data class SessionError(val properties: Props) : BusEvent {

78@SerialName("session.updated")

79data class SessionUpdated(val properties: Props) : BusEvent {

80 @Serializable

81− data class Props(val info: Session)

81+ data class Props(val info: Session? = null, val sessionID: String? = null)

82}

84@Serializable

app/src/main/java/soy/iko/opencode/data/network/OpencodeApiClient.kt+22−6

@@ -216,18 +216,34 @@ open class OpencodeApiClient private constructor(

216 }

217

218 /** Revert the session to just before [messageId] (optionally part [partId]); the

219− * server hides everything after the checkpoint. Returns the updated session. Naturally

219+ * server hides everything after the checkpoint. Returns the updated session.

220− * idempotent (reverting to the same point twice yields the same state). */

220+ *

221+ * NOT retried (single attempt), mirroring [abort]: a lost response followed by a retry

222+ * would POST revert again, and if a new run started in the gap the retry would hide

223+ * that new run's messages too (until the user noticed and called [unrevert]). There's

224+ * no Idempotency-Key the server could dedupe on, so a single attempt is the safe

225+ * choice — the SSE stream reflects the actual revert state regardless. */

221− open suspend fun revert(sessionId: String, messageId: String, partId: String? = null): Session = withRetry {

226+ open suspend fun revert(sessionId: String, messageId: String, partId: String? = null): Session = withRetry(maxAttempts = 1) {

222 client!!.post("session/${encode(sessionId)}/revert") {

227 client!!.post("session/${encode(sessionId)}/revert") {

223 contentType(ContentType.Application.Json)

228 contentType(ContentType.Application.Json)

224 setBody(RevertRequest(messageID = messageId, partID = partId))

229 setBody(RevertRequest(messageID = messageId, partID = partId))

225 }.body()

230 }.body()

226 }

231 }

227

232

228− /** Undo the active revert checkpoint, restoring the hidden messages. */

233+ /** Undo the active revert checkpoint, restoring the hidden messages.

234+ *

235+ * Mint an Idempotency-Key *before* withRetry so a retried POST (first request reached

236+ * the server but its response was lost) doesn't trigger a second unrevert on a session

237+ * whose checkpoint was already cleared — the second attempt would hit a session with no

238+ * active revert and the server could return a non-retryable 4xx, surfacing a spurious

239+ * error for an operation that actually succeeded. The key lets the server dedupe. */

229− open suspend fun unrevert(sessionId: String): Session = withRetry {

240+ open suspend fun unrevert(sessionId: String): Session {

241+ val idempotencyKey = java.util.UUID.randomUUID().toString()

242+ return withRetry {

230− client!!.post("session/${encode(sessionId)}/unrevert").body()

243+ client!!.post("session/${encode(sessionId)}/unrevert") {

244+ header("Idempotency-Key", idempotencyKey)

245+ }.body()

246+ }

231 }

247 }

232

248

233 /** Create a public share link for the session. Returns the session with [Session.share] set.

249 /** Create a public share link for the session. Returns the session with [Session.share] set.

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

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

130 /** Deferred deletions keyed by file name, so an Undo can cancel one before it fires. */

131 private val pendingDeletes = java.util.concurrent.ConcurrentHashMap<String, Job>()

132

133+ /** Files whose scheduled delete has already claimed (passed its delay and removed its

134+ * map entry) and is about to delete or has just deleted. [cancelScheduledDelete]

135+ * consults this to honor its contract: once a delete has fired, undo returns false

136+ * even if a reschedule installed a newer job in the map in the gap between the claim

137+ * and the cancel. Without this, the cancel would remove the newer job and report

138+ * "undo succeeded" while the older job's delete proceeds — contradicting the user

139+ * who tapped Undo and the Diagnostics screen that then shows the file gone. */

140+ private val claimedDeletes = java.util.concurrent.ConcurrentHashMap<String, Boolean>()

141+

133 /**

142 /**

134 * Schedule [fileName] for deletion after [delayMs], cancellable via

143 * Schedule [fileName] for deletion after [delayMs], cancellable via

135 * [cancelScheduledDelete]. Runs on the logger's own long-lived scope — not the

144 * [cancelScheduledDelete]. Runs on the logger's own long-lived scope — not the

@@ -149,11 +158,20 @@ class CrashLogger private constructor(private val appContext: Context) {

149 // remove(name, thisJob) also no-ops for a stale job after a reschedule.

158 // remove(name, thisJob) also no-ops for a stale job after a reschedule.

150 val claimed = coroutineContext[Job]?.let { pendingDeletes.remove(fileName, it) } == true

159 val claimed = coroutineContext[Job]?.let { pendingDeletes.remove(fileName, it) } == true

151 if (!claimed) return@launch

160 if (!claimed) return@launch

161+ // Mark this delete as fired so a cancel that arrives AFTER the claim (but before

162+ // the file.delete() completes) honestly reports "already fired" instead of

163+ // "undo succeeded". A reschedule that installed a newer job in the map in that

164+ // gap would otherwise make cancelScheduledDelete remove the newer job and return

165+ // true, while this coroutine still deletes — violating the documented contract.

166+ claimedDeletes[fileName] = true

152 val file = File(crashDir, fileName).canonicalFile

167 val file = File(crashDir, fileName).canonicalFile

153 if (file.path.startsWith(crashDir.canonicalPath + File.separator)) {

168 if (file.path.startsWith(crashDir.canonicalPath + File.separator)) {

154 file.delete()

169 file.delete()

155 }

170 }

156 refresh()

171 refresh()

172+ // Clear the fired marker after the delete + refresh so a future scheduleDelete

173+ // for the same name (e.g. a re-created report) starts with a clean slate.

174+ claimedDeletes.remove(fileName)

157 }

175 }

158 pendingDeletes.put(fileName, job)?.cancel()

176 pendingDeletes.put(fileName, job)?.cancel()

159 }

177 }

@@ -161,6 +179,9 @@ class CrashLogger private constructor(private val appContext: Context) {

161 /** Cancel a pending deferred delete (the Undo action). Returns true if it was still

179 /** Cancel a pending deferred delete (the Undo action). Returns true if it was still

162 * pending (undo succeeded), false if the delete had already fired. */

180 * pending (undo succeeded), false if the delete had already fired. */

163 fun cancelScheduledDelete(fileName: String): Boolean {

181 fun cancelScheduledDelete(fileName: String): Boolean {

182+ // If the delete has already claimed (passed its delay), the undo is too late —

183+ // report false even if a reschedule installed a newer job that's still in the map.

184+ if (claimedDeletes.containsKey(fileName)) return false

164 val job = pendingDeletes.remove(fileName) ?: return false

185 val job = pendingDeletes.remove(fileName) ?: return false

165 job.cancel()

186 job.cancel()

166 return true

187 return true

app/src/main/java/soy/iko/opencode/data/repo/DraftStore.kt+13−2

@@ -278,7 +278,18 @@ open class DraftStore private constructor(

278 open fun shutdown() {

279 flushExecutor.shutdown()

280 // Await pending flushDraft writes so drafts aren't lost if the process exits

281− // immediately after shutdown (e.g. ANR-triggered process kill).

281+ // immediately after shutdown (e.g. ANR-triggered process kill). Loop until the

282+ // executor is truly idle (or 5s elapses) rather than a single 2s wait: a slow disk

283+ // with a deep queue of keystroke-triggered flushes can exceed 2s, and dropping the

284+ // tail would lose the user's most recent edits. The JVM shutdown hook's own deadline

285+ // is the hard limit; this loop just gives the executor the best chance to drain.

286+ var waitedMs = 0L

287+ while (waitedMs < 5000L) {

288+ if (flushExecutor.isTerminated) break

289+ runCatching {

282− runCatching { flushExecutor.awaitTermination(2, java.util.concurrent.TimeUnit.SECONDS) }

290+ flushExecutor.awaitTermination(500, java.util.concurrent.TimeUnit.MILLISECONDS)

291+ }

292+ waitedMs += 500L

293+ }

283 }

294 }

284}

295}

app/src/main/java/soy/iko/opencode/data/repo/ErrorKind.kt+4−1

@@ -51,8 +51,11 @@ fun classifyError(t: Throwable): ErrorKind {

51 is SocketTimeoutException, is HttpRequestTimeoutException ->

52 return ErrorKind.TIMEOUT

53 is ServerResponseException -> return ErrorKind.SERVER

54− is ClientRequestException -> return ErrorKind.CLIENT

54+ // RedirectResponseException extends ClientRequestException, so it must be checked

55+ // first: a 3xx response is semantically distinct from a 4xx, and matching the

56+ // parent class first would make this branch unreachable dead code.

55 is RedirectResponseException -> return ErrorKind.CLIENT

57 is RedirectResponseException -> return ErrorKind.CLIENT

58+ is ClientRequestException -> return ErrorKind.CLIENT

56 is IOException -> return ErrorKind.NETWORK

59 is IOException -> return ErrorKind.NETWORK

57 else -> {

60 else -> {

58 // Any other ResponseException (unrecognized status family).

61 // Any other ResponseException (unrecognized status family).

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

@@ -80,11 +80,17 @@ object RecentSessionsStore {

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.

83+ // Write directly to target, accepting a brief non-atomic window (a

84− target.delete()

84+ // concurrent reader could see a partially-written file). The previous

85− tmp2.renameTo(target)

85+ // delete-then-rename here was worse: between the delete and the rename

86+ // a reader saw NO file (empty recents), and if the rename failed too,

87+ // target was left missing and tmp2 orphaned. writeText at least keeps

88+ // either the old or the new content visible, never a gap. Clean up

89+ // both temps afterwards.

90+ target.writeText(encoded)

86 }

91 }

87 tmp.delete()

92 tmp.delete()

93+ tmp2.delete()

88 }

94 }

89 }

95 }

90 }

96 }

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

@@ -221,28 +221,37 @@ open class SessionRepository(

221 store.beginReseed()

222 ++seedGeneration

223 }

224− val fresh = runCatchingCancellable { api.listMessages(sessionId) }

224+ val freshResult = runCatchingCancellable { api.listMessages(sessionId) }

225 .onFailure { Log.w("SessionRepository", "Re-seed message load failed for $sessionId; relying on SSE: ${safeExceptionSummary(it)}") }

226− .getOrDefault(emptyList())

226+ val fresh = freshResult.getOrNull()

227 var seedChanged = false

228 lock.withLock {

229 if (generation == seedGeneration) {

230+ // Only prune when the fetch SUCCEEDED. A failed fetch yields

231+ // fresh == null; treating null as an empty REST snapshot would

232+ // make pruneStaleCacheSeeded remove EVERY cache-seeded message

233+ // (none are in the empty retain-set), wiping the whole cached

234+ // conversation on a transient network error — the cache is the

235+ // only thing keeping the session readable until SSE repopulates.

236+ var pruned = false

237+ if (fresh != null) {

230− // Prune cache-seeded ghosts here too, not only in the initial

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

231− // REST seed. If the SSE stream connected→dropped→reconnected

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

232− // before the initial listMessages returned, seedGeneration was

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

233− // already >0 by the time the initial seed ran, so its prune was

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

234− // skipped (the `seedGeneration == 0` guard below). Without this

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

235− // prune, messages deleted server-side while the app was closed

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

236− // would survive as ghosts until a future app restart where the

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

237− // timing happened not to recur.

245+ // timing happened not to recur.

238− val pruned = store.pruneStaleCacheSeeded(cacheSeededIds, fresh)

246+ pruned = store.pruneStaleCacheSeeded(cacheSeededIds, fresh)

247+ }

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

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

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

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

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

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

242 // Stale messages from deletions during the disconnect

251 // Stale messages from deletions during the disconnect

243 // gap are eventually removed via MessageRemoved events

252 // gap are eventually removed via MessageRemoved events

244 // or on the next app restart.

253 // or on the next app restart.

245− val seeded = store.seed(fresh, prune = false)

254+ val seeded = if (fresh != null) store.seed(fresh, prune = false) else false

246 seedChanged = pruned || seeded

255 seedChanged = pruned || seeded

247 }

256 }

248 }

257 }

@@ -349,7 +358,7 @@ open class SessionRepository(

349 (part.sessionID ?: event.properties.sessionID) == sessionId

358 (part.sessionID ?: event.properties.sessionID) == sessionId

350 }

359 }

351 is MessageUpdated -> {

360 is MessageUpdated -> {

352− val info = event.properties.info

361+ val info = event.properties.info ?: return false

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

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

354 }

363 }

355 else -> false

364 else -> false

@@ -441,6 +450,10 @@ internal class MessageStore {

441 messages.keys.retainAll(incomingKeys)

450 messages.keys.retainAll(incomingKeys)

442 if (messages.size != before) changed = true

451 if (messages.size != before) changed = true

443 }

452 }

453+ // An authoritative seed supersedes any eviction record: a message re-introduced by

454+ // this seed is back in memory, so a later MessagePartUpdated should update it, not

455+ // be dropped as "late for an evicted message". Clearing keeps the set bounded too.

456+ evictedMessageIds.clear()

444

457

445 for (m in initial) {

458 for (m in initial) {

446 val key = syntheticKeyFor(m)

459 val key = syntheticKeyFor(m)

@@ -621,7 +634,7 @@ internal class MessageStore {

621 }

634 }

622

635

623 private fun handleMessageUpdated(sessionId: String, event: MessageUpdated): Boolean {

636 private fun handleMessageUpdated(sessionId: String, event: MessageUpdated): Boolean {

624− val info = event.properties.info

637+ val info = event.properties.info ?: return false

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

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

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

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

627 // field) would collide with other such messages in the map. It carries no id, only

640 // field) would collide with other such messages in the map. It carries no id, only

@@ -640,6 +653,10 @@ internal class MessageStore {

640 // A server re-sending byte-identical info is a no-op: skip it so we don't publish a

653 // A server re-sending byte-identical info is a no-op: skip it so we don't publish a

641 // redundant snapshot (mirrors upsertPart's identical-part guard).

654 // redundant snapshot (mirrors upsertPart's identical-part guard).

642 if (existing != null && existing.info == info) return false

655 if (existing != null && existing.info == info) return false

656+ // A MessageUpdated re-introduces the message into the store (it was either evicted

657+ // by evictOldMessages or genuinely new). Drop any eviction record so a following

658+ // MessagePartUpdated updates this holder instead of being dropped as "late".

659+ info.id.takeIf { it.isNotEmpty() }?.let { evictedMessageIds.remove(it) }

643 if (existing == null) {

660 if (existing == null) {

644 messages[key] = Holder(info, LinkedHashMap())

661 messages[key] = Holder(info, LinkedHashMap())

645 } else {

662 } else {

@@ -685,6 +702,10 @@ internal class MessageStore {

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

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

686 }

703 }

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

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

705+ // The message is gone from the store; drop any eviction record for it so the set

706+ // doesn't retain ids of long-deleted messages. (A re-seed would also clear this,

707+ // but a quiet session that never reconnects could otherwise accumulate entries.)

708+ removed.info.id.takeIf { it.isNotEmpty() }?.let { evictedMessageIds.remove(it) }

688 // Derive the same key handleMessageUpdated would have stored this holder under. For an

709 // Derive the same key handleMessageUpdated would have stored this holder under. For an

689 // UnknownMessage with an empty id, that's the synthetic "unknown-c<created>" /

710 // UnknownMessage with an empty id, that's the synthetic "unknown-c<created>" /

690 // "unknown-h<hash>" key (not the raw empty id) — removing the raw id here would leave

711 // "unknown-h<hash>" key (not the raw empty id) — removing the raw id here would leave

@@ -712,6 +733,13 @@ internal class MessageStore {

712 current.parts[part.id] = part

733 current.parts[part.id] = part

713 current.invalidate()

734 current.invalidate()

714 } else {

735 } else {

736+ // A late part update for a message that was evicted by evictOldMessages() (the

737+ // store caps at maxMessages) would otherwise re-insert the message with only

738+ // this single part — a partial, contextless bubble at the end of the list. The

739+ // evicted message's other parts are gone from memory, so re-inserting can't

740+ // restore them; ignore the late part instead. (A later MessageUpdated for the

741+ // same id — which always carries full info — re-creates the holder correctly.)

742+ if (evictedMessageIds.contains(messageId)) return false

715 val parts = LinkedHashMap<String, Part>()

743 val parts = LinkedHashMap<String, Part>()

716 parts[part.id] = part

744 parts[part.id] = part

717 messages[messageId] = Holder(

745 messages[messageId] = Holder(

@@ -739,6 +767,15 @@ internal class MessageStore {

739 return true

767 return true

740 }

768 }

741

769

770+ /** Message ids evicted by [evictOldMessages] (capped at [maxMessages]). A late

771+ * [MessagePartUpdated] for one of these would otherwise re-insert the message under

772+ * [upsertPart] with only the late part — a partial, contextless bubble at the end of

773+ * the list. Membership here tells upsertPart to drop the late part instead. Bounded by

774+ * the same cap (entries are removed when the message is re-seeded via [seed] or removed

775+ * via [handleMessageRemoved]); a long-lived session that cycles past maxMessages many

776+ * times never grows this beyond the most recent eviction batch. */

777+ private val evictedMessageIds = mutableSetOf<String>()

778+

742 /** Evict the oldest messages when the store exceeds [maxMessages], keeping memory bounded.

779 /** Evict the oldest messages when the store exceeds [maxMessages], keeping memory bounded.

743 * Returns true if any messages were evicted. */

780 * Returns true if any messages were evicted. */

744 private fun evictOldMessages(): Boolean {

781 private fun evictOldMessages(): Boolean {

@@ -746,6 +783,13 @@ internal class MessageStore {

746 while (messages.size > maxMessages) {

783 while (messages.size > maxMessages) {

747 val oldestKey = messages.keys.iterator().next()

784 val oldestKey = messages.keys.iterator().next()

748 val removed = messages.remove(oldestKey)

785 val removed = messages.remove(oldestKey)

786+ // Record the evicted message's id so a late MessagePartUpdated for it is dropped

787+ // instead of re-inserting a single-part bubble (see upsertPart). Also clear any

788+ // prior eviction record for this id from an older cycle (the message re-entered

789+ // via seed/MessageUpdated and has now been evicted again).

790+ removed?.info?.id?.takeIf { it.isNotEmpty() }?.let { id ->

791+ evictedMessageIds.add(id)

792+ }

749 // Prune the evicted message's pivot bookkeeping so these sets don't grow

793 // Prune the evicted message's pivot bookkeeping so these sets don't grow

750 // unbounded over a long-running session that never reconnects (they're only

794 // unbounded over a long-running session that never reconnects (they're only

751 // otherwise cleared on reseed/seed).

795 // otherwise cleared on reseed/seed).

app/src/main/java/soy/iko/opencode/di/AppContainer.kt+9−5

@@ -870,7 +870,7 @@ open class AppContainer private constructor(

870 // to clear it — pinning anyRunActive true and the foreground notification indefinitely.

871 // Mirrors SessionRepository.isRunActivity's guard exactly.

872 if (event is MessageUpdated) {

873− val info = event.properties.info

873+ val info = event.properties.info ?: return false

874 return info is AssistantMessage && !info.isComplete && info.error == null

875 }

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

@@ -1473,7 +1473,10 @@ open class AppContainer private constructor(

1473 val status = responseStatusCode(e)

1474 if (status == 401 || status == 403) false else throw e

1475 } finally {

1476+ // runCatchingCancellable (not plain runCatching): this is a suspend finally, and

1477+ // plain runCatching swallows CancellationException — if the caller's scope is

1478+ // cancelled while client.close() is in flight, the cancellation must propagate.

1476− runCatching { client.close() }

1479+ runCatchingCancellable { client.close() }

1477 }

1480 }

1478 }

1481 }

1479

1482

@@ -1493,7 +1496,8 @@ open class AppContainer private constructor(

1493 ProbeResult.Unreachable(friendlyErrorFor(e, profile.baseUrl))

1496 ProbeResult.Unreachable(friendlyErrorFor(e, profile.baseUrl))

1494 }

1497 }

1495 } finally {

1498 } finally {

1499+ // runCatchingCancellable: see probeWithCredentials for the rationale.

1496− runCatching { client.close() }

1500+ runCatchingCancellable { client.close() }

1497 }

1501 }

1498 }

1502 }

1499

1503

@@ -1541,7 +1545,7 @@ open class AppContainer private constructor(

1541 * [Context] (the surrounding [AppContainer] needs one).

1545 * [Context] (the surrounding [AppContainer] needs one).

1542 */

1546 */

1543internal fun sessionOfEvent(event: BusEvent): String? = when (event) {

1547internal fun sessionOfEvent(event: BusEvent): String? = when (event) {

1544− is MessageUpdated -> event.properties.info.sessionID

1548+ is MessageUpdated -> event.properties.info?.sessionID

1545 is MessagePartUpdated -> event.properties.part.sessionID ?: event.properties.sessionID

1549 is MessagePartUpdated -> event.properties.part.sessionID ?: event.properties.sessionID

1546 else -> null

1550 else -> null

1547}

1551}

@@ -1552,7 +1556,7 @@ internal fun sessionOfEvent(event: BusEvent): String? = when (event) {

1552 * message id (those fall back to being counted individually).

1556 * message id (those fall back to being counted individually).

1553 */

1557 */

1554internal fun messageIdOfEvent(event: BusEvent): String? = when (event) {

1558internal fun messageIdOfEvent(event: BusEvent): String? = when (event) {

1555− is MessageUpdated -> event.properties.info.id

1559+ is MessageUpdated -> event.properties.info?.id

1556 is MessagePartUpdated -> event.properties.messageID ?: event.properties.part.messageID

1560 is MessagePartUpdated -> event.properties.messageID ?: event.properties.part.messageID

1557 else -> null

1561 else -> null

1558}

1562}

app/src/main/java/soy/iko/opencode/notification/RunForegroundService.kt+59−13

@@ -16,6 +16,7 @@ import androidx.core.app.NotificationManagerCompat

16import androidx.core.app.ServiceCompat

17import soy.iko.opencode.MainActivity

18import soy.iko.opencode.R

19+import java.security.MessageDigest

20/**

21/**

21 * A foreground service kept alive while an opencode agent is actively running. Holding a

22 * A foreground service kept alive while an opencode agent is actively running. Holding a

@@ -37,7 +38,12 @@ class RunForegroundService : Service() {

37 // recreated service instance and leave the chronometer pinned to a previous run's start

38 // recreated service instance and leave the chronometer pinned to a previous run's start

38 // if onDestroy hadn't run yet when the next run's first intent arrived. As an instance

39 // if onDestroy hadn't run yet when the next run's first intent arrived. As an instance

39 // field it's reset by construction on every new Service instance.

40 // field it's reset by construction on every new Service instance.

41+ //

42+ // @Volatile: onStartCommand/onDestroy write these on the main thread, but updateProgress

43+ // (reached via the companion start() from AppContainer.observeRunForegroundService, which

44+ // runs on Dispatchers.IO) reads them. Without @Volatile there's no happens-before edge,

45+ // so a 64-bit Long (runStartMillis) can be torn and the booleans/strings can be stale.

40− private var runStartMillis: Long = 0L

46+ @Volatile private var runStartMillis: Long = 0L

41 // Whether *this* service instance has successfully called startForeground. Tracked so

47 // Whether *this* service instance has successfully called startForeground. Tracked so

42 // progress-driven re-starts (which re-enter onStartCommand) don't re-call startForeground

48 // progress-driven re-starts (which re-enter onStartCommand) don't re-call startForeground

43 // and re-bind the 5s obligation each time — that would let a backgrounded re-start throw

49 // and re-bind the 5s obligation each time — that would let a backgrounded re-start throw

@@ -45,14 +51,14 @@ class RunForegroundService : Service() {

45 // foreground. After the first successful startForeground, progress updates refresh the

51 // foreground. After the first successful startForeground, progress updates refresh the

46 // notification via NotificationManager.notify directly (see [updateProgress]), which

52 // notification via NotificationManager.notify directly (see [updateProgress]), which

47 // never throws the background-start exception.

53 // never throws the background-start exception.

48− private var isInForeground: Boolean = false

54+ @Volatile private var isInForeground: Boolean = false

49 // The most recent session title/id/profile/progress handed to onStartCommand, retained so

55 // The most recent session title/id/profile/progress handed to onStartCommand, retained so

50 // [updateProgress] can rebuild the notification with a new progress string without a fresh

56 // [updateProgress] can rebuild the notification with a new progress string without a fresh

51 // startForegroundService intent (which can throw when backgrounded).

57 // startForegroundService intent (which can throw when backgrounded).

52− private var lastTitle: String? = null

58+ @Volatile private var lastTitle: String? = null

53− private var lastSessionId: String? = null

59+ @Volatile private var lastSessionId: String? = null

54− private var lastProfileId: String? = null

60+ @Volatile private var lastProfileId: String? = null

55− private var lastProgress: String? = null

61+ @Volatile private var lastProgress: String? = null

57 @android.annotation.SuppressLint("MissingPermission")

63 @android.annotation.SuppressLint("MissingPermission")

58 override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {

64 override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {

@@ -113,19 +119,33 @@ class RunForegroundService : Service() {

113 * re-entering [onStartCommand] (and therefore without calling [Context.startForegroundService],

119 * re-entering [onStartCommand] (and therefore without calling [Context.startForegroundService],

114 * which can throw [android.app.ForegroundServiceStartNotAllowedException] on Android 12+ when

120 * which can throw [android.app.ForegroundServiceStartNotAllowedException] on Android 12+ when

115 * the app is backgrounded — even if the service is already in the foreground). Called directly

121 * the app is backgrounded — even if the service is already in the foreground). Called directly

116− * by the companion [updateProgress] when a progress refresh is needed while the service is

122+ * by the companion [start] when a progress refresh is needed while the service is already

117− * already running; falls back to [start] (which goes through startForegroundService) when the

123+ * running; falls back to [start] (which goes through startForegroundService) when the

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

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

125+ *

126+ * Forwards the full set of run fields (title/sessionId/profileId/progress), not just the

127+ * progress string, so a late-resolved session title or a second run starting after the

128+ * first updates the notification text AND the Stop action's target session/profile.

129+ * Without this, the Stop action would keep targeting the originally-captured session even

130+ * after a profile switch (mirroring the SessionNotifications abort-routing guard). */

119 @SuppressLint("MissingPermission")

131 @SuppressLint("MissingPermission")

120− private fun updateProgress(progress: String?) {

132+ private fun updateProgress(

133+ progress: String?,

134+ sessionTitle: String? = lastTitle,

135+ sessionId: String? = lastSessionId,

136+ profileId: String? = lastProfileId,

137+ ) {

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

138 // 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)

139 // 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

140 // 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

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

125 // and avoids touching NotificationManager during teardown.

142 // and avoids touching NotificationManager during teardown.

126 if (!isInForeground) return

143 if (!isInForeground) return

144+ lastTitle = sessionTitle

145+ lastSessionId = sessionId

146+ lastProfileId = profileId

127 lastProgress = progress

147 lastProgress = progress

128− val notification = buildNotification(lastTitle, lastSessionId, lastProfileId, progress)

148+ val notification = buildNotification(sessionTitle, sessionId, profileId, progress)

129 runCatching {

149 runCatching {

130 NotificationManagerCompat.from(this).notify(NOTIF_ID, notification)

150 NotificationManagerCompat.from(this).notify(NOTIF_ID, notification)

131 }.onFailure { Log.w(TAG, "Failed to update foreground notification progress", it) }

151 }.onFailure { Log.w(TAG, "Failed to update foreground notification progress", it) }

@@ -181,8 +201,15 @@ class RunForegroundService : Service() {

181 putExtra(NotificationActionReceiver.EXTRA_SESSION_ID, sessionId)

201 putExtra(NotificationActionReceiver.EXTRA_SESSION_ID, sessionId)

182 profileId?.let { putExtra(NotificationActionReceiver.EXTRA_PROFILE_ID, it) }

202 profileId?.let { putExtra(NotificationActionReceiver.EXTRA_PROFILE_ID, it) }

183 }

203 }

204+ // Derive the request code from a SHA-256 of the session id (truncated to 31 bits),

205+ // NOT String.hashCode(): two distinct session ids can share a 32-bit hashCode, and

206+ // with FLAG_UPDATE_CURRENT the second registration overwrites the first's extras —

207+ // so a hash collision between two concurrent runs would make tapping Stop on one

208+ // session's notification broadcast the OTHER session's id. Mirrors SessionNotifications.

209+ // notifId's collision-resistant derivation.

210+ val requestCode = stopRequestCode(sessionId)

184 val stopPending = PendingIntent.getBroadcast(

211 val stopPending = PendingIntent.getBroadcast(

185− this, sessionId.hashCode(), stopIntent,

212+ this, requestCode, stopIntent,

186 PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE,

213 PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE,

187 )

214 )

188 builder.addAction(

215 builder.addAction(

@@ -263,6 +290,21 @@ class RunForegroundService : Service() {

263 private const val TAG = "RunForegroundService"

290 private const val TAG = "RunForegroundService"

264 private const val NOTIF_ID = 1

291 private const val NOTIF_ID = 1

265 private const val NOTIF_TIMEOUT_ID = 2

292 private const val NOTIF_TIMEOUT_ID = 2

293+

294+ /** Derive a stable, collision-resistant PendingIntent request code from a session id.

295+ * String.hashCode() can collide for distinct session ids; a SHA-256 truncated to 31

296+ * bits makes accidental collisions astronomically unlikely, so two concurrent runs'

297+ * Stop actions can't overwrite each other's PendingIntent extras. Mirrors

298+ * [SessionNotifications.notifId]'s derivation. */

299+ private fun stopRequestCode(sessionId: String): Int {

300+ val digest = runCatching { MessageDigest.getInstance("SHA-256") }

301+ .getOrNull()?.digest(sessionId.toByteArray()) ?: return sessionId.hashCode()

302+ val hash = (((digest[0].toInt() and 0xFF) shl 24) or

303+ ((digest[1].toInt() and 0xFF) shl 16) or

304+ ((digest[2].toInt() and 0xFF) shl 8) or

305+ (digest[3].toInt() and 0xFF)).and(0x7FFFFFFF)

306+ return hash

307+ }

266 // Brand accent for the foreground notifications' small-icon tint circle. Resolved

308 // Brand accent for the foreground notifications' small-icon tint circle. Resolved

267 // from resources (R.color.notif_brand) so it follows the app theme (light/dark)

309 // from resources (R.color.notif_brand) so it follows the app theme (light/dark)

268 // instead of a hardcoded constant. Lazily computed on first use from the app context

310 // instead of a hardcoded constant. Lazily computed on first use from the app context

@@ -304,7 +346,11 @@ class RunForegroundService : Service() {

304 // notify never throws, so progress refreshes always land while the service runs.

346 // notify never throws, so progress refreshes always land while the service runs.

305 val live = instance

347 val live = instance

306 if (live != null) {

348 if (live != null) {

349+ // Forward the full set of run fields (not just progress) so a late-resolved

350+ // session title or a second run updates the notification text AND the Stop

351+ // action's target session/profile — otherwise the Stop action would keep

352+ // targeting the originally-captured session even after a profile switch.

307− live.updateProgress(progress)

353+ live.updateProgress(progress, sessionTitle, sessionId, profileId)

308 return

354 return

309 }

355 }

310 // startForegroundService can throw ForegroundServiceStartNotAllowedException

356 // startForegroundService can throw ForegroundServiceStartNotAllowedException

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

@@ -547,7 +547,17 @@ fun ChatScreen(

547 }

548 }

549

550+ // The in-conversation search bar is an AnimatedVisibility overlay (not a Dialog/Dropdown),

551+ // so it has no built-in back handling. Close it first on back press, before the exit

552+ // confirmation or navigation-away can fire — standard Android "back closes the topmost

553+ // overlay" behavior. Declared before the exit-confirm BackHandler so it takes precedence

554+ // when both could be active (search open + a run in progress).

555+ BackHandler(enabled = searchActive) {

556+ searchActive = false

557+ chatSearch = ""

558+ }

559+

550− BackHandler(enabled = running && !showModelPicker && !showAgentPicker && !showCommandPicker) { showExitConfirm = true }

560+ BackHandler(enabled = running && !showModelPicker && !showAgentPicker && !showCommandPicker && !searchActive) { showExitConfirm = true }

551

561

552 Scaffold(

562 Scaffold(

553 // Hardware-keyboard shortcuts (tablets / DeX / Chromebooks): Ctrl+K opens the command

563 // Hardware-keyboard shortcuts (tablets / DeX / Chromebooks): Ctrl+K opens the command

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

@@ -957,14 +957,15 @@ class ChatViewModel(

957 if (event.properties.sessionID == sessionId) enqueuePermission(event.properties)

958 is PermissionReplied ->

959 if (event.properties.sessionID == sessionId) event.properties.permissionID?.let { resolvePermission(it) }

960− is SessionUpdated ->

960+ is SessionUpdated -> {

961+ val info = event.properties.info

961− if (event.properties.info.id == sessionId) {

962+ if (info != null && info.id == sessionId) {

962− val info = event.properties.info

963 _sessionTitle.value = info.displayTitle

964 _reverted.value = info.isReverted

965 _revertDiff.value = info.revert?.diff?.takeIf { it.isNotBlank() }

966 _shareUrl.value = info.share?.url?.takeIf { it.isNotBlank() }

967 }

968+ }

968 is SessionDeleted ->

969 is SessionDeleted ->

969 if (event.properties.info?.id == sessionId || event.properties.sessionID == sessionId) {

970 if (event.properties.info?.id == sessionId || event.properties.sessionID == sessionId) {

970 _sessionDeleted.value = true

971 _sessionDeleted.value = true

app/src/main/java/soy/iko/opencode/ui/file/FileViewScreen.kt+14−2

@@ -163,8 +163,11 @@ fun FileViewScreen(

163 var matchPos by rememberSaveable(path) { mutableIntStateOf(0) }

164 // Seed the per-file wrap toggle from the user's global code-wrap preference so the viewer

165 // respects it by default; they can still flip it per-file (persisted across rotation).

166+ // Key on BOTH path and codeWrap so a change to the global preference while the file is

167+ // open re-seeds the toggle — without the codeWrap key, rememberSaveable(path) would keep

168+ // the old value and ignore the new preference until the file were closed and reopened.

166 val codeWrap = LocalCodeWrap.current

169 val codeWrap = LocalCodeWrap.current

167− var wrap by rememberSaveable(path) { mutableStateOf(codeWrap) }

170+ var wrap by rememberSaveable(path, codeWrap) { mutableStateOf(codeWrap) }

168 val listState = rememberLazyListState()

171 val listState = rememberLazyListState()

169 val rawText = state.content?.content.orEmpty()

172 val rawText = state.content?.content.orEmpty()

170 // Directory portion of the path, shown under the filename in the top bar so a user who

173 // Directory portion of the path, shown under the filename in the top bar so a user who

@@ -279,7 +282,16 @@ fun FileViewScreen(

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

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

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

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

281 highlightLine.value = null

284 highlightLine.value = null

285+ scope.launch {

286+ // When the render limit was just expanded above, the LazyColumn hasn't recomposed

287+ // yet (Dispatchers.Main.immediate runs this body before recomposition), so

288+ // animateScrollToItem would clamp to the last currently-rendered item. A short

289+ // delay lets the state change propagate and the LazyColumn measure the expanded

290+ // item count before the scroll runs. Mirrors rememberJumpToLineHighlight's

291+ // identical race fix. The delay is harmless when no expansion was needed.

292+ kotlinx.coroutines.delay(1)

282− scope.launch { runCatchingCancellable { listState.animateScrollToItem(target) } }

293+ runCatchingCancellable { listState.animateScrollToItem(target) }

294+ }

283 }

295 }

284 Scaffold(

296 Scaffold(

285 topBar = {

297 topBar = {

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

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

444 conn.events.events.collect { event ->

445 when (event) {

446 is SessionUpdated -> {

447− val session = event.properties.info

447+ val session = event.properties.info ?: return@collect

448 // Ignore updates for a session the user just deleted (still

449 // within its undo window): the server hasn't deleted it yet

450 // and can legitimately emit updates, but buffering them here