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`