🥇 export from upstream (56cc965)
This commit is contained in:
@@ -1,234 +0,0 @@
|
||||
# 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.
|
||||
@@ -1,74 +0,0 @@
|
||||
# Mount Tilde Expansion
|
||||
|
||||
**Date:** 2026-05-23
|
||||
**Status:** Approved
|
||||
|
||||
## Problem
|
||||
|
||||
Mount paths in `.sandcage.yml` use `~` for portability (e.g. `~/.ssh:/home/agent/.ssh:ro`).
|
||||
Sandcage passes these strings verbatim as `-v` flags to `docker compose run` via Rust's
|
||||
`Command::new()`, which does not invoke a shell. Since `~` is a shell expansion feature,
|
||||
Docker receives the literal string `~/.ssh` and either:
|
||||
|
||||
- **Windows:** Errors with "invalid characters for a local volume name"
|
||||
- **Linux/macOS:** Creates a literal `~` directory relative to CWD
|
||||
|
||||
## Platform Matrix
|
||||
|
||||
| Environment | `dirs::home_dir()` | Docker path format | Notes |
|
||||
|---|---|---|---|
|
||||
| Native Windows (PowerShell/cmd) | `C:\Users\<user>` | `C:/Users/<user>/...` | Forward slashes required |
|
||||
| MINGW / Git Bash | `C:\Users\<user>` (via Rust) | `C:/Users/<user>/...` | Rust bypasses MSYS path mangling |
|
||||
| WSL (Linux binary) | `/home/<user>` | `/home/<user>/...` | Native Linux paths |
|
||||
| Native Linux | `/home/<user>` | `/home/<user>/...` | Straightforward |
|
||||
| macOS | `/Users/<user>` | `/Users/<user>/...` | Straightforward |
|
||||
|
||||
Key insight: Rust's `Command::new()` spawns Docker directly — no shell involved — so
|
||||
the environment (PowerShell, Git Bash, etc.) is irrelevant. `dirs::home_dir()` returns
|
||||
the correct native path on every platform.
|
||||
|
||||
## Design
|
||||
|
||||
### Approach: Expand at mount-pass-through time
|
||||
|
||||
Config files keep the portable `~` form. Expansion happens only when building Docker
|
||||
CLI args.
|
||||
|
||||
### Implementation
|
||||
|
||||
Add `expand_mount_path(mount: &str) -> String` in `docker.rs`:
|
||||
|
||||
1. Split mount on `:` to extract host path, container path, and optional mode.
|
||||
2. If host path starts with `~/` or equals `~`, replace the `~` prefix with
|
||||
`dirs::home_dir()`.
|
||||
3. On Windows (`cfg!(windows)`), normalize host path separators to forward slashes.
|
||||
4. Rejoin parts with `:` and return.
|
||||
|
||||
Call site: `build_run_args()` in `docker.rs`, replacing the current
|
||||
`args.push(mount.clone())` with `args.push(expand_mount_path(mount))`.
|
||||
|
||||
### Edge Cases
|
||||
|
||||
| Input | Output (Linux) | Output (Windows) |
|
||||
|---|---|---|
|
||||
| `~/.ssh:/home/agent/.ssh:ro` | `/home/user/.ssh:/home/agent/.ssh:ro` | `C:/Users/user/.ssh:/home/agent/.ssh:ro` |
|
||||
| `~:/container` | `/home/user:/container` | `C:/Users/user:/container` |
|
||||
| `/absolute:/container:ro` | `/absolute:/container:ro` (unchanged) | `/absolute:/container:ro` (unchanged) |
|
||||
| `./relative:/container` | `./relative:/container` (unchanged) | `./relative:/container` (unchanged) |
|
||||
|
||||
### Not in Scope
|
||||
|
||||
- `$HOME` / `${HOME}` expansion
|
||||
- `~otheruser/` expansion
|
||||
- Environment variable substitution in mount paths
|
||||
|
||||
### Testing
|
||||
|
||||
- Unit tests for `expand_mount_path` covering all edge cases above.
|
||||
- Update existing `build_run_args` tests that assert literal `~/.ssh` to assert the
|
||||
expanded absolute path.
|
||||
|
||||
### Files Changed
|
||||
|
||||
- `crates/sandcage/src/docker.rs` — add `expand_mount_path`, call it in `build_run_args`
|
||||
- Existing tests in `docker.rs` — update mount assertions
|
||||
Reference in New Issue
Block a user