🥇 export from upstream (3b6ef04)
This commit is contained in:
@@ -0,0 +1,234 @@
|
||||
# Mount Tilde Expansion Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Expand `~` in mount paths to the user's home directory at Docker invocation time, so mounts work on all platforms without relying on shell expansion.
|
||||
|
||||
**Architecture:** A single pure function `expand_mount_path` in `docker.rs` replaces `~` with `dirs::home_dir()` and normalizes Windows separators. Called from `build_run_args` where mounts become `-v` flags. Config files keep the portable `~` form.
|
||||
|
||||
**Tech Stack:** Rust, `dirs` crate (already a dependency)
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Add `expand_mount_path` function with TDD
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/sandcage/src/docker.rs:227-233` (call site)
|
||||
- Modify: `crates/sandcage/src/docker.rs` (add function + tests at end of file)
|
||||
|
||||
- [ ] **Step 1: Write the failing tests for `expand_mount_path`**
|
||||
|
||||
Add these tests inside the existing `#[cfg(test)] mod tests` block in `docker.rs`:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn expand_mount_path_expands_tilde_slash() {
|
||||
let home = dirs::home_dir().unwrap();
|
||||
let home_str = home.to_string_lossy().replace('\\', "/");
|
||||
let result = expand_mount_path("~/.ssh:/home/agent/.ssh:ro");
|
||||
assert_eq!(result, format!("{home_str}/.ssh:/home/agent/.ssh:ro"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn expand_mount_path_expands_bare_tilde() {
|
||||
let home = dirs::home_dir().unwrap();
|
||||
let home_str = home.to_string_lossy().replace('\\', "/");
|
||||
let result = expand_mount_path("~:/container");
|
||||
assert_eq!(result, format!("{home_str}:/container"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn expand_mount_path_ignores_absolute() {
|
||||
let result = expand_mount_path("/absolute:/container:ro");
|
||||
assert_eq!(result, "/absolute:/container:ro");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn expand_mount_path_ignores_relative() {
|
||||
let result = expand_mount_path("./relative:/container");
|
||||
assert_eq!(result, "./relative:/container");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn expand_mount_path_ignores_no_colon() {
|
||||
let result = expand_mount_path("no-colon");
|
||||
assert_eq!(result, "no-colon");
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `cargo test -p sandcage expand_mount_path`
|
||||
Expected: compilation error — `expand_mount_path` is not defined.
|
||||
|
||||
- [ ] **Step 3: Implement `expand_mount_path`**
|
||||
|
||||
Add this function in `docker.rs`, just above `build_run_args`:
|
||||
|
||||
```rust
|
||||
fn expand_mount_path(mount: &str) -> String {
|
||||
let Some(colon_pos) = mount.find(':') else {
|
||||
return mount.to_string();
|
||||
};
|
||||
|
||||
let host_path = &mount[..colon_pos];
|
||||
let rest = &mount[colon_pos..];
|
||||
|
||||
let expanded = if host_path == "~" {
|
||||
match dirs::home_dir() {
|
||||
Some(home) => home.to_string_lossy().into_owned(),
|
||||
None => return mount.to_string(),
|
||||
}
|
||||
} else if let Some(suffix) = host_path.strip_prefix("~/") {
|
||||
match dirs::home_dir() {
|
||||
Some(home) => format!("{}/{suffix}", home.to_string_lossy()),
|
||||
None => return mount.to_string(),
|
||||
}
|
||||
} else {
|
||||
return mount.to_string();
|
||||
};
|
||||
|
||||
let normalized = expanded.replace('\\', "/");
|
||||
format!("{normalized}{rest}")
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `cargo test -p sandcage expand_mount_path`
|
||||
Expected: all 5 tests PASS.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add crates/sandcage/src/docker.rs
|
||||
git commit -m "✨ Add expand_mount_path for cross-platform tilde expansion"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Wire `expand_mount_path` into `build_run_args` and update existing tests
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/sandcage/src/docker.rs:229-231` (call site)
|
||||
- Modify: `crates/sandcage/src/docker.rs` (existing tests at lines 866-921)
|
||||
|
||||
- [ ] **Step 1: Replace `mount.clone()` with `expand_mount_path(mount)` in `build_run_args`**
|
||||
|
||||
Change lines 229-231 from:
|
||||
|
||||
```rust
|
||||
if mount.contains(':') {
|
||||
args.push("-v".to_string());
|
||||
args.push(mount.clone());
|
||||
}
|
||||
```
|
||||
|
||||
To:
|
||||
|
||||
```rust
|
||||
if mount.contains(':') {
|
||||
args.push("-v".to_string());
|
||||
args.push(expand_mount_path(mount));
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update `build_run_args_with_mounts` test**
|
||||
|
||||
The test at line 866 currently asserts literal `~/.ssh` strings. Replace the assertions:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn build_run_args_with_mounts() {
|
||||
let home = dirs::home_dir().unwrap();
|
||||
let home_str = home.to_string_lossy().replace('\\', "/");
|
||||
let config = SandcageConfig {
|
||||
mounts: Some(vec![
|
||||
"~/.ssh:/home/agent/.ssh:ro".to_string(),
|
||||
"~/.gitconfig:/home/agent/.gitconfig:ro".to_string(),
|
||||
]),
|
||||
..Default::default()
|
||||
};
|
||||
let args = build_run_args("claude", "/tmp/compose.yml", &config, false, &[]);
|
||||
|
||||
assert!(args.contains(&"-v".to_string()), "should contain -v flag");
|
||||
let expected_ssh = format!("{home_str}/.ssh:/home/agent/.ssh:ro");
|
||||
let expected_git = format!("{home_str}/.gitconfig:/home/agent/.gitconfig:ro");
|
||||
assert!(args.contains(&expected_ssh), "SSH mount should be expanded");
|
||||
assert!(args.contains(&expected_git), "gitconfig mount should be expanded");
|
||||
|
||||
let service_pos = args.iter().position(|a| a == "claude").unwrap();
|
||||
let first_v = args.iter().position(|a| a == "-v").unwrap();
|
||||
assert!(first_v < service_pos, "-v flags must come before the service name");
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Update `build_run_args_mounts_with_env_and_extra_args` test**
|
||||
|
||||
The test at line 902 asserts literal `~/.ssh`. Replace the mount assertion:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn build_run_args_mounts_with_env_and_extra_args() {
|
||||
let home = dirs::home_dir().unwrap();
|
||||
let home_str = home.to_string_lossy().replace('\\', "/");
|
||||
let mut env = HashMap::new();
|
||||
env.insert("FOO".to_string(), "bar".to_string());
|
||||
let config = SandcageConfig {
|
||||
env: Some(env),
|
||||
mounts: Some(vec!["~/.ssh:/home/agent/.ssh:ro".to_string()]),
|
||||
..Default::default()
|
||||
};
|
||||
let args = build_run_args("claude", "/tmp/compose.yml", &config, false, &["--resume".to_string()]);
|
||||
|
||||
let service_pos = args.iter().position(|a| a == "claude").unwrap();
|
||||
let env_pos = args.iter().position(|a| a == "FOO=bar").unwrap();
|
||||
let expected_mount = format!("{home_str}/.ssh:/home/agent/.ssh:ro");
|
||||
let mount_pos = args.iter().position(|a| *a == expected_mount).unwrap();
|
||||
let resume_pos = args.iter().position(|a| a == "--resume").unwrap();
|
||||
|
||||
assert!(env_pos < service_pos, "env before service");
|
||||
assert!(mount_pos < service_pos, "mounts before service");
|
||||
assert!(resume_pos > service_pos, "extra args after service");
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the full test suite**
|
||||
|
||||
Run: `cargo test -p sandcage`
|
||||
Expected: all tests PASS.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add crates/sandcage/src/docker.rs
|
||||
git commit -m "🐛 Expand tilde in mount paths before passing to Docker"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Clean up the literal `~` directory artifact
|
||||
|
||||
- [ ] **Step 1: Remove the literal `~` directory from the project root**
|
||||
|
||||
```bash
|
||||
rm -rf "~"
|
||||
```
|
||||
|
||||
Verify it's gone:
|
||||
|
||||
```bash
|
||||
ls -la "~" 2>&1
|
||||
```
|
||||
|
||||
Expected: "No such file or directory"
|
||||
|
||||
- [ ] **Step 2: Verify `.gitignore` doesn't need updating**
|
||||
|
||||
Check if the `~` directory was tracked by git:
|
||||
|
||||
```bash
|
||||
git status
|
||||
```
|
||||
|
||||
If it shows as untracked (it should, since git wouldn't track a `~` directory), no `.gitignore` change is needed.
|
||||
Reference in New Issue
Block a user