diff --git a/core/archipelago/src/api/rpc/assistant_chat.rs b/core/archipelago/src/api/rpc/assistant_chat.rs index 898ea66a..e26d0f6c 100644 --- a/core/archipelago/src/api/rpc/assistant_chat.rs +++ b/core/archipelago/src/api/rpc/assistant_chat.rs @@ -187,6 +187,21 @@ impl RpcHandler { grants.set(category, granted); grants.save(self.data_dir()).await?; + // Keep the legacy UI-store file in step — same downgrade-safety as + // `ai.permissions.set` — so no reader of either file sees a stale + // set, and a future `ai_grants_unified` migration (should the + // grants file ever be removed) cannot resurrect an old state. + let names: Vec = grants + .categories() + .iter() + .filter_map(|c| serde_json::to_value(c).ok()?.as_str().map(str::to_string)) + .collect(); + let _ = crate::settings::ai_permissions::save( + self.data_dir(), + crate::settings::ai_permissions::AiPermissions { granted: names }, + ) + .await; + Ok(serde_json::json!({ "categories": grants_categories_json(&grants) })) } diff --git a/core/archipelago/src/api/rpc/system/handlers.rs b/core/archipelago/src/api/rpc/system/handlers.rs index cd097797..6afe0cca 100644 --- a/core/archipelago/src/api/rpc/system/handlers.rs +++ b/core/archipelago/src/api/rpc/system/handlers.rs @@ -1076,24 +1076,48 @@ impl RpcHandler { /// The node's AI grants, as category id strings. /// - /// Reads the assistant's grants file — the one that actually gates tools — - /// and folds in anything still recorded only in the older - /// settings/ai_permissions.json, so grants made before the two were - /// unified are not silently revoked on upgrade. + /// The assistant's grants file is the AUTHORITY — it is what gates the + /// tools, so it alone answers whenever it exists. The older + /// settings/ai_permissions.json is consulted only when no assistant + /// file exists yet (upgrade from pre-unification versions), and that + /// read migrates forward: the legacy set is persisted as assistant + /// grants, ending the union. A permanent union resurrected a revoked + /// category on every read whenever the two files drifted — observed + /// live on archi-dev-box, where legacy said all-ten and grants.json + /// said four, so the UI showed every toggle ON while the assistant + /// refused six of them. async fn ai_grants_unified(data_dir: &std::path::Path) -> Vec { - let grants = crate::assistant::grants::Grants::load(data_dir).await; - let mut out: Vec = grants - .categories() - .iter() - .filter_map(|c| serde_json::to_value(c).ok()?.as_str().map(str::to_string)) - .collect(); + if crate::assistant::grants::Grants::exists(data_dir).await { + let grants = crate::assistant::grants::Grants::load(data_dir).await; + let mut out: Vec = grants + .categories() + .iter() + .filter_map(|c| serde_json::to_value(c).ok()?.as_str().map(str::to_string)) + .collect(); + out.sort(); + return out; + } + // One-time upgrade fold: no assistant file yet — carry the legacy + // UI-store grants forward so pre-unification grants are not silently + // revoked, persist them as the authoritative file, and let the + // legacy file become inert (every set path rewrites it in step for + // downgrade-safety). let legacy = crate::settings::ai_permissions::load(data_dir).await; - for name in legacy.granted { - if !out.contains(&name) { - out.push(name); + if legacy.granted.is_empty() { + return Vec::new(); + } + let mut grants = crate::assistant::grants::Grants::default_closed(); + let mut out: Vec = Vec::new(); + for name in &legacy.granted { + if let Ok(cat) = serde_json::from_value::( + serde_json::Value::String(name.clone()), + ) { + grants.set(cat, true); + out.push(name.clone()); } } + let _ = grants.save(data_dir).await; out.sort(); out.dedup(); out @@ -1607,3 +1631,58 @@ mod host_secrets_tests { assert_eq!(v["rotated_at"], "2026-08-02T12:04:00Z"); } } + +#[cfg(test)] +mod ai_grants_tests { + use super::*; + + /// The assistant grants file is the authority: when it exists, the + /// legacy UI-store file must NOT widen it. Regression test for the + /// live archi-dev-box desync — legacy said all-ten, grants.json said + /// four, and the union resurrected the six closed categories into the + /// UI on every read. + #[tokio::test] + async fn existing_grants_file_outranks_legacy() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join("assistant")).unwrap(); + std::fs::create_dir_all(dir.path().join("settings")).unwrap(); + std::fs::write( + dir.path().join("assistant/grants.json"), + r#"{"categories":["apps"]}"#, + ) + .unwrap(); + std::fs::write( + dir.path().join("settings/ai_permissions.json"), + r#"{"granted":["apps","wallet"]}"#, + ) + .unwrap(); + + let got = RpcHandler::ai_grants_unified(dir.path()).await; + assert_eq!(got, vec!["apps".to_string()], "legacy file must not widen the authority"); + } + + /// With no assistant grants file yet (pre-unification upgrade), the + /// legacy set folds forward and is persisted as the authoritative + /// file, so subsequent reads answer from it. + #[tokio::test] + async fn missing_grants_file_migrates_legacy_forward() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join("settings")).unwrap(); + std::fs::write( + dir.path().join("settings/ai_permissions.json"), + r#"{"granted":["apps","wallet"]}"#, + ) + .unwrap(); + + let got = RpcHandler::ai_grants_unified(dir.path()).await; + assert_eq!(got, vec!["apps".to_string(), "wallet".to_string()]); + assert!( + dir.path().join("assistant/grants.json").exists(), + "migration must persist the authoritative file" + ); + // Second read answers from the migrated file even if legacy is gone. + std::fs::remove_file(dir.path().join("settings/ai_permissions.json")).unwrap(); + let got2 = RpcHandler::ai_grants_unified(dir.path()).await; + assert_eq!(got2, vec!["apps".to_string(), "wallet".to_string()]); + } +} diff --git a/core/archipelago/src/assistant/grants.rs b/core/archipelago/src/assistant/grants.rs index aff16401..80f60bdb 100644 --- a/core/archipelago/src/assistant/grants.rs +++ b/core/archipelago/src/assistant/grants.rs @@ -60,6 +60,8 @@ impl Grants { /// data layer; `CallerScope::granted_categories` has no other source of /// authority to fall back to. pub async fn load(data_dir: &Path) -> Grants { + + let path = data_dir.join(GRANTS_FILE); let Ok(content) = tokio::fs::read_to_string(&path).await else { return Grants::default_closed(); @@ -67,6 +69,15 @@ impl Grants { serde_json::from_str(&content).unwrap_or_else(|_| Grants::default_closed()) } + /// Whether a grants file exists on disk at all. `load` cannot say this — + /// it maps "absent" and "present but empty" to the same value, and the + /// unified `ai.permissions.get` reader needs the distinction: an existing + /// file is authoritative, while an absent one triggers the one-time + /// legacy migration. + pub(crate) async fn exists(data_dir: &Path) -> bool { + tokio::fs::metadata(data_dir.join(GRANTS_FILE)).await.is_ok() + } + /// Persist the grants for this node, 0600 (following /// `streaming/session.rs`'s `data_dir`-scoped persisted-state /// convention, and this codebase's convention of keeping