mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Upgrade Git marketplaces from merged configuration (#42149)
## Why Configured Git marketplaces may be defined outside the user config, but those definitions could not download or update their snapshots. ## What changed - Select marketplace source, ref, and sparse-path settings from the effective merged configuration for startup synchronization and explicit upgrades. - Preserve the initiating operation's configuration loader and overrides when reloading before activation. - Roll back activation if the marketplace definition changed or configuration can no longer be loaded, without copying settings into the user config. ## Testing - Cover system-defined marketplaces, CLI overrides, startup fallback after invalid user configuration, and rollback after configuration changes. GitOrigin-RevId: 53d1eecf68a91ce2aa8430e5749d78697bfcd821
This commit is contained in:
committed by
copyberry
parent
e576993911
commit
68c9556cdf
@@ -339,8 +339,14 @@ sources before returning; ordinary listing schedules the same work in the backgr
|
||||
Remote catalog settings and feature gating remain request-wide rather than being
|
||||
selected from the requested repos. Search continues to report `enabled: false`.
|
||||
|
||||
Marketplace definitions can come from system configuration, but configured Git
|
||||
marketplaces currently require an existing downloaded snapshot.
|
||||
Marketplace definitions can come from system configuration. Startup synchronization
|
||||
and `marketplace/upgrade` download or update configured Git marketplaces using the
|
||||
merged source, ref, and sparse-path settings. Snapshot metadata stays with the
|
||||
downloaded files; configuration is not copied into the user layer. Pure catalog
|
||||
listing does not wait for missing snapshots to download.
|
||||
Activation reloads configuration with the operation's original load settings and
|
||||
rolls back if the marketplace definition changed or the reload fails. User files
|
||||
ignored at startup remain ignored during this check.
|
||||
|
||||
`marketplace/remove` rejects removal when the marketplace name is defined in another
|
||||
enabled layer of the operation's loaded config stack. Otherwise it removes the
|
||||
|
||||
@@ -63,6 +63,7 @@ use crate::outgoing_message::OutgoingEnvelope;
|
||||
use crate::outgoing_message::OutgoingMessage;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use crate::outgoing_message::QueuedOutgoingMessage;
|
||||
use crate::plugin_config_reload::PluginStartupConfig;
|
||||
use crate::transport::CHANNEL_CAPACITY;
|
||||
use crate::transport::OutboundConnectionState;
|
||||
use crate::transport::route_outgoing_envelope;
|
||||
@@ -486,7 +487,7 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult<InProcessClie
|
||||
code_mode_session_provider: None,
|
||||
rpc_transport: AppServerRpcTransport::InProcess,
|
||||
remote_control_handle: None,
|
||||
plugin_startup_tasks: crate::PluginStartupTasks::Start,
|
||||
plugin_startup_tasks: Some(PluginStartupConfig::Current),
|
||||
}));
|
||||
let mut thread_created_rx = processor.thread_created_receiver();
|
||||
let session = Arc::new(ConnectionSessionState::new());
|
||||
|
||||
@@ -31,6 +31,7 @@ use crate::outgoing_message::ConnectionId;
|
||||
use crate::outgoing_message::OutgoingEnvelope;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use crate::outgoing_message::QueuedOutgoingMessage;
|
||||
use crate::plugin_config_reload::PluginStartupConfig;
|
||||
use crate::transport::CHANNEL_CAPACITY;
|
||||
use crate::transport::ConnectionOrigin;
|
||||
use crate::transport::ConnectionState;
|
||||
@@ -122,6 +123,7 @@ mod models_refresh_worker;
|
||||
mod notification_media;
|
||||
mod otel_reloader;
|
||||
mod outgoing_message;
|
||||
mod plugin_config_reload;
|
||||
mod request_processors;
|
||||
mod request_serialization;
|
||||
mod server_request_error;
|
||||
@@ -524,6 +526,7 @@ pub async fn run_main_with_transport_options(
|
||||
}
|
||||
};
|
||||
let mut config_warnings = Vec::new();
|
||||
let mut plugin_startup_config = PluginStartupConfig::Current;
|
||||
let config = match config_manager
|
||||
.load_latest_config(/*fallback_cwd*/ None)
|
||||
.await
|
||||
@@ -539,6 +542,7 @@ pub async fn run_main_with_transport_options(
|
||||
|
||||
let message = config_warning_from_error("Invalid configuration; using defaults.", &err);
|
||||
config_warnings.push(message);
|
||||
plugin_startup_config = PluginStartupConfig::Defaults;
|
||||
config_manager.load_default_config().await.map_err(|e| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidData,
|
||||
@@ -917,7 +921,11 @@ pub async fn run_main_with_transport_options(
|
||||
code_mode_session_provider,
|
||||
rpc_transport: analytics_rpc_transport(&transport),
|
||||
remote_control_handle: Some(remote_control_handle.clone()),
|
||||
plugin_startup_tasks: runtime_options.plugin_startup_tasks,
|
||||
plugin_startup_tasks: matches!(
|
||||
runtime_options.plugin_startup_tasks,
|
||||
PluginStartupTasks::Start
|
||||
)
|
||||
.then_some(plugin_startup_config),
|
||||
}));
|
||||
let mut thread_created_rx = processor.thread_created_receiver();
|
||||
let mut running_turn_count_rx = processor.subscribe_running_assistant_turn_count();
|
||||
|
||||
@@ -22,6 +22,8 @@ use crate::outgoing_message::ConnectionId;
|
||||
use crate::outgoing_message::ConnectionRequestId;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use crate::outgoing_message::RequestContext;
|
||||
use crate::plugin_config_reload;
|
||||
use crate::plugin_config_reload::PluginStartupConfig;
|
||||
use crate::request_processors::AccountRequestProcessor;
|
||||
use crate::request_processors::AppsRequestProcessor;
|
||||
use crate::request_processors::CatalogRequestProcessor;
|
||||
@@ -258,7 +260,8 @@ pub(crate) struct MessageProcessorArgs {
|
||||
pub(crate) code_mode_session_provider: Option<Arc<dyn CodeModeSessionProvider>>,
|
||||
pub(crate) rpc_transport: AppServerRpcTransport,
|
||||
pub(crate) remote_control_handle: Option<RemoteControlHandle>,
|
||||
pub(crate) plugin_startup_tasks: crate::PluginStartupTasks,
|
||||
/// `None` skips startup tasks; otherwise preserve the initial config-loading path.
|
||||
pub(crate) plugin_startup_tasks: Option<PluginStartupConfig>,
|
||||
}
|
||||
|
||||
impl MessageProcessor {
|
||||
@@ -519,14 +522,23 @@ impl MessageProcessor {
|
||||
Arc::clone(&skills_watcher),
|
||||
turn_cost_worker.as_ref().map(TurnCostWorker::handle),
|
||||
);
|
||||
if matches!(plugin_startup_tasks, crate::PluginStartupTasks::Start) {
|
||||
if let Some(startup_config) = plugin_startup_tasks {
|
||||
// Keep plugin startup warmups aligned at app-server startup.
|
||||
let reload_config = match startup_config {
|
||||
PluginStartupConfig::Current => {
|
||||
plugin_config_reload::for_cwd(config_manager.clone(), config.cwd.clone())
|
||||
}
|
||||
PluginStartupConfig::Defaults => {
|
||||
plugin_config_reload::defaults(config_manager.clone())
|
||||
}
|
||||
};
|
||||
let on_effective_plugins_changed =
|
||||
plugin_processor.effective_plugins_changed_callback();
|
||||
thread_manager
|
||||
.plugins_manager()
|
||||
.maybe_start_plugin_startup_tasks_for_config(
|
||||
&config.plugins_config_input(),
|
||||
reload_config,
|
||||
Some(on_effective_plugins_changed),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ use crate::analytics_utils::analytics_events_client_from_config;
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::outgoing_message::ConnectionId;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use crate::plugin_config_reload::PluginStartupConfig;
|
||||
use crate::transport::AppServerTransport;
|
||||
use anyhow::Result;
|
||||
use app_test_support::create_mock_responses_server_repeating_assistant;
|
||||
@@ -269,7 +270,7 @@ async fn build_test_processor(
|
||||
code_mode_session_provider: None,
|
||||
rpc_transport: AppServerRpcTransport::Stdio,
|
||||
remote_control_handle: None,
|
||||
plugin_startup_tasks: crate::PluginStartupTasks::Start,
|
||||
plugin_startup_tasks: Some(PluginStartupConfig::Current),
|
||||
}));
|
||||
(processor, outgoing_rx)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
//! Adapts existing config loaders for blocking plugin-upgrade workers.
|
||||
//! Reloads must use the same normal or fallback entry point as the initial load.
|
||||
|
||||
use crate::config_manager::ConfigManager;
|
||||
use codex_core_plugins::ConfigLayerReload;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// The config-loading path used to select marketplaces for startup tasks.
|
||||
pub(crate) enum PluginStartupConfig {
|
||||
Current,
|
||||
Defaults,
|
||||
}
|
||||
|
||||
pub(crate) fn for_cwd(manager: ConfigManager, cwd: AbsolutePathBuf) -> ConfigLayerReload {
|
||||
let runtime = tokio::runtime::Handle::current();
|
||||
Arc::new(move || runtime.block_on(manager.load_config_layers_for_cwd(cwd.clone())))
|
||||
}
|
||||
|
||||
pub(crate) fn defaults(manager: ConfigManager) -> ConfigLayerReload {
|
||||
let runtime = tokio::runtime::Handle::current();
|
||||
Arc::new(move || {
|
||||
runtime
|
||||
.block_on(manager.load_default_config())
|
||||
.map(|config| config.config_layer_stack)
|
||||
})
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::plugin_config_reload;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct MarketplaceRequestProcessor {
|
||||
@@ -78,11 +79,14 @@ impl MarketplaceRequestProcessor {
|
||||
let plugins_manager = self.thread_manager.plugins_manager();
|
||||
let MarketplaceUpgradeParams { marketplace_name } = params;
|
||||
let plugins_input = config.plugins_config_input();
|
||||
let reload_config =
|
||||
plugin_config_reload::for_cwd(self.config_manager.clone(), config.cwd.clone());
|
||||
|
||||
let outcome = tokio::task::spawn_blocking(move || {
|
||||
plugins_manager.upgrade_configured_marketplaces_for_config(
|
||||
&plugins_input,
|
||||
marketplace_name.as_deref(),
|
||||
&reload_config,
|
||||
)
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -19,6 +19,7 @@ use codex_config::record_user_marketplace;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use test_case::test_case;
|
||||
use tokio::time::timeout;
|
||||
|
||||
#[cfg(windows)]
|
||||
@@ -152,17 +153,17 @@ async fn marketplace_upgrade_all_configured_git_marketplaces() -> Result<()> {
|
||||
debug_source.path(),
|
||||
Some(&debug_new_revision),
|
||||
)?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"tools",
|
||||
tools_source.path(),
|
||||
Some(&tools_new_revision),
|
||||
)?;
|
||||
let tools_override = format!(
|
||||
"marketplaces.tools={{ source_type = \"git\", source = {}, ref = {} }}",
|
||||
serde_json::to_string(&tools_source.path().to_string_lossy())?,
|
||||
serde_json::to_string(&tools_new_revision)?,
|
||||
);
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
let config_before = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.with_args(&["--config", &tools_override])
|
||||
.without_auto_env()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
@@ -204,9 +205,12 @@ async fn marketplace_upgrade_all_configured_git_marketplaces() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(false; "user config")]
|
||||
#[test_case(true; "invalid user config fallback")]
|
||||
#[tokio::test]
|
||||
async fn automatic_upgrade_isolates_git_while_explicit_install_preserves_configuration()
|
||||
-> Result<()> {
|
||||
async fn automatic_upgrade_isolates_git_while_explicit_install_preserves_configuration(
|
||||
invalid_user_config: bool,
|
||||
) -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let source = TempDir::new()?;
|
||||
let plugin_source = TempDir::new()?;
|
||||
@@ -266,14 +270,30 @@ async fn automatic_upgrade_isolates_git_while_explicit_install_preserves_configu
|
||||
|
||||
let manual_plugin_alias = "https://manual.example/plugin.git";
|
||||
let manual_plugin_rewrite = format!("url.{}.insteadOf", plugin_source.path().display());
|
||||
let mut server = TestAppServer::builder()
|
||||
let config_before = std::fs::read_to_string(&config_path)?;
|
||||
let mut builder = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.with_plugin_startup_tasks()
|
||||
.with_env_overrides(&[
|
||||
("GIT_CONFIG_COUNT", Some("1")),
|
||||
("GIT_CONFIG_KEY_0", Some(&manual_plugin_rewrite)),
|
||||
("GIT_CONFIG_VALUE_0", Some(manual_plugin_alias)),
|
||||
])
|
||||
]);
|
||||
if invalid_user_config {
|
||||
// Runtime definitions remain available when startup ignores malformed user config.
|
||||
std::fs::write(&config_path, "invalid = [")?;
|
||||
let marketplace_override = format!(
|
||||
"marketplaces.trusted={{ source_type = 'git', source = {} }}",
|
||||
serde_json::to_string(&source_path)?,
|
||||
);
|
||||
builder = builder.with_args(&[
|
||||
"--config",
|
||||
&marketplace_override,
|
||||
"--config",
|
||||
"plugins={\"toolkit@trusted\"={enabled=true}}",
|
||||
]);
|
||||
}
|
||||
let mut server = builder
|
||||
.build_initialized_with_timeout(DEFAULT_TIMEOUT)
|
||||
.await?;
|
||||
let plugin_root = codex_home
|
||||
@@ -286,6 +306,10 @@ async fn automatic_upgrade_isolates_git_while_explicit_install_preserves_configu
|
||||
})
|
||||
.await?;
|
||||
|
||||
if invalid_user_config {
|
||||
// Subsequent explicit operations must pick up a repaired user config normally.
|
||||
std::fs::write(&config_path, config_before)?;
|
||||
}
|
||||
let marketplace_path = marketplace_install_root(codex_home.path())
|
||||
.join("trusted/.agents/plugins/marketplace.json");
|
||||
std::fs::write(
|
||||
|
||||
@@ -16,6 +16,17 @@ pub(crate) async fn load_config(
|
||||
config_overrides: &CliConfigOverrides,
|
||||
loader_overrides: LoaderOverrides,
|
||||
) -> Result<Config> {
|
||||
config_builder(config_overrides, loader_overrides)
|
||||
.await?
|
||||
.build()
|
||||
.await
|
||||
.context("failed to load configuration")
|
||||
}
|
||||
|
||||
pub(crate) async fn config_builder(
|
||||
config_overrides: &CliConfigOverrides,
|
||||
loader_overrides: LoaderOverrides,
|
||||
) -> Result<ConfigBuilder> {
|
||||
let cli_overrides = config_overrides
|
||||
.parse_overrides()
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
@@ -41,12 +52,10 @@ pub(crate) async fn load_config(
|
||||
.await
|
||||
.context("failed to initialize cloud configuration authentication")?;
|
||||
|
||||
ConfigBuilder::default()
|
||||
Ok(ConfigBuilder::default()
|
||||
.codex_home(codex_home.to_path_buf())
|
||||
.cli_overrides(cli_overrides)
|
||||
.loader_overrides(loader_overrides)
|
||||
.cloud_config_bundle(cloud_config_bundle)
|
||||
.build()
|
||||
.await
|
||||
.context("failed to load configuration")
|
||||
.fallback_cwd(Some(cwd.to_path_buf())))
|
||||
}
|
||||
|
||||
@@ -128,13 +128,14 @@ impl MarketplaceCli {
|
||||
subcommand,
|
||||
} = self;
|
||||
|
||||
let config =
|
||||
cloud_config::load_config(&config_overrides, LoaderOverrides::default()).await?;
|
||||
let builder =
|
||||
cloud_config::config_builder(&config_overrides, LoaderOverrides::default()).await?;
|
||||
let config = builder.clone().build().await?;
|
||||
|
||||
match subcommand {
|
||||
MarketplaceSubcommand::Add(args) => run_add(config, args).await?,
|
||||
MarketplaceSubcommand::List(args) => run_list(config, args).await?,
|
||||
MarketplaceSubcommand::Upgrade(args) => run_upgrade(config, args).await?,
|
||||
MarketplaceSubcommand::Upgrade(args) => run_upgrade(config, args, builder).await?,
|
||||
MarketplaceSubcommand::Remove(args) => run_remove(config, args).await?,
|
||||
}
|
||||
|
||||
@@ -358,16 +359,33 @@ fn configured_marketplace_sources_by_root(
|
||||
.collect()
|
||||
}
|
||||
|
||||
async fn run_upgrade(config: Config, args: UpgradeMarketplaceArgs) -> Result<()> {
|
||||
async fn run_upgrade(
|
||||
config: Config,
|
||||
args: UpgradeMarketplaceArgs,
|
||||
builder: codex_core::config::ConfigBuilder,
|
||||
) -> Result<()> {
|
||||
let UpgradeMarketplaceArgs {
|
||||
marketplace_name,
|
||||
json,
|
||||
} = args;
|
||||
let manager = plugins_manager_for_config(&config, load_cli_auth_manager(&config).await?);
|
||||
let plugins_input = config.plugins_config_input();
|
||||
let outcome = manager
|
||||
.upgrade_configured_marketplaces_for_config(&plugins_input, marketplace_name.as_deref())
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
let runtime = tokio::runtime::Handle::current();
|
||||
let reload_config: codex_core_plugins::ConfigLayerReload = std::sync::Arc::new(move || {
|
||||
runtime
|
||||
.block_on(builder.clone().build())
|
||||
.map(|config| config.config_layer_stack)
|
||||
});
|
||||
let requested_name = marketplace_name.clone();
|
||||
let outcome = tokio::task::spawn_blocking(move || {
|
||||
manager.upgrade_configured_marketplaces_for_config(
|
||||
&plugins_input,
|
||||
requested_name.as_deref(),
|
||||
&reload_config,
|
||||
)
|
||||
})
|
||||
.await?
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
if json {
|
||||
print_upgrade_outcome_json(&outcome)
|
||||
} else {
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use anyhow::Result;
|
||||
use codex_core_plugins::installed_marketplaces::marketplace_install_root;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use predicates::str::contains;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
@@ -27,22 +29,65 @@ async fn marketplace_upgrade_runs_under_plugin() -> Result<()> {
|
||||
#[tokio::test]
|
||||
async fn marketplace_upgrade_json_prints_upgrade_outcome() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let source = TempDir::new()?;
|
||||
let manifest = r#"{"name":"debug","plugins":[]}"#;
|
||||
std::fs::create_dir_all(source.path().join(".agents/plugins"))?;
|
||||
std::fs::write(
|
||||
source.path().join(".agents/plugins/marketplace.json"),
|
||||
manifest,
|
||||
)?;
|
||||
for args in [
|
||||
vec!["init", "--quiet"],
|
||||
vec!["add", "."],
|
||||
vec![
|
||||
"-c",
|
||||
"user.name=Codex Tests",
|
||||
"-c",
|
||||
"user.email=codex@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"marketplace",
|
||||
],
|
||||
] {
|
||||
let output = std::process::Command::new("git")
|
||||
.current_dir(source.path())
|
||||
.args(args)
|
||||
.output()?;
|
||||
anyhow::ensure!(
|
||||
output.status.success(),
|
||||
"git fixture failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
let source_arg = serde_json::to_string(&source.path().to_string_lossy())?;
|
||||
let marketplace_override =
|
||||
format!("marketplaces.debug={{source_type=\"git\",source={source_arg}}}");
|
||||
|
||||
let assert = codex_command(codex_home.path())?
|
||||
.current_dir(codex_home.path())
|
||||
.args(["--config", &marketplace_override])
|
||||
.args(["plugin", "marketplace", "upgrade", "--json"])
|
||||
.assert()
|
||||
.success();
|
||||
let stdout = assert.get_output().stdout.as_slice();
|
||||
let actual: serde_json::Value = serde_json::from_slice(stdout)?;
|
||||
let installed_root = AbsolutePathBuf::try_from(
|
||||
marketplace_install_root(&codex_home.path().canonicalize()?).join("debug"),
|
||||
)?;
|
||||
|
||||
assert_eq!(
|
||||
actual,
|
||||
json!({
|
||||
"selectedMarketplaces": [],
|
||||
"upgradedRoots": [],
|
||||
"selectedMarketplaces": ["debug"],
|
||||
"upgradedRoots": [installed_root],
|
||||
"errors": [],
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(installed_root.join(".agents/plugins/marketplace.json"))?,
|
||||
manifest
|
||||
);
|
||||
assert!(!codex_home.path().join("config.toml").exists());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -89,6 +89,7 @@ pub use manager::RemotePluginOperationError;
|
||||
pub use manager::RemotePluginOperationErrorKind;
|
||||
pub use manager::RemotePluginUninstallOutcome;
|
||||
pub use marketplace_policy::allowed_configured_marketplace_names;
|
||||
pub use marketplace_upgrade::ConfigLayerReload;
|
||||
pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeError as PluginMarketplaceUpgradeError;
|
||||
pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarketplaceUpgradeOutcome;
|
||||
pub use plugin_metrics::PluginMeasurementDefinition;
|
||||
|
||||
@@ -58,6 +58,7 @@ use crate::marketplace::list_marketplaces_with_home;
|
||||
use crate::marketplace::plugin_interface_with_marketplace_category;
|
||||
use crate::marketplace_policy::MarketplacePolicy;
|
||||
use crate::marketplace_policy::configured_plugins_from_stack;
|
||||
use crate::marketplace_upgrade::ConfigLayerReload;
|
||||
use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError;
|
||||
use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome;
|
||||
use crate::marketplace_upgrade::upgrade_configured_git_marketplaces_with_mode;
|
||||
@@ -2734,6 +2735,7 @@ impl PluginsManager {
|
||||
pub fn maybe_start_plugin_startup_tasks_for_config(
|
||||
self: &Arc<Self>,
|
||||
config: &PluginsConfigInput,
|
||||
reload_config: ConfigLayerReload,
|
||||
on_effective_plugins_changed: Option<EffectivePluginsChangedCallback>,
|
||||
) {
|
||||
if config.plugins_enabled {
|
||||
@@ -2765,6 +2767,7 @@ impl PluginsManager {
|
||||
&config,
|
||||
/*marketplace_name*/ None,
|
||||
PluginGitMode::Automatic,
|
||||
&reload_config,
|
||||
);
|
||||
match outcome {
|
||||
Ok(outcome) => {
|
||||
@@ -2860,11 +2863,13 @@ impl PluginsManager {
|
||||
&self,
|
||||
config: &PluginsConfigInput,
|
||||
marketplace_name: Option<&str>,
|
||||
reload_config: &ConfigLayerReload,
|
||||
) -> Result<ConfiguredMarketplaceUpgradeOutcome, String> {
|
||||
self.upgrade_configured_marketplaces_for_config_with_mode(
|
||||
config,
|
||||
marketplace_name,
|
||||
PluginGitMode::Manual,
|
||||
reload_config,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -2874,12 +2879,14 @@ impl PluginsManager {
|
||||
config: &PluginsConfigInput,
|
||||
marketplace_name: Option<&str>,
|
||||
mode: PluginGitMode,
|
||||
reload_config: &ConfigLayerReload,
|
||||
) -> Result<ConfiguredMarketplaceUpgradeOutcome, String> {
|
||||
let mut outcome = upgrade_configured_git_marketplaces_with_mode(
|
||||
self.codex_home.as_path(),
|
||||
&config.config_layer_stack,
|
||||
marketplace_name,
|
||||
mode,
|
||||
reload_config,
|
||||
);
|
||||
if let Some(marketplace_name) = marketplace_name
|
||||
&& outcome.selected_marketplaces.is_empty()
|
||||
|
||||
@@ -5313,8 +5313,14 @@ source = "{remote_repo_url}"
|
||||
|
||||
let manager = test_plugins_manager(tmp.path().to_path_buf());
|
||||
let config = load_config(tmp.path(), tmp.path()).await;
|
||||
let reload_stack = config.config_layer_stack.clone();
|
||||
let reload_config: ConfigLayerReload = Arc::new(move || Ok(reload_stack.clone()));
|
||||
let initial_upgrade = manager
|
||||
.upgrade_configured_marketplaces_for_config(&config, /*marketplace_name*/ None)
|
||||
.upgrade_configured_marketplaces_for_config(
|
||||
&config,
|
||||
/*marketplace_name*/ None,
|
||||
&reload_config,
|
||||
)
|
||||
.expect("initial marketplace install should succeed");
|
||||
assert_eq!(initial_upgrade.errors, Vec::new());
|
||||
assert_eq!(initial_upgrade.upgraded_roots.len(), 1);
|
||||
@@ -5350,7 +5356,7 @@ source = "{remote_repo_url}"
|
||||
run_git(&remote_repo, &["add", "."]);
|
||||
run_git(&remote_repo, &["commit", "-m", "update plugin"]);
|
||||
let upgrade = manager
|
||||
.upgrade_configured_marketplaces_for_config(&config, Some("debug"))
|
||||
.upgrade_configured_marketplaces_for_config(&config, Some("debug"), &reload_config)
|
||||
.expect("marketplace upgrade should succeed");
|
||||
assert_eq!(upgrade.errors, Vec::new());
|
||||
assert_eq!(upgrade.upgraded_roots.len(), 1);
|
||||
|
||||
@@ -690,6 +690,8 @@ enabled = true
|
||||
|
||||
#[test]
|
||||
fn blocked_or_reserved_upgrade_is_rejected_before_marketplace_installation() {
|
||||
let reload_config: crate::ConfigLayerReload =
|
||||
std::sync::Arc::new(|| panic!("blocked or reserved marketplace must not reload config"));
|
||||
let codex_home = TempDir::new().expect("create Codex home");
|
||||
let config_file = AbsolutePathBuf::try_from(codex_home.path().join("config.toml"))
|
||||
.expect("absolute config path");
|
||||
@@ -708,7 +710,12 @@ source = "https://github.com/example/blocked.git"
|
||||
)),
|
||||
);
|
||||
|
||||
let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &stack, Some("debug"));
|
||||
let outcome = upgrade_configured_git_marketplaces(
|
||||
codex_home.path(),
|
||||
&stack,
|
||||
Some("debug"),
|
||||
&reload_config,
|
||||
);
|
||||
|
||||
assert_eq!(outcome.selected_marketplaces, vec!["debug".to_string()]);
|
||||
assert_eq!(outcome.upgraded_roots, Vec::new());
|
||||
@@ -731,6 +738,7 @@ source = "https://github.com/example/blocked.git"
|
||||
codex_home.path(),
|
||||
&stack,
|
||||
Some(crate::OPENAI_BUNDLED_MARKETPLACE_NAME),
|
||||
&reload_config,
|
||||
);
|
||||
assert!(outcome.errors[0].message.contains("is reserved"));
|
||||
assert!(!marketplace_install_root(codex_home.path()).exists());
|
||||
|
||||
@@ -13,15 +13,18 @@ use crate::marketplace::validate_marketplace_root;
|
||||
use crate::marketplace_add::MarketplaceSource;
|
||||
use crate::marketplace_policy::MarketplacePolicy;
|
||||
use crate::marketplace_policy::validate_marketplace_name_for_add;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::types::MarketplaceConfig;
|
||||
use codex_config::types::MarketplaceSourceType;
|
||||
use codex_plugin::validate_plugin_segment;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
/// Reloads configuration with the initiating operation's settings. Called only on a blocking worker.
|
||||
pub type ConfigLayerReload = Arc<dyn Fn() -> std::io::Result<ConfigLayerStack> + Send + Sync>;
|
||||
|
||||
const MARKETPLACE_UPGRADE_GIT_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -71,12 +74,14 @@ pub fn upgrade_configured_git_marketplaces(
|
||||
codex_home: &Path,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
marketplace_name: Option<&str>,
|
||||
reload_config: &ConfigLayerReload,
|
||||
) -> ConfiguredMarketplaceUpgradeOutcome {
|
||||
upgrade_configured_git_marketplaces_with_mode(
|
||||
codex_home,
|
||||
config_layer_stack,
|
||||
marketplace_name,
|
||||
PluginGitMode::Manual,
|
||||
reload_config,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -86,6 +91,7 @@ pub(crate) fn upgrade_configured_git_marketplaces_with_mode(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
marketplace_name: Option<&str>,
|
||||
mode: PluginGitMode,
|
||||
reload_config: &ConfigLayerReload,
|
||||
) -> ConfiguredMarketplaceUpgradeOutcome {
|
||||
let loaded = load_configured_git_marketplaces(config_layer_stack);
|
||||
let marketplaces = loaded
|
||||
@@ -131,6 +137,7 @@ pub(crate) fn upgrade_configured_git_marketplaces_with_mode(
|
||||
codex_home,
|
||||
&install_root,
|
||||
&marketplace,
|
||||
reload_config,
|
||||
normalized_source.as_ref(),
|
||||
mode,
|
||||
) {
|
||||
@@ -155,10 +162,8 @@ pub(crate) fn upgrade_configured_git_marketplaces_with_mode(
|
||||
fn load_configured_git_marketplaces(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> ConfiguredGitMarketplaceLoadOutcome {
|
||||
let Some(user_config) = config_layer_stack.effective_user_config() else {
|
||||
return ConfiguredGitMarketplaceLoadOutcome::default();
|
||||
};
|
||||
let Some(marketplaces) = user_config
|
||||
let effective_config = config_layer_stack.effective_config();
|
||||
let Some(marketplaces) = effective_config
|
||||
.get("marketplaces")
|
||||
.and_then(toml::Value::as_table)
|
||||
else {
|
||||
@@ -221,6 +226,7 @@ fn upgrade_configured_git_marketplace(
|
||||
codex_home: &Path,
|
||||
install_root: &Path,
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
reload_config: &ConfigLayerReload,
|
||||
normalized_source: Option<&MarketplaceSource>,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<Option<AbsolutePathBuf>, String> {
|
||||
@@ -284,7 +290,7 @@ fn upgrade_configured_git_marketplace(
|
||||
}
|
||||
write_installed_marketplace_metadata(staged_dir.path(), marketplace, &activated_revision)?;
|
||||
activate_marketplace_root(&destination, staged_dir, &previous_snapshot, || {
|
||||
ensure_configured_git_marketplace_unchanged(codex_home, marketplace)
|
||||
ensure_configured_git_marketplace_unchanged(reload_config, marketplace)
|
||||
})?;
|
||||
|
||||
AbsolutePathBuf::try_from(destination)
|
||||
@@ -292,10 +298,10 @@ fn upgrade_configured_git_marketplace(
|
||||
.map_err(|err| format!("upgraded marketplace path is not absolute: {err}"))
|
||||
}
|
||||
fn ensure_configured_git_marketplace_unchanged(
|
||||
codex_home: &Path,
|
||||
reload_config: &ConfigLayerReload,
|
||||
expected: &ConfiguredGitMarketplace,
|
||||
) -> Result<(), String> {
|
||||
let current = read_configured_git_marketplace(codex_home, &expected.name)?;
|
||||
let current = read_configured_git_marketplace(reload_config, &expected.name)?;
|
||||
match current {
|
||||
Some(current) if current == *expected => Ok(()),
|
||||
Some(_) => Err(format!(
|
||||
@@ -310,27 +316,15 @@ fn ensure_configured_git_marketplace_unchanged(
|
||||
}
|
||||
|
||||
fn read_configured_git_marketplace(
|
||||
codex_home: &Path,
|
||||
reload_config: &ConfigLayerReload,
|
||||
marketplace_name: &str,
|
||||
) -> Result<Option<ConfiguredGitMarketplace>, String> {
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let raw_config = match std::fs::read_to_string(&config_path) {
|
||||
Ok(raw_config) => raw_config,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None),
|
||||
Err(err) => {
|
||||
return Err(format!(
|
||||
"failed to read user config {} while checking marketplace auto-upgrade: {err}",
|
||||
config_path.display()
|
||||
));
|
||||
}
|
||||
};
|
||||
let config: toml::Value = toml::from_str(&raw_config).map_err(|err| {
|
||||
format!(
|
||||
"failed to parse user config {} while checking marketplace auto-upgrade: {err}",
|
||||
config_path.display()
|
||||
)
|
||||
})?;
|
||||
let Some(marketplace) = config
|
||||
let effective_config = reload_config()
|
||||
.map_err(|err| {
|
||||
format!("failed to reload config while checking marketplace upgrade: {err}")
|
||||
})?
|
||||
.effective_config();
|
||||
let Some(marketplace) = effective_config
|
||||
.get("marketplaces")
|
||||
.and_then(toml::Value::as_table)
|
||||
.and_then(|marketplaces| marketplaces.get(marketplace_name))
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
use super::*;
|
||||
use crate::PluginGitMode;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::loader::load_config_layers_state;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -31,7 +31,7 @@ last_revision = "abc123"
|
||||
.expect("write config");
|
||||
|
||||
assert_eq!(
|
||||
read_configured_git_marketplace(codex_home.path(), "good")
|
||||
read_configured_git_marketplace(&config_reloader(codex_home.path()), "good")
|
||||
.expect("read configured marketplace"),
|
||||
Some(ConfiguredGitMarketplace {
|
||||
name: "good".to_string(),
|
||||
@@ -65,12 +65,14 @@ source = {good_url:?}
|
||||
"#
|
||||
);
|
||||
std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), &config).expect("write config");
|
||||
let stack = config_layer_stack(codex_home.path(), &config);
|
||||
let reload_config = config_reloader(codex_home.path());
|
||||
let stack = reload_config().expect("load config");
|
||||
|
||||
let outcome = upgrade_configured_git_marketplaces(
|
||||
codex_home.path(),
|
||||
&stack,
|
||||
/*marketplace_name*/ None,
|
||||
&reload_config,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
@@ -103,12 +105,14 @@ fn automatic_marketplace_git_ignores_inherited_repository_configuration() {
|
||||
let config =
|
||||
format!("[marketplaces.trusted]\nsource_type = \"git\"\nsource = {source:?}\n");
|
||||
std::fs::write(codex_home.join(CONFIG_TOML_FILE), &config).expect("write config");
|
||||
let stack = config_layer_stack(&codex_home, &config);
|
||||
let reload_config = config_reloader(&codex_home);
|
||||
let stack = reload_config().expect("load config");
|
||||
let outcome = upgrade_configured_git_marketplaces_with_mode(
|
||||
&codex_home,
|
||||
&stack,
|
||||
/*marketplace_name*/ None,
|
||||
PluginGitMode::Automatic,
|
||||
&reload_config,
|
||||
);
|
||||
assert_eq!(
|
||||
outcome,
|
||||
@@ -255,6 +259,7 @@ ref = "missing-ref"
|
||||
codex_home.path(),
|
||||
&install_root,
|
||||
&marketplace,
|
||||
&config_reloader(codex_home.path()),
|
||||
Some(&normalized_source),
|
||||
PluginGitMode::Manual,
|
||||
)
|
||||
@@ -299,6 +304,7 @@ fn up_to_date_fast_path_validates_marketplace_name() {
|
||||
codex_home.path(),
|
||||
&install_root,
|
||||
&marketplace,
|
||||
&config_reloader(codex_home.path()),
|
||||
Some(&normalized_source),
|
||||
PluginGitMode::Manual,
|
||||
)
|
||||
@@ -380,21 +386,149 @@ fn stale_activation_restores_newer_concurrently_installed_marketplace() {
|
||||
));
|
||||
}
|
||||
|
||||
fn config_layer_stack(codex_home: &Path, config: &str) -> ConfigLayerStack {
|
||||
let config_file =
|
||||
AbsolutePathBuf::try_from(codex_home.join(CONFIG_TOML_FILE)).expect("absolute config path");
|
||||
ConfigLayerStack::new(
|
||||
vec![ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: config_file,
|
||||
profile: None,
|
||||
},
|
||||
toml::from_str(config).expect("parse config"),
|
||||
)],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("build config layer stack")
|
||||
fn config_reloader(codex_home: &Path) -> ConfigLayerReload {
|
||||
let codex_home = codex_home.to_path_buf();
|
||||
let options = LoaderOverrides {
|
||||
system_config_path: Some(codex_home.join("system.toml")),
|
||||
managed_config_path: Some(codex_home.join("managed.toml")),
|
||||
system_requirements_path: Some(codex_home.join("requirements.toml")),
|
||||
..LoaderOverrides::without_managed_config_for_tests()
|
||||
};
|
||||
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()
|
||||
.expect("create config loading runtime");
|
||||
Arc::new(move || {
|
||||
runtime.block_on(load_config_layers_state(
|
||||
LOCAL_FS.as_ref(),
|
||||
&codex_home,
|
||||
Some(AbsolutePathBuf::try_from(codex_home.join("project"))?),
|
||||
&[],
|
||||
options.clone(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
fn system_marketplace_stack(codex_home: &Path, system: &str, user: &str) -> ConfigLayerStack {
|
||||
let system_file = AbsolutePathBuf::try_from(codex_home.join("system.toml")).unwrap();
|
||||
std::fs::write(&system_file, system).unwrap();
|
||||
if !user.is_empty() {
|
||||
std::fs::write(codex_home.join(CONFIG_TOML_FILE), user).unwrap();
|
||||
}
|
||||
config_reloader(codex_home)().expect("load system and user config")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn system_marketplace_downloads_and_updates_without_writing_user_config() {
|
||||
for user_config in ["", "[marketplaces.good]\nsparse_paths = []\n"] {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let remote = TempDir::new().unwrap();
|
||||
init_marketplace_repo(remote.path(), "good");
|
||||
run_git(remote.path(), &["checkout", "-b", "release"]);
|
||||
std::fs::write(remote.path().join("marker"), "release").unwrap();
|
||||
run_git(remote.path(), &["add", "."]);
|
||||
run_git(remote.path(), &["commit", "-m", "release"]);
|
||||
run_git(remote.path(), &["checkout", "-b", "other"]);
|
||||
std::fs::write(remote.path().join("marker"), "wrong branch").unwrap();
|
||||
run_git(remote.path(), &["commit", "-am", "other"]);
|
||||
let source = url::Url::from_directory_path(remote.path())
|
||||
.unwrap()
|
||||
.to_string();
|
||||
let system = format!(
|
||||
"[marketplaces.good]\nsource_type = \"git\"\nsource = {source:?}\nref = \"release\"\n"
|
||||
);
|
||||
let stack = system_marketplace_stack(codex_home.path(), &system, user_config);
|
||||
let reload_config = config_reloader(codex_home.path());
|
||||
let root =
|
||||
AbsolutePathBuf::try_from(marketplace_install_root(codex_home.path()).join("good"))
|
||||
.unwrap();
|
||||
for marker in ["release", "updated"] {
|
||||
if marker == "updated" {
|
||||
run_git(remote.path(), &["checkout", "release"]);
|
||||
std::fs::write(remote.path().join("marker"), marker).unwrap();
|
||||
run_git(remote.path(), &["commit", "-am", "update release"]);
|
||||
}
|
||||
assert_eq!(
|
||||
upgrade_configured_git_marketplaces_with_mode(
|
||||
codex_home.path(),
|
||||
&stack,
|
||||
Some("good"),
|
||||
PluginGitMode::Automatic,
|
||||
&reload_config,
|
||||
),
|
||||
ConfiguredMarketplaceUpgradeOutcome {
|
||||
selected_marketplaces: vec!["good".to_string()],
|
||||
upgraded_roots: vec![root.clone()],
|
||||
errors: Vec::new(),
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(root.join("marker")).unwrap(),
|
||||
marker
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(codex_home.path().join("system.toml")).unwrap(),
|
||||
system
|
||||
);
|
||||
let user_file = codex_home.path().join(CONFIG_TOML_FILE);
|
||||
if user_config.is_empty() {
|
||||
assert!(!user_file.exists());
|
||||
} else {
|
||||
assert_eq!(std::fs::read_to_string(user_file).unwrap(), user_config);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn changed_config_rolls_back_marketplace_activation() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let remote = TempDir::new().unwrap();
|
||||
init_marketplace_repo(remote.path(), "good");
|
||||
let source = url::Url::from_directory_path(remote.path())
|
||||
.unwrap()
|
||||
.to_string();
|
||||
let system = format!("[marketplaces.good]\nsource_type = \"git\"\nsource = {source:?}\n");
|
||||
let stack = system_marketplace_stack(codex_home.path(), &system, "");
|
||||
let reload_config = config_reloader(codex_home.path());
|
||||
let initial = upgrade_configured_git_marketplaces(
|
||||
codex_home.path(),
|
||||
&stack,
|
||||
Some("good"),
|
||||
&reload_config,
|
||||
);
|
||||
assert!(initial.all_succeeded());
|
||||
let root = &initial.upgraded_roots[0];
|
||||
let original_metadata = std::fs::read(root.join(".codex-marketplace-install.json")).unwrap();
|
||||
std::fs::write(remote.path().join("new-file"), "new revision").unwrap();
|
||||
run_git(remote.path(), &["add", "."]);
|
||||
run_git(remote.path(), &["commit", "-m", "update"]);
|
||||
for (file, changed_config) in [
|
||||
("system.toml", format!("{system}ref = \"different\"\n")),
|
||||
("system.toml", String::new()),
|
||||
(
|
||||
CONFIG_TOML_FILE,
|
||||
"[marketplaces.good]\nref = \"user-override\"\n".to_string(),
|
||||
),
|
||||
(CONFIG_TOML_FILE, "invalid = [".to_string()),
|
||||
] {
|
||||
std::fs::write(codex_home.path().join("system.toml"), &system).unwrap();
|
||||
std::fs::write(codex_home.path().join(file), changed_config).unwrap();
|
||||
let outcome = upgrade_configured_git_marketplaces(
|
||||
codex_home.path(),
|
||||
&stack,
|
||||
Some("good"),
|
||||
&reload_config,
|
||||
);
|
||||
assert_eq!(outcome.upgraded_roots, Vec::new());
|
||||
assert_eq!(outcome.errors.len(), 1);
|
||||
assert!(!root.join("new-file").exists());
|
||||
assert_eq!(
|
||||
std::fs::read(root.join(".codex-marketplace-install.json")).unwrap(),
|
||||
original_metadata
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn init_marketplace_repo(repo: &Path, marketplace_name: &str) {
|
||||
|
||||
Reference in New Issue
Block a user