fix: bugs around serialization (#1812)

Author: brandonskiser

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

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<Context>, updates: Option<SharedWriter>) {
+        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()
     }

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<Instant>,
 }
 
 /// Maps a hook name to a [`CachedHook`]
-#[derive(Debug, Clone, Default, Serialize, Deserialize)]
+#[derive(Debug, Clone, Default)]
 pub struct HookExecutor {
     pub global_cache: HashMap<String, CachedHook>,
     pub profile_cache: HashMap<String, CachedHook>,

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,