From 8a5364782e04ebe9bf16a70f1f184a1eb2661e14 Mon Sep 17 00:00:00 2001 From: Sebastian Maniak Date: Fri, 23 Oct 2026 14:35:14 -0400 Subject: [PATCH] Let a system daemon manage Pi for one configured user Pi keeps its configuration only under the user's ~/.pi/agent, so a system daemon skipped it and a device needed a second, user-mode daemon just for Pi while Claude Desktop, Codex and Grok need the system daemon. A system daemon now manages Pi when daemon.pi.models and daemon.pi.settings name that user's files (both are required). The plan hands the files, their state sidecars and any directories it creates to the owner of the nearest existing ancestor, so Pi can read them and the credential command stays private to the user. Without daemon.pi paths a system daemon still skips Pi. ReconcilePlan gains an optional plan-wide owner (Unix), applied after each write and to created directories; a write is not skipped as already in place while the file's owner differs. Co-Authored-By: Claude Opus 5.5 --- crates/agent/src/daemon.rs | 48 +++++++++------ crates/agent/src/provider/pi/mod.rs | 93 ++++++++++++++++++++++++++--- crates/agent/src/reconcile/mod.rs | 5 ++ crates/agent/src/reconcile/plan.rs | 87 +++++++++++++++++++++++++-- crates/core/src/config.rs | 7 ++- schema/daemon-config.json | 4 +- schema/daemon-config.md | 2 +- 7 files changed, 209 insertions(+), 37 deletions(-) diff --git a/crates/agent/src/daemon.rs b/crates/agent/src/daemon.rs index bbb8a35..5962094 100644 --- a/crates/agent/src/daemon.rs +++ b/crates/agent/src/daemon.rs @@ -227,12 +227,20 @@ impl DaemonArgs { vscode_chat_models: None, vscode_settings: None, pi: { - if startup.pi.models.is_some() || startup.pi.settings.is_some() { - bail!("daemon.pi paths require --user (or daemon.user: true)"); - } - ResolvedPiStartupConfig { - models: None, - settings: None, + // Pi has no system-wide configuration. A system daemon manages + // it only for the one user whose files are named here. + match (startup.pi.models, startup.pi.settings) { + (None, None) => ResolvedPiStartupConfig { + models: None, + settings: None, + }, + (Some(models), Some(settings)) => ResolvedPiStartupConfig { + models: Some(models), + settings: Some(settings), + }, + _ => bail!( + "daemon.pi.models and daemon.pi.settings must both be set to manage Pi from a system daemon" + ), } }, reconcile_interval: startup.reconcile_interval, @@ -1408,7 +1416,7 @@ mod tests { use tokio::sync::{mpsc, watch}; #[test] - fn pi_startup_paths_require_user_mode() { + fn pi_startup_paths_resolve_in_both_modes() { for user in [false, true] { let file = tempfile::NamedTempFile::new().unwrap(); std::fs::write( @@ -1430,17 +1438,8 @@ mod tests { file.path().to_path_buf(), agentdesktop_core::DEFAULT_SOCKET_PATH.into(), ); - if !user { - assert!( - result - .err() - .unwrap() - .to_string() - .contains("daemon.pi paths require --user") - ); - continue; - } - let resolved = result.unwrap(); + // A system daemon uses them to manage that one user's Pi. + let resolved = result.unwrap_or_else(|error| panic!("user={user}: {error:#}")); assert_eq!( resolved.pi.models.as_deref(), Some(Path::new("custom/pi-models.json")) @@ -2323,6 +2322,19 @@ programs: assert_eq!(resolved.reconcile_interval, Some(Duration::from_secs(120))); } + #[test] + fn system_mode_rejects_a_single_pi_path() { + let mut startup = config::DaemonStartupConfig::default(); + startup.pi.models = Some(PathBuf::from("/home/dev/.pi/agent/models.json")); + let error = resolve_error(daemon_args(false), startup); + assert!( + error + .to_string() + .contains("daemon.pi.models and daemon.pi.settings must both be set"), + "{error:#}" + ); + } + #[test] fn system_mode_rejects_a_vscode_settings_path() { let mut startup = config::DaemonStartupConfig::default(); diff --git a/crates/agent/src/provider/pi/mod.rs b/crates/agent/src/provider/pi/mod.rs index ff03146..542add6 100644 --- a/crates/agent/src/provider/pi/mod.rs +++ b/crates/agent/src/provider/pi/mod.rs @@ -11,9 +11,13 @@ mod reconcile; #[derive(Clone, Default)] pub struct Pi { - /// User paths only; system daemons have no Pi files to manage. + /// User daemons always have these. A system daemon has them only when + /// `daemon.pi.models` and `daemon.pi.settings` name one user's files. pub models_path: Option, pub settings_path: Option, + /// Set for a system daemon configured with `daemon.pi` paths: manage that + /// one user's Pi files instead of skipping Pi as user-only. + pub manage_for_user: bool, } impl Pi { @@ -36,11 +40,15 @@ impl Provider for Pi { } fn plan(&self, ctx: &ReconcileContext, config: &DaemonConfig) -> anyhow::Result { - if let Some(plan) = ctx.skip_unless( - DaemonScope::User, - config.programs.pi.is_some(), - Self::DISPLAY_NAME, - ) { + // A system daemon manages Pi only for a user whose files are configured. + let for_user = !ctx.merge_user_settings && self.manage_for_user; + if !for_user + && let Some(plan) = ctx.skip_unless( + DaemonScope::User, + config.programs.pi.is_some(), + Self::DISPLAY_NAME, + ) + { return Ok(plan); } let plan = ReconcilePlan::default(); @@ -55,6 +63,11 @@ impl Provider for Pi { .filter(|_| provider.use_llm_gateway); (provider, gateway) }); + if for_user { + // Pi reads these as the user, so files and any directories the plan + // creates belong to whoever owns the user's Pi directory. + plan.set_owner(owner_of_nearest(models_path)); + } reconcile::plan( models_path, settings_path, @@ -67,6 +80,28 @@ impl Provider for Pi { } } +/// Owner of the closest existing ancestor of `path`, so a system daemon can +/// hand Pi's files to the user they belong to. `None` off Unix or if nothing +/// along the path can be inspected. +fn owner_of_nearest(path: &std::path::Path) -> Option { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + path.ancestors() + .skip(1) + .find_map(|dir| std::fs::metadata(dir).ok()) + .map(|metadata| crate::reconcile::plan::FileOwner { + uid: metadata.uid(), + gid: metadata.gid(), + }) + } + #[cfg(not(unix))] + { + let _ = path; + None + } +} + pub fn user_pi_agent_dir(home: &std::path::Path) -> PathBuf { std::env::var_os("PI_CODING_AGENT_DIR") .filter(|value| !value.is_empty()) @@ -91,9 +126,11 @@ mod tests { #[test] fn skips_configured_pi_in_system_mode() { let root = tempfile::tempdir().unwrap(); + // User paths alone do not opt a system daemon in (daemon.pi does). let pi = Pi { models_path: Some(root.path().join("models.json")), settings_path: Some(root.path().join("settings.json")), + manage_for_user: false, }; let ctx = ReconcileContext { llm_proxy: None, @@ -109,8 +146,8 @@ mod tests { .expect("Pi is skipped in system mode"); assert!(plan.inactive_reason().unwrap().contains("--user")); plan.apply().unwrap(); - assert!(!pi.models_path.as_ref().unwrap().exists()); - assert!(!pi.settings_path.as_ref().unwrap().exists()); + assert!(!root.path().join("models.json").exists()); + assert!(!root.path().join("settings.json").exists()); } #[test] @@ -119,6 +156,7 @@ mod tests { let pi = Pi { models_path: Some(root.path().join("models.json")), settings_path: Some(root.path().join("settings.json")), + manage_for_user: false, }; let mut ctx = ReconcileContext { llm_proxy: None, @@ -132,7 +170,8 @@ mod tests { assert!(pi.models_path.as_ref().unwrap().exists()); ctx.merge_user_settings = false; - pi.plan(&ctx, &DaemonConfig::default()) + Pi::default() + .plan(&ctx, &DaemonConfig::default()) .unwrap() .apply() .unwrap(); @@ -153,4 +192,40 @@ mod tests { .apply() .unwrap(); } + + #[test] + fn system_mode_manages_a_configured_users_pi_files() { + let root = tempfile::tempdir().unwrap(); + let agent_dir = root.path().join("home/dev/.pi/agent"); + let pi = Pi { + models_path: Some(agent_dir.join("models.json")), + settings_path: Some(agent_dir.join("settings.json")), + manage_for_user: true, + }; + let ctx = ReconcileContext { + llm_proxy: None, + merge_user_settings: false, + credential_helper: root.path().join("agentdesktop"), + socket: root.path().join("agentdesktop.sock"), + }; + let config = + parse_daemon("programs:\n pi:\n managedConfig:\n providers: {}\n").unwrap(); + + let plan = pi.plan(&ctx, &config).unwrap(); + assert!(plan.inactive_reason().is_none()); + plan.apply().unwrap(); + assert!(pi.models_path.as_ref().unwrap().exists()); + + // Files and the directories created for them belong to the owner of the + // nearest existing ancestor (here the test user). + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let owner = std::fs::metadata(root.path()).unwrap(); + for path in [pi.models_path.as_ref().unwrap(), &agent_dir] { + let metadata = std::fs::metadata(path).unwrap(); + assert_eq!((metadata.uid(), metadata.gid()), (owner.uid(), owner.gid())); + } + } + } } diff --git a/crates/agent/src/reconcile/mod.rs b/crates/agent/src/reconcile/mod.rs index e7be69b..94574e5 100644 --- a/crates/agent/src/reconcile/mod.rs +++ b/crates/agent/src/reconcile/mod.rs @@ -107,6 +107,11 @@ impl Reconciler { providers_path: copilot_providers_path, }), Box::new(Pi { + // A system daemon gets Pi paths only from daemon.pi, which + // opts it in to managing that one user's Pi. + manage_for_user: !merge_user_settings + && pi_models_path.is_some() + && pi_settings_path.is_some(), models_path: pi_models_path, settings_path: pi_settings_path, }), diff --git a/crates/agent/src/reconcile/plan.rs b/crates/agent/src/reconcile/plan.rs index 5f135eb..b9d4b86 100644 --- a/crates/agent/src/reconcile/plan.rs +++ b/crates/agent/src/reconcile/plan.rs @@ -3,7 +3,7 @@ use super::DryRunReport; use super::registry::RegistryKeyPlan; use anyhow::Context; use std::{ - cell::RefCell, + cell::{Cell, RefCell}, collections::BTreeMap, fs, io, path::{Path, PathBuf}, @@ -26,6 +26,17 @@ pub struct ReconcilePlan { /// Why the configured program cannot be managed in this daemon's mode or /// cannot use the gateway (the loopback proxy is absent). inactive: RefCell>, + /// Owner for files and directories this plan creates, when a system daemon + /// manages a file in one user's profile. `None` keeps the daemon's owner. + owner: Cell>, +} + +/// The user and group a planned file or directory is handed to after it is +/// written. Applied on Unix only. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct FileOwner { + pub uid: u32, + pub gid: u32, } /// What one provider's plan contains, taken before it is appended to the @@ -61,13 +72,14 @@ struct FileChange { path: PathBuf, contents: Option>, permissions: u32, + owner: Option, } /// A directory to create owner-only before the file changes are applied. /// Used for a directory that will hold a secret-bearing file and does not /// exist yet; an existing directory is left as it is. #[derive(Default)] -struct PrivateDirs(RefCell>); +struct PrivateDirs(RefCell)>>); impl ReconcilePlan { pub fn render(&self) -> String { @@ -221,9 +233,11 @@ impl ReconcilePlan { )); } } - for dir in self.private_dirs.0.into_inner() { + for (dir, owner) in self.private_dirs.0.into_inner() { if !dir.exists() { + let created = missing_dirs(&dir); crate::secure_fs::ensure_private_dir(&dir) + .and_then(|()| apply_owner(&created, owner)) .map_err(|error| (error, ApplyStop::PrivateDir(dir.clone())))?; } } @@ -231,6 +245,7 @@ impl ReconcilePlan { for (index, change) in self.operations.into_inner().into_iter().enumerate() { if let Some(contents) = &change.contents && already_in_place(&change.path, contents, change.permissions, &observed) + && owned_by(&change.path, change.owner) { continue; } @@ -242,9 +257,17 @@ impl ReconcilePlan { .parent() .filter(|p| !p.as_os_str().is_empty()) .unwrap_or_else(|| Path::new(".")); + let created = missing_dirs(parent); fs::create_dir_all(parent) .with_context(|| format!("create directory {}", parent.display()))?; - crate::secure_fs::atomic_write(&change.path, &contents, change.permissions) + crate::secure_fs::atomic_write( + &change.path, + &contents, + change.permissions, + )?; + let mut owned = created; + owned.push(change.path.clone()); + apply_owner(&owned, change.owner) } None => fs::remove_file(&change.path) .with_context(|| format!("remove {}", change.path.display())), @@ -359,13 +382,23 @@ impl ReconcilePlan { path: path.to_owned(), contents: Some(contents.to_vec()), permissions, + owner: self.owner.get(), }); Ok(()) } /// Create `dir` owner-only at apply time if it does not exist by then. pub(crate) fn ensure_private_dir(&self, dir: &Path) { - self.private_dirs.0.borrow_mut().push(dir.to_owned()); + self.private_dirs + .0 + .borrow_mut() + .push((dir.to_owned(), self.owner.get())); + } + + /// Hand every file and directory this plan creates to `owner` (Unix). + /// Used when a system daemon manages a file inside one user's profile. + pub(crate) fn set_owner(&self, owner: Option) { + self.owner.set(owner); } /// Plan one Windows registry key's changes, applied after the file @@ -387,6 +420,7 @@ impl ReconcilePlan { path: path.to_owned(), contents: None, permissions: 0, + owner: None, }); Ok(()) } @@ -446,6 +480,49 @@ fn already_in_place( /// Whether the file at `path` grants a permission bit outside `mode`. /// Unix: the current mode has a bit outside `mode`. A missing file, and any /// file on non-Unix, is false. +/// Directories from `dir` upward that do not exist yet, deepest first, so the +/// ones a write creates can be handed to the plan's owner afterwards. +fn missing_dirs(dir: &Path) -> Vec { + let mut missing = Vec::new(); + let mut current = Some(dir); + while let Some(path) = current { + if path.as_os_str().is_empty() || path.exists() { + break; + } + missing.push(path.to_path_buf()); + current = path.parent(); + } + missing +} + +/// Give `paths` to `owner`. A no-op without an owner and on non-Unix systems. +fn apply_owner(paths: &[PathBuf], owner: Option) -> anyhow::Result<()> { + #[cfg(unix)] + if let Some(owner) = owner { + for path in paths { + std::os::unix::fs::chown(path, Some(owner.uid), Some(owner.gid)) + .with_context(|| format!("set owner of {}", path.display()))?; + } + } + #[cfg(not(unix))] + let _ = (paths, owner); + Ok(()) +} + +/// Whether `path` already belongs to `owner` (always true without an owner). +fn owned_by(path: &Path, owner: Option) -> bool { + #[cfg(unix)] + if let Some(owner) = owner { + use std::os::unix::fs::MetadataExt; + return fs::metadata(path) + .map(|metadata| metadata.uid() == owner.uid && metadata.gid() == owner.gid) + .unwrap_or(false); + } + #[cfg(not(unix))] + let _ = (path, owner); + true +} + pub(crate) fn grants_beyond(path: &Path, mode: u32) -> bool { #[cfg(unix)] { diff --git a/crates/core/src/config.rs b/crates/core/src/config.rs index 83c7c12..2149028 100644 --- a/crates/core/src/config.rs +++ b/crates/core/src/config.rs @@ -205,7 +205,9 @@ pub struct OpenCodeStartupConfig { #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct PiStartupConfig { - /// Model catalog file. + /// Model catalog file. A user daemon defaults to Pi's agent directory. A + /// system daemon manages Pi only when this and `settings` name one user's + /// files; it writes them as the owner of that user's Pi directory. #[serde(default, skip_serializing_if = "Option::is_none")] pub models: Option, /// Settings file. @@ -1189,7 +1191,8 @@ impl VsCodeConfig { /// /// Generated `providers.agentdesktop` values take precedence over overlapping /// `managedConfig` keys. Pi stores configuration under `~/.pi/agent` (or -/// `PI_CODING_AGENT_DIR`), so management requires `--user`. System daemons skip it. +/// `PI_CODING_AGENT_DIR`), so a user daemon manages it; a system daemon manages +/// it only for the user named by `daemon.pi.models` and `daemon.pi.settings`. #[derive(Clone, Debug, Deserialize, Serialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] diff --git a/schema/daemon-config.json b/schema/daemon-config.json index 377fc81..2542b06 100644 --- a/schema/daemon-config.json +++ b/schema/daemon-config.json @@ -593,7 +593,7 @@ "additionalProperties": false }, "PiConfig": { - "description": "Settings reconciled into Pi's user `models.json` and `settings.json`.\n\nGenerated `providers.agentdesktop` values take precedence over overlapping\n`managedConfig` keys. Pi stores configuration under `~/.pi/agent` (or\n`PI_CODING_AGENT_DIR`), so management requires `--user`. System daemons skip it.", + "description": "Settings reconciled into Pi's user `models.json` and `settings.json`.\n\nGenerated `providers.agentdesktop` values take precedence over overlapping\n`managedConfig` keys. Pi stores configuration under `~/.pi/agent` (or\n`PI_CODING_AGENT_DIR`), so a user daemon manages it; a system daemon manages\nit only for the user named by `daemon.pi.models` and `daemon.pi.settings`.", "type": "object", "properties": { "api": { @@ -632,7 +632,7 @@ "type": "object", "properties": { "models": { - "description": "Model catalog file.", + "description": "Model catalog file. A user daemon defaults to Pi's agent directory. A\nsystem daemon manages Pi only when this and `settings` name one user's\nfiles; it writes them as the owner of that user's Pi directory.", "type": [ "string", "null" diff --git a/schema/daemon-config.md b/schema/daemon-config.md index 81e5fe7..c1806f5 100644 --- a/schema/daemon-config.md +++ b/schema/daemon-config.md @@ -27,7 +27,7 @@ |`daemon.openCode.config`|string|Configuration file.| |`daemon.openCode.plugin`|string|Credential plugin path.| |`daemon.pi`|object|Pi paths.| -|`daemon.pi.models`|string|Model catalog file.| +|`daemon.pi.models`|string|Model catalog file. A user daemon defaults to Pi's agent directory. A
system daemon manages Pi only when this and `settings` name one user's
files; it writes them as the owner of that user's Pi directory.| |`daemon.pi.settings`|string|Settings file.| |`daemon.reconcileInterval`|string|Opt-in interval between periodic re-applies of the current
configuration, which repair drift (a managed file deleted or edited by
hand, a fixed conflict, a loosened mode) without rewriting anything
unchanged. Unset means no periodic re-apply; a controller-managed
device still re-applies the controller's configuration on every
reconnect. Must be greater than zero and at most 30 days; read at
startup only (restart the daemon after changing it).| |`daemon.socket`|string|Local API Unix socket or Windows named pipe.|