From 82570302802bff515f929c77cb5dde7beff321a5 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 21:57:36 -0400 Subject: [PATCH 1/2] Never let a dropped event stream close the app Both screens that follow a stream retried an `ApiException` and let everything else through, and `Sse.run` opened its connection on a line outside the `try` that maps failures onto that type. So a failure at open time, or anything the framing did not expect, reached the top of the app and closed it -- from a screen whose own comment says failures there are deliberately quiet, because the listing already carries every state the stream would have brought. The open moves inside the guarded region, and both loops now retry on any exception while rethrowing `CancellationException`, which is the screen leaving rather than a failure -- swallowing that one would leave the loop reconnecting to a stream nobody is watching. This is hardening on the path that runs when a screen with a stream opens, not a diagnosed fix: an import list loading against a server missing the events route, and against 121 real transcripts, does not crash here. Co-Authored-By: Claude Opus 5 --- .../main/kotlin/com/example/aiapp/ImportScreen.kt | 12 +++++++++++- .../main/kotlin/com/example/aiapp/SessionScreen.kt | 11 +++++++++-- .../src/main/kotlin/com/example/aiapp/Sse.kt | 13 ++++++++++--- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt index af9297a..9f4fc7f 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt @@ -323,8 +323,18 @@ fun ImportScreen(settings: ServerSettings, reloadToken: Int, onImported: (Sessio } } } - } catch (_: ApiException) { + } catch (e: kotlinx.coroutines.CancellationException) { + // The screen leaving, not a failure -- and swallowing it would leave this + // loop reconnecting to a stream nobody is watching. + throw e + } catch (_: Exception) { // Retried below; the listing is the truth in the meantime. + // + // Any failure, not only an [ApiException]. A stream is an optimisation over + // the listing here, so nothing it can do is worth taking the app down for -- + // and catching only the failure that was expected means an unexpected one + // reaches the top of the app and closes it, from a screen that is merely + // loading a list. } finally { stream.close() } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index ce673f8..e523c0d 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -718,8 +718,15 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () apply(entry) } } - } catch (e: ApiException) { - streamError = e.message + } catch (e: kotlinx.coroutines.CancellationException) { + // Leaving the screen or going below STARTED. Not a failure, and + // swallowing it would leave this loop reconnecting forever. + throw e + } catch (e: Exception) { + // Any failure, not only an [ApiException]: the stream reconnects from its + // cursor, so there is nothing a failure here can cost that is worth + // closing the app over. Reported on the screen either way. + streamError = e.message ?: e::class.simpleName } finally { stream.close() } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Sse.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Sse.kt index 1e1897f..bbfbd16 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Sse.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Sse.kt @@ -47,9 +47,16 @@ class Sse(private val settings: ServerSettings) { * recovered from, indefinitely. */ fun run(path: String, onOpen: () -> Unit, onFrame: (name: String?, data: String) -> Unit) { - val connection = URL("${settings.baseUrl}$path").openConnection() as HttpURLConnection - this.connection = connection + // Opening is inside the try, not before it. Everything this method can fail at owes the + // caller the same kind of failure -- both callers retry an [ApiException] and let anything + // else reach the top of the app -- and a connection that could not even be constructed + // used to escape as a raw `IOException` from a line no `catch` covered. + var connection: HttpURLConnection? = null try { + connection = + (URL("${settings.baseUrl}$path").openConnection() as HttpURLConnection).also { + this.connection = it + } connection.applyPinnedTls() connection.connectTimeout = CONNECT_TIMEOUT_MS // No read timeout: between events there is nothing to read for as long as the thing @@ -90,7 +97,7 @@ class Sse(private val settings: ServerSettings) { ) } } finally { - connection.disconnect() + connection?.disconnect() this.connection = null } } From 7a8811aab3ab661f23fddc4e7d64a0ef59293596 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 22:12:49 -0400 Subject: [PATCH 2/2] Offer a session once when two project folders hold it, and never crash on a repeat Resuming a Claude Code session from a different working directory makes the CLI write a second transcript with the same id under that directory's project folder. This machine has one: 160 KB under `-home-bob-repos-tdep-survey` and a 614-byte stub under `-home-bob-repos-tdep`. Everything downstream addresses a session by id -- `--resume` takes it, the delete glob resolves it, the in-flight registry is keyed on it -- so two rows sharing an id are two rows no operation can tell apart, and the phone keys its list on it, so scrolling to them closed the app on Compose's duplicate-key throw. The listing now keeps the copy with the most in it. Size rather than recency, because the stub is often the newer of the two, and picking it describes the session by the wrong size, the wrong cwd and the wrong title. Deleting removes every copy rather than stopping at the first, which had left the row to come back on the next listing after a delete that reported success. The phone's half is `uniqueItems`: every list keyed on a server-chosen id goes through it, since none of them could rule the repeat out locally and a data problem must not be able to close the app. Verified both ways against the real duplicate -- the unguarded build reproduces the reported stack on the same id, the guarded one scrolls the whole list. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 14 +++++ .../kotlin/com/example/aiapp/ImportScreen.kt | 3 +- .../kotlin/com/example/aiapp/ModelsScreen.kt | 8 +-- .../com/example/aiapp/SessionListScreen.kt | 3 +- .../kotlin/com/example/aiapp/SetupsScreen.kt | 3 +- .../kotlin/com/example/aiapp/UniqueItems.kt | 44 ++++++++++++++ server/src/session/import.rs | 60 ++++++++++++++++++- 7 files changed, 124 insertions(+), 11 deletions(-) create mode 100644 app/androidApp/src/main/kotlin/com/example/aiapp/UniqueItems.kt diff --git a/AGENTS.md b/AGENTS.md index e9c8a95..bdf14db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -255,6 +255,20 @@ first if a remote spawn ever mangles an argument. be there to import again" is exactly the one the switch makes false. The server deletes the machine's copy *first*, so a machine it cannot reach leaves the session where it was instead of half-deleted. +- **One Claude Code session id can name two files, and the listing offers + it once.** Resuming a session from a different working directory makes + the CLI write a second transcript with the same id under that + directory's project folder -- an ordinary state of a machine, not + corruption. Everything downstream addresses a session by id (`--resume`, + the delete glob, the in-flight registry) and the phone keyed its list on + it, so two rows sharing one *closed the app* on a Compose duplicate-key + throw. `parse_listing` keeps the copy with the most lines, because the + other is usually a few-hundred-byte stub and is often the *newer* of the + two -- so recency is the wrong key. Deleting removes every copy rather + than the first, or the row came back after a delete that reported + success. The phone's half is `uniqueItems`, which every list keyed on a + server-chosen id goes through: a repeat there must never be able to + close the app, whatever produced it. - **A markdown table wraps its cells and never cuts one off.** The renderer's own defaults draw every cell at one line with an ellipsis, which on a phone loses most of a table -- and an elided cell looks diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt index 9f4fc7f..d1cfd65 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt @@ -14,7 +14,6 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items import androidx.compose.material3.AlertDialog import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults @@ -551,7 +550,7 @@ private fun ImportableList( Modifier.fillMaxSize(), contentPadding = PaddingValues(bottom = bottomInset), ) { - items(state.value, key = { it.id }) { session -> + uniqueItems(state.value, key = { it.id }) { session -> val picked = session.id in selected BusyItem(label = running[session.id]) { Card( diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ModelsScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ModelsScreen.kt index 43e2816..234fc9a 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ModelsScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ModelsScreen.kt @@ -8,7 +8,6 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items import androidx.compose.material3.Card import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.LinearProgressIndicator @@ -114,7 +113,8 @@ fun ModelsScreen(settings: ServerSettings, reloadToken: Int) { is LoadState.Loaded -> { if (current.value.downloads.isNotEmpty()) { item { SectionLabel("Downloading") } - items(current.value.downloads, key = { it.key + it.run }) { download -> + uniqueItems(current.value.downloads, key = { it.key + it.run }) { download + -> DownloadCard(download) { scope.launch { actionError = @@ -139,7 +139,7 @@ fun ModelsScreen(settings: ServerSettings, reloadToken: Int) { ) } } - items(current.value.local, key = { it.key }) { model -> + uniqueItems(current.value.local, key = { it.key }) { model -> LocalModelCard(model) { scope.launch { actionError = @@ -164,7 +164,7 @@ fun ModelsScreen(settings: ServerSettings, reloadToken: Int) { is LoadState.Error -> item { Text(found.message, color = MaterialTheme.colorScheme.error) } is LoadState.Loaded -> - items(found.value, key = { it.id }) { repo -> + uniqueItems(found.value, key = { it.id }) { repo -> val open = openRepo == repo.id RepoRow(repo, expanded = open) { if (open) { diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt index d0476c8..f24bfa8 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt @@ -12,7 +12,6 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items import androidx.compose.material3.AlertDialog import androidx.compose.material3.Card import androidx.compose.material3.CircularProgressIndicator @@ -117,7 +116,7 @@ fun SessionListScreen( .thenByDescending { it.lastActivity } ) LazyColumn { - items(ordered, key = { it.id }) { session -> + uniqueItems(ordered, key = { it.id }) { session -> SessionCard( session = session, error = deleteErrors[session.id], diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SetupsScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SetupsScreen.kt index c85da27..ace30b0 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SetupsScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SetupsScreen.kt @@ -8,7 +8,6 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items import androidx.compose.material3.AlertDialog import androidx.compose.material3.Card import androidx.compose.material3.CircularProgressIndicator @@ -83,7 +82,7 @@ fun SetupsScreen(settings: ServerSettings, reloadToken: Int) { is LoadState.Error -> Text(current.message, color = MaterialTheme.colorScheme.error) is LoadState.Loaded -> LazyColumn(Modifier.fillMaxSize()) { - items(current.value, key = { it.id }) { setup -> + uniqueItems(current.value, key = { it.id }) { setup -> SetupCard( setup = setup, onRename = { renaming = setup }, diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/UniqueItems.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/UniqueItems.kt new file mode 100644 index 0000000..d53df3b --- /dev/null +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/UniqueItems.kt @@ -0,0 +1,44 @@ +package com.example.aiapp + +import androidx.compose.foundation.lazy.LazyItemScope +import androidx.compose.foundation.lazy.LazyListScope +import androidx.compose.foundation.lazy.items +import androidx.compose.runtime.Composable + +/** + * Keyed [items], with anything repeating a key already used left out. + * + * A lazy list throws when two of its items claim the same key, and the throw happens during measure + * on the main thread -- so it is not an error the screen can show, it closes the app. That is a + * disproportionate answer to a list with a repeat in it, and it lands on the reader rather than on + * whoever produced the repeat: on 2026-08-31 the import list crashed on a Claude Code session id + * recorded under two project directories, which is an ordinary state of a machine and not something + * the phone did. + * + * Every list in this app keyed on an id keyed it on an id *the server chose*, so all of them shared + * the hazard and none of them could rule it out locally. Hence one function they all go through + * rather than a `distinctBy` remembered at each call site. + * + * Dropping the repeat is the right answer here because the key is the whole identity: two rows with + * one id are two rows every action would treat as the same thing, so there is nothing to show about + * the second that the first is not already showing. Where the duplicate means something -- the + * import list's did -- the fix belongs at the source, and this is only what stops a data problem + * from being a crash. It is counted so the render report says it happened rather than leaving a + * silently shorter list. + * + * The transcript's own list is deliberately not on this: its keys are made here rather than + * received, and it is the one list where an extra pass over the items is measurable. + */ +inline fun LazyListScope.uniqueItems( + items: List, + crossinline key: (T) -> Any, + noinline contentType: (T) -> Any? = { null }, + crossinline itemContent: @Composable LazyItemScope.(T) -> Unit, +) { + val seen = HashSet(items.size) + val unique = items.filter { seen.add(key(it)) } + if (unique.size != items.size) { + DebugStats.count("list items dropped for a repeated key") + } + items(unique, key = { key(it) }, contentType = contentType) { itemContent(it) } +} diff --git a/server/src/session/import.rs b/server/src/session/import.rs index bf85892..74bd783 100644 --- a/server/src/session/import.rs +++ b/server/src/session/import.rs @@ -257,6 +257,30 @@ fn parse_listing(found: &str) -> Result> { (false, false) => InUse::Unknown, }; } + // One row per session id, because the id is what everything downstream + // addresses: `--resume` takes it, deleting globs for it, and the + // in-flight registry is keyed on it. So two rows sharing an id are two + // rows that no operation can tell apart -- and the phone keys its list + // on it too, which turned this into a crash rather than a confusion. + // + // It is a real state of the machine, not corruption: resuming a session + // from a different working directory makes the CLI write a second file + // under that directory's project folder with the same id. One of the two + // is then usually a stub of a few hundred bytes and the other is the + // conversation somebody means. + // + // So the copy with the most in it wins, and the row's `cwd` comes from + // that same copy -- which is the directory `--resume` will find it under. + // Ties go to the more recent, and the *stub* is often the more recent, so + // the size has to be the first key rather than the tie-break. + sessions.sort_by(|a, b| { + b.lines + .cmp(&a.lines) + .then_with(|| b.modified.total_cmp(&a.modified)) + }); + let mut seen = std::collections::HashSet::new(); + sessions.retain(|session| seen.insert(session.id.clone())); + // Most recent first, and only that. Naming was tried as the first key // and is a worse list: it buries what somebody was just doing under // everything they ever named, and the reason to open this screen is @@ -672,12 +696,15 @@ pub async fn delete(transport: &Transport, id: &str) -> Result<()> { // `context_of` below already resolved an id the cheap way; this is the // same lookup, and the two now agree. ensure!(is_session_id(id), "not a Claude Code session id: {id}"); + // Every copy, not the first. The same id can name a file under two + // project directories -- see the de-duplication in `parse_listing` -- + // and stopping at the first left the other behind, so the row came back + // on the next listing after a delete that had reported success. let script = r#" for f in "$HOME"/.claude/projects/*/"$1".jsonl; do [ -f "$f" ] || continue rm -f "$f" || exit 1 printf '%s\n' "$f" - exit 0 done "#; let launch = Launch::new( @@ -852,6 +879,37 @@ pub async fn replay_after( mod tests { use super::*; + /// One session id, two files, one row. + /// + /// Resuming a session from a different working directory makes the CLI + /// write a second file with the same id under that directory's project + /// folder, so this is an ordinary state of a machine rather than a + /// corrupt one. Everything downstream addresses a session by id, and + /// the phone keys its list on it, so two rows sharing one was a crash. + /// + /// The stub is deliberately the *newer* of the two here, because that + /// is how the real case looked: ordering by recency alone picks the + /// near-empty copy and describes the session by the wrong cwd. + #[test] + fn a_session_recorded_under_two_projects_is_offered_once() { + let id = "3114dee1-2f95-4de0-9c04-3d6fcc594afe"; + let said = r#"{"cwd":"/home/bob/repos/survey","message":{"role":"user","content":"the real conversation"}}"#; + let stub = r#"{"cwd":"/home/bob/repos/elsewhere","message":{"role":"user","content":"resumed here once"}}"#; + let listing = format!( + "LIVEKNOWN\n\ + 1000.0\t412\t160638\t\t/home/bob/.claude/projects/-home-bob-repos-survey/{id}.jsonl\t{said}\n\ + 2000.0\t4\t614\t\t/home/bob/.claude/projects/-home-bob-repos-elsewhere/{id}.jsonl\t{stub}\n" + ); + + let rows = parse_listing(&listing).expect("parse"); + + assert_eq!(rows.len(), 1, "one id is one row: {rows:#?}"); + assert_eq!(rows[0].lines, 412, "the conversation, not the stub"); + // The cwd has to come from the copy that was kept, because that is + // the directory `--resume` will find those 412 lines under. + assert_eq!(rows[0].cwd, "/home/bob/repos/survey"); + } + /// The guard on the only thing this module ever puts in a glob. /// /// Worth a test of its own because what it protects is a `rm`: `delete`