From 774ab6ad61c62d9185515e3648a2a7964cca70a8 Mon Sep 17 00:00:00 2001 From: Adam Dalloul <47503782+Adam-Dalloul@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:46:35 -0700 Subject: [PATCH] fix(acp): journal sent prompts so an interrupted chat cannot reopen blank --- src-tauri/src/acp/connection.rs | 78 +++++++++++++++ src-tauri/src/commands/conversations.rs | 126 ++++++++++++++++++++++-- src-tauri/src/paths.rs | 28 ++++++ 3 files changed, 223 insertions(+), 9 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index fb09a6ced..0db72f5bf 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -631,6 +631,55 @@ async fn record_prompt(agent_type: AgentType, session_id: &str, blocks: &[Conten let _ = tokio::time::timeout(std::time::Duration::from_millis(2000), ack).await; } +/// Journal directory for an agent whose prompts codeg keeps a safety copy of, +/// or `None` for custom agents, whose prompts [`record_prompt`] already stores +/// in their full transcript. +/// +/// The inverse gate of [`transcript_dir_for`], for the inverse reason: a +/// built-in's own store is written by the agent, so codeg holds NO copy of a +/// sent prompt — and an agent that dies before flushing (a wedged CLI, a crash +/// on the first turn, a session file that was never created) takes the user's +/// message with it. The journal is the one line per turn that makes that +/// impossible. It records to its own root (`paths::codeg_prompt_journal_root`) +/// so none of the transcript-store consumers start seeing built-in files. +fn prompt_journal_dir_for(agent_type: AgentType) -> Option<&'static str> { + match agent_type.custom_id() { + Some(_) => None, + None => Some(registry::registry_id_for(agent_type)), + } +} + +/// Record an outgoing prompt for a BUILT-IN agent into the prompt journal, and +/// wait (briefly) for it to land. No-op for custom agents. +/// +/// Written before the request is dispatched, like [`record_prompt`], and +/// bound-waited for the same reason record_prompt is: the reader is a +/// different party (the conversation read path's journal fallback), and a +/// prompt that is still in the writer's queue when the app is torn down is +/// exactly the prompt this journal exists to keep. +/// +/// The journal deliberately records prompts ONLY — never the agent's output. +/// The agent's own store stays the source of truth for the conversation; the +/// journal is read purely as a fallback when that store has nothing for the +/// session (see `get_folder_conversation_core`), so the two can never disagree +/// about a turn both of them have. +async fn record_prompt_journal(agent_type: AgentType, session_id: &str, blocks: &[ContentBlock]) { + let Some(dir) = prompt_journal_dir_for(agent_type) else { + return; + }; + let Ok(payload) = serde_json::to_value(blocks) else { + return; + }; + let ack = crate::acp_transcript::record_entry_in( + &crate::paths::codeg_prompt_journal_root(), + dir, + session_id, + crate::acp_transcript::EntryKind::Prompt, + payload, + ); + let _ = tokio::time::timeout(std::time::Duration::from_millis(2000), ack).await; +} + /// Record a turn's completion for a custom agent, and wait (briefly) for it to /// land. No-op for agents with their own store. /// @@ -7443,6 +7492,7 @@ async fn run_conversation_loop<'a>( // instantly — and awaited, so the replay gate can never see // this conversation as transcript-less (see `record_prompt`). record_prompt(agent_type, &sid.0, &prompt_blocks).await; + record_prompt_journal(agent_type, &sid.0, &prompt_blocks).await; let turn_started_at_ms = crate::acp_transcript::now_epoch_ms(); let prompt_request = PromptRequest::new(sid.clone(), prompt_blocks); // Snapshot the stderr write position BEFORE the request is @@ -11304,6 +11354,34 @@ mod tests { use super::*; use sacp::schema::{Diff, SessionConfigId}; + // ── Prompt journal gate ───────────────────────────────────────────────── + // + // The journal and the custom-agent transcript are complementary by + // construction: every agent type lands in exactly one of the two + // recorders, so no prompt is stored twice and none is stored nowhere. + + #[test] + fn prompt_journal_gate_covers_built_ins_and_skips_custom() { + for built_in in [AgentType::ClaudeCode, AgentType::Codex, AgentType::Grok] { + assert_eq!( + prompt_journal_dir_for(built_in), + Some(registry::registry_id_for(built_in)), + "a built-in agent's prompts must be journaled" + ); + assert!( + transcript_dir_for(built_in).is_none(), + "a built-in agent must not also get a full codeg transcript" + ); + } + let custom = AgentType::Custom("my-agent"); + assert_eq!( + prompt_journal_dir_for(custom), + None, + "a custom agent's prompts already live in its full transcript" + ); + assert!(transcript_dir_for(custom).is_some()); + } + /// Unwrap a select selector. The Grok synthesizers below only ever build /// selects, so any other kind is a test failure rather than a branch to /// handle — this keeps the assertions as terse as the irrefutable `let` diff --git a/src-tauri/src/commands/conversations.rs b/src-tauri/src/commands/conversations.rs index dbddadf5e..328522885 100644 --- a/src-tauri/src/commands/conversations.rs +++ b/src-tauri/src/commands/conversations.rs @@ -1144,14 +1144,25 @@ pub async fn get_folder_conversation_core( AgentType::Custom(_) => Box::new(AcpNativeParser::new(at)), }; match parser.get_conversation(&eid) { - Ok(d) => Ok(( - d.turns, - d.session_stats, - None, - d.summary.title, - d.summary.model, - d.transcript_watermark, - )), + Ok(d) => { + // A session file that exists but yielded no turns (e.g. an + // agent that wrote its header and died before the first + // flush) reads like a blank chat. The journal has the one + // thing codeg knows the agent was sent: the user's prompt. + let turns = if d.turns.is_empty() { + prompt_journal_turns(at, &eid) + } else { + d.turns + }; + Ok(( + turns, + d.session_stats, + None, + d.summary.title, + d.summary.model, + d.transcript_watermark, + )) + } Err(crate::parsers::ParseError::ConversationNotFound(_)) => { // The external_id may no longer match any local file — // e.g. an ACP session UUID (OpenClaw, Cline) or a stale @@ -1196,7 +1207,13 @@ pub async fn get_folder_conversation_core( } } } - Ok((vec![], None, None, None, None, None)) + // The agent has nothing for this session — a wedged CLI + // that never started the turn, a crash before the session + // file was created, a store the agent's own retention + // pruned. Fall back to codeg's prompt journal so the chat + // reopens with the user's sent message(s) instead of + // rendering blank. + Ok((prompt_journal_turns(at, &eid), None, None, None, None, None)) } Err(e) => Err(parse_error_to_app_error(e)), } @@ -1459,6 +1476,38 @@ fn apply_turn_window( detail.uncovered_prefix_max_ts = meta.uncovered_prefix_max_ts; } +/// Turns from codeg's prompt journal for this session, or empty. +/// +/// The read half of `acp::connection::record_prompt_journal`, consulted ONLY +/// when the agent's own store yielded no turns for a bound session. The +/// journal holds outgoing prompts alone — never agent output — so what this +/// returns can never disagree with, or duplicate, a history the agent +/// actually has: either the agent's parser produced turns (journal unread), +/// or it produced none (journal is all there is). +fn prompt_journal_turns(agent_type: AgentType, session_id: &str) -> Vec { + prompt_journal_turns_in( + crate::paths::codeg_prompt_journal_root(), + agent_type, + session_id, + ) +} + +/// Root-injectable core of [`prompt_journal_turns`]. +fn prompt_journal_turns_in( + root: std::path::PathBuf, + agent_type: AgentType, + session_id: &str, +) -> Vec { + // The journal shares the transcript file format, so the custom-agent + // parser pointed at the journal root reads it as a (prompt-only) + // transcript. Any error — no journal for this session included — means + // "nothing to show", which is exactly what the caller already had. + AcpNativeParser::new_in(agent_type, root) + .get_conversation(session_id) + .map(|d| d.turns) + .unwrap_or_default() +} + /// `get_folder_conversation_core` plus live in-flight correlation: when a turn is /// currently running on the conversation's connection, stamp the persisted /// in-flight user turn with the broadcast `message_id` so a cross-client viewer @@ -2394,6 +2443,65 @@ mod tests { /// locks can't deadlock. Held for the whole test body. static IMPORT_GUARD_SERIALIZER: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + // ────────────────────────────────────────────────────────────────────── + // Prompt-journal fallback (`prompt_journal_turns_in`): a conversation + // whose agent store has nothing must reopen with the user's sent + // message(s) from the journal instead of rendering blank. + // ────────────────────────────────────────────────────────────────────── + + /// One journal line, exactly as `record_prompt_journal` serializes it. + fn journal_prompt_line(t: u64, text: &str) -> String { + serde_json::to_string(&crate::acp_transcript::TranscriptEntry { + t, + k: crate::acp_transcript::EntryKind::Prompt, + p: serde_json::json!([{ "type": "text", "text": text }]), + }) + .expect("serialize journal entry") + } + + #[test] + fn prompt_journal_turns_reads_recorded_prompts_as_user_turns() { + let root = tempfile::tempdir().expect("tempdir"); + let dir = crate::acp::registry::registry_id_for(AgentType::Grok); + crate::acp_transcript::append_line_in( + root.path(), + dir, + "sess-1", + &journal_prompt_line(1_000, "first message"), + ); + crate::acp_transcript::append_line_in( + root.path(), + dir, + "sess-1", + &journal_prompt_line(2_000, "second message"), + ); + + let turns = + prompt_journal_turns_in(root.path().to_path_buf(), AgentType::Grok, "sess-1"); + assert_eq!(turns.len(), 2, "every journaled prompt becomes a user turn"); + for (turn, expected) in turns.iter().zip(["first message", "second message"]) { + assert!(matches!(turn.role, TurnRole::User)); + match &turn.blocks[0] { + ContentBlock::Text { text } => assert_eq!(text, expected), + other => panic!("expected a text block, got {other:?}"), + } + } + } + + #[test] + fn prompt_journal_turns_is_empty_for_an_unjournaled_session() { + let root = tempfile::tempdir().expect("tempdir"); + // No file at all — the common case for every conversation recorded + // before the journal existed, and for custom agents, which never + // write here. + assert!(prompt_journal_turns_in( + root.path().to_path_buf(), + AgentType::Grok, + "sess-missing" + ) + .is_empty()); + } + // ────────────────────────────────────────────────────────────────────── // Delegation meta injection for historical reload. Parsers always emit // `ContentBlock::ToolUse { meta: None }`; without this helper, a diff --git a/src-tauri/src/paths.rs b/src-tauri/src/paths.rs index d7f76631e..3692c70e5 100644 --- a/src-tauri/src/paths.rs +++ b/src-tauri/src/paths.rs @@ -13,6 +13,7 @@ const UPLOADS_DIR_NAME: &str = "uploads"; const LOGS_DIR_NAME: &str = "logs"; const TURN_TIMINGS_DIR_NAME: &str = "turn-timings"; const ACP_TRANSCRIPTS_DIR_NAME: &str = "acp-transcripts"; +const PROMPT_JOURNAL_DIR_NAME: &str = "acp-prompts"; const BACKGROUNDS_DIR_NAME: &str = "backgrounds"; /// `$CODEG_HOME` if set (and non-empty), else `~/.codeg/`. @@ -169,6 +170,33 @@ pub fn codeg_acp_transcripts_root() -> PathBuf { .unwrap_or_else(|| PathBuf::from(CODEG_DIR_NAME).join(ACP_TRANSCRIPTS_DIR_NAME)) } +/// Root directory for codeg's prompt journal: the outgoing `session/prompt` +/// content recorded for **built-in** agents (see +/// `crate::acp::connection::record_prompt_journal`). +/// +/// Deliberately a SIBLING of [`codeg_acp_transcripts_root`] rather than a +/// subdirectory of it: everything that walks the transcript root — the +/// `session/load` replay gate, continuation chains, the custom-agent parser's +/// conversation listing — assumes those files are the full history of a custom +/// agent's session. Prompt-only journals for built-ins must stay invisible to +/// all of them. +/// +/// Resolution mirrors [`codeg_acp_transcripts_root`]: +/// 1. `$CODEG_HOME/acp-prompts` +/// 2. `$CODEG_DATA_DIR/acp-prompts` (server-mode data directory) +/// 3. `~/.codeg/acp-prompts` (desktop default) +pub fn codeg_prompt_journal_root() -> PathBuf { + if let Some(custom) = std::env::var_os("CODEG_HOME").filter(|s| !s.is_empty()) { + return PathBuf::from(custom).join(PROMPT_JOURNAL_DIR_NAME); + } + if let Some(data) = std::env::var_os("CODEG_DATA_DIR").filter(|s| !s.is_empty()) { + return PathBuf::from(data).join(PROMPT_JOURNAL_DIR_NAME); + } + dirs::home_dir() + .map(|h| h.join(CODEG_DIR_NAME).join(PROMPT_JOURNAL_DIR_NAME)) + .unwrap_or_else(|| PathBuf::from(CODEG_DIR_NAME).join(PROMPT_JOURNAL_DIR_NAME)) +} + /// Single source of truth for "where does the database live, and where /// do `paths::*` resolve their roots against." ///