mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Persist remote plugin identities with cached bundles
This commit is contained in:
@@ -199,6 +199,16 @@ pub async fn sync_remote_installed_plugin_bundles_once(
|
||||
.map(str::trim)
|
||||
.filter(|version| !version.is_empty());
|
||||
if store.active_plugin_version(&plugin_id).as_deref() == release_version {
|
||||
if let Err(err) = store.write_remote_plugin_id(&plugin_id, &plugin.id) {
|
||||
warn!(
|
||||
remote_plugin_id = %plugin.id,
|
||||
plugin = %plugin.name,
|
||||
marketplace = %marketplace_name,
|
||||
error = %err,
|
||||
"failed to persist identity for cached remote installed plugin"
|
||||
);
|
||||
failed_remote_plugin_ids.insert(plugin.id);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -34,6 +34,7 @@ const TEST_ALLOW_LOOPBACK_HTTP_REMOTE_PLUGIN_BUNDLES_ENV: &str =
|
||||
pub struct ValidatedRemotePluginBundle {
|
||||
pub plugin_id: PluginId,
|
||||
pub plugin_version: String,
|
||||
remote_plugin_id: String,
|
||||
app_manifest: Option<JsonValue>,
|
||||
bundle_download_url: String,
|
||||
}
|
||||
@@ -190,6 +191,7 @@ pub fn validate_remote_plugin_bundle(
|
||||
Ok(ValidatedRemotePluginBundle {
|
||||
plugin_id,
|
||||
plugin_version,
|
||||
remote_plugin_id: remote_plugin_id.to_string(),
|
||||
app_manifest,
|
||||
bundle_download_url,
|
||||
})
|
||||
@@ -379,9 +381,12 @@ fn install_remote_plugin_bundle(
|
||||
})?;
|
||||
|
||||
let store = PluginStore::try_new(codex_home)?;
|
||||
store
|
||||
let remote_plugin_id = bundle.remote_plugin_id;
|
||||
let result = store
|
||||
.install_with_version(plugin_root, bundle.plugin_id, bundle.plugin_version)
|
||||
.map_err(RemotePluginBundleInstallError::from)
|
||||
.map_err(RemotePluginBundleInstallError::from)?;
|
||||
store.write_remote_plugin_id(&result.plugin_id, &remote_plugin_id)?;
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
fn extract_remote_plugin_bundle_to_path(
|
||||
@@ -721,6 +726,29 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_persists_remote_plugin_identity() {
|
||||
let codex_home = tempdir().expect("tempdir");
|
||||
let bundle = valid_remote_plugin_bundle();
|
||||
|
||||
let result = install_remote_plugin_bundle(
|
||||
codex_home.path().to_path_buf(),
|
||||
bundle,
|
||||
tar_gz_bytes(&[(
|
||||
".codex-plugin/plugin.json",
|
||||
br#"{"name":"linear","version":"1.2.3"}"#,
|
||||
/*mode*/ 0o644,
|
||||
)]),
|
||||
)
|
||||
.expect("install bundle");
|
||||
let store = PluginStore::new(codex_home.path().to_path_buf());
|
||||
|
||||
assert_eq!(
|
||||
store.remote_plugin_id(&result.plugin_id).unwrap(),
|
||||
Some(REMOTE_PLUGIN_ID.to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_preserves_non_global_bundle_manifest_metadata() {
|
||||
let codex_home = tempdir().expect("tempdir");
|
||||
|
||||
@@ -6,16 +6,25 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::find_plugin_manifest_path;
|
||||
use semver::Version;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::cmp::Ordering;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub const DEFAULT_PLUGIN_VERSION: &str = "local";
|
||||
pub const PLUGINS_CACHE_DIR: &str = "plugins/cache";
|
||||
pub const PLUGINS_DATA_DIR: &str = "plugins/data";
|
||||
const REMOTE_PLUGIN_IDENTITY_FILE: &str = ".remote-plugin.json";
|
||||
|
||||
#[derive(Debug, Deserialize, Serialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct RemotePluginIdentity {
|
||||
remote_plugin_id: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct PluginInstallResult {
|
||||
@@ -99,6 +108,83 @@ impl PluginStore {
|
||||
self.active_plugin_version(plugin_id).is_some()
|
||||
}
|
||||
|
||||
pub fn remote_plugin_id(
|
||||
&self,
|
||||
plugin_id: &PluginId,
|
||||
) -> Result<Option<String>, PluginStoreError> {
|
||||
if !self.is_installed(plugin_id) {
|
||||
return Ok(None);
|
||||
}
|
||||
let path = self.remote_plugin_identity_path(plugin_id);
|
||||
let contents = match fs::read_to_string(path.as_path()) {
|
||||
Ok(contents) => contents,
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
|
||||
Err(err) => {
|
||||
return Err(PluginStoreError::io(
|
||||
"failed to read remote plugin identity",
|
||||
err,
|
||||
));
|
||||
}
|
||||
};
|
||||
let identity: RemotePluginIdentity = serde_json::from_str(&contents).map_err(|err| {
|
||||
PluginStoreError::Invalid(format!("failed to parse remote plugin identity: {err}"))
|
||||
})?;
|
||||
let remote_plugin_id = identity.remote_plugin_id.trim();
|
||||
if remote_plugin_id.is_empty() {
|
||||
return Err(PluginStoreError::Invalid(
|
||||
"invalid remote plugin identity: remote plugin id must not be blank".to_string(),
|
||||
));
|
||||
}
|
||||
Ok(Some(remote_plugin_id.to_string()))
|
||||
}
|
||||
|
||||
pub fn write_remote_plugin_id(
|
||||
&self,
|
||||
plugin_id: &PluginId,
|
||||
remote_plugin_id: &str,
|
||||
) -> Result<(), PluginStoreError> {
|
||||
if !self.is_installed(plugin_id) {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"cannot write remote identity for uninstalled plugin `{}`",
|
||||
plugin_id.as_key()
|
||||
)));
|
||||
}
|
||||
let remote_plugin_id = remote_plugin_id.trim();
|
||||
if remote_plugin_id.is_empty() {
|
||||
return Err(PluginStoreError::Invalid(
|
||||
"invalid remote plugin identity: remote plugin id must not be blank".to_string(),
|
||||
));
|
||||
}
|
||||
let path = self.remote_plugin_identity_path(plugin_id);
|
||||
let parent = path.as_path().parent().ok_or_else(|| {
|
||||
PluginStoreError::Invalid(format!(
|
||||
"remote plugin identity path has no parent: {}",
|
||||
path.display()
|
||||
))
|
||||
})?;
|
||||
let mut contents = serde_json::to_vec(&RemotePluginIdentity {
|
||||
remote_plugin_id: remote_plugin_id.to_string(),
|
||||
})
|
||||
.map_err(|err| {
|
||||
PluginStoreError::Invalid(format!("failed to serialize remote plugin identity: {err}"))
|
||||
})?;
|
||||
contents.push(b'\n');
|
||||
let mut temporary = tempfile::NamedTempFile::new_in(parent).map_err(|err| {
|
||||
PluginStoreError::io("failed to create temporary remote plugin identity", err)
|
||||
})?;
|
||||
temporary
|
||||
.write_all(&contents)
|
||||
.map_err(|err| PluginStoreError::io("failed to write remote plugin identity", err))?;
|
||||
temporary
|
||||
.as_file_mut()
|
||||
.flush()
|
||||
.map_err(|err| PluginStoreError::io("failed to flush remote plugin identity", err))?;
|
||||
temporary.persist(path.as_path()).map_err(|err| {
|
||||
PluginStoreError::io("failed to persist remote plugin identity", err.error)
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn install(
|
||||
&self,
|
||||
source_path: AbsolutePathBuf,
|
||||
@@ -135,6 +221,7 @@ impl PluginStore {
|
||||
self.plugin_base_root(&plugin_id).as_path(),
|
||||
&plugin_version,
|
||||
)?;
|
||||
self.remove_remote_plugin_identity(&plugin_id)?;
|
||||
|
||||
Ok(PluginInstallResult {
|
||||
plugin_id,
|
||||
@@ -146,6 +233,23 @@ impl PluginStore {
|
||||
pub fn uninstall(&self, plugin_id: &PluginId) -> Result<(), PluginStoreError> {
|
||||
remove_existing_target(self.plugin_base_root(plugin_id).as_path())
|
||||
}
|
||||
|
||||
fn remote_plugin_identity_path(&self, plugin_id: &PluginId) -> AbsolutePathBuf {
|
||||
self.plugin_base_root(plugin_id)
|
||||
.join(REMOTE_PLUGIN_IDENTITY_FILE)
|
||||
}
|
||||
|
||||
fn remove_remote_plugin_identity(&self, plugin_id: &PluginId) -> Result<(), PluginStoreError> {
|
||||
let path = self.remote_plugin_identity_path(plugin_id);
|
||||
match fs::remove_file(path.as_path()) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()),
|
||||
Err(err) => Err(PluginStoreError::io(
|
||||
"failed to remove remote plugin identity",
|
||||
err,
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
|
||||
@@ -151,6 +151,50 @@ fn install_with_version_uses_requested_cache_version() {
|
||||
assert!(installed_path.join(".codex-plugin/plugin.json").is_file());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_plugin_identity_follows_installed_cache_lifecycle() {
|
||||
let tmp = tempdir().unwrap();
|
||||
write_plugin(tmp.path(), "sample-plugin", "sample-plugin");
|
||||
let plugin_id = PluginId::new(
|
||||
"sample-plugin".to_string(),
|
||||
"openai-curated-remote".to_string(),
|
||||
)
|
||||
.unwrap();
|
||||
let store = PluginStore::new(tmp.path().to_path_buf());
|
||||
let source = AbsolutePathBuf::try_from(tmp.path().join("sample-plugin")).unwrap();
|
||||
|
||||
store
|
||||
.install(source.clone(), plugin_id.clone())
|
||||
.expect("install plugin");
|
||||
assert_eq!(store.remote_plugin_id(&plugin_id).unwrap(), None);
|
||||
|
||||
store
|
||||
.write_remote_plugin_id(&plugin_id, "plugins~Plugin_sample")
|
||||
.expect("write remote identity");
|
||||
assert_eq!(
|
||||
store.remote_plugin_id(&plugin_id).unwrap(),
|
||||
Some("plugins~Plugin_sample".to_string())
|
||||
);
|
||||
store
|
||||
.write_remote_plugin_id(&plugin_id, "plugins~Plugin_updated")
|
||||
.expect("replace remote identity");
|
||||
assert_eq!(
|
||||
store.remote_plugin_id(&plugin_id).unwrap(),
|
||||
Some("plugins~Plugin_updated".to_string())
|
||||
);
|
||||
|
||||
store
|
||||
.install(source, plugin_id.clone())
|
||||
.expect("replace with local install");
|
||||
assert_eq!(store.remote_plugin_id(&plugin_id).unwrap(), None);
|
||||
|
||||
store
|
||||
.write_remote_plugin_id(&plugin_id, "plugins~Plugin_sample")
|
||||
.expect("restore remote identity");
|
||||
store.uninstall(&plugin_id).expect("uninstall plugin");
|
||||
assert_eq!(store.remote_plugin_id(&plugin_id).unwrap(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_uses_manifest_version_when_present() {
|
||||
let tmp = tempdir().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user