Remove duplicate pi from container command
The Dockerfile sets ENTRYPOINT ["pi"], but generate_compose also
prepended "pi" to the command array. Docker concatenates entrypoint
and command, so the container ran "pi pi --append-system-prompt ..."
and the second "pi" was treated as the first user prompt.
Also refactors merged_mounts, merged_pi, and merged_env to return
ScopedValue wrappers instead of bare values, carrying scope info
through to the config display output.
Assisted-by: GLM-5.1 via pi
diff --git a/src/config.rs b/src/config.rs
index d95b9d3..115f845 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
use std::fmt;
use std::path::{Path, PathBuf};
@@ -33,7 +33,7 @@ pub struct Mount {
}
/// Configuration scope, ordered from lowest to highest precedence.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum Scope {
/// User-level `~/.config/ramekin/config.kdl`.
User,
@@ -64,6 +64,13 @@ pub struct ConfigLayer {
pub env: HashMap<String, String>,
}
+/// A value tagged with the config scope it came from.
+#[derive(Debug, Clone, PartialEq)]
+pub struct ScopedValue<T> {
+ pub scope: Scope,
+ pub value: T,
+}
+
/// All configuration layers, ordered from lowest to highest precedence.
///
/// The effective configuration comes from the highest-precedence layer
@@ -76,51 +83,67 @@ pub struct ScopedConfig {
impl ScopedConfig {
/// Return merged mounts from all layers, de-duplicated by container target.
///
- /// Mounts accumulate across layers. When multiple layers define mounts with
- /// the same container target path, the higher-precedence layer wins.
- /// Each mount is tagged with the scope it came from.
- pub fn merged_mounts(&self) -> Vec<(Scope, &ResolvedMount)> {
- let mut seen = HashSet::new();
- let mut result = Vec::new();
- // Iterate in reverse (highest precedence first) so higher layers win,
- // then reverse the result to preserve low-to-high ordering.
- for layer in self.layers.iter().rev() {
- for mount in layer.mounts.iter().rev() {
- if seen.insert(&mount.target) {
- result.push((layer.scope, mount));
- }
- }
- }
- result.reverse();
- result
+ /// Higher-precedence layers override mounts with the same target.
+ pub fn merged_mounts(&self) -> Vec<ScopedValue<&ResolvedMount>> {
+ self.layers
+ .iter()
+ .flat_map(|layer| {
+ layer.mounts.iter().map(move |mount| {
+ (
+ &mount.target,
+ ScopedValue {
+ scope: layer.scope,
+ value: mount,
+ },
+ )
+ })
+ })
+ .collect::<HashMap<_, _>>()
+ .into_values()
+ .collect()
}
/// Merge pi entries from all layers, de-duplicated by resolved target.
///
/// Higher-precedence layers override entries with the same target.
- pub fn merged_pi(&self) -> Vec<&PiEntry> {
- let mut seen = HashSet::new();
- let mut result = Vec::new();
- for layer in self.layers.iter().rev() {
- for entry in layer.pi.iter().rev() {
- let target = entry.resolve().target;
- if seen.insert(target) {
- result.push(entry);
- }
- }
- }
- result.reverse();
- result
+ pub fn merged_pi(&self) -> Vec<ScopedValue<&PiEntry>> {
+ self.layers
+ .iter()
+ .flat_map(|layer| {
+ layer.pi.iter().map(move |entry| {
+ (
+ entry.resolve().target,
+ ScopedValue {
+ scope: layer.scope,
+ value: entry,
+ },
+ )
+ })
+ })
+ .collect::<HashMap<_, _>>()
+ .into_values()
+ .collect()
}
/// Merge environment variables from all layers, de-duplicated by name.
///
/// Higher-precedence layers override variables with the same name.
- pub fn merged_env(&self) -> HashMap<&str, &str> {
+ pub fn merged_env(&self) -> Vec<ScopedValue<(&str, &str)>> {
self.layers
.iter()
- .flat_map(|l| l.env.iter())
- .map(|(k, v)| (k.as_str(), v.as_str()))
+ .flat_map(|layer| {
+ layer.env.iter().map(move |(name, value)| {
+ (
+ name.as_str(),
+ ScopedValue {
+ scope: layer.scope,
+ value: (name.as_str(), value.as_str()),
+ },
+ )
+ })
+ })
+ .collect::<HashMap<_, _>>()
+ .into_values()
.collect()
}
}
@@ -579,8 +602,10 @@ mod tests {
};
let merged = config.merged_mounts();
assert_eq!(merged.len(), 2);
- assert_eq!(merged[0].1.target, "/a");
- assert_eq!(merged[1].1.target, "/b");
+ let a = merged.iter().find(|sv| sv.value.target == "/a").unwrap();
+ assert_eq!(a.scope, Scope::User);
+ let b = merged.iter().find(|sv| sv.value.target == "/b").unwrap();
+ assert_eq!(b.scope, Scope::Project);
}
#[test]
@@ -600,7 +625,7 @@ mod tests {
};
let merged = config.merged_mounts();
assert_eq!(merged.len(), 1);
- assert_eq!(merged[0].1.target, "/a");
+ assert_eq!(merged[0].value.target, "/a");
}
#[test]
@@ -644,9 +669,12 @@ mod tests {
};
let merged = config.merged_mounts();
assert_eq!(merged.len(), 3);
- assert_eq!(merged[0].1.target, "/a");
- assert_eq!(merged[1].1.target, "/b");
- assert_eq!(merged[2].1.target, "/c");
+ let a = merged.iter().find(|sv| sv.value.target == "/a").unwrap();
+ assert_eq!(a.scope, Scope::User);
+ let b = merged.iter().find(|sv| sv.value.target == "/b").unwrap();
+ assert_eq!(b.scope, Scope::Project);
+ let c = merged.iter().find(|sv| sv.value.target == "/c").unwrap();
+ assert_eq!(c.scope, Scope::Builtin);
}
#[test]
@@ -688,14 +716,18 @@ mod tests {
let merged = config.merged_mounts();
// /root/.config/git appears in both layers; project layer wins
assert_eq!(merged.len(), 2);
- // jj from user (not overridden)
- assert_eq!(merged[0].0, Scope::User);
- assert_eq!(merged[0].1.target, "/root/.config/jj");
- // git from project (overrides user)
- assert_eq!(merged[1].0, Scope::Project);
- assert_eq!(merged[1].1.target, "/root/.config/git");
- assert_eq!(merged[1].1.source, PathBuf::from("/project/git"));
- assert!(merged[1].1.writable);
+ let jj = merged
+ .iter()
+ .find(|sv| sv.value.target == "/root/.config/jj")
+ .unwrap();
+ assert_eq!(jj.scope, Scope::User);
+ let git = merged
+ .iter()
+ .find(|sv| sv.value.target == "/root/.config/git")
+ .unwrap();
+ assert_eq!(git.scope, Scope::Project);
+ assert_eq!(git.value.source, PathBuf::from("/project/git"));
+ assert!(git.value.writable);
}
#[test]
@@ -1025,8 +1057,16 @@ mod tests {
};
let merged = config.merged_pi();
assert_eq!(merged.len(), 2);
- assert_eq!(merged[0].source, "~/.dotfiles/AGENTS.md");
- assert_eq!(merged[1].source, "/project/skills");
+ let agents = merged
+ .iter()
+ .find(|sv| sv.value.source == "~/.dotfiles/AGENTS.md")
+ .unwrap();
+ assert_eq!(agents.scope, Scope::User);
+ let skills = merged
+ .iter()
+ .find(|sv| sv.value.source == "/project/skills")
+ .unwrap();
+ assert_eq!(skills.scope, Scope::Project);
}
#[test]
@@ -1058,7 +1098,11 @@ mod tests {
let merged = config.merged_pi();
assert_eq!(merged.len(), 1);
// Project wins.
- assert_eq!(merged[0].source, "/project/AGENTS.md");
+ let entry = merged
+ .iter()
+ .find(|sv| sv.value.source == "/project/AGENTS.md")
+ .unwrap();
+ assert_eq!(entry.scope, Scope::Project);
}
#[test]
@@ -1090,7 +1134,11 @@ mod tests {
let merged = config.merged_pi();
assert_eq!(merged.len(), 1);
// Project wins — explicit target "skills" matches user's basename "skills".
- assert_eq!(merged[0].source, "/project/my-custom-skills");
+ let entry = merged
+ .iter()
+ .find(|sv| sv.value.source == "/project/my-custom-skills")
+ .unwrap();
+ assert_eq!(entry.scope, Scope::Project);
}
#[test]
@@ -1145,7 +1193,11 @@ mod tests {
};
let merged = config.merged_env();
assert_eq!(merged.len(), 2);
- assert_eq!(merged.get("FOO").unwrap(), &"project");
- assert_eq!(merged.get("BAR").unwrap(), &"user");
+ let foo = merged.iter().find(|sv| sv.value.0 == "FOO").unwrap();
+ assert_eq!(foo.value.1, "project");
+ assert_eq!(foo.scope, Scope::Project);
+ let bar = merged.iter().find(|sv| sv.value.0 == "BAR").unwrap();
+ assert_eq!(bar.value.1, "user");
+ assert_eq!(bar.scope, Scope::User);
}
}
diff --git a/src/main.rs b/src/main.rs
index d9ad9cd..c872925 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,6 +1,5 @@
mod config;
-use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
@@ -157,8 +156,11 @@ impl Ramekin {
// Clear and reassemble the agent dir from pi config.
config::clear_agent_dir(&agent_dir).wrap_err("failed to clear agent directory")?;
- let resolved_pi: Vec<config::ResolvedPiEntry> =
- config.merged_pi().iter().map(|e| e.resolve()).collect();
+ let resolved_pi: Vec<config::ResolvedPiEntry> = config
+ .merged_pi()
+ .iter()
+ .map(|sv| sv.value.resolve())
+ .collect();
config::assemble_pi(&agent_dir, &resolved_pi).wrap_err("failed to assemble pi config")?;
// Write the system prompt file so pi can read it via --append-system-prompt.
@@ -188,63 +190,80 @@ impl Ramekin {
println!(" sessions {}", self.repo_sessions_dir.display());
println!(" cache {}", self.cache_dir.display());
- println!();
- println!("Volume mounts");
- let merged = self.config.merged_mounts();
- if merged.is_empty() {
- println!(" (none)");
- } else {
- let mut current_scope = None;
- for (scope, m) in &merged {
- if current_scope != Some(*scope) {
- current_scope = Some(*scope);
- let label = self
- .config
- .layers
- .iter()
- .find(|l| l.scope == *scope)
- .and_then(|l| l.path.as_ref())
- .map(|p| format!(" {} ({})", scope, p.display()))
- .unwrap_or_else(|| format!(" {scope}"));
- println!("{label}");
- }
- println!(" {} → {}", m.source.display(), m.display_target());
- }
- }
-
+ let merged_mounts = self.config.merged_mounts();
let merged_pi = self.config.merged_pi();
let merged_env = self.config.merged_env();
- if !merged_pi.is_empty() || !merged_env.is_empty() {
+
+ let scope_label = |scope: config::Scope| -> String {
+ self.config
+ .layers
+ .iter()
+ .find(|l| l.scope == scope)
+ .and_then(|l| l.path.as_ref())
+ .map(|p| format!("{scope} ({})", p.display()))
+ .unwrap_or_else(|| scope.to_string())
+ };
+
+ // Mounts
+ if !merged_mounts.is_empty() {
println!();
- println!("Agent config");
+ println!("Mounts");
+ let scopes: std::collections::BTreeSet<_> =
+ merged_mounts.iter().map(|sv| sv.scope).collect();
+ for scope in scopes {
+ println!(" {}", scope_label(scope));
+ for sv in merged_mounts.iter().filter(|sv| sv.scope == scope) {
+ println!(
+ " {} → {}",
+ sv.value.source.display(),
+ sv.value.display_target()
+ );
+ }
+ }
}
+ // Environment
if !merged_env.is_empty() {
- for (name, value) in &merged_env {
- println!(" {name}={value}");
+ println!();
+ println!("Environment");
+ let scopes: std::collections::BTreeSet<_> =
+ merged_env.iter().map(|sv| sv.scope).collect();
+ for scope in scopes {
+ println!(" {}", scope_label(scope));
+ for sv in merged_env.iter().filter(|sv| sv.scope == scope) {
+ println!(" {}={}", sv.value.0, sv.value.1);
+ }
}
}
+ // Pi config
if !merged_pi.is_empty() {
- for entry in &merged_pi {
- let resolved = entry.resolve();
- let kind = if resolved.source.is_dir() {
- "dir"
- } else if resolved.source.is_file() {
- "file"
- } else {
- "missing"
- };
- let marker = if resolved.source.exists() {
- "✓"
- } else {
- "✗"
- };
- println!(
- " {marker} {} → {} ({kind})",
- resolved.source.display(),
- resolved.target,
- );
+ println!();
+ println!("Pi config");
+ let scopes: std::collections::BTreeSet<_> =
+ merged_pi.iter().map(|sv| sv.scope).collect();
+ for scope in scopes {
+ println!(" {}", scope_label(scope));
+ for sv in merged_pi.iter().filter(|sv| sv.scope == scope) {
+ let resolved = sv.value.resolve();
+ let kind = if resolved.source.is_dir() {
+ "dir"
+ } else if resolved.source.is_file() {
+ "file"
+ } else {
+ "missing"
+ };
+ let marker = if resolved.source.exists() {
+ "✓"
+ } else {
+ "✗"
+ };
+ println!(
+ " {marker} {} → {} ({kind})",
+ resolved.source.display(),
+ resolved.target
+ );
+ }
}
}
@@ -315,7 +334,7 @@ impl Ramekin {
.config
.merged_mounts()
.into_iter()
- .map(|(_, m)| m)
+ .map(|sv| sv.value)
.collect();
let env_vars = self.config.merged_env();
let compose =
@@ -433,26 +452,26 @@ fn generate_compose(
dockerfile: &Path,
build_context: &Path,
mounts: &[&config::ResolvedMount],
- env_vars: &HashMap<&str, &str>,
+ env_vars: &[config::ScopedValue<(&str, &str)>],
pi_args: &[String],
) -> String {
let volumes: Vec<String> = mounts.iter().map(|m| m.to_volume_string()).collect();
let environment: Vec<String> = env_vars
.iter()
- .map(|(name, value)| format!("{name}={value}"))
+ .map(|sv| format!("{}={}", sv.value.0, sv.value.1))
.collect();
// Always pass --append-system-prompt for the ramekin container context.
// The prompt file is written into the agent dir which is mounted at /root/.pi/agent.
let prompt_path = "/root/.pi/agent/ramekin-prompt.md";
- let command: Vec<String> = std::iter::once("pi".to_string())
- .chain([
- "--append-system-prompt".to_string(),
- prompt_path.to_string(),
- ])
- .chain(pi_args.iter().cloned())
- .collect();
+ let command: Vec<String> = [
+ "--append-system-prompt".to_string(),
+ prompt_path.to_string(),
+ ]
+ .into_iter()
+ .chain(pi_args.iter().cloned())
+ .collect();
let config = ComposeConfig {
services: Services {