mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
use config write lock for specialized writers
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
use codex_config::types::PluginConfig;
|
||||
use codex_config::with_config_write_lock;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core_plugins::PluginInstallRequest;
|
||||
@@ -793,30 +794,7 @@ impl ExternalAgentConfigService {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(target_parent) = target_config.parent() else {
|
||||
return Err(invalid_data_error("config target path has no parent"));
|
||||
};
|
||||
fs::create_dir_all(target_parent)?;
|
||||
if !target_config.exists() {
|
||||
write_toml_file(&target_config, &migrated)?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let existing_raw = fs::read_to_string(&target_config)?;
|
||||
let mut existing = if existing_raw.trim().is_empty() {
|
||||
TomlValue::Table(Default::default())
|
||||
} else {
|
||||
toml::from_str::<TomlValue>(&existing_raw)
|
||||
.map_err(|err| invalid_data_error(format!("invalid existing config.toml: {err}")))?
|
||||
};
|
||||
|
||||
let changed = merge_missing_toml_values(&mut existing, &migrated)?;
|
||||
if !changed {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
write_toml_file(&target_config, &existing)?;
|
||||
Ok(())
|
||||
merge_missing_toml_file(&target_config, &migrated)
|
||||
}
|
||||
|
||||
fn import_mcp_server_config(&self, cwd: Option<&Path>) -> io::Result<()> {
|
||||
@@ -847,26 +825,7 @@ impl ExternalAgentConfigService {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(target_parent) = target_config.parent() else {
|
||||
return Err(invalid_data_error("config target path has no parent"));
|
||||
};
|
||||
fs::create_dir_all(target_parent)?;
|
||||
if !target_config.exists() {
|
||||
write_toml_file(&target_config, &migrated)?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let existing_raw = fs::read_to_string(&target_config)?;
|
||||
let mut existing = if existing_raw.trim().is_empty() {
|
||||
TomlValue::Table(Default::default())
|
||||
} else {
|
||||
toml::from_str::<TomlValue>(&existing_raw)
|
||||
.map_err(|err| invalid_data_error(format!("invalid existing config.toml: {err}")))?
|
||||
};
|
||||
if merge_missing_toml_values(&mut existing, &migrated)? {
|
||||
write_toml_file(&target_config, &existing)?;
|
||||
}
|
||||
Ok(())
|
||||
merge_missing_toml_file(&target_config, &migrated)
|
||||
}
|
||||
|
||||
fn import_subagents(&self, cwd: Option<&Path>) -> io::Result<usize> {
|
||||
@@ -1538,6 +1497,35 @@ fn merge_missing_toml_values(existing: &mut TomlValue, incoming: &TomlValue) ->
|
||||
}
|
||||
}
|
||||
|
||||
fn merge_missing_toml_file(target_config: &Path, incoming: &TomlValue) -> io::Result<()> {
|
||||
with_config_write_lock(target_config, |write_paths| {
|
||||
let Some(target_parent) = write_paths.write_path.parent() else {
|
||||
return Err(invalid_data_error("config target path has no parent"));
|
||||
};
|
||||
fs::create_dir_all(target_parent)?;
|
||||
let Some(read_path) = write_paths.read_path.as_deref() else {
|
||||
return write_toml_file(&write_paths.write_path, incoming);
|
||||
};
|
||||
let existing_raw = match fs::read_to_string(read_path) {
|
||||
Ok(raw) => raw,
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => {
|
||||
return write_toml_file(&write_paths.write_path, incoming);
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
let mut existing = if existing_raw.trim().is_empty() {
|
||||
TomlValue::Table(Default::default())
|
||||
} else {
|
||||
toml::from_str::<TomlValue>(&existing_raw)
|
||||
.map_err(|err| invalid_data_error(format!("invalid existing config.toml: {err}")))?
|
||||
};
|
||||
if merge_missing_toml_values(&mut existing, incoming)? {
|
||||
write_toml_file(&write_paths.write_path, &existing)?;
|
||||
}
|
||||
Ok(())
|
||||
})
|
||||
}
|
||||
|
||||
fn write_toml_file(path: &Path, value: &TomlValue) -> io::Result<()> {
|
||||
let serialized = toml::to_string_pretty(value)
|
||||
.map_err(|err| invalid_data_error(format!("failed to serialize config.toml: {err}")))?;
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_config::merge_toml_values;
|
||||
use codex_config::with_config_write_lock;
|
||||
use codex_core::config::deserialize_config_toml_with_base;
|
||||
use codex_core::config::edit::ConfigEdit;
|
||||
use codex_core::config::edit::ConfigEditsBuilder;
|
||||
@@ -399,10 +400,14 @@ async fn create_empty_user_layer(
|
||||
}
|
||||
|
||||
async fn write_empty_user_config(write_path: PathBuf) -> Result<(), ConfigManagerError> {
|
||||
task::spawn_blocking(move || write_atomically(&write_path, ""))
|
||||
.await
|
||||
.map_err(|err| ConfigManagerError::anyhow("config persistence task panicked", err.into()))?
|
||||
.map_err(|err| ConfigManagerError::io("failed to create empty user config.toml", err))
|
||||
task::spawn_blocking(move || {
|
||||
with_config_write_lock(&write_path, |write_paths| {
|
||||
write_atomically(&write_paths.write_path, "")
|
||||
})
|
||||
})
|
||||
.await
|
||||
.map_err(|err| ConfigManagerError::anyhow("config persistence task panicked", err.into()))?
|
||||
.map_err(|err| ConfigManagerError::io("failed to create empty user config.toml", err))
|
||||
}
|
||||
|
||||
fn parse_value(value: JsonValue) -> Result<Option<TomlValue>, String> {
|
||||
|
||||
@@ -9,6 +9,7 @@ use toml_edit::Value as TomlValue;
|
||||
use toml_edit::value;
|
||||
|
||||
use crate::CONFIG_TOML_FILE;
|
||||
use crate::with_config_write_lock;
|
||||
|
||||
pub struct MarketplaceConfigUpdate<'a> {
|
||||
pub last_updated: &'a str,
|
||||
@@ -32,10 +33,17 @@ pub fn record_user_marketplace(
|
||||
update: &MarketplaceConfigUpdate<'_>,
|
||||
) -> std::io::Result<()> {
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let mut doc = read_or_create_document(&config_path)?;
|
||||
upsert_marketplace(&mut doc, marketplace_name, update);
|
||||
fs::create_dir_all(codex_home)?;
|
||||
fs::write(config_path, doc.to_string())
|
||||
with_config_write_lock(&config_path, |write_paths| {
|
||||
let mut doc = read_or_create_document(
|
||||
write_paths
|
||||
.read_path
|
||||
.as_deref()
|
||||
.unwrap_or(&write_paths.write_path),
|
||||
)?;
|
||||
upsert_marketplace(&mut doc, marketplace_name, update);
|
||||
fs::create_dir_all(codex_home)?;
|
||||
fs::write(&write_paths.write_path, doc.to_string())
|
||||
})
|
||||
}
|
||||
|
||||
pub fn remove_user_marketplace(codex_home: &Path, marketplace_name: &str) -> std::io::Result<bool> {
|
||||
@@ -48,24 +56,31 @@ pub fn remove_user_marketplace_config(
|
||||
marketplace_name: &str,
|
||||
) -> std::io::Result<RemoveMarketplaceConfigOutcome> {
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let mut doc = match fs::read_to_string(&config_path) {
|
||||
Ok(raw) => raw
|
||||
.parse::<DocumentMut>()
|
||||
.map_err(|err| std::io::Error::new(ErrorKind::InvalidData, err))?,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => {
|
||||
return Ok(RemoveMarketplaceConfigOutcome::NotFound);
|
||||
with_config_write_lock(&config_path, |write_paths| {
|
||||
let mut doc = match fs::read_to_string(
|
||||
write_paths
|
||||
.read_path
|
||||
.as_deref()
|
||||
.unwrap_or(&write_paths.write_path),
|
||||
) {
|
||||
Ok(raw) => raw
|
||||
.parse::<DocumentMut>()
|
||||
.map_err(|err| std::io::Error::new(ErrorKind::InvalidData, err))?,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => {
|
||||
return Ok(RemoveMarketplaceConfigOutcome::NotFound);
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
|
||||
let outcome = remove_marketplace(&mut doc, marketplace_name);
|
||||
if outcome != RemoveMarketplaceConfigOutcome::Removed {
|
||||
return Ok(outcome);
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
|
||||
let outcome = remove_marketplace(&mut doc, marketplace_name);
|
||||
if outcome != RemoveMarketplaceConfigOutcome::Removed {
|
||||
return Ok(outcome);
|
||||
}
|
||||
|
||||
fs::create_dir_all(codex_home)?;
|
||||
fs::write(config_path, doc.to_string())?;
|
||||
Ok(RemoveMarketplaceConfigOutcome::Removed)
|
||||
fs::create_dir_all(codex_home)?;
|
||||
fs::write(&write_paths.write_path, doc.to_string())?;
|
||||
Ok(RemoveMarketplaceConfigOutcome::Removed)
|
||||
})
|
||||
}
|
||||
|
||||
fn read_or_create_document(config_path: &Path) -> std::io::Result<DocumentMut> {
|
||||
|
||||
@@ -16,6 +16,7 @@ use crate::CONFIG_TOML_FILE;
|
||||
use crate::McpServerConfig;
|
||||
use crate::McpServerEnvVar;
|
||||
use crate::McpServerTransportConfig;
|
||||
use crate::with_config_write_lock;
|
||||
|
||||
pub async fn load_global_mcp_servers(
|
||||
codex_home: &Path,
|
||||
@@ -87,12 +88,19 @@ impl ConfigEditsBuilder {
|
||||
|
||||
fn apply_blocking(self) -> std::io::Result<()> {
|
||||
let config_path = self.codex_home.join(CONFIG_TOML_FILE);
|
||||
let mut doc = read_or_create_document(&config_path)?;
|
||||
if let Some(servers) = self.mcp_servers.as_ref() {
|
||||
replace_mcp_servers(&mut doc, servers);
|
||||
}
|
||||
fs::create_dir_all(&self.codex_home)?;
|
||||
fs::write(config_path, doc.to_string())
|
||||
with_config_write_lock(&config_path, |write_paths| {
|
||||
let mut doc = read_or_create_document(
|
||||
write_paths
|
||||
.read_path
|
||||
.as_deref()
|
||||
.unwrap_or(&write_paths.write_path),
|
||||
)?;
|
||||
if let Some(servers) = self.mcp_servers.as_ref() {
|
||||
replace_mcp_servers(&mut doc, servers);
|
||||
}
|
||||
fs::create_dir_all(&self.codex_home)?;
|
||||
fs::write(&write_paths.write_path, doc.to_string())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,7 +2,6 @@ use std::fs;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
|
||||
use codex_utils_path::resolve_symlink_write_paths;
|
||||
use codex_utils_path::write_atomically;
|
||||
use tokio::task;
|
||||
use toml_edit::DocumentMut;
|
||||
@@ -11,6 +10,7 @@ use toml_edit::Table as TomlTable;
|
||||
use toml_edit::value;
|
||||
|
||||
use crate::CONFIG_TOML_FILE;
|
||||
use crate::with_config_write_lock;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum PluginConfigEdit {
|
||||
@@ -56,22 +56,23 @@ fn apply_user_plugin_config_edits_blocking(
|
||||
}
|
||||
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let write_paths = resolve_symlink_write_paths(&config_path)?;
|
||||
let mut doc = read_or_create_document(write_paths.read_path.as_deref())?;
|
||||
let mut mutated = false;
|
||||
for edit in edits {
|
||||
mutated |= match edit {
|
||||
PluginConfigEdit::SetEnabled {
|
||||
plugin_key,
|
||||
enabled,
|
||||
} => set_plugin_enabled(&mut doc, &plugin_key, enabled),
|
||||
PluginConfigEdit::Clear { plugin_key } => clear_plugin(&mut doc, &plugin_key),
|
||||
};
|
||||
}
|
||||
if !mutated {
|
||||
return Ok(());
|
||||
}
|
||||
write_atomically(&write_paths.write_path, &doc.to_string())
|
||||
with_config_write_lock(&config_path, |write_paths| {
|
||||
let mut doc = read_or_create_document(write_paths.read_path.as_deref())?;
|
||||
let mut mutated = false;
|
||||
for edit in edits {
|
||||
mutated |= match edit {
|
||||
PluginConfigEdit::SetEnabled {
|
||||
plugin_key,
|
||||
enabled,
|
||||
} => set_plugin_enabled(&mut doc, &plugin_key, enabled),
|
||||
PluginConfigEdit::Clear { plugin_key } => clear_plugin(&mut doc, &plugin_key),
|
||||
};
|
||||
}
|
||||
if !mutated {
|
||||
return Ok(());
|
||||
}
|
||||
write_atomically(&write_paths.write_path, &doc.to_string())
|
||||
})
|
||||
}
|
||||
|
||||
fn read_or_create_document(config_path: Option<&Path>) -> std::io::Result<DocumentMut> {
|
||||
|
||||
Reference in New Issue
Block a user