From cdca1a741d3f2a06245e2eeb2fd2f368b4900fe5 Mon Sep 17 00:00:00 2001 From: Brandon Kiser Date: Tue, 13 May 2025 14:45:30 -0700 Subject: [PATCH] fix: bugs around serialization --- crates/chat-cli/src/cli/chat/context.rs | 8 +++++ .../src/cli/chat/conversation_state.rs | 31 +++++++++++++++++++ crates/chat-cli/src/cli/chat/hooks.rs | 5 ++- crates/chat-cli/src/cli/chat/mod.rs | 25 +++++++++------ 4 files changed, 56 insertions(+), 13 deletions(-) diff --git a/crates/chat-cli/src/cli/chat/context.rs b/crates/chat-cli/src/cli/chat/context.rs index c3abf7967c..b7b84e8e3c 100644 --- a/crates/chat-cli/src/cli/chat/context.rs +++ b/crates/chat-cli/src/cli/chat/context.rs @@ -59,6 +59,7 @@ pub struct ContextManager { /// Context configuration for the current profile. pub profile_config: ContextConfig, + #[serde(skip)] pub hook_executor: HookExecutor, } @@ -128,6 +129,13 @@ impl ContextManager { Ok(()) } + /// Reloads the global and profile config from disk. + pub async fn reload_config(&mut self) -> Result<()> { + self.global_config = load_global_config(&self.ctx).await?; + self.profile_config = load_profile_config(&self.ctx, &self.current_profile).await?; + Ok(()) + } + /// Add paths to the context configuration. /// /// # Arguments diff --git a/crates/chat-cli/src/cli/chat/conversation_state.rs b/crates/chat-cli/src/cli/chat/conversation_state.rs index da0c5941dd..3428af29c7 100644 --- a/crates/chat-cli/src/cli/chat/conversation_state.rs +++ b/crates/chat-cli/src/cli/chat/conversation_state.rs @@ -156,6 +156,37 @@ impl ConversationState { } } + /// Reloads necessary fields after being deserialized. This should be called after + /// deserialization. + pub async fn reload_serialized_state(&mut self, ctx: Arc, updates: Option) { + self.updates = updates; + + // Try to reload ContextManager, but do not return an error if we fail. + // TODO: Currently the failure modes around ContextManager is unclear, and we don't return + // errors in most cases. Thus, we try to preserve the same behavior here and simply have + // self.context_manager equal to None if any errors are encountered. This needs to be + // refactored. + let mut failed = false; + if let Some(context_manager) = self.context_manager.as_mut() { + match context_manager.reload_config().await { + Ok(_) => (), + Err(err) => { + error!(?err, "failed to reload context config"); + match ContextManager::new(ctx, None).await { + Ok(v) => *context_manager = v, + Err(err) => { + failed = true; + error!(?err, "failed to construct context manager"); + }, + } + }, + } + } + if failed { + self.context_manager.take(); + } + } + pub fn latest_summary(&self) -> Option<&str> { self.latest_summary.as_deref() } diff --git a/crates/chat-cli/src/cli/chat/hooks.rs b/crates/chat-cli/src/cli/chat/hooks.rs index f6d857ba38..036195ba17 100644 --- a/crates/chat-cli/src/cli/chat/hooks.rs +++ b/crates/chat-cli/src/cli/chat/hooks.rs @@ -119,15 +119,14 @@ pub enum HookTrigger { PerPrompt, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone)] pub struct CachedHook { output: String, - #[serde(skip)] expiry: Option, } /// Maps a hook name to a [`CachedHook`] -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Default)] pub struct HookExecutor { pub global_cache: HashMap, pub profile_cache: HashMap, diff --git a/crates/chat-cli/src/cli/chat/mod.rs b/crates/chat-cli/src/cli/chat/mod.rs index d547d6ced8..47479cab27 100644 --- a/crates/chat-cli/src/cli/chat/mod.rs +++ b/crates/chat-cli/src/cli/chat/mod.rs @@ -543,6 +543,9 @@ impl ChatContext { { Some(mut prior) => { existing_conversation = true; + prior + .reload_serialized_state(Arc::clone(&ctx), Some(output.clone())) + .await; input = Some(input.unwrap_or("In a few words, summarize our conversation so far.".to_owned())); prior.tool_manager = tool_manager; prior @@ -1315,14 +1318,6 @@ impl ChatContext { // Otherwise continue with normal chat on 'n' or other responses self.tool_use_status = ToolUseStatus::Idle; - if self.interactive { - queue!(self.output, style::SetForegroundColor(Color::Magenta))?; - queue!(self.output, style::SetForegroundColor(Color::Reset))?; - queue!(self.output, cursor::Hide)?; - execute!(self.output, style::Print("\n"))?; - self.spinner = Some(Spinner::new(Spinners::Dots, "Thinking...".to_owned())); - } - if pending_tool_index.is_some() { self.conversation_state.abandon_tool_use(tool_uses, user_input); } else { @@ -1332,6 +1327,14 @@ impl ChatContext { let conv_state = self.conversation_state.as_sendable_conversation_state(true).await; self.send_tool_use_telemetry(telemetry).await; + if self.interactive { + queue!(self.output, style::SetForegroundColor(Color::Magenta))?; + queue!(self.output, style::SetForegroundColor(Color::Reset))?; + queue!(self.output, cursor::Hide)?; + execute!(self.output, style::Print("\n"))?; + self.spinner = Some(Spinner::new(Spinners::Dots, "Thinking...".to_owned())); + } + ChatState::HandleResponseStream(self.client.send_message(conv_state).await?) }, Command::Execute { command } => { @@ -2808,9 +2811,11 @@ impl ChatContext { } let contents = tri!(self.ctx.fs().read_to_string(&path).await); - let new_state: ConversationState = tri!(serde_json::from_str(&contents)); + let mut new_state: ConversationState = tri!(serde_json::from_str(&contents)); + new_state + .reload_serialized_state(Arc::clone(&self.ctx), Some(self.output.clone())) + .await; self.conversation_state = new_state; - self.conversation_state.updates = Some(self.output.clone()); execute!( self.output,