diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt index 49ad60a..613f39f 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt @@ -125,6 +125,16 @@ data class SessionSummary( val provider: String, val title: String, val model: String?, + /** + * Whether the conversation would outlive deleting this session, decided by the server from the + * provider's kind rather than here from its name. + * + * What it licenses is narrow, and the delete dialog is worded to match: the driver keeps its + * own record of the conversation somewhere this app's delete does not reach. It is not a + * promise that the file is still there, and re-importing is not a restore -- this app's + * transcript holds things that record does not. + */ + val keepsOwnTranscript: Boolean, /** How much the session asks before acting; null when it was never set. */ val permissionMode: String?, /** @@ -139,6 +149,7 @@ private fun parseSession(session: JSONObject) = SessionSummary( id = session.getString("id"), setup = session.getString("setup"), + keepsOwnTranscript = session.optBoolean("keepsOwnTranscript", false), setupName = session.getString("setupName"), provider = session.getString("provider"), title = session.getString("title"), 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 0416e5b..683ce5c 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ImportScreen.kt @@ -374,7 +374,12 @@ private fun statsOf(session: Importable, importing: String?): String = */ private fun warningOf(session: Importable): String? = when (session.inUse) { - "yes" -> "open in a terminal — close it there first" + // What was measured is that a live process on that machine holds this session open. Which + // process is not measured, so it isn't claimed: "a terminal — close it there first" sent + // people looking for a window that need not exist. It is just as likely another agent, or + // this app on a session it spawned. Naming a place the reader then can't find turns a + // correct refusal into a wrong instruction. + "yes" -> "something on that machine is running it" "unknown" -> "can't tell if it's open" else -> null } 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 3fd93cd..de72668 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt @@ -168,16 +168,33 @@ fun SessionListScreen( onDismissRequest = { confirmingDelete = null }, title = { Text("Delete \"${session.title}\"?") }, text = { - // Two different acts behind one button, so it says which one this is. An - // imported session's real transcript belongs to Claude Code and outlives - // this: removing it here undoes a view. A session started here has no copy - // anywhere, and removing it ends the conversation. Warning "this cannot be - // undone" of both would make the warning worthless where it is true. + // Two different acts behind one button, so it says which one this is. What + // separates them is whether the *driver* keeps its own record of the + // conversation -- the Claude Code CLI does, under ~/.claude/projects, whether + // this app spawned the session or imported it; echo and llama.cpp do not, and + // for those the app's transcript is the only copy there is. + // + // This used to branch on `imported`, above a comment asserting that "a session + // started here has no copy anywhere". That was simply false for every + // claude-cli session this app spawned, and the two warnings disagreed about + // sessions that were equally recoverable. Getting it wrong in that direction + // is the expensive one: "this can't be undone", said of something that can, + // spends the credibility the sentence needs on the sessions where it is true. + // + // Neither branch promises a restore. The recoverable one says what is known -- + // the driver keeps its own record -- rather than that the file is still there, + // which nothing here checked; and it names what goes either way, because this + // app's transcript holds images, peer messages and commands that the CLI's own + // record never had. Text( - if (session.imported) - "Stops the process and removes this app's copy. The conversation " + - "itself stays on the machine and can be imported again." - else "Kills the process and deletes its transcript. This can't be undone." + if (session.keepsOwnTranscript) + "Stops the process and deletes this app's copy of the conversation, " + + "including any images, peer messages and commands recorded only " + + "here. Claude Code keeps its own transcript on the machine, so the " + + "conversation itself should still be there to import again." + else + "Kills the process and deletes the conversation. Nothing else keeps a " + + "copy, so this can't be undone." ) }, confirmButton = { diff --git a/server/src/config.rs b/server/src/config.rs index 6870817..a77a264 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -134,6 +134,30 @@ pub enum DriverKind { ClaudeCli, } +impl DriverKind { + /// Whether the conversation exists outside this app, so that deleting + /// the session here does not end it. + /// + /// The Claude Code CLI owns its own transcript under + /// `~/.claude/projects/` and is resumable from it whatever started + /// it -- so a session this app spawned is every bit as recoverable as + /// one it imported, and the difference between those two is only how + /// it got here. Echo has nothing to keep, and a llama session's + /// conversation is folded out of *this* app's transcript, so for both + /// of those a delete is the end of it. + /// + /// Asked before warning somebody that a deletion cannot be undone, + /// which is the one sentence that has to be true: said of a session + /// that can in fact be brought back, it spends the credibility the + /// warning needs on the sessions where it is real. + pub fn keeps_own_transcript(self) -> bool { + match self { + Self::ClaudeCli => true, + Self::Echo | Self::LlamaCpp => false, + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct TokenEntry { diff --git a/server/src/routes.rs b/server/src/routes.rs index cb9d22f..130d269 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -431,7 +431,7 @@ async fn list_importable( // Joined here because the importer knows about files and the manager // knows about sessions, and putting the two together is the route's // job rather than either one's. - found.retain(|candidate| manager.session_importing(&candidate.id).is_none()); + found.retain(|candidate| manager.session_driving(&candidate.id).is_none()); Ok(axum::Json(found)) } @@ -477,7 +477,7 @@ async fn spawn_session( body.setup )) })?; - if let Some(existing) = manager.session_importing(want) { + if let Some(existing) = manager.session_driving(want) { return Err(ApiError::BadRequest(format!( "session {existing} is already continuing that one -- delete it first if you \ want a fresh copy. Deleting it here does not touch the conversation itself, \ diff --git a/server/src/session/claude.rs b/server/src/session/claude.rs index 3631fd4..721b00f 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -992,7 +992,7 @@ fn create_log(path: &Path) -> Result { .with_context(|| format!("creating {}", path.display())) } -fn read_resume_token(session_dir: &Path) -> Option { +pub(super) fn read_resume_token(session_dir: &Path) -> Option { let text = std::fs::read_to_string(session_dir.join(RESUME_FILE)).ok()?; serde_json::from_str::(&text) .ok()? diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index a608d6f..e72ec69 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -77,6 +77,16 @@ pub struct SessionInfo { pub title: String, #[serde(skip_serializing_if = "Option::is_none")] pub model: Option, + /// Whether the conversation would survive deleting this session -- + /// see [`DriverKind::keeps_own_transcript`]. + /// + /// Reported rather than worked out on the phone, because the phone + /// has the provider's *name* and this is a property of its *kind*: a + /// provider can be called anything, so a client deciding by name + /// would get the answer wrong for anyone who renamed one. It decides + /// what the delete confirmation says will happen, so it is not a + /// field to guess at. + pub keeps_own_transcript: bool, /// How much this session asks before acting. Reported so the phone can /// *show* the current mode rather than assume one -- a picker that /// guesses its own value is how you end up changing something you @@ -300,7 +310,7 @@ impl LiveSession { /// `setup_name` is passed in rather than stored: only the manager /// holds the config, and the label can change under a running session. - fn info(&self, setup_name: &str, imported: bool) -> SessionInfo { + fn info(&self, setup_name: &str, imported: bool, keeps_own_transcript: bool) -> SessionInfo { SessionInfo { id: self.meta.id.clone(), provider: self.meta.provider.clone(), @@ -310,6 +320,7 @@ impl LiveSession { model: self.shared.model.lock().unwrap().clone(), permission_mode: self.shared.permission_mode.lock().unwrap().clone(), imported, + keeps_own_transcript, cwd: self.meta.cwd.clone(), status: *self.shared.status.lock().unwrap(), last_activity: *self.shared.last_activity.lock().unwrap(), @@ -581,24 +592,36 @@ impl SessionManager { ); } - /// The session already continuing `source`, if there is one. + /// The session already driving `source`, if there is one. /// - /// Importing the same Claude Code session twice would leave two - /// `--resume` processes appending to one transcript, each seeing the - /// other's writes as work done elsewhere and replaying them. Nothing - /// is corrupted, but both sessions show a conversation neither of them - /// is having, which is worse than a refusal. - pub fn session_importing(&self, source: &str) -> Option { + /// Two `--resume` processes on one transcript each see the other's + /// writes as work done elsewhere and replay them, so both sessions + /// show a conversation neither is having -- worse than a refusal. + /// + /// Two ways to already be driving one, and only the first used to + /// count. An **imported** session records a cursor naming the file it + /// follows. A session this app **spawned** has no cursor at all, but + /// it has a resume token, which is the CLI's own id for the + /// conversation and is exactly the thing being asked about. Matching + /// only the cursor left every spawned session looking like somebody + /// else's: it appeared in the import list, marked as in use, telling + /// the reader to go and close it somewhere -- and the somewhere was + /// this app. + pub fn session_driving(&self, source: &str) -> Option { let inner = self.inner.read().unwrap(); inner.config.sessions.iter().find_map(|meta| { - let cursor = import::read_cursor(&self.data_dir.join(&meta.id))?; - cursor - .path - .rsplit('/') - .next()? - .strip_suffix(".jsonl") - .filter(|found| *found == source) - .map(|_| meta.id.clone()) + let dir = self.data_dir.join(&meta.id); + let followed = import::read_cursor(&dir).and_then(|cursor| { + cursor + .path + .rsplit('/') + .next() + .and_then(|name| name.strip_suffix(".jsonl")) + .map(str::to_string) + }); + let resuming = claude::read_resume_token(&dir); + (followed.as_deref() == Some(source) || resuming.as_deref() == Some(source)) + .then(|| meta.id.clone()) }) } @@ -614,6 +637,7 @@ impl SessionManager { Some(session) => session.info( label_of(&inner.config, &meta.setup), import::read_cursor(&self.data_dir.join(&meta.id)).is_some(), + keeps_own_transcript(&inner.config, &meta.setup, &meta.provider), ), None => SessionInfo { id: meta.id.clone(), @@ -624,6 +648,11 @@ impl SessionManager { model: meta.model.clone(), permission_mode: meta.permission_mode.clone(), imported: import::read_cursor(&self.data_dir.join(&meta.id)).is_some(), + keeps_own_transcript: keeps_own_transcript( + &inner.config, + &meta.setup, + &meta.provider, + ), cwd: meta.cwd.clone(), status: status_of_unlaunched(&self.data_dir.join(&meta.id)), last_activity: meta.created, @@ -737,6 +766,7 @@ impl SessionManager { let info = session.info( &setup.name, import::read_cursor(&self.data_dir.join(&id)).is_some(), + provider.kind.keeps_own_transcript(), ); inner.live.insert(id, session); Ok(info) @@ -904,6 +934,21 @@ fn label_of<'a>(config: &'a Config, id: &'a str) -> &'a str { config.setup(id).map_or(id, |setup| setup.name.as_str()) } +/// Whether this session's provider keeps the conversation somewhere this +/// app's delete cannot reach. +/// +/// False when the provider can't be found, which is the safe way round: a +/// setup or provider removed from the config leaves sessions naming one +/// that is gone, and the warning that then shows is the strong one. Saying +/// "this can be brought back" on no evidence is the answer that loses +/// somebody's conversation. +fn keeps_own_transcript(config: &Config, setup: &str, provider: &str) -> bool { + config + .setup(setup) + .and_then(|setup| setup.providers.iter().find(|it| it.name == provider)) + .is_some_and(|provider| provider.kind.keeps_own_transcript()) +} + /// Names for a failure message: what there is, so the reader can see what /// they meant instead of only that they were wrong. fn names<'a>(all: impl Iterator) -> String { @@ -1307,6 +1352,56 @@ mod tests { .expect("seed config"); } + /// Deleting an echo session ends the conversation; deleting a + /// claude-cli one does not, because the CLI keeps its own transcript + /// whether this app spawned the session or imported it. + /// + /// The delete confirmation is worded off this, so getting it backwards + /// either loses a conversation somebody was told they could recover, + /// or cries wolf about one they can. + #[test] + fn only_a_driver_that_keeps_its_own_record_survives_deletion() { + assert!(DriverKind::ClaudeCli.keeps_own_transcript()); + assert!(!DriverKind::Echo.keeps_own_transcript()); + assert!(!DriverKind::LlamaCpp.keeps_own_transcript()); + } + + /// A session this app *spawned* is one it is driving, and used to look + /// like somebody else's. + /// + /// Only imported sessions leave an import cursor, so matching on that + /// alone missed every spawned session: each one stayed in the import + /// list, marked in use, telling the reader to close it wherever it was + /// open -- and it was open here. The resume token is the CLI's own id + /// for the conversation, which both kinds have. + #[tokio::test] + async fn a_spawned_session_counts_as_one_we_are_already_driving() { + let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join("config.ron"); + let data_dir = dir.path().join("sessions"); + seed_echo_only(&config_path); + let manager = SessionManager::new( + config_path.clone(), + data_dir.clone(), + data_dir.join("models"), + ) + .expect("manager"); + let info = manager.spawn_session(echo_spec()).expect("spawn"); + + // No cursor -- nothing was imported -- so this is the case the old + // check could not see. + assert!(import::read_cursor(&data_dir.join(&info.id)).is_none()); + assert_eq!(manager.session_driving("5ecf21da-d53f"), None); + + // What a claude session records once the CLI names itself. + claude::write_resume_token(&data_dir.join(&info.id), "5ecf21da-d53f"); + assert_eq!( + manager.session_driving("5ecf21da-d53f").as_deref(), + Some(info.id.as_str()) + ); + assert_eq!(manager.session_driving("some-other-session"), None); + } + #[test] fn a_session_we_are_not_driving_says_exited_only_when_it_is_gone() { let dir = tempfile::tempdir().expect("tempdir");