mirror of
https://github.com/farion1231/cc-switch.git
synced 2026-08-03 19:12:04 +08:00
fix(pi): reconcile native resources across directory moves
This commit is contained in:
@@ -28,9 +28,6 @@ impl PromptService {
|
||||
state: &AppState,
|
||||
app: AppType,
|
||||
) -> Result<IndexMap<String, Prompt>, AppError> {
|
||||
if matches!(app, AppType::Pi) {
|
||||
Self::reconcile_pi_portable_import(state)?;
|
||||
}
|
||||
state.db.get_prompts(app.as_str())
|
||||
}
|
||||
|
||||
@@ -697,4 +694,55 @@ mod tests {
|
||||
assert!(prompts.values().all(|prompt| !prompt.enabled));
|
||||
assert!(!temp.path().join("AGENTS.md").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn reading_pi_prompts_does_not_adopt_external_agents_drift() {
|
||||
let temp = tempfile::tempdir().expect("tempdir");
|
||||
let _restore = EnvRestore::set("PI_CODING_AGENT_DIR", temp.path());
|
||||
std::fs::write(temp.path().join("AGENTS.md"), "managed-before").expect("seed AGENTS.md");
|
||||
let state = AppState::new(Arc::new(Database::memory().expect("database")));
|
||||
state
|
||||
.db
|
||||
.save_prompt(
|
||||
AppType::Pi.as_str(),
|
||||
&Prompt {
|
||||
id: "managed".to_string(),
|
||||
name: "Managed".to_string(),
|
||||
content: "managed-before".to_string(),
|
||||
description: None,
|
||||
enabled: true,
|
||||
created_at: Some(1),
|
||||
updated_at: Some(1),
|
||||
},
|
||||
)
|
||||
.expect("seed prompt");
|
||||
state
|
||||
.db
|
||||
.save_prompt(
|
||||
AppType::Pi.as_str(),
|
||||
&Prompt {
|
||||
id: "other".to_string(),
|
||||
name: "Other".to_string(),
|
||||
content: "other-content".to_string(),
|
||||
description: None,
|
||||
enabled: false,
|
||||
created_at: Some(2),
|
||||
updated_at: Some(2),
|
||||
},
|
||||
)
|
||||
.expect("seed alternate prompt");
|
||||
|
||||
std::fs::write(temp.path().join("AGENTS.md"), "external-after").expect("external edit");
|
||||
let prompts = PromptService::get_prompts(&state, AppType::Pi).expect("read prompt list");
|
||||
assert_eq!(prompts["managed"].content, "managed-before");
|
||||
assert!(
|
||||
PromptService::enable_prompt(&state, AppType::Pi, "other").is_err(),
|
||||
"the write boundary must still report the external drift conflict"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(temp.path().join("AGENTS.md")).expect("live AGENTS.md"),
|
||||
"external-after"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -722,6 +722,26 @@ impl ProxyService {
|
||||
return crate::settings::update_settings(next);
|
||||
}
|
||||
|
||||
let direct_patch = direct_pi_projection_patch(self.db.as_ref())?;
|
||||
let new_native_before = crate::pi_config::document::snapshot_pi_provider_values(
|
||||
&new_models_path,
|
||||
direct_patch.keys().cloned(),
|
||||
)?;
|
||||
for (provider_key, expected) in &direct_patch {
|
||||
if let Some(existing_value) = new_native_before
|
||||
.values
|
||||
.get(provider_key)
|
||||
.and_then(Option::as_ref)
|
||||
{
|
||||
if Some(existing_value) != expected.as_ref() {
|
||||
return Err(AppError::Conflict(format!(
|
||||
"Pi directory change would overwrite unowned provider key '{provider_key}' in {}",
|
||||
new_models_path.display()
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Prompt operations and directory ownership share one sendable mutex.
|
||||
// Holding it across runtime publication prevents a prompt write from
|
||||
// committing against the new root while a failed directory move is
|
||||
@@ -731,6 +751,19 @@ impl ProxyService {
|
||||
|
||||
if !existing.pi_takeover_enabled {
|
||||
crate::settings::update_settings(next)?;
|
||||
if let Err(error) =
|
||||
crate::pi_config::document::apply_pi_provider_patch(&new_models_path, &direct_patch)
|
||||
{
|
||||
let settings_restored = crate::settings::update_settings(existing.clone()).is_ok();
|
||||
let new_native_restored = crate::pi_config::document::restore_pi_provider_values(
|
||||
&new_models_path,
|
||||
&new_native_before,
|
||||
)
|
||||
.is_ok();
|
||||
return Err(AppError::Config(format!(
|
||||
"failed to publish managed Pi providers in the new directory: {error}; rollback: settings={settings_restored}, native={new_native_restored}"
|
||||
)));
|
||||
}
|
||||
if let Err(error) =
|
||||
PromptService::reconcile_pi_native_under_guard(self.db.as_ref(), &prompt_guard)
|
||||
{
|
||||
@@ -739,8 +772,35 @@ impl ProxyService {
|
||||
.db
|
||||
.save_prompt_selection(AppType::Pi.as_str(), &previous_prompts)
|
||||
.is_ok();
|
||||
let new_native_restored = crate::pi_config::document::restore_pi_provider_values(
|
||||
&new_models_path,
|
||||
&new_native_before,
|
||||
)
|
||||
.is_ok();
|
||||
return Err(AppError::Config(format!(
|
||||
"failed to reconcile Pi prompts in the new directory: {error}; rollback: settings={settings_restored}, prompts={prompts_restored}"
|
||||
"failed to reconcile Pi prompts in the new directory: {error}; rollback: settings={settings_restored}, prompts={prompts_restored}, native={new_native_restored}"
|
||||
)));
|
||||
}
|
||||
if let Err(error) =
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::reconcile_all(&self.db)
|
||||
{
|
||||
let settings_restored = crate::settings::update_settings(existing.clone()).is_ok();
|
||||
let prompts_restored = self
|
||||
.db
|
||||
.save_prompt_selection(AppType::Pi.as_str(), &previous_prompts)
|
||||
.is_ok();
|
||||
let new_native_restored = crate::pi_config::document::restore_pi_provider_values(
|
||||
&new_models_path,
|
||||
&new_native_before,
|
||||
)
|
||||
.is_ok();
|
||||
let skills_restored = settings_restored
|
||||
&& crate::services::skill_deployment::PiSkillDeploymentService::reconcile_all(
|
||||
&self.db,
|
||||
)
|
||||
.is_ok();
|
||||
return Err(AppError::Config(format!(
|
||||
"failed to reconcile Pi Skills in the new directory: {error}; rollback: settings={settings_restored}, prompts={prompts_restored}, native={new_native_restored}, skills={skills_restored}"
|
||||
)));
|
||||
}
|
||||
return Ok(());
|
||||
@@ -776,28 +836,69 @@ impl ProxyService {
|
||||
.db
|
||||
.save_prompt_selection(AppType::Pi.as_str(), &previous_prompts)
|
||||
.is_ok();
|
||||
let new_native_restored = crate::pi_config::document::restore_pi_provider_values(
|
||||
&new_models_path,
|
||||
&new_native_before,
|
||||
)
|
||||
.is_ok();
|
||||
let skills_restored = settings_restored
|
||||
&& crate::services::skill_deployment::PiSkillDeploymentService::reconcile_all(
|
||||
&self.db,
|
||||
)
|
||||
.is_ok();
|
||||
let runtime_restored = self.reconcile_pi_runtime_at_epoch(epoch).await.is_ok();
|
||||
return Err(AppError::Config(format!(
|
||||
"failed to reconcile Pi prompts in the new directory: {error}; rollback: settings={settings_restored}, prompts={prompts_restored}, gateway={runtime_restored}"
|
||||
"failed to reconcile Pi prompts in the new directory: {error}; rollback: settings={settings_restored}, prompts={prompts_restored}, native={new_native_restored}, skills={skills_restored}, gateway={runtime_restored}"
|
||||
)));
|
||||
}
|
||||
|
||||
if let Err(error) = self.reconcile_pi_runtime_at_epoch(epoch).await {
|
||||
let new_direct_restored = self
|
||||
.restore_pi_direct_projection_at(&new_models_path)
|
||||
.is_ok();
|
||||
if let Err(error) =
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::reconcile_all(&self.db)
|
||||
{
|
||||
let settings_restored = crate::settings::update_settings(existing.clone()).is_ok();
|
||||
let prompts_restored = self
|
||||
.db
|
||||
.save_prompt_selection(AppType::Pi.as_str(), &previous_prompts)
|
||||
.is_ok();
|
||||
let new_native_restored = crate::pi_config::document::restore_pi_provider_values(
|
||||
&new_models_path,
|
||||
&new_native_before,
|
||||
)
|
||||
.is_ok();
|
||||
let skills_restored = settings_restored
|
||||
&& crate::services::skill_deployment::PiSkillDeploymentService::reconcile_all(
|
||||
&self.db,
|
||||
)
|
||||
.is_ok();
|
||||
let runtime_restored = self.reconcile_pi_runtime_at_epoch(epoch).await.is_ok();
|
||||
return Err(AppError::Config(format!(
|
||||
"failed to reconcile Pi Skills in the new directory: {error}; rollback: settings={settings_restored}, prompts={prompts_restored}, native={new_native_restored}, skills={skills_restored}, gateway={runtime_restored}"
|
||||
)));
|
||||
}
|
||||
|
||||
if let Err(error) = self.reconcile_pi_runtime_at_epoch(epoch).await {
|
||||
let settings_restored = crate::settings::update_settings(existing.clone()).is_ok();
|
||||
let prompts_restored = self
|
||||
.db
|
||||
.save_prompt_selection(AppType::Pi.as_str(), &previous_prompts)
|
||||
.is_ok();
|
||||
let new_native_restored = crate::pi_config::document::restore_pi_provider_values(
|
||||
&new_models_path,
|
||||
&new_native_before,
|
||||
)
|
||||
.is_ok();
|
||||
let skills_restored = settings_restored
|
||||
&& crate::services::skill_deployment::PiSkillDeploymentService::reconcile_all(
|
||||
&self.db,
|
||||
)
|
||||
.is_ok();
|
||||
let old_gateway_restored = if settings_restored {
|
||||
self.reconcile_pi_runtime_at_epoch(epoch).await.is_ok()
|
||||
} else {
|
||||
self.pi_runtime.republish_current(epoch).await.is_ok()
|
||||
};
|
||||
return Err(AppError::Config(format!(
|
||||
"failed to publish Pi in the new native directory: {error}; rollback: new_direct={new_direct_restored}, settings={settings_restored}, prompts={prompts_restored}, old_gateway={old_gateway_restored}"
|
||||
"failed to publish Pi in the new native directory: {error}; rollback: native={new_native_restored}, settings={settings_restored}, prompts={prompts_restored}, skills={skills_restored}, old_gateway={old_gateway_restored}"
|
||||
)));
|
||||
}
|
||||
|
||||
@@ -4001,6 +4102,10 @@ mod tests {
|
||||
Some(value) => env::set_var("CC_SWITCH_TEST_HOME", value),
|
||||
None => env::remove_var("CC_SWITCH_TEST_HOME"),
|
||||
}
|
||||
// Tests mutate the process-global settings cache after redirecting
|
||||
// the home directory. Restore the cache after the environment so
|
||||
// later serial tests do not inherit a dead temporary Pi override.
|
||||
let _ = crate::settings::reload_settings();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4123,6 +4228,33 @@ mod tests {
|
||||
crate::settings::update_settings(settings).expect("set old Pi directory");
|
||||
|
||||
let db = Arc::new(Database::memory().expect("in-memory database"));
|
||||
let skill_source = crate::services::skill::SkillService::get_ssot_dir()
|
||||
.expect("SSOT")
|
||||
.join("directory-move");
|
||||
std::fs::create_dir_all(&skill_source).expect("skill source");
|
||||
std::fs::write(
|
||||
skill_source.join("SKILL.md"),
|
||||
"---\nname: directory-move\ndescription: directory move\n---\n",
|
||||
)
|
||||
.expect("skill manifest");
|
||||
db.save_skill(&crate::app_config::InstalledSkill {
|
||||
id: "local:directory-move".to_string(),
|
||||
name: "Directory move".to_string(),
|
||||
description: Some("directory move".to_string()),
|
||||
directory: "directory-move".to_string(),
|
||||
repo_owner: None,
|
||||
repo_name: None,
|
||||
repo_branch: None,
|
||||
readme_url: None,
|
||||
apps: crate::app_config::SkillApps::only(&AppType::Pi),
|
||||
installed_at: 1,
|
||||
content_hash: None,
|
||||
updated_at: 1,
|
||||
})
|
||||
.expect("save skill");
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::reconcile_all(&db)
|
||||
.expect("deploy skill in old root");
|
||||
assert!(old_dir.join("skills").join("directory-move").exists());
|
||||
db.save_prompt(
|
||||
AppType::Pi.as_str(),
|
||||
&crate::prompt::Prompt {
|
||||
@@ -4223,6 +4355,18 @@ mod tests {
|
||||
std::fs::read_to_string(new_dir.join("AGENTS.md")).expect("new AGENTS"),
|
||||
"new-root-agents"
|
||||
);
|
||||
assert!(
|
||||
!old_dir.join("skills").join("directory-move").exists(),
|
||||
"verified stale Skill ownership must be cleaned after new deployment"
|
||||
);
|
||||
assert!(new_dir.join("skills").join("directory-move").exists());
|
||||
let deployments = db
|
||||
.get_pi_skill_deployments("local:directory-move")
|
||||
.expect("skill deployments");
|
||||
assert_eq!(deployments.len(), 1);
|
||||
assert!(deployments[0]
|
||||
.destination
|
||||
.contains(new_dir.to_string_lossy().as_ref()));
|
||||
|
||||
service
|
||||
.set_takeover_for_app("pi", false)
|
||||
@@ -4230,6 +4374,84 @@ mod tests {
|
||||
.expect("disable Pi takeover");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn changing_pi_directory_rejects_an_unowned_managed_key_before_side_effects() {
|
||||
let home = TempHome::new();
|
||||
crate::settings::reload_settings().expect("reload isolated settings");
|
||||
let old_dir = home.dir.path().join("old-pi");
|
||||
let new_dir = home.dir.path().join("new-pi");
|
||||
std::fs::create_dir_all(&old_dir).expect("old Pi directory");
|
||||
std::fs::create_dir_all(&new_dir).expect("new Pi directory");
|
||||
|
||||
let managed = json!({
|
||||
"name": "Managed Pi",
|
||||
"api": "openai-responses",
|
||||
"baseUrl": "https://managed.example/v1",
|
||||
"apiKey": "managed-key",
|
||||
"models": [{"id": "model-a", "name": "Model A"}]
|
||||
});
|
||||
let foreign = json!({
|
||||
"api": "anthropic-messages",
|
||||
"baseUrl": "https://foreign.example",
|
||||
"apiKey": "foreign-secret",
|
||||
"models": [{"id": "foreign-model"}]
|
||||
});
|
||||
let foreign_document = serde_json::to_vec_pretty(&json!({
|
||||
"providers": {"managed-pi": foreign}
|
||||
}))
|
||||
.expect("foreign models");
|
||||
std::fs::write(new_dir.join("models.json"), &foreign_document).expect("seed foreign key");
|
||||
|
||||
let mut settings = crate::settings::get_settings();
|
||||
settings.pi_config_dir = Some(old_dir.to_string_lossy().into_owned());
|
||||
settings.pi_takeover_enabled = false;
|
||||
crate::settings::update_settings(settings).expect("old Pi settings");
|
||||
let db = Arc::new(Database::memory().expect("database"));
|
||||
db.create_pi_catalog_provider(
|
||||
NewProviderAggregate::from_input(
|
||||
AppType::Pi.as_str(),
|
||||
ProviderMutationInput {
|
||||
id: "managed-pi".to_string(),
|
||||
name: "Managed Pi".to_string(),
|
||||
settings_config: managed,
|
||||
website_url: None,
|
||||
category: None,
|
||||
created_at: None,
|
||||
sort_index: Some(0),
|
||||
notes: None,
|
||||
meta: None,
|
||||
icon: Some("pi".to_string()),
|
||||
icon_color: None,
|
||||
in_failover_queue: false,
|
||||
},
|
||||
)
|
||||
.expect("aggregate"),
|
||||
"managed-pi",
|
||||
)
|
||||
.expect("managed provider");
|
||||
|
||||
let service = ProxyService::new(db);
|
||||
let switch_guard = service.lock_switch_for_app(AppType::Pi.as_str()).await;
|
||||
let existing = crate::settings::get_settings();
|
||||
let mut next = existing.clone();
|
||||
next.pi_config_dir = Some(new_dir.to_string_lossy().into_owned());
|
||||
let error = service
|
||||
.replace_settings_with_pi_directory_boundary_under_lock(&switch_guard, &existing, next)
|
||||
.await
|
||||
.expect_err("unowned target key must reject the directory move");
|
||||
assert!(error.to_string().contains("overwrite unowned provider key"));
|
||||
assert_eq!(
|
||||
crate::settings::get_settings().pi_config_dir,
|
||||
existing.pi_config_dir,
|
||||
"preflight rejection must not switch the native authority"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read(new_dir.join("models.json")).expect("foreign models remain"),
|
||||
foreign_document
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn changing_pi_directory_without_takeover_reconciles_missing_agents_truth() {
|
||||
@@ -4247,6 +4469,35 @@ mod tests {
|
||||
crate::settings::update_settings(settings).expect("old directory settings");
|
||||
|
||||
let db = Arc::new(Database::memory().expect("database"));
|
||||
let direct = json!({
|
||||
"name": "Managed direct Pi",
|
||||
"api": "openai-responses",
|
||||
"baseUrl": "https://direct.example/v1",
|
||||
"apiKey": "direct-key",
|
||||
"models": [{"id": "direct-model", "name": "Direct model"}]
|
||||
});
|
||||
db.create_pi_catalog_provider(
|
||||
NewProviderAggregate::from_input(
|
||||
AppType::Pi.as_str(),
|
||||
ProviderMutationInput {
|
||||
id: "managed-direct".to_string(),
|
||||
name: "Managed direct Pi".to_string(),
|
||||
settings_config: direct.clone(),
|
||||
website_url: None,
|
||||
category: None,
|
||||
created_at: None,
|
||||
sort_index: Some(0),
|
||||
notes: None,
|
||||
meta: None,
|
||||
icon: Some("pi".to_string()),
|
||||
icon_color: None,
|
||||
in_failover_queue: false,
|
||||
},
|
||||
)
|
||||
.expect("aggregate"),
|
||||
"managed-direct",
|
||||
)
|
||||
.expect("managed direct provider");
|
||||
db.save_prompt(
|
||||
AppType::Pi.as_str(),
|
||||
&crate::prompt::Prompt {
|
||||
@@ -4282,6 +4533,15 @@ mod tests {
|
||||
"old-only"
|
||||
);
|
||||
assert!(!new_dir.join("AGENTS.md").exists());
|
||||
let new_models: Value = serde_json::from_slice(
|
||||
&std::fs::read(new_dir.join("models.json")).expect("new direct models"),
|
||||
)
|
||||
.expect("parse new direct models");
|
||||
assert_eq!(
|
||||
new_models.pointer("/providers/managed-direct"),
|
||||
Some(&direct),
|
||||
"a direct-mode directory change must publish every managed provider in the new root"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
+109
-14
@@ -3012,6 +3012,35 @@ impl SkillService {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Copy into a unique sibling and publish with an OS no-replace rename.
|
||||
/// A concurrent installer can win, but its directory is never overwritten.
|
||||
fn copy_dir_noreplace(src: &Path, dest: &Path) -> Result<()> {
|
||||
let parent = dest
|
||||
.parent()
|
||||
.ok_or_else(|| anyhow!("Skill destination has no parent: {}", dest.display()))?;
|
||||
fs::create_dir_all(parent)?;
|
||||
let name = dest
|
||||
.file_name()
|
||||
.and_then(|value| value.to_str())
|
||||
.ok_or_else(|| anyhow!("Skill destination has an invalid name: {}", dest.display()))?;
|
||||
let staged = parent.join(format!(
|
||||
".{name}.cc-switch-install-{}",
|
||||
uuid::Uuid::new_v4().simple()
|
||||
));
|
||||
if let Err(error) = Self::copy_dir_recursive(src, &staged) {
|
||||
let _ = fs::remove_dir_all(&staged);
|
||||
return Err(error);
|
||||
}
|
||||
if let Err(error) = crate::pi_config::shared_file::publish_path_noreplace(&staged, dest) {
|
||||
let _ = fs::remove_dir_all(&staged);
|
||||
return Err(anyhow!(
|
||||
"Skill destination was created concurrently ({}): {error}",
|
||||
dest.display()
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn resolve_uninstall_backup_source(skill: &InstalledSkill) -> Result<Option<PathBuf>> {
|
||||
// 返回值会被整目录复制进 ~/.cc-switch/skill-backups/ 并由 get_skill_backups
|
||||
// 在界面上列出——脏 directory 在这里等于任意文件读取 + 外泄通道。
|
||||
@@ -3382,6 +3411,16 @@ impl SkillService {
|
||||
|
||||
let deployment_guard = matches!(current_app, AppType::Pi)
|
||||
.then(crate::services::skill_deployment::PiSkillDeploymentService::operation_guard);
|
||||
let pi_source_digest = if deployment_guard.is_some() {
|
||||
Some(
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::source_digest(
|
||||
&skill_dir,
|
||||
)
|
||||
.map_err(|error| anyhow!(error.to_string()))?,
|
||||
)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// 复制到 SSOT
|
||||
let dest = ssot_dir.join(&install_name);
|
||||
@@ -3392,7 +3431,7 @@ impl SkillService {
|
||||
Some("uninstallFirst"),
|
||||
)));
|
||||
}
|
||||
Self::copy_dir_recursive(&skill_dir, &dest)?;
|
||||
Self::copy_dir_noreplace(&skill_dir, &dest)?;
|
||||
|
||||
// 计算内容哈希
|
||||
let content_hash = Self::compute_dir_hash(&dest).ok();
|
||||
@@ -3418,28 +3457,56 @@ impl SkillService {
|
||||
// evidence together. Until then the portable row is inert.
|
||||
skill.apps.pi = false;
|
||||
if let Err(error) = db.save_skill(&skill) {
|
||||
let _ = fs::remove_dir_all(&dest);
|
||||
return Err(error.into());
|
||||
let cleanup =
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::remove_source_if_unchanged(
|
||||
&dest,
|
||||
pi_source_digest
|
||||
.as_deref()
|
||||
.expect("Pi ZIP publication has a source digest"),
|
||||
);
|
||||
return match cleanup {
|
||||
Ok(()) => Err(error.into()),
|
||||
Err(cleanup) => Err(anyhow!(
|
||||
"Pi Skill ZIP database write failed ({error}); SSOT rollback failed ({cleanup})"
|
||||
)),
|
||||
};
|
||||
}
|
||||
if let Err(error) =
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::toggle_under_guard(
|
||||
guard, db, &mut skill, true,
|
||||
)
|
||||
{
|
||||
if let Err(deployment_rollback) =
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::remove_before_uninstall_under_guard(
|
||||
guard, db, &skill,
|
||||
)
|
||||
{
|
||||
return Err(anyhow!(
|
||||
"Pi Skill ZIP install failed ({error}); native ownership rollback failed ({deployment_rollback}); the DB row and SSOT were retained as recovery evidence"
|
||||
));
|
||||
}
|
||||
let db_rollback = db.delete_skill(&skill.id);
|
||||
let file_rollback = fs::remove_dir_all(&dest);
|
||||
return match (db_rollback, file_rollback) {
|
||||
(Ok(true), Ok(())) => Err(anyhow!(error.to_string())),
|
||||
(db_result, file_result) => Err(anyhow!(
|
||||
"Pi Skill ZIP install failed ({error}); DB rollback: {}; SSOT rollback: {}",
|
||||
match db_result {
|
||||
Ok(true) => "ok".to_string(),
|
||||
if !matches!(&db_rollback, Ok(true)) {
|
||||
return Err(anyhow!(
|
||||
"Pi Skill ZIP install failed ({error}); DB rollback failed ({}); SSOT was retained",
|
||||
match db_rollback {
|
||||
Ok(false) => "row missing".to_string(),
|
||||
Err(value) => value.to_string(),
|
||||
},
|
||||
file_result
|
||||
.err()
|
||||
.map_or_else(|| "ok".to_string(), |value| value.to_string())
|
||||
Ok(true) => unreachable!(),
|
||||
}
|
||||
));
|
||||
}
|
||||
let file_rollback =
|
||||
crate::services::skill_deployment::PiSkillDeploymentService::remove_source_if_unchanged(
|
||||
&dest,
|
||||
pi_source_digest
|
||||
.as_deref()
|
||||
.expect("Pi ZIP publication has a source digest"),
|
||||
);
|
||||
return match file_rollback {
|
||||
Ok(()) => Err(anyhow!(error.to_string())),
|
||||
Err(file_error) => Err(anyhow!(
|
||||
"Pi Skill ZIP install failed ({error}); SSOT rollback failed ({file_error})"
|
||||
)),
|
||||
};
|
||||
}
|
||||
@@ -4359,6 +4426,34 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_replace_directory_publish_preserves_an_existing_destination() {
|
||||
let temp = tempdir().expect("tempdir");
|
||||
let source = temp.path().join("source");
|
||||
let destination = temp.path().join("destination");
|
||||
fs::create_dir(&source).expect("source");
|
||||
fs::create_dir(&destination).expect("destination");
|
||||
fs::write(source.join("SKILL.md"), "managed").expect("source manifest");
|
||||
fs::write(destination.join("SKILL.md"), "external").expect("external manifest");
|
||||
|
||||
SkillService::copy_dir_noreplace(&source, &destination)
|
||||
.expect_err("an existing destination must win atomically");
|
||||
assert_eq!(
|
||||
fs::read_to_string(destination.join("SKILL.md")).expect("external destination"),
|
||||
"external"
|
||||
);
|
||||
assert!(
|
||||
fs::read_dir(temp.path())
|
||||
.expect("temp root")
|
||||
.all(|entry| !entry
|
||||
.expect("entry")
|
||||
.file_name()
|
||||
.to_string_lossy()
|
||||
.contains("cc-switch-install")),
|
||||
"a rejected staged publication must be cleaned"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_local_zip_hands_back_a_guard_that_owns_the_tree() {
|
||||
use std::io::Write;
|
||||
|
||||
@@ -70,10 +70,10 @@ impl PiSkillDeploymentService {
|
||||
skill: &mut InstalledSkill,
|
||||
enabled: bool,
|
||||
) -> Result<(), AppError> {
|
||||
let destination = skill_destination(skill)?;
|
||||
let destination_key = destination_key(&destination);
|
||||
let existing = db.get_pi_skill_deployment(&skill.id, &destination_key)?;
|
||||
if enabled {
|
||||
let destination = skill_destination(skill)?;
|
||||
let destination_key = destination_key(&destination);
|
||||
let existing = db.get_pi_skill_deployment(&skill.id, &destination_key)?;
|
||||
let source = skill_source(skill)?;
|
||||
deploy(
|
||||
db,
|
||||
@@ -84,15 +84,9 @@ impl PiSkillDeploymentService {
|
||||
existing,
|
||||
Some(true),
|
||||
)?;
|
||||
cleanup_stale_deployments(db, skill, &destination_key)?;
|
||||
} else {
|
||||
remove_owned(
|
||||
db,
|
||||
skill,
|
||||
&destination,
|
||||
&destination_key,
|
||||
existing,
|
||||
Some(false),
|
||||
)?;
|
||||
remove_all_recorded_deployments(db, skill, Some(false))?;
|
||||
}
|
||||
skill.apps.pi = enabled;
|
||||
Ok(())
|
||||
@@ -130,15 +124,9 @@ impl PiSkillDeploymentService {
|
||||
Some(true),
|
||||
)?;
|
||||
}
|
||||
cleanup_stale_deployments(db, skill, &destination_key)?;
|
||||
} else {
|
||||
remove_owned(
|
||||
db,
|
||||
skill,
|
||||
&destination,
|
||||
&destination_key,
|
||||
existing,
|
||||
Some(false),
|
||||
)?;
|
||||
remove_all_recorded_deployments(db, skill, Some(false))?;
|
||||
}
|
||||
skill.apps.pi = enabled;
|
||||
Ok(())
|
||||
@@ -188,10 +176,7 @@ impl PiSkillDeploymentService {
|
||||
db: &Arc<Database>,
|
||||
skill: &InstalledSkill,
|
||||
) -> Result<(), AppError> {
|
||||
let destination = skill_destination(skill)?;
|
||||
let key = destination_key(&destination);
|
||||
let existing = db.get_pi_skill_deployment(&skill.id, &key)?;
|
||||
remove_owned(db, skill, &destination, &key, existing, None)
|
||||
remove_all_recorded_deployments(db, skill, None)
|
||||
}
|
||||
|
||||
pub(crate) fn inspect_all(
|
||||
@@ -206,6 +191,35 @@ impl PiSkillDeploymentService {
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) fn source_digest(path: &Path) -> Result<String, AppError> {
|
||||
tree_digest(path)
|
||||
}
|
||||
|
||||
/// Remove a just-published SSOT tree only if the exact bytes installed by
|
||||
/// this operation still own the path. The namespace move precedes digest
|
||||
/// validation so an external replacement is restored, never recursively
|
||||
/// deleted after a check/use race.
|
||||
pub(crate) fn remove_source_if_unchanged(
|
||||
path: &Path,
|
||||
expected_digest: &str,
|
||||
) -> Result<(), AppError> {
|
||||
let staged = stage_destination(path)?;
|
||||
let observed = tree_digest(&staged);
|
||||
if !matches!(observed, Ok(ref digest) if digest == expected_digest) {
|
||||
restore_staged_destination(&staged, path).map_err(|rollback| {
|
||||
AppError::Conflict(format!(
|
||||
"Pi Skill SSOT ownership changed and rollback failed ({rollback}); recovery tree: {}",
|
||||
staged.display()
|
||||
))
|
||||
})?;
|
||||
return Err(AppError::Conflict(format!(
|
||||
"Pi Skill SSOT changed before rollback: {}",
|
||||
path.display()
|
||||
)));
|
||||
}
|
||||
remove_path(&staged)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -323,18 +337,27 @@ fn inspect_skill_status(
|
||||
) -> Result<SkillAppStatus, AppError> {
|
||||
let destination = skill_destination(skill)?;
|
||||
let destination_key = destination_key(&destination);
|
||||
let deployment = db.get_pi_skill_deployment(&skill.id, &destination_key)?;
|
||||
let deployments = db.get_pi_skill_deployments(&skill.id)?;
|
||||
let deployment = deployments
|
||||
.iter()
|
||||
.find(|deployment| deployment.destination_key == destination_key);
|
||||
let has_stale_destination = deployments
|
||||
.iter()
|
||||
.any(|deployment| deployment.destination_key != destination_key);
|
||||
let manifest = destination.join("SKILL.md");
|
||||
let discovered = discovery.by_manifest.get(&manifest);
|
||||
let destination_exists = fs::symlink_metadata(&destination).is_ok();
|
||||
let owned_deployment = deployment
|
||||
.as_ref()
|
||||
.is_some_and(|deployment| verify_owned_destination(deployment, &destination).is_ok());
|
||||
let ownership = match (deployment.is_some(), destination_exists, owned_deployment) {
|
||||
(_, _, true) => PiSkillOwnership::Owned,
|
||||
(true, _, false) => PiSkillOwnership::Stale,
|
||||
(false, true, false) => PiSkillOwnership::Foreign,
|
||||
(false, false, false) => PiSkillOwnership::Absent,
|
||||
let ownership = if has_stale_destination {
|
||||
PiSkillOwnership::Stale
|
||||
} else {
|
||||
match (deployment.is_some(), destination_exists, owned_deployment) {
|
||||
(_, _, true) => PiSkillOwnership::Owned,
|
||||
(true, _, false) => PiSkillOwnership::Stale,
|
||||
(false, true, false) => PiSkillOwnership::Foreign,
|
||||
(false, false, false) => PiSkillOwnership::Absent,
|
||||
}
|
||||
};
|
||||
let (discovery_status, discovery_issue) = discovered.cloned().unwrap_or_else(|| {
|
||||
(
|
||||
@@ -343,14 +366,18 @@ fn inspect_skill_status(
|
||||
)
|
||||
});
|
||||
let effectively_discovered = discovery_status == PiSkillDiscovery::Active;
|
||||
let issue = match ownership {
|
||||
PiSkillOwnership::Stale => {
|
||||
Some("recorded Pi deployment no longer matches the live filesystem".to_string())
|
||||
let issue = if has_stale_destination {
|
||||
Some("recorded Pi deployment remains at a previous agent root".to_string())
|
||||
} else {
|
||||
match ownership {
|
||||
PiSkillOwnership::Stale => {
|
||||
Some("recorded Pi deployment no longer matches the live filesystem".to_string())
|
||||
}
|
||||
PiSkillOwnership::Foreign if skill.apps.pi => {
|
||||
Some("desired Pi Skill collides with an unowned live destination".to_string())
|
||||
}
|
||||
_ => discovery_issue,
|
||||
}
|
||||
PiSkillOwnership::Foreign if skill.apps.pi => {
|
||||
Some("desired Pi Skill collides with an unowned live destination".to_string())
|
||||
}
|
||||
_ => discovery_issue,
|
||||
};
|
||||
Ok(SkillAppStatus {
|
||||
desired_enabled: skill.apps.pi,
|
||||
@@ -370,10 +397,10 @@ fn deployment_lock() -> std::sync::MutexGuard<'static, ()> {
|
||||
}
|
||||
|
||||
fn reconcile_skill_unlocked(db: &Arc<Database>, skill: &InstalledSkill) -> Result<(), AppError> {
|
||||
let destination = skill_destination(skill)?;
|
||||
let destination_key = destination_key(&destination);
|
||||
let existing = db.get_pi_skill_deployment(&skill.id, &destination_key)?;
|
||||
if skill.apps.pi {
|
||||
let destination = skill_destination(skill)?;
|
||||
let destination_key = destination_key(&destination);
|
||||
let existing = db.get_pi_skill_deployment(&skill.id, &destination_key)?;
|
||||
let source = skill_source(skill)?;
|
||||
deploy(
|
||||
db,
|
||||
@@ -383,9 +410,64 @@ fn reconcile_skill_unlocked(db: &Arc<Database>, skill: &InstalledSkill) -> Resul
|
||||
&destination_key,
|
||||
existing,
|
||||
None,
|
||||
)
|
||||
)?;
|
||||
cleanup_stale_deployments(db, skill, &destination_key)
|
||||
} else {
|
||||
remove_owned(db, skill, &destination, &destination_key, existing, None)
|
||||
remove_all_recorded_deployments(db, skill, None)
|
||||
}
|
||||
}
|
||||
|
||||
fn cleanup_stale_deployments(
|
||||
db: &Arc<Database>,
|
||||
skill: &InstalledSkill,
|
||||
current_destination_key: &str,
|
||||
) -> Result<(), AppError> {
|
||||
for deployment in db.get_pi_skill_deployments(&skill.id)? {
|
||||
if deployment.destination_key != current_destination_key {
|
||||
remove_recorded_deployment(db, skill, deployment)?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_all_recorded_deployments(
|
||||
db: &Arc<Database>,
|
||||
skill: &InstalledSkill,
|
||||
desired_enabled: Option<bool>,
|
||||
) -> Result<(), AppError> {
|
||||
if let Some(desired_enabled) = desired_enabled {
|
||||
// Desired state is one row-level authority, independent of how many
|
||||
// old agent roots still have device-local ownership evidence.
|
||||
db.set_pi_skill_desired(&skill.id, desired_enabled)?;
|
||||
}
|
||||
for deployment in db.get_pi_skill_deployments(&skill.id)? {
|
||||
remove_recorded_deployment(db, skill, deployment)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_recorded_deployment(
|
||||
db: &Arc<Database>,
|
||||
skill: &InstalledSkill,
|
||||
deployment: SkillDeployment,
|
||||
) -> Result<(), AppError> {
|
||||
let destination = PathBuf::from(&deployment.destination);
|
||||
if destination_key(&destination) != deployment.destination_key {
|
||||
return Err(AppError::Conflict(format!(
|
||||
"Pi Skill '{}' has inconsistent recorded destination identity",
|
||||
skill.id
|
||||
)));
|
||||
}
|
||||
match fs::symlink_metadata(&destination) {
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
|
||||
db.delete_pi_skill_deployment(&skill.id, &deployment.destination_key)?;
|
||||
Ok(())
|
||||
}
|
||||
Err(error) => Err(AppError::io(&destination, error)),
|
||||
Ok(_) => {
|
||||
let key = deployment.destination_key.clone();
|
||||
remove_owned(db, skill, &destination, &key, Some(deployment), None)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1076,4 +1158,84 @@ mod tests {
|
||||
.as_deref()
|
||||
.is_some_and(|issue| issue.contains("description")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial_test::serial]
|
||||
fn agent_root_relocation_deploys_new_destination_before_cleaning_old_ownership() {
|
||||
struct EnvGuard {
|
||||
key: &'static str,
|
||||
previous: Option<std::ffi::OsString>,
|
||||
}
|
||||
impl EnvGuard {
|
||||
fn set(key: &'static str, value: &Path) -> Self {
|
||||
let previous = std::env::var_os(key);
|
||||
std::env::set_var(key, value);
|
||||
Self { key, previous }
|
||||
}
|
||||
}
|
||||
impl Drop for EnvGuard {
|
||||
fn drop(&mut self) {
|
||||
match self.previous.take() {
|
||||
Some(value) => std::env::set_var(self.key, value),
|
||||
None => std::env::remove_var(self.key),
|
||||
}
|
||||
if self.key == "CC_SWITCH_TEST_HOME" {
|
||||
let _ = crate::settings::reload_settings();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let temp = tempfile::tempdir().expect("tempdir");
|
||||
let _home = EnvGuard::set("CC_SWITCH_TEST_HOME", temp.path());
|
||||
crate::settings::reload_settings().expect("reload settings");
|
||||
let old_root = temp.path().join("old-pi");
|
||||
let new_root = temp.path().join("new-pi");
|
||||
let _pi_root = EnvGuard::set("PI_CODING_AGENT_DIR", &old_root);
|
||||
|
||||
let source = SkillService::get_ssot_dir()
|
||||
.expect("SSOT")
|
||||
.join("relocated");
|
||||
fs::create_dir_all(&source).expect("source");
|
||||
fs::write(
|
||||
source.join("SKILL.md"),
|
||||
"---\nname: relocated\ndescription: relocation test\n---\n",
|
||||
)
|
||||
.expect("manifest");
|
||||
let skill = InstalledSkill {
|
||||
id: "local:relocated".to_string(),
|
||||
name: "Relocated".to_string(),
|
||||
description: Some("relocation test".to_string()),
|
||||
directory: "relocated".to_string(),
|
||||
repo_owner: None,
|
||||
repo_name: None,
|
||||
repo_branch: None,
|
||||
readme_url: None,
|
||||
apps: SkillApps::only(&AppType::Pi),
|
||||
installed_at: 1,
|
||||
content_hash: None,
|
||||
updated_at: 1,
|
||||
};
|
||||
let db = Arc::new(Database::memory().expect("database"));
|
||||
db.save_skill(&skill).expect("save skill");
|
||||
PiSkillDeploymentService::reconcile_skill(&db, &skill).expect("old deployment");
|
||||
let old_destination = old_root.join("skills").join("relocated");
|
||||
assert!(fs::symlink_metadata(&old_destination).is_ok());
|
||||
|
||||
std::env::set_var("PI_CODING_AGENT_DIR", &new_root);
|
||||
PiSkillDeploymentService::reconcile_skill(&db, &skill).expect("relocate deployment");
|
||||
let new_destination = new_root.join("skills").join("relocated");
|
||||
assert!(fs::symlink_metadata(&new_destination).is_ok());
|
||||
assert!(
|
||||
fs::symlink_metadata(&old_destination).is_err(),
|
||||
"the verified old deployment must be cleaned only after new publication"
|
||||
);
|
||||
let deployments = db
|
||||
.get_pi_skill_deployments(&skill.id)
|
||||
.expect("deployment ledger");
|
||||
assert_eq!(deployments.len(), 1);
|
||||
assert_eq!(
|
||||
deployments[0].destination_key,
|
||||
destination_key(&new_destination)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user