fix(config): avoid overwriting config defaults from legacy json (tailcallhq#2748)

Author: laststylebender14

crates/forge_config/src/config.rs

@@ -16,79 +16,104 @@ use crate::{AutoDumpFormat, Compact, Decimal, HttpConfig, ModelConfig, RetryConf
 #[setters(strip_option)]
 pub struct ForgeConfig {
  /// Retry settings applied at the system level to all IO operations.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub retry: Option<RetryConfig>,
  /// Maximum number of lines returned by a single file search operation.
- pub max_search_lines: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_search_lines: Option<usize>,
  /// Maximum number of bytes returned by a single file search operation.
- pub max_search_result_bytes: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_search_result_bytes: Option<usize>,
  /// Maximum number of characters returned from a URL fetch.
- pub max_fetch_chars: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_fetch_chars: Option<usize>,
  /// Maximum number of lines captured from the leading portion of shell
  /// command output.
- pub max_stdout_prefix_lines: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_stdout_prefix_lines: Option<usize>,
  /// Maximum number of lines captured from the trailing portion of shell
  /// command output.
- pub max_stdout_suffix_lines: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_stdout_suffix_lines: Option<usize>,
  /// Maximum number of characters per line in shell command output.
- pub max_stdout_line_chars: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_stdout_line_chars: Option<usize>,
  /// Maximum number of characters per line when reading a file.
- pub max_line_chars: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_line_chars: Option<usize>,
  /// Maximum number of lines read from a file in a single operation.
- pub max_read_lines: u64,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_read_lines: Option<u64>,
  /// Maximum number of files read in a single batch operation.
- pub max_file_read_batch_size: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_file_read_batch_size: Option<usize>,
  /// HTTP client settings including proxy, TLS, and timeout configuration.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub http: Option<HttpConfig>,
  /// Maximum file size in bytes permitted for read operations.
- pub max_file_size_bytes: u64,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_file_size_bytes: Option<u64>,
  /// Maximum image file size in bytes permitted for read operations.
- pub max_image_size_bytes: u64,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_image_size_bytes: Option<u64>,
  /// Maximum time in seconds a single tool call may run before being
  /// cancelled.
- pub tool_timeout_secs: u64,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub tool_timeout_secs: Option<u64>,
  /// Whether to automatically open HTML dump files in the browser after
  /// creation.
- pub auto_open_dump: bool,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub auto_open_dump: Option<bool>,
  /// Directory where debug request files are written; disabled when absent.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub debug_requests: Option<PathBuf>,
  /// Path to the conversation history file; defaults to the global history
  /// location when absent.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub custom_history_path: Option<PathBuf>,
  /// Maximum number of conversations shown in the conversation list.
- pub max_conversations: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_conversations: Option<usize>,
  /// Maximum number of candidate results returned from the initial semantic
  /// search vector query.
- pub max_sem_search_results: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_sem_search_results: Option<usize>,
  /// Number of top results retained after re-ranking in semantic search.
- pub sem_search_top_k: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub sem_search_top_k: Option<usize>,
  /// Base URL of the Forge services API used for semantic search and
  /// indexing.
- #[dummy(expr = "\"https://example.com/api\".to_string()")]
- pub services_url: String,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ #[dummy(expr = "Some(\"https://example.com/api\".to_string())")]
+ pub services_url: Option<String>,
  /// Maximum number of file extensions included in the agent system prompt.
- pub max_extensions: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_extensions: Option<usize>,
  /// Format used when automatically creating a session dump after task
  /// completion; disabled when absent.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub auto_dump: Option<AutoDumpFormat>,
  /// Maximum number of files read concurrently during batch operations.
- pub max_parallel_file_reads: usize,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub max_parallel_file_reads: Option<usize>,
  /// Time-to-live in seconds for the cached model API list.
- pub model_cache_ttl_secs: u64,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub model_cache_ttl_secs: Option<u64>,
  /// Default model and provider configuration used when not overridden by
- /// individual agents.
- #[serde(default)]
+ /// individual agents. 
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub session: Option<ModelConfig>,
  /// Model and provider configuration used for commit message generation.
- #[serde(default)]
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub commit: Option<ModelConfig>,
  /// Model and provider configuration used for shell command suggestion
  /// generation.
- #[serde(default)]
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub suggest: Option<ModelConfig>,
 
  // --- Workflow fields ---
  /// Configuration for automatic Forge updates.
- #[serde(skip_serializing_if = "Option::is_none")]
+ #[serde(default, skip_serializing_if = "Option::is_none")]
  pub updates: Option<Update>,
 
  /// Output randomness for all agents; lower values are deterministic, higher
@@ -127,11 +152,13 @@ pub struct ForgeConfig {
 
  /// Whether restricted mode is active; when enabled, tool execution requires
  /// explicit permission grants.
- pub restricted: bool,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub restricted: Option<bool>,
 
  /// Whether tool use is supported in the current environment; when false,
  /// all tool calls are disabled.
- pub tool_supported: bool,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub tool_supported: Option<bool>,
 }
 
 #[cfg(test)]

crates/forge_config/src/legacy.rs

@@ -38,6 +38,10 @@ impl LegacyConfig {
  /// Reads the legacy `~/forge/.config.json` file at `path`, parses it, and
  /// returns the equivalent TOML representation as a [`String`].
  ///
+ /// Because every field in [`ForgeConfig`] is `Option`, fields not covered
+ /// by the legacy format are `None` and omitted from the serialized TOML,
+ /// so they cannot overwrite values from lower-priority config layers.
+ ///
  /// # Errors
  ///
  /// Returns an error if the file cannot be read, the JSON is invalid, or the
@@ -51,7 +55,7 @@ impl LegacyConfig {
  }
 
  /// Converts a [`LegacyConfig`] into the fields of [`ForgeConfig`] that it
- /// covers, leaving all other fields at their defaults.
+ /// covers, leaving all other fields at their defaults (`None`).
  fn into_forge_config(self) -> ForgeConfig {
  let session = self.provider.as_deref().map(|provider_id| {
  let model_id = self.model.get(provider_id).cloned();

crates/forge_config/src/reader.rs

@@ -170,6 +170,43 @@ mod tests {
  assert!(actual.is_ok(), "read() failed: {:?}", actual.err());
  }
 
+ #[test]
+ fn test_legacy_layer_does_not_overwrite_defaults() {
+ // Simulate what `read_legacy` does: serialize a ForgeConfig that only
+ // carries session/commit/suggest (all other fields are None) and layer
+ // it on top of the embedded defaults. The default values must survive.
+ let legacy = ForgeConfig {
+ session: Some(ModelConfig {
+ provider_id: Some("anthropic".to_string()),
+ model_id: Some("claude-3".to_string()),
+ }),
+            ..Default::default()
+ };
+ let legacy_toml = toml_edit::ser::to_string_pretty(&legacy).unwrap();
+
+ let actual = ConfigReader::default()
+ .read_defaults()
+ .read_toml(&legacy_toml)
+ .build()
+ .unwrap();
+
+ // Session should come from the legacy layer
+ assert_eq!(
+            actual.session,
+ Some(ModelConfig {
+                provider_id: Some("anthropic".to_string()),
+                model_id: Some("claude-3".to_string()),
+ })
+ );
+
+ // Default values from .forge.toml must be retained, not reset to zero
+ assert_eq!(actual.max_parallel_file_reads, Some(64));
+ assert_eq!(actual.max_read_lines, Some(2000));
+ assert_eq!(actual.tool_timeout_secs, Some(300));
+ assert_eq!(actual.max_search_lines, Some(1000));
+ assert_eq!(actual.tool_supported, Some(true));
+ }
+
  #[test]
  fn test_read_session_from_env_vars() {
  let _guard = EnvGuard::set(&[

crates/forge_infra/src/env.rs

@@ -135,36 +135,39 @@ fn to_environment(fc: ForgeConfig, cwd: PathBuf) -> Environment {
 
  // --- ForgeConfig-mapped fields ---
  retry_config: fc.retry.map(to_retry_config).unwrap_or_default(),
- max_search_lines: fc.max_search_lines,
- max_search_result_bytes: fc.max_search_result_bytes,
- fetch_truncation_limit: fc.max_fetch_chars,
- stdout_max_prefix_length: fc.max_stdout_prefix_lines,
- stdout_max_suffix_length: fc.max_stdout_suffix_lines,
- stdout_max_line_length: fc.max_stdout_line_chars,
- max_line_length: fc.max_line_chars,
- max_read_size: fc.max_read_lines,
- max_file_read_batch_size: fc.max_file_read_batch_size,
+ max_search_lines: fc.max_search_lines.unwrap_or_default(),
+ max_search_result_bytes: fc.max_search_result_bytes.unwrap_or_default(),
+ fetch_truncation_limit: fc.max_fetch_chars.unwrap_or_default(),
+ stdout_max_prefix_length: fc.max_stdout_prefix_lines.unwrap_or_default(),
+ stdout_max_suffix_length: fc.max_stdout_suffix_lines.unwrap_or_default(),
+ stdout_max_line_length: fc.max_stdout_line_chars.unwrap_or_default(),
+ max_line_length: fc.max_line_chars.unwrap_or_default(),
+ max_read_size: fc.max_read_lines.unwrap_or_default(),
+ max_file_read_batch_size: fc.max_file_read_batch_size.unwrap_or_default(),
  http: fc.http.map(to_http_config).unwrap_or_default(),
- max_file_size: fc.max_file_size_bytes,
- max_image_size: fc.max_image_size_bytes,
- tool_timeout: fc.tool_timeout_secs,
- auto_open_dump: fc.auto_open_dump,
+ max_file_size: fc.max_file_size_bytes.unwrap_or_default(),
+ max_image_size: fc.max_image_size_bytes.unwrap_or_default(),
+ tool_timeout: fc.tool_timeout_secs.unwrap_or_default(),
+ auto_open_dump: fc.auto_open_dump.unwrap_or_default(),
  debug_requests: fc.debug_requests,
  custom_history_path: fc.custom_history_path,
- max_conversations: fc.max_conversations,
- sem_search_limit: fc.max_sem_search_results,
- sem_search_top_k: fc.sem_search_top_k,
- service_url: Url::parse(fc.services_url.as_str())
- .unwrap_or_else(|_| Url::parse("https://api.forgecode.dev").unwrap()),
- max_extensions: fc.max_extensions,
+ max_conversations: fc.max_conversations.unwrap_or_default(),
+ sem_search_limit: fc.max_sem_search_results.unwrap_or_default(),
+ sem_search_top_k: fc.sem_search_top_k.unwrap_or_default(),
+ service_url: fc
+ .services_url
+ .as_deref()
+ .and_then(|s| Url::parse(s).ok())
+ .unwrap_or_else(|| Url::parse("https://api.forgecode.dev").unwrap()),
+ max_extensions: fc.max_extensions.unwrap_or_default(),
  auto_dump: fc.auto_dump.map(to_auto_dump_format),
- parallel_file_reads: fc.max_parallel_file_reads,
- model_cache_ttl: fc.model_cache_ttl_secs,
+ parallel_file_reads: fc.max_parallel_file_reads.unwrap_or_default(),
+ model_cache_ttl: fc.model_cache_ttl_secs.unwrap_or_default(),
  session: fc.session.as_ref().map(to_session_config),
  commit: fc.commit.as_ref().map(to_session_config),
  suggest: fc.suggest.as_ref().map(to_session_config),
- is_restricted: fc.restricted,
- tool_supported: fc.tool_supported,
+ is_restricted: fc.restricted.unwrap_or_default(),
+ tool_supported: fc.tool_supported.unwrap_or_default(),
  temperature: fc
  .temperature
  .and_then(|v| Temperature::new(v.value() as f32).ok()),
@@ -293,37 +296,37 @@ fn to_forge_config(env: &Environment) -> ForgeConfig {
  } else {
  Some(from_retry_config(&env.retry_config))
  };
-    fc.max_search_lines = env.max_search_lines;
-    fc.max_search_result_bytes = env.max_search_result_bytes;
-    fc.max_fetch_chars = env.fetch_truncation_limit;
-    fc.max_stdout_prefix_lines = env.stdout_max_prefix_length;
-    fc.max_stdout_suffix_lines = env.stdout_max_suffix_length;
-    fc.max_stdout_line_chars = env.stdout_max_line_length;
-    fc.max_line_chars = env.max_line_length;
-    fc.max_read_lines = env.max_read_size;
-    fc.max_file_read_batch_size = env.max_file_read_batch_size;
+    fc.max_search_lines = Some(env.max_search_lines);
+    fc.max_search_result_bytes = Some(env.max_search_result_bytes);
+    fc.max_fetch_chars = Some(env.fetch_truncation_limit);
+    fc.max_stdout_prefix_lines = Some(env.stdout_max_prefix_length);
+    fc.max_stdout_suffix_lines = Some(env.stdout_max_suffix_length);
+    fc.max_stdout_line_chars = Some(env.stdout_max_line_length);
+    fc.max_line_chars = Some(env.max_line_length);
+    fc.max_read_lines = Some(env.max_read_size);
+    fc.max_file_read_batch_size = Some(env.max_file_read_batch_size);
  let default_http = HttpConfig::default();
     fc.http = if env.http == default_http {
  None
  } else {
  Some(from_http_config(&env.http))
  };
-    fc.max_file_size_bytes = env.max_file_size;
-    fc.max_image_size_bytes = env.max_image_size;
-    fc.tool_timeout_secs = env.tool_timeout;
-    fc.auto_open_dump = env.auto_open_dump;
+    fc.max_file_size_bytes = Some(env.max_file_size);
+    fc.max_image_size_bytes = Some(env.max_image_size);
+    fc.tool_timeout_secs = Some(env.tool_timeout);
+    fc.auto_open_dump = Some(env.auto_open_dump);
     fc.debug_requests = env.debug_requests.clone();
     fc.custom_history_path = env.custom_history_path.clone();
-    fc.max_conversations = env.max_conversations;
-    fc.max_sem_search_results = env.sem_search_limit;
-    fc.sem_search_top_k = env.sem_search_top_k;
-    fc.services_url = env.service_url.to_string();
-    fc.max_extensions = env.max_extensions;
+    fc.max_conversations = Some(env.max_conversations);
+    fc.max_sem_search_results = Some(env.sem_search_limit);
+    fc.sem_search_top_k = Some(env.sem_search_top_k);
+    fc.services_url = Some(env.service_url.to_string());
+    fc.max_extensions = Some(env.max_extensions);
     fc.auto_dump = env.auto_dump.as_ref().map(from_auto_dump_format);
-    fc.max_parallel_file_reads = env.parallel_file_reads;
-    fc.model_cache_ttl_secs = env.model_cache_ttl;
-    fc.restricted = env.is_restricted;
-    fc.tool_supported = env.tool_supported;
+    fc.max_parallel_file_reads = Some(env.parallel_file_reads);
+    fc.model_cache_ttl_secs = Some(env.model_cache_ttl);
+    fc.restricted = Some(env.is_restricted);
+    fc.tool_supported = Some(env.tool_supported);
 
  // --- Workflow fields ---
     fc.temperature = env