From cc6c912fe2ad3ae11ecad93e015008cddee72a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabor=20K=C3=B6rber?= Date: Sun, 24 May 2026 18:47:38 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=A5=87=20export=20from=20upstream=20(56cc?= =?UTF-8?q?965)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../plans/2026-05-23-mount-tilde-expansion.md | 234 ------------------ ...2026-05-23-mount-tilde-expansion-design.md | 74 ------ 2 files changed, 308 deletions(-) delete mode 100644 docs/superpowers/plans/2026-05-23-mount-tilde-expansion.md delete mode 100644 docs/superpowers/specs/2026-05-23-mount-tilde-expansion-design.md diff --git a/docs/superpowers/plans/2026-05-23-mount-tilde-expansion.md b/docs/superpowers/plans/2026-05-23-mount-tilde-expansion.md deleted file mode 100644 index 512d7b2..0000000 --- a/docs/superpowers/plans/2026-05-23-mount-tilde-expansion.md +++ /dev/null @@ -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. diff --git a/docs/superpowers/specs/2026-05-23-mount-tilde-expansion-design.md b/docs/superpowers/specs/2026-05-23-mount-tilde-expansion-design.md deleted file mode 100644 index d2120ca..0000000 --- a/docs/superpowers/specs/2026-05-23-mount-tilde-expansion-design.md +++ /dev/null @@ -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\` | `C:/Users//...` | Forward slashes required | -| MINGW / Git Bash | `C:\Users\` (via Rust) | `C:/Users//...` | Rust bypasses MSYS path mangling | -| WSL (Linux binary) | `/home/` | `/home//...` | Native Linux paths | -| Native Linux | `/home/` | `/home//...` | Straightforward | -| macOS | `/Users/` | `/Users//...` | 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