From 188c1a64f9ba6dde937f1c039fa3f01c3b611087 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 9 May 2026 19:06:09 -0700 Subject: [PATCH 01/19] fix: generate SKILL.md with all tags active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SKILL.md lives in the repo and is read by agents on all platforms. Excluding WSL tools on Linux builds meant Windows agents couldn't see them. Since tool registration is already #[cfg]-gated in Rust source, SKILL.md should document everything — agents that call unavailable tools get an actionable error ('WSL not found'). --- SKILL.md | 54 ++++++++++++++++++++++++++++++++ crates/devcontainer-mcp/build.rs | 29 ++++++----------- 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/SKILL.md b/SKILL.md index 3842368..a9f8937 100644 --- a/SKILL.md +++ b/SKILL.md @@ -47,6 +47,15 @@ tools: - devpod_file_write - devpod_file_edit - devpod_file_list + - wsl_list + - wsl_exec + - wsl_terminate + - wsl_shutdown + - wsl_set_default + - wsl_file_read + - wsl_file_write + - wsl_file_edit + - wsl_file_list --- # DevContainer MCP Skill @@ -166,6 +175,51 @@ codespaces_ssh(auth: "github-USERNAME", codespace: "codespace-name", command: "n codespaces_stop(auth: "github-USERNAME", codespace: "codespace-name") ``` +## Workflow: WSL (Windows only) + +> **Use these tools — not raw `wsl.exe` or PowerShell `wsl` commands.** When a user asks to work "in WSL" or "directly in WSL", use `wsl_exec` and the WSL file tools below — these ARE the way to work in WSL through MCP. + +WSL tools let you clone repos, build projects, and run commands inside any WSL distribution — without devcontainers or Docker. + +### 1. List available distributions +``` +wsl_list() +``` + +### 2. Clone and build a repo in WSL +``` +wsl_exec(distro: "Ubuntu", command: "git clone https://github.com/org/repo.git /home/user/repo") +wsl_exec(distro: "Ubuntu", command: "cd /home/user/repo && cargo build") +``` + +### 3. Execute any command inside a distribution +``` +wsl_exec(distro: "Ubuntu", command: "apt list --installed") +``` + +### 4. Set the default distribution +``` +wsl_set_default(distro: "Ubuntu") +``` + +### 5. Stop a distribution +``` +wsl_terminate(distro: "Ubuntu") +``` + +### 6. Shut down all WSL distributions +``` +wsl_shutdown() +``` + +### File operations in WSL +``` +wsl_file_read(distro: "Ubuntu", path: "/home/user/project/src/main.rs") +wsl_file_write(distro: "Ubuntu", path: "/home/user/file.txt", content: "fn main() {}") +wsl_file_edit(distro: "Ubuntu", path: "/home/user/file.txt", old_str: "fn main", new_str: "fn start") +wsl_file_list(distro: "Ubuntu", path: "/home/user/project") +``` + ## Self-Healing If `devpod_up`, `devcontainer_up`, or `codespaces_create` returns errors: diff --git a/crates/devcontainer-mcp/build.rs b/crates/devcontainer-mcp/build.rs index 90f23ac..256ba3f 100644 --- a/crates/devcontainer-mcp/build.rs +++ b/crates/devcontainer-mcp/build.rs @@ -7,27 +7,19 @@ const FRONTMATTER_DESC: &str = "Manage dev container environments via MCP tools (DevPod, devcontainer CLI, Codespaces)"; /// Resolve the set of active tags for the current build target. +/// +/// SKILL.md is read from the repo by all platforms, so we activate all tags +/// to ensure every tool is documented. The actual tool registration is +/// controlled by #[cfg(target_os)] in Rust source — build.rs only assembles +/// the documentation. fn active_tags() -> HashSet { let mut tags = HashSet::new(); tags.insert("core".to_string()); - - let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default(); - match target_os.as_str() { - "windows" => { - tags.insert("windows".to_string()); - tags.insert("docker-desktop".to_string()); - tags.insert("wsl".to_string()); - } - "macos" => { - tags.insert("macos".to_string()); - tags.insert("docker-desktop".to_string()); - } - "linux" => { - tags.insert("linux".to_string()); - } - _ => {} - } - + tags.insert("linux".to_string()); + tags.insert("macos".to_string()); + tags.insert("windows".to_string()); + tags.insert("docker-desktop".to_string()); + tags.insert("wsl".to_string()); tags } @@ -174,5 +166,4 @@ fn main() { // --- Incremental build support ---------------------------------------------- println!("cargo:rerun-if-changed={}", skills_dir.display()); - println!("cargo:rerun-if-env-changed=CARGO_CFG_TARGET_OS"); } From e2234bdea02a9bded833b275f0377bc32ac129b2 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 9 May 2026 19:32:15 -0700 Subject: [PATCH 02/19] =?UTF-8?q?refactor:=20remove=20WSL=20tools=20?= =?UTF-8?q?=E2=80=94=20will=20be=20a=20separate=20wsl-mcp=20project?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WSL management is a different concern from dev containers. Bundling them forced platform detection complexity that didn't fit the deployment model (Linux binary running inside WSL via bridge). Removed: - crates/devcontainer-mcp-core/src/wsl.rs - crates/devcontainer-mcp/src/tools/wsl/ (6 files, 9 tools) - skills/wsl.md and skills/_tools/wsl.txt - CliBinary::Wsl and Error::WslNotFound - Windows native build job in release.yml - WSL references in skill constraint text Kept: - Composable #[tool_router] architecture - Tag-based skill composition (build.rs) - shell-escape crate fix - Strengthened skill constraints --- .github/workflows/release.yml | 23 --- SKILL.md | 58 +----- crates/devcontainer-mcp-core/src/cli.rs | 7 - crates/devcontainer-mcp-core/src/error.rs | 4 - crates/devcontainer-mcp-core/src/lib.rs | 2 - crates/devcontainer-mcp-core/src/wsl.rs | 182 ------------------ crates/devcontainer-mcp/build.rs | 1 - crates/devcontainer-mcp/src/tools/mod.rs | 9 +- crates/devcontainer-mcp/src/tools/wsl/exec.rs | 27 --- .../devcontainer-mcp/src/tools/wsl/files.rs | 138 ------------- crates/devcontainer-mcp/src/tools/wsl/list.rs | 18 -- crates/devcontainer-mcp/src/tools/wsl/mod.rs | 21 -- .../src/tools/wsl/set_default.rs | 25 --- .../src/tools/wsl/shutdown.rs | 18 -- crates/devcontainer-mcp/src/tools/wsl/stop.rs | 25 --- skills/_tools/wsl.txt | 12 -- skills/core-rule.md | 2 +- skills/footer.md | 2 +- skills/wsl.md | 48 ----- 19 files changed, 6 insertions(+), 616 deletions(-) delete mode 100644 crates/devcontainer-mcp-core/src/wsl.rs delete mode 100644 crates/devcontainer-mcp/src/tools/wsl/exec.rs delete mode 100644 crates/devcontainer-mcp/src/tools/wsl/files.rs delete mode 100644 crates/devcontainer-mcp/src/tools/wsl/list.rs delete mode 100644 crates/devcontainer-mcp/src/tools/wsl/mod.rs delete mode 100644 crates/devcontainer-mcp/src/tools/wsl/set_default.rs delete mode 100644 crates/devcontainer-mcp/src/tools/wsl/shutdown.rs delete mode 100644 crates/devcontainer-mcp/src/tools/wsl/stop.rs delete mode 100644 skills/_tools/wsl.txt delete mode 100644 skills/wsl.md diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d90e382..a4eea12 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -88,26 +88,3 @@ jobs: files: | install.sh install.ps1 - - build-windows: - name: Build devcontainer-mcp-windows-x64 - runs-on: windows-latest - steps: - - uses: actions/checkout@v5 - - uses: dtolnay/rust-toolchain@stable - with: - targets: x86_64-pc-windows-msvc - - uses: Swatinem/rust-cache@v2 - with: - key: x86_64-pc-windows-msvc - - - name: Build - run: cargo build --release --target x86_64-pc-windows-msvc -p devcontainer-mcp - - - name: Package binary - run: Compress-Archive -Path target/x86_64-pc-windows-msvc/release/devcontainer-mcp.exe -DestinationPath devcontainer-mcp-windows-x64.zip - - - name: Upload release asset - uses: softprops/action-gh-release@v3 - with: - files: devcontainer-mcp-windows-x64.zip diff --git a/SKILL.md b/SKILL.md index a9f8937..367de62 100644 --- a/SKILL.md +++ b/SKILL.md @@ -47,15 +47,6 @@ tools: - devpod_file_write - devpod_file_edit - devpod_file_list - - wsl_list - - wsl_exec - - wsl_terminate - - wsl_shutdown - - wsl_set_default - - wsl_file_read - - wsl_file_write - - wsl_file_edit - - wsl_file_list --- # DevContainer MCP Skill @@ -69,7 +60,7 @@ You have access to `devcontainer-mcp`, an MCP server that manages dev container **If a project has `.devcontainer/devcontainer.json`, ALL work MUST happen inside a dev container — never install dependencies, run builds, or execute code directly on the host.** -**Use ONLY the MCP tools listed here.** Do not invoke `docker`, `devcontainer`, `devpod`, `gh`, or `wsl` CLI commands directly — the MCP tools wrap these CLIs with proper error handling, auth resolution, and escaping. Direct CLI usage bypasses these safeguards. This applies even when the user asks to work "directly in WSL" or "not in a devcontainer" — use `wsl_exec` and WSL file tools instead of raw `wsl` commands. +**Use ONLY the MCP tools listed here.** Do not invoke `docker`, `devcontainer`, `devpod`, or `gh` CLI commands directly — the MCP tools wrap these CLIs with proper error handling, auth resolution, and escaping. Direct CLI usage bypasses these safeguards. ## Authentication @@ -175,51 +166,6 @@ codespaces_ssh(auth: "github-USERNAME", codespace: "codespace-name", command: "n codespaces_stop(auth: "github-USERNAME", codespace: "codespace-name") ``` -## Workflow: WSL (Windows only) - -> **Use these tools — not raw `wsl.exe` or PowerShell `wsl` commands.** When a user asks to work "in WSL" or "directly in WSL", use `wsl_exec` and the WSL file tools below — these ARE the way to work in WSL through MCP. - -WSL tools let you clone repos, build projects, and run commands inside any WSL distribution — without devcontainers or Docker. - -### 1. List available distributions -``` -wsl_list() -``` - -### 2. Clone and build a repo in WSL -``` -wsl_exec(distro: "Ubuntu", command: "git clone https://github.com/org/repo.git /home/user/repo") -wsl_exec(distro: "Ubuntu", command: "cd /home/user/repo && cargo build") -``` - -### 3. Execute any command inside a distribution -``` -wsl_exec(distro: "Ubuntu", command: "apt list --installed") -``` - -### 4. Set the default distribution -``` -wsl_set_default(distro: "Ubuntu") -``` - -### 5. Stop a distribution -``` -wsl_terminate(distro: "Ubuntu") -``` - -### 6. Shut down all WSL distributions -``` -wsl_shutdown() -``` - -### File operations in WSL -``` -wsl_file_read(distro: "Ubuntu", path: "/home/user/project/src/main.rs") -wsl_file_write(distro: "Ubuntu", path: "/home/user/file.txt", content: "fn main() {}") -wsl_file_edit(distro: "Ubuntu", path: "/home/user/file.txt", old_str: "fn main", new_str: "fn start") -wsl_file_list(distro: "Ubuntu", path: "/home/user/project") -``` - ## Self-Healing If `devpod_up`, `devcontainer_up`, or `codespaces_create` returns errors: @@ -233,7 +179,7 @@ If `devpod_up`, `devcontainer_up`, or `codespaces_create` returns errors: - ❌ Do NOT install packages on the host - ❌ Do NOT run builds on the host - ❌ Do NOT modify the host's global config -- ❌ Do NOT run `docker`, `devcontainer`, `devpod`, `gh`, or `wsl` CLI commands directly — use the MCP tools +- ❌ Do NOT run `docker`, `devcontainer`, `devpod`, or `gh` CLI commands directly — use the MCP tools - ✅ DO authenticate before using codespaces tools - ✅ DO ask the user which account/machine type to use - ✅ DO use `devpod_ssh`, `devcontainer_exec`, or `codespaces_ssh` for everything diff --git a/crates/devcontainer-mcp-core/src/cli.rs b/crates/devcontainer-mcp-core/src/cli.rs index 13b1457..cee82ec 100644 --- a/crates/devcontainer-mcp-core/src/cli.rs +++ b/crates/devcontainer-mcp-core/src/cli.rs @@ -29,9 +29,6 @@ pub enum CliBinary { Gcloud, /// Kubernetes CLI Kubectl, - #[cfg(target_os = "windows")] - /// Windows Subsystem for Linux - Wsl, } impl CliBinary { @@ -44,8 +41,6 @@ impl CliBinary { CliBinary::Aws => "aws", CliBinary::Gcloud => "gcloud", CliBinary::Kubectl => "kubectl", - #[cfg(target_os = "windows")] - CliBinary::Wsl => "wsl", } } @@ -58,8 +53,6 @@ impl CliBinary { CliBinary::Aws => Error::AwsCliNotFound, CliBinary::Gcloud => Error::GcloudCliNotFound, CliBinary::Kubectl => Error::KubectlNotFound, - #[cfg(target_os = "windows")] - CliBinary::Wsl => Error::WslNotFound, } } } diff --git a/crates/devcontainer-mcp-core/src/error.rs b/crates/devcontainer-mcp-core/src/error.rs index 549b840..ec3105c 100644 --- a/crates/devcontainer-mcp-core/src/error.rs +++ b/crates/devcontainer-mcp-core/src/error.rs @@ -27,10 +27,6 @@ pub enum Error { #[error("kubectl not found. Install from: https://kubernetes.io/docs/tasks/tools/")] KubectlNotFound, - #[cfg(target_os = "windows")] - #[error("WSL (wsl.exe) not found. WSL must be installed: https://learn.microsoft.com/en-us/windows/wsl/install")] - WslNotFound, - #[error("DevPod command failed (exit code {exit_code}): {stderr}")] DevPodCommand { exit_code: i32, stderr: String }, diff --git a/crates/devcontainer-mcp-core/src/lib.rs b/crates/devcontainer-mcp-core/src/lib.rs index 0177a10..3189d28 100644 --- a/crates/devcontainer-mcp-core/src/lib.rs +++ b/crates/devcontainer-mcp-core/src/lib.rs @@ -6,5 +6,3 @@ pub mod devpod; pub mod docker; pub mod error; pub mod file_ops; -#[cfg(target_os = "windows")] -pub mod wsl; diff --git a/crates/devcontainer-mcp-core/src/wsl.rs b/crates/devcontainer-mcp-core/src/wsl.rs deleted file mode 100644 index 735a051..0000000 --- a/crates/devcontainer-mcp-core/src/wsl.rs +++ /dev/null @@ -1,182 +0,0 @@ -//! WSL (Windows Subsystem for Linux) backend. -//! -//! Wraps the `wsl.exe` CLI to manage WSL distributions, execute commands, -//! and perform file operations inside Linux distros. - -use serde::Serialize; - -use crate::cli::{run_cli, CliBinary, CliOutput}; -use crate::error::{Error, Result}; - -/// A WSL distribution parsed from `wsl --list --verbose`. -#[derive(Debug, Clone, Serialize)] -pub struct WslDistro { - pub name: String, - pub state: String, - pub version: u8, - pub is_default: bool, -} - -/// Run a WSL CLI command with the given args. -async fn run_wsl(args: &[&str], parse_json: bool) -> Result { - run_cli(&CliBinary::Wsl, args, parse_json).await -} - -// --------------------------------------------------------------------------- -// Distribution management -// --------------------------------------------------------------------------- - -/// `wsl --list --verbose` — list installed distributions with state and version. -/// -/// Parses the tabular output into structured [`WslDistro`] entries and returns -/// them as a JSON array in `CliOutput::json`. -pub async fn list() -> Result { - let mut output = run_wsl(&["--list", "--verbose"], false).await?; - - if output.exit_code == 0 { - let distros = parse_list_output(&output.stdout); - output.json = Some(serde_json::to_value(&distros).unwrap_or_default()); - } - - Ok(output) -} - -/// Parse the tabular output of `wsl --list --verbose`. -/// -/// Example output: -/// ```text -/// NAME STATE VERSION -/// * Ubuntu Running 2 -/// Debian Stopped 2 -/// ``` -fn parse_list_output(stdout: &str) -> Vec { - let mut distros = Vec::new(); - - for line in stdout.lines().skip(1) { - let trimmed = line.trim(); - if trimmed.is_empty() { - continue; - } - - let is_default = trimmed.starts_with('*'); - let clean = trimmed.trim_start_matches('*').trim(); - - let parts: Vec<&str> = clean.split_whitespace().collect(); - if parts.len() >= 3 { - if let Ok(version) = parts[parts.len() - 1].parse::() { - let state = parts[parts.len() - 2].to_string(); - let name = parts[..parts.len() - 2].join(" "); - distros.push(WslDistro { - name, - state, - version, - is_default, - }); - } - } - } - - distros -} - -/// `wsl --set-default ` — set the default WSL distribution. -pub async fn set_default(distro: &str) -> Result { - run_wsl(&["--set-default", distro], false).await -} - -/// `wsl --terminate ` — stop a running distribution. -pub async fn terminate(distro: &str) -> Result { - run_wsl(&["--terminate", distro], false).await -} - -/// `wsl --shutdown` — shut down all running WSL distributions. -pub async fn shutdown() -> Result { - run_wsl(&["--shutdown"], false).await -} - -// --------------------------------------------------------------------------- -// Command execution -// --------------------------------------------------------------------------- - -/// `wsl -d -- sh -c ` — execute a command inside a distro. -pub async fn exec(distro: &str, command: &str) -> Result { - run_wsl(&["-d", distro, "--", "sh", "-c", command], false).await -} - -// --------------------------------------------------------------------------- -// File operations -// --------------------------------------------------------------------------- - -/// Read a file from a WSL distro. -pub async fn file_read(distro: &str, path: &str) -> Result { - let cmd = crate::file_ops::read_file_command(path); - exec(distro, &cmd).await -} - -/// Write (create or overwrite) a file in a WSL distro. -pub async fn file_write(distro: &str, path: &str, content: &str) -> Result { - let cmd = crate::file_ops::write_file_command(path, content); - exec(distro, &cmd).await -} - -/// Surgical edit: replace exactly one occurrence of `old_str` with `new_str`. -pub async fn file_edit(distro: &str, path: &str, old_str: &str, new_str: &str) -> Result { - let read_output = file_read(distro, path).await?; - if read_output.exit_code != 0 { - return Err(Error::FileRead(format!( - "Failed to read {path}: {}", - read_output.stderr.trim() - ))); - } - - let modified = crate::file_ops::apply_edit(&read_output.stdout, old_str, new_str)?; - - let write_output = file_write(distro, path, &modified).await?; - if write_output.exit_code != 0 { - return Err(Error::FileEdit(format!( - "Failed to write {path}: {}", - write_output.stderr.trim() - ))); - } - - Ok(format!("Edit applied to {path}")) -} - -/// List directory contents in a WSL distro. -pub async fn file_list(distro: &str, path: &str) -> Result { - let cmd = crate::file_ops::list_dir_command(path); - exec(distro, &cmd).await -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_parse_list_output_typical() { - let output = "\ - NAME STATE VERSION -* Ubuntu Running 2 - Debian Stopped 2 -"; - let distros = parse_list_output(output); - assert_eq!(distros.len(), 2); - - assert_eq!(distros[0].name, "Ubuntu"); - assert_eq!(distros[0].state, "Running"); - assert_eq!(distros[0].version, 2); - assert!(distros[0].is_default); - - assert_eq!(distros[1].name, "Debian"); - assert_eq!(distros[1].state, "Stopped"); - assert_eq!(distros[1].version, 2); - assert!(!distros[1].is_default); - } - - #[test] - fn test_parse_list_output_empty() { - let output = " NAME STATE VERSION\n"; - let distros = parse_list_output(output); - assert!(distros.is_empty()); - } -} diff --git a/crates/devcontainer-mcp/build.rs b/crates/devcontainer-mcp/build.rs index 256ba3f..1fc7a64 100644 --- a/crates/devcontainer-mcp/build.rs +++ b/crates/devcontainer-mcp/build.rs @@ -19,7 +19,6 @@ fn active_tags() -> HashSet { tags.insert("macos".to_string()); tags.insert("windows".to_string()); tags.insert("docker-desktop".to_string()); - tags.insert("wsl".to_string()); tags } diff --git a/crates/devcontainer-mcp/src/tools/mod.rs b/crates/devcontainer-mcp/src/tools/mod.rs index 4e6f4f1..54ed9f3 100644 --- a/crates/devcontainer-mcp/src/tools/mod.rs +++ b/crates/devcontainer-mcp/src/tools/mod.rs @@ -3,8 +3,6 @@ mod codespaces; pub mod common; mod devcontainer; mod devpod; -#[cfg(target_os = "windows")] -mod wsl; use rmcp::handler::server::router::tool::ToolRouter; use rmcp::model::{ServerCapabilities, ServerInfo}; @@ -19,13 +17,10 @@ impl DevContainerMcp { } fn combined_router() -> ToolRouter { - let r = Self::devpod_router() + Self::devpod_router() + Self::devcontainer_router() + Self::codespaces_router() - + Self::auth_router(); - #[cfg(target_os = "windows")] - let r = r + Self::wsl_router(); - r + + Self::auth_router() } } diff --git a/crates/devcontainer-mcp/src/tools/wsl/exec.rs b/crates/devcontainer-mcp/src/tools/wsl/exec.rs deleted file mode 100644 index 92f3a32..0000000 --- a/crates/devcontainer-mcp/src/tools/wsl/exec.rs +++ /dev/null @@ -1,27 +0,0 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; -use devcontainer_mcp_core::wsl; -use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; - -#[derive(serde::Deserialize, schemars::JsonSchema)] -struct WslExecParams { - #[schemars(description = "WSL distribution name")] - distro: String, - #[schemars(description = "Command to execute")] - command: String, -} - -#[tool_router(router = wsl_exec_router, vis = "pub(super)")] -impl DevContainerMcp { - #[tool( - name = "wsl_exec", - description = "Execute a command inside a WSL distribution. Returns stdout, stderr, and exit code." - )] - async fn wsl_exec(&self, Parameters(params): Parameters) -> String { - match wsl::exec(¶ms.distro, ¶ms.command).await { - Ok(output) => format_output(&output), - Err(e) => format!("Error: {e}"), - } - } -} diff --git a/crates/devcontainer-mcp/src/tools/wsl/files.rs b/crates/devcontainer-mcp/src/tools/wsl/files.rs deleted file mode 100644 index 25376fa..0000000 --- a/crates/devcontainer-mcp/src/tools/wsl/files.rs +++ /dev/null @@ -1,138 +0,0 @@ -use crate::tools::DevContainerMcp; -use devcontainer_mcp_core::{file_ops, wsl}; -use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; - -#[derive(serde::Deserialize, schemars::JsonSchema)] -struct WslFileReadParams { - #[schemars(description = "WSL distribution name")] - distro: String, - #[schemars(description = "Path to the file inside the distribution")] - path: String, - #[serde(default)] - #[schemars(description = "Start line number (1-based, inclusive)")] - start_line: Option, - #[serde(default)] - #[schemars( - description = "End line number (1-based, inclusive). Use -1 or omit for end of file" - )] - end_line: Option, -} - -#[derive(serde::Deserialize, schemars::JsonSchema)] -struct WslFileWriteParams { - #[schemars(description = "WSL distribution name")] - distro: String, - #[schemars(description = "Path to the file inside the distribution")] - path: String, - #[schemars(description = "File content to write")] - content: String, -} - -#[derive(serde::Deserialize, schemars::JsonSchema)] -struct WslFileEditParams { - #[schemars(description = "WSL distribution name")] - distro: String, - #[schemars(description = "Path to the file inside the distribution")] - path: String, - #[schemars(description = "The exact string in the file to replace. Must match exactly once.")] - old_str: String, - #[schemars(description = "The new string to replace old_str with")] - new_str: String, -} - -#[derive(serde::Deserialize, schemars::JsonSchema)] -struct WslFileListParams { - #[schemars(description = "WSL distribution name")] - distro: String, - #[serde(default)] - #[schemars(description = "Path to the directory inside the distribution (defaults to '.')")] - path: Option, -} - -#[tool_router(router = wsl_files_router, vis = "pub(super)")] -impl DevContainerMcp { - #[tool( - name = "wsl_file_read", - description = "Read file content from a WSL distribution. Returns content with line numbers. Supports optional line range." - )] - async fn wsl_file_read(&self, Parameters(params): Parameters) -> String { - match wsl::file_read(¶ms.distro, ¶ms.path).await { - Ok(output) => { - if output.exit_code != 0 { - return format!( - "Error (exit {}): {}", - output.exit_code, - output.stderr.trim() - ); - } - let end = params - .end_line - .and_then(|e| if e < 0 { None } else { Some(e as usize) }); - file_ops::format_with_line_numbers(&output.stdout, params.start_line, end) - } - Err(e) => format!("Error: {e}"), - } - } - - #[tool( - name = "wsl_file_write", - description = "Create or overwrite a file in a WSL distribution. Creates parent directories automatically." - )] - async fn wsl_file_write(&self, Parameters(params): Parameters) -> String { - match wsl::file_write(¶ms.distro, ¶ms.path, ¶ms.content).await { - Ok(output) => { - if output.exit_code != 0 { - format!( - "Error (exit {}): {}", - output.exit_code, - output.stderr.trim() - ) - } else { - format!("File written: {}", params.path) - } - } - Err(e) => format!("Error: {e}"), - } - } - - #[tool( - name = "wsl_file_edit", - description = "Make a surgical edit to a file in a WSL distribution. Replaces exactly one occurrence of old_str with new_str. The old_str must match exactly one location in the file — include enough surrounding context to make it unique." - )] - async fn wsl_file_edit(&self, Parameters(params): Parameters) -> String { - match wsl::file_edit( - ¶ms.distro, - ¶ms.path, - ¶ms.old_str, - ¶ms.new_str, - ) - .await - { - Ok(msg) => msg, - Err(e) => format!("Error: {e}"), - } - } - - #[tool( - name = "wsl_file_list", - description = "List directory contents in a WSL distribution. Shows non-hidden files up to 2 levels deep." - )] - async fn wsl_file_list(&self, Parameters(params): Parameters) -> String { - let dir = params.path.as_deref().unwrap_or("."); - match wsl::file_list(¶ms.distro, dir).await { - Ok(output) => { - if output.exit_code != 0 { - format!( - "Error (exit {}): {}", - output.exit_code, - output.stderr.trim() - ) - } else { - output.stdout - } - } - Err(e) => format!("Error: {e}"), - } - } -} diff --git a/crates/devcontainer-mcp/src/tools/wsl/list.rs b/crates/devcontainer-mcp/src/tools/wsl/list.rs deleted file mode 100644 index c7a28d3..0000000 --- a/crates/devcontainer-mcp/src/tools/wsl/list.rs +++ /dev/null @@ -1,18 +0,0 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; -use devcontainer_mcp_core::wsl; -use rmcp::{tool, tool_router}; - -#[tool_router(router = wsl_list_router, vis = "pub(super)")] -impl DevContainerMcp { - #[tool( - name = "wsl_list", - description = "List WSL distributions with their state and version." - )] - async fn wsl_list(&self) -> String { - match wsl::list().await { - Ok(output) => format_output(&output), - Err(e) => format!("Error: {e}"), - } - } -} diff --git a/crates/devcontainer-mcp/src/tools/wsl/mod.rs b/crates/devcontainer-mcp/src/tools/wsl/mod.rs deleted file mode 100644 index c3f3f60..0000000 --- a/crates/devcontainer-mcp/src/tools/wsl/mod.rs +++ /dev/null @@ -1,21 +0,0 @@ -mod exec; -mod files; -mod list; -mod set_default; -mod shutdown; -mod stop; - -use rmcp::handler::server::router::tool::ToolRouter; - -use super::DevContainerMcp; - -impl DevContainerMcp { - pub(super) fn wsl_router() -> ToolRouter { - Self::wsl_list_router() - + Self::wsl_exec_router() - + Self::wsl_stop_router() - + Self::wsl_shutdown_router() - + Self::wsl_set_default_router() - + Self::wsl_files_router() - } -} diff --git a/crates/devcontainer-mcp/src/tools/wsl/set_default.rs b/crates/devcontainer-mcp/src/tools/wsl/set_default.rs deleted file mode 100644 index e407643..0000000 --- a/crates/devcontainer-mcp/src/tools/wsl/set_default.rs +++ /dev/null @@ -1,25 +0,0 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; -use devcontainer_mcp_core::wsl; -use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; - -#[derive(serde::Deserialize, schemars::JsonSchema)] -struct WslSetDefaultParams { - #[schemars(description = "WSL distribution name to set as default")] - distro: String, -} - -#[tool_router(router = wsl_set_default_router, vis = "pub(super)")] -impl DevContainerMcp { - #[tool( - name = "wsl_set_default", - description = "Set the default WSL distribution." - )] - async fn wsl_set_default(&self, Parameters(params): Parameters) -> String { - match wsl::set_default(¶ms.distro).await { - Ok(output) => format_output(&output), - Err(e) => format!("Error: {e}"), - } - } -} diff --git a/crates/devcontainer-mcp/src/tools/wsl/shutdown.rs b/crates/devcontainer-mcp/src/tools/wsl/shutdown.rs deleted file mode 100644 index 5266870..0000000 --- a/crates/devcontainer-mcp/src/tools/wsl/shutdown.rs +++ /dev/null @@ -1,18 +0,0 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; -use devcontainer_mcp_core::wsl; -use rmcp::{tool, tool_router}; - -#[tool_router(router = wsl_shutdown_router, vis = "pub(super)")] -impl DevContainerMcp { - #[tool( - name = "wsl_shutdown", - description = "Shut down all running WSL distributions." - )] - async fn wsl_shutdown(&self) -> String { - match wsl::shutdown().await { - Ok(output) => format_output(&output), - Err(e) => format!("Error: {e}"), - } - } -} diff --git a/crates/devcontainer-mcp/src/tools/wsl/stop.rs b/crates/devcontainer-mcp/src/tools/wsl/stop.rs deleted file mode 100644 index f275f55..0000000 --- a/crates/devcontainer-mcp/src/tools/wsl/stop.rs +++ /dev/null @@ -1,25 +0,0 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; -use devcontainer_mcp_core::wsl; -use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; - -#[derive(serde::Deserialize, schemars::JsonSchema)] -struct WslTerminateParams { - #[schemars(description = "WSL distribution name to terminate")] - distro: String, -} - -#[tool_router(router = wsl_stop_router, vis = "pub(super)")] -impl DevContainerMcp { - #[tool( - name = "wsl_terminate", - description = "Terminate (stop) a running WSL distribution." - )] - async fn wsl_terminate(&self, Parameters(params): Parameters) -> String { - match wsl::terminate(¶ms.distro).await { - Ok(output) => format_output(&output), - Err(e) => format!("Error: {e}"), - } - } -} diff --git a/skills/_tools/wsl.txt b/skills/_tools/wsl.txt deleted file mode 100644 index 279ee46..0000000 --- a/skills/_tools/wsl.txt +++ /dev/null @@ -1,12 +0,0 @@ ---- -tags: [wsl] ---- -wsl_list -wsl_exec -wsl_terminate -wsl_shutdown -wsl_set_default -wsl_file_read -wsl_file_write -wsl_file_edit -wsl_file_list diff --git a/skills/core-rule.md b/skills/core-rule.md index e08986d..bec14b5 100644 --- a/skills/core-rule.md +++ b/skills/core-rule.md @@ -6,4 +6,4 @@ order: 20 **If a project has `.devcontainer/devcontainer.json`, ALL work MUST happen inside a dev container — never install dependencies, run builds, or execute code directly on the host.** -**Use ONLY the MCP tools listed here.** Do not invoke `docker`, `devcontainer`, `devpod`, `gh`, or `wsl` CLI commands directly — the MCP tools wrap these CLIs with proper error handling, auth resolution, and escaping. Direct CLI usage bypasses these safeguards. This applies even when the user asks to work "directly in WSL" or "not in a devcontainer" — use `wsl_exec` and WSL file tools instead of raw `wsl` commands. +**Use ONLY the MCP tools listed here.** Do not invoke `docker`, `devcontainer`, `devpod`, or `gh` CLI commands directly — the MCP tools wrap these CLIs with proper error handling, auth resolution, and escaping. Direct CLI usage bypasses these safeguards. diff --git a/skills/footer.md b/skills/footer.md index a14a9b5..285c808 100644 --- a/skills/footer.md +++ b/skills/footer.md @@ -7,7 +7,7 @@ order: 90 - ❌ Do NOT install packages on the host - ❌ Do NOT run builds on the host - ❌ Do NOT modify the host's global config -- ❌ Do NOT run `docker`, `devcontainer`, `devpod`, `gh`, or `wsl` CLI commands directly — use the MCP tools +- ❌ Do NOT run `docker`, `devcontainer`, `devpod`, or `gh` CLI commands directly — use the MCP tools - ✅ DO authenticate before using codespaces tools - ✅ DO ask the user which account/machine type to use - ✅ DO use `devpod_ssh`, `devcontainer_exec`, or `codespaces_ssh` for everything diff --git a/skills/wsl.md b/skills/wsl.md deleted file mode 100644 index 4329fd3..0000000 --- a/skills/wsl.md +++ /dev/null @@ -1,48 +0,0 @@ ---- -tags: [wsl] -order: 75 ---- -## Workflow: WSL (Windows only) - -> **Use these tools — not raw `wsl.exe` or PowerShell `wsl` commands.** When a user asks to work "in WSL" or "directly in WSL", use `wsl_exec` and the WSL file tools below — these ARE the way to work in WSL through MCP. - -WSL tools let you clone repos, build projects, and run commands inside any WSL distribution — without devcontainers or Docker. - -### 1. List available distributions -``` -wsl_list() -``` - -### 2. Clone and build a repo in WSL -``` -wsl_exec(distro: "Ubuntu", command: "git clone https://github.com/org/repo.git /home/user/repo") -wsl_exec(distro: "Ubuntu", command: "cd /home/user/repo && cargo build") -``` - -### 3. Execute any command inside a distribution -``` -wsl_exec(distro: "Ubuntu", command: "apt list --installed") -``` - -### 4. Set the default distribution -``` -wsl_set_default(distro: "Ubuntu") -``` - -### 5. Stop a distribution -``` -wsl_terminate(distro: "Ubuntu") -``` - -### 6. Shut down all WSL distributions -``` -wsl_shutdown() -``` - -### File operations in WSL -``` -wsl_file_read(distro: "Ubuntu", path: "/home/user/project/src/main.rs") -wsl_file_write(distro: "Ubuntu", path: "/home/user/file.txt", content: "fn main() {}") -wsl_file_edit(distro: "Ubuntu", path: "/home/user/file.txt", old_str: "fn main", new_str: "fn start") -wsl_file_list(distro: "Ubuntu", path: "/home/user/project") -``` From 811c613f51cad0eae9d9864a480b40666357e473 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 9 May 2026 20:19:39 -0700 Subject: [PATCH 03/19] Enhance README with demo GIFs, client compatibility, and hardware scaling point --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index f1931ac..063d10e 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,12 @@ `devcontainer-mcp` is an MCP server that lets AI coding agents create, manage, and work inside [dev containers](https://containers.dev/) across three backends: local Docker, [DevPod](https://devpod.sh/), and [GitHub Codespaces](https://github.com/features/codespaces). The agent builds, tests, and ships code in an isolated container — your laptop stays clean. +

+ devcontainer-mcp local Docker demo +

+ +> Works with **GitHub Copilot**, **Claude**, **Cursor**, and any MCP-compatible client. + ## The Problem When AI agents write code, they need to run it somewhere. Today that means your host machine: @@ -14,6 +20,7 @@ When AI agents write code, they need to run it somewhere. Today that means your - 🔴 **"Works on my machine"** — agents assume your local toolchain matches production - 🔴 **No isolation** — one project's dependencies break another - 🔴 **Security risk** — agents run arbitrary commands with your user privileges +- 🔴 **Hardware constraints** — you're limited to your local machine's resources ## The Solution @@ -36,6 +43,10 @@ Agent: "Let me build this project..." ## Quick Install +

+ devcontainer-mcp install demo +

+ ### Linux / macOS ```bash @@ -79,6 +90,10 @@ graph TD ## Three Backends, One Interface +

+ devcontainer-mcp Codespaces demo +

+ | Backend | Best for | Requires | Auth needed? | |---------|----------|----------|:---:| | **devcontainer CLI** (`devcontainer_*`) | Local Docker — fast, simple | [@devcontainers/cli](https://github.com/devcontainers/cli) + Docker | No | From 12054c5ecf25c082c46cf073eb94d79cabb2ec1d Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 9 May 2026 20:23:55 -0700 Subject: [PATCH 04/19] Add website badge to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 063d10e..88638ea 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # devcontainer-mcp [![CI](https://github.com/aniongithub/devcontainer-mcp/actions/workflows/ci.yml/badge.svg)](https://github.com/aniongithub/devcontainer-mcp/actions/workflows/ci.yml) +[![Website](https://img.shields.io/badge/website-anonline.me-blue)](https://www.anonline.me/devcontainer-mcp) **Give your AI agent its own dev environment — not yours.** From a140bfdd23c439cd954ea053ea8c82f30ebad721 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 9 May 2026 20:25:36 -0700 Subject: [PATCH 05/19] Fix website badge text to show devcontainer-mcp --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 88638ea..b7f12fd 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # devcontainer-mcp [![CI](https://github.com/aniongithub/devcontainer-mcp/actions/workflows/ci.yml/badge.svg)](https://github.com/aniongithub/devcontainer-mcp/actions/workflows/ci.yml) -[![Website](https://img.shields.io/badge/website-anonline.me-blue)](https://www.anonline.me/devcontainer-mcp) +[![Website](https://img.shields.io/badge/website-devcontainer--mcp-blue)](https://www.anonline.me/devcontainer-mcp) **Give your AI agent its own dev environment — not yours.** From fec91c3886f4362884755444efbdb0f8ed1177af Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 9 May 2026 20:26:56 -0700 Subject: [PATCH 06/19] Fix website URL typo: anonline -> anionline --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b7f12fd..8c63ed7 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # devcontainer-mcp [![CI](https://github.com/aniongithub/devcontainer-mcp/actions/workflows/ci.yml/badge.svg)](https://github.com/aniongithub/devcontainer-mcp/actions/workflows/ci.yml) -[![Website](https://img.shields.io/badge/website-devcontainer--mcp-blue)](https://www.anonline.me/devcontainer-mcp) +[![Website](https://img.shields.io/badge/website-devcontainer--mcp-blue)](https://www.anionline.me/devcontainer-mcp) **Give your AI agent its own dev environment — not yours.** From 0f803e2ee407f9b7f2240e086eaadca2a2d1d91b Mon Sep 17 00:00:00 2001 From: Ani Date: Mon, 11 May 2026 06:54:32 +0000 Subject: [PATCH 07/19] Add host-protection hooks for Claude Code and Copilot CLI PreToolUse hooks that block bash/shell tool calls when a .devcontainer/devcontainer.json exists, forcing agents to use devcontainer-mcp MCP tools instead of executing on the host. - hooks/devcontainer-guard.sh: shared hook script supporting both Claude Code and Copilot CLI payload formats - .github/hooks/devcontainer-guard.json: Copilot CLI hook config for this repo - install.sh/install.ps1: auto-install hooks and configure agent environments during installation - 10 integration tests validating block/allow/bypass behavior - SKILL.md footer updated to mention hook enforcement Bypass: USER_CONFIRMED_HOST_OPERATION=1 in the command allows through, designed as a semantic tripwire that LLMs cannot honestly generate. --- .github/hooks/devcontainer-guard.json | 12 ++ SKILL.md | 2 + crates/devcontainer-mcp-core/src/file_ops.rs | 5 +- .../tests/hook_guard_test.rs | 172 ++++++++++++++++++ hooks/devcontainer-guard.sh | 72 ++++++++ install.ps1 | 92 ++++++++++ install.sh | 110 +++++++++++ skills/footer.md | 2 + 8 files changed, 463 insertions(+), 4 deletions(-) create mode 100644 .github/hooks/devcontainer-guard.json create mode 100644 crates/devcontainer-mcp-core/tests/hook_guard_test.rs create mode 100755 hooks/devcontainer-guard.sh diff --git a/.github/hooks/devcontainer-guard.json b/.github/hooks/devcontainer-guard.json new file mode 100644 index 0000000..24c6fa2 --- /dev/null +++ b/.github/hooks/devcontainer-guard.json @@ -0,0 +1,12 @@ +{ + "version": 1, + "hooks": { + "preToolUse": [ + { + "type": "command", + "bash": "./hooks/devcontainer-guard.sh", + "timeoutSec": 5 + } + ] + } +} diff --git a/SKILL.md b/SKILL.md index 367de62..9e09880 100644 --- a/SKILL.md +++ b/SKILL.md @@ -185,6 +185,8 @@ If `devpod_up`, `devcontainer_up`, or `codespaces_create` returns errors: - ✅ DO use `devpod_ssh`, `devcontainer_exec`, or `codespaces_ssh` for everything - ✅ DO check `.devcontainer/devcontainer.json` first +> **Note:** Host-protection hooks are installed for supported agent environments (Claude Code, GitHub Copilot CLI) that automatically block shell commands when a devcontainer is detected. If a command is blocked, use the appropriate MCP tool instead. + ## File Operations **All backends support built-in file operations — no need to construct shell commands.** diff --git a/crates/devcontainer-mcp-core/src/file_ops.rs b/crates/devcontainer-mcp-core/src/file_ops.rs index 0efaaab..c79ccee 100644 --- a/crates/devcontainer-mcp-core/src/file_ops.rs +++ b/crates/devcontainer-mcp-core/src/file_ops.rs @@ -75,10 +75,7 @@ pub fn write_file_command(path: &str, content: &str) -> String { /// Build a shell command that lists a directory (non-hidden, up to 2 levels). pub fn list_dir_command(path: &str) -> String { - format!( - "find {} -maxdepth 2 -not -path '*/.*' | sort", - quote(path) - ) + format!("find {} -maxdepth 2 -not -path '*/.*' | sort", quote(path)) } #[cfg(test)] diff --git a/crates/devcontainer-mcp-core/tests/hook_guard_test.rs b/crates/devcontainer-mcp-core/tests/hook_guard_test.rs new file mode 100644 index 0000000..cb30138 --- /dev/null +++ b/crates/devcontainer-mcp-core/tests/hook_guard_test.rs @@ -0,0 +1,172 @@ +//! Integration tests for the devcontainer-guard.sh hook script. +//! +//! These tests verify that the hook correctly blocks/allows tool calls +//! based on the agent format, tool name, devcontainer presence, and bypass. + +use std::process::Command; + +fn repo_root() -> std::path::PathBuf { + let manifest = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")); + manifest + .parent() + .and_then(|p| p.parent()) + .expect("could not find repo root") + .to_path_buf() +} + +fn hook_path() -> std::path::PathBuf { + repo_root().join("hooks/devcontainer-guard.sh") +} + +/// Run the hook script with the given JSON input and return (stdout, exit_code). +fn run_hook(json_input: &str) -> (String, i32) { + let output = Command::new("bash") + .arg(hook_path()) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .and_then(|mut child| { + use std::io::Write; + child + .stdin + .take() + .unwrap() + .write_all(json_input.as_bytes()) + .unwrap(); + child.wait_with_output() + }) + .expect("failed to run hook script"); + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let code = output.status.code().unwrap_or(-1); + (stdout, code) +} + +fn cwd_with_devcontainer() -> String { + repo_root().to_string_lossy().to_string() +} + +// ----------------------------------------------------------------------- +// Copilot CLI format +// ----------------------------------------------------------------------- + +#[test] +fn copilot_cli_bash_with_devcontainer_denies() { + let input = format!( + r#"{{"toolName":"bash","toolArgs":{{"command":"cargo build"}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, code) = run_hook(&input); + assert_eq!(code, 0); + assert!( + stdout.contains(r#""permissionDecision":"deny""#) + || stdout.contains(r#""permissionDecision": "deny""#) + ); + // Copilot CLI format should NOT have hookSpecificOutput + assert!(!stdout.contains("hookSpecificOutput")); +} + +#[test] +fn copilot_cli_shell_with_devcontainer_denies() { + let input = format!( + r#"{{"toolName":"shell","toolArgs":{{"command":"make"}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, _) = run_hook(&input); + assert!(stdout.contains("deny")); +} + +#[test] +fn copilot_cli_powershell_with_devcontainer_denies() { + let input = format!( + r#"{{"toolName":"powershell","toolArgs":{{"command":"dir"}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, _) = run_hook(&input); + assert!(stdout.contains("deny")); +} + +#[test] +fn copilot_cli_view_tool_allows() { + let input = format!( + r#"{{"toolName":"view","toolArgs":{{"path":"src/main.rs"}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, code) = run_hook(&input); + assert_eq!(code, 0); + assert!(stdout.is_empty(), "non-bash tool should produce no output"); +} + +#[test] +fn copilot_cli_no_devcontainer_allows() { + let input = r#"{"toolName":"bash","toolArgs":{"command":"ls"},"cwd":"/tmp"}"#; + let (stdout, code) = run_hook(input); + assert_eq!(code, 0); + assert!(stdout.is_empty()); +} + +// ----------------------------------------------------------------------- +// Claude Code format +// ----------------------------------------------------------------------- + +#[test] +fn claude_code_bash_with_devcontainer_denies() { + let input = format!( + r#"{{"tool_name":"Bash","tool_input":{{"command":"npm install"}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, code) = run_hook(&input); + assert_eq!(code, 0); + assert!(stdout.contains("hookSpecificOutput")); + assert!(stdout.contains("deny")); +} + +#[test] +fn claude_code_edit_tool_allows() { + let input = format!( + r#"{{"tool_name":"Edit","tool_input":{{"path":"src/main.rs"}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, code) = run_hook(&input); + assert_eq!(code, 0); + assert!(stdout.is_empty()); +} + +// ----------------------------------------------------------------------- +// Bypass +// ----------------------------------------------------------------------- + +#[test] +fn bypass_string_allows_through() { + let input = format!( + r#"{{"tool_name":"Bash","tool_input":{{"command":"USER_CONFIRMED_HOST_OPERATION=1 cargo build"}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, code) = run_hook(&input); + assert_eq!(code, 0); + assert!(stdout.is_empty(), "bypass should produce no output"); +} + +// ----------------------------------------------------------------------- +// Edge cases +// ----------------------------------------------------------------------- + +#[test] +fn missing_cwd_allows() { + let input = r#"{"toolName":"bash","toolArgs":{"command":"ls"}}"#; + let (stdout, code) = run_hook(input); + assert_eq!(code, 0); + assert!(stdout.is_empty()); +} + +#[test] +fn empty_tool_name_allows() { + let input = format!( + r#"{{"toolName":"","toolArgs":{{}},"cwd":"{}"}}"#, + cwd_with_devcontainer() + ); + let (stdout, code) = run_hook(&input); + assert_eq!(code, 0); + assert!(stdout.is_empty()); +} diff --git a/hooks/devcontainer-guard.sh b/hooks/devcontainer-guard.sh new file mode 100755 index 0000000..fb78b19 --- /dev/null +++ b/hooks/devcontainer-guard.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# devcontainer-guard.sh — PreToolUse hook for Claude Code & GitHub Copilot CLI +# +# Blocks bash/shell tool calls when .devcontainer/devcontainer.json exists in +# the working directory, forcing agents to use devcontainer-mcp MCP tools +# instead of running commands directly on the host. +# +# Read-only tools (view, grep, glob) and file edits are allowed through — only +# command execution is blocked. +# +# Bypass: include USER_CONFIRMED_HOST_OPERATION=1 in the command. +# +# Supports both agent payload formats: +# Claude Code: { tool_name, tool_input, cwd, ... } +# Copilot CLI: { toolName, toolArgs, cwd, ... } + +set -euo pipefail + +INPUT=$(cat) + +# --- Detect agent format and extract fields --- + +# Try Claude Code fields first (snake_case), fall back to Copilot CLI (camelCase) +TOOL_NAME=$(echo "$INPUT" | jq -r '.tool_name // .toolName // empty') +CWD=$(echo "$INPUT" | jq -r '.cwd // empty') + +# Only guard bash/shell tool calls — allow everything else through +case "$TOOL_NAME" in + Bash|bash|shell|powershell|Shell|PowerShell) ;; + *) exit 0 ;; +esac + +TOOL_INPUT=$(echo "$INPUT" | jq -r '(.tool_input // .toolArgs // {}) | tostring') + +# Check for the bypass string anywhere in the tool input +if echo "$TOOL_INPUT" | grep -q 'USER_CONFIRMED_HOST_OPERATION=1'; then + exit 0 +fi + +# Check if a devcontainer exists in the working directory +if [ -z "$CWD" ]; then + # No cwd in payload — can't determine context, allow through + exit 0 +fi + +if [ ! -f "${CWD}/.devcontainer/devcontainer.json" ]; then + # No devcontainer — allow through + exit 0 +fi + +# --- Devcontainer exists: block the tool call --- + +DENY_REASON="Host execution blocked. This project has a devcontainer. Use devcontainer-mcp tools (devcontainer_exec, devpod_ssh, codespaces_ssh, and file operation tools) instead of running commands directly on the host." + +# Detect which agent format to use for the response +if echo "$INPUT" | jq -e '.tool_name // empty' >/dev/null 2>&1 && \ + [ -n "$(echo "$INPUT" | jq -r '.tool_name // empty')" ]; then + # Claude Code format + jq -n --arg reason "$DENY_REASON" '{ + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: $reason + } + }' +else + # Copilot CLI format + jq -n --arg reason "$DENY_REASON" '{ + permissionDecision: "deny", + permissionDecisionReason: $reason + }' +fi diff --git a/install.ps1 b/install.ps1 index 9f44397..8e61b7c 100644 --- a/install.ps1 +++ b/install.ps1 @@ -123,6 +123,98 @@ foreach ($dir in $skillDirs) { } } +# --------------------------------------------------------------------------- +# 3b. Install host-protection hooks +# --------------------------------------------------------------------------- + +Write-Step "Installing host-protection hook..." + +$hookUrl = "https://raw.githubusercontent.com/$Repo/main/hooks/devcontainer-guard.sh" +$WslHookDir = "~/.local/share/devcontainer-mcp/hooks" +$WslHookPath = "$WslHookDir/devcontainer-guard.sh" + +$hookResult = wsl -d $WslDistro bash -c "mkdir -p '$WslHookDir' && curl -fsSL -o '$WslHookPath' '$hookUrl' && chmod +x '$WslHookPath' && echo OK" 2>&1 +if ($hookResult -match "OK") { + Write-Ok "Hook script installed in WSL at $WslHookPath" +} else { + Write-Warn "Could not install hook script in WSL" +} + +# Configure Claude Code PreToolUse hook (Windows-side) +Write-Step "Configuring agent host-protection hooks..." + +$claudeSettings = "$env:USERPROFILE\.claude\settings.json" +try { + $hookEntry = @{ + matcher = "Bash" + hooks = @( + @{ + type = "command" + command = "wsl $WslHookPath" + timeout = 5 + } + ) + } + + if (Test-Path $claudeSettings) { + $content = Get-Content -Raw $claudeSettings | ConvertFrom-Json + if (-not $content.hooks) { + $content | Add-Member -NotePropertyName "hooks" -NotePropertyValue ([PSCustomObject]@{}) + } + if (-not $content.hooks.PreToolUse) { + $content.hooks | Add-Member -NotePropertyName "PreToolUse" -NotePropertyValue @() + } + # Check if already configured + $already = $false + foreach ($group in $content.hooks.PreToolUse) { + foreach ($h in $group.hooks) { + if ($h.command -match "devcontainer-guard") { $already = $true; break } + } + } + if (-not $already) { + $content.hooks.PreToolUse += [PSCustomObject]$hookEntry + $content | ConvertTo-Json -Depth 10 | Set-Content $claudeSettings -Encoding UTF8 + Write-Ok "Claude Code — added hook to $claudeSettings" + } else { + Write-Ok "Claude Code — hook already configured" + } + } else { + $dir = Split-Path $claudeSettings -Parent + if ($dir) { New-Item -ItemType Directory -Path $dir -Force | Out-Null } + $config = [PSCustomObject]@{ + hooks = [PSCustomObject]@{ + PreToolUse = @([PSCustomObject]$hookEntry) + } + } + $config | ConvertTo-Json -Depth 10 | Set-Content $claudeSettings -Encoding UTF8 + Write-Ok "Claude Code — created $claudeSettings" + } +} catch { + Write-Warn "Claude Code — could not configure hooks" +} + +# Configure Copilot CLI preToolUse hook (Windows-side) +$copilotHooksDir = "$env:USERPROFILE\.copilot\hooks" +try { + New-Item -ItemType Directory -Path $copilotHooksDir -Force | Out-Null + $copilotHook = [PSCustomObject]@{ + version = 1 + hooks = [PSCustomObject]@{ + preToolUse = @( + [PSCustomObject]@{ + type = "command" + bash = "wsl $WslHookPath" + timeoutSec = 5 + } + ) + } + } + $copilotHook | ConvertTo-Json -Depth 10 | Set-Content "$copilotHooksDir\devcontainer-guard.json" -Encoding UTF8 + Write-Ok "Copilot CLI — created $copilotHooksDir\devcontainer-guard.json" +} catch { + Write-Warn "Copilot CLI — could not configure hooks" +} + # --------------------------------------------------------------------------- # 4. Detect backend CLIs available in WSL # --------------------------------------------------------------------------- diff --git a/install.sh b/install.sh index b3db7cf..13d07ce 100755 --- a/install.sh +++ b/install.sh @@ -116,6 +116,116 @@ for dir in "${SKILL_DIRS[@]}"; do echo " ${dir}/SKILL.md" || true done +# --------------------------------------------------------------------------- +# Install host-protection hooks +# --------------------------------------------------------------------------- + +HOOK_URL="https://raw.githubusercontent.com/${REPO}/main/hooks/devcontainer-guard.sh" +HOOK_DIR="${HOME}/.local/share/devcontainer-mcp/hooks" +HOOK_PATH="${HOOK_DIR}/devcontainer-guard.sh" + +echo "" +echo "==> Installing host-protection hook..." +mkdir -p "$HOOK_DIR" +curl -fsSL -o "$HOOK_PATH" "$HOOK_URL" 2>/dev/null && \ + chmod +x "$HOOK_PATH" && \ + echo " ${HOOK_PATH}" || echo " ⚠ Could not download hook script" + +# Configure Claude Code PreToolUse hook +configure_claude_hook() { + local settings_file="${HOME}/.claude/settings.json" + local hook_command="$HOOK_PATH" + + if [ ! -f "$settings_file" ]; then + mkdir -p "$(dirname "$settings_file")" + cat > "$settings_file" << CLAUDEEOF +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "${hook_command}", + "timeout": 5 + } + ] + } + ] + } +} +CLAUDEEOF + echo " ✓ Claude Code — created ${settings_file}" + elif command -v python3 >/dev/null 2>&1; then + python3 -c " +import json, sys + +path = '${settings_file}' +hook_cmd = '${hook_command}' + +with open(path) as f: + data = json.load(f) + +hooks = data.setdefault('hooks', {}) +pre_tool = hooks.setdefault('PreToolUse', []) + +# Check if devcontainer-guard is already configured +already = False +for group in pre_tool: + for h in group.get('hooks', []): + if 'devcontainer-guard' in h.get('command', ''): + already = True + break + +if not already: + pre_tool.append({ + 'matcher': 'Bash', + 'hooks': [{ + 'type': 'command', + 'command': hook_cmd, + 'timeout': 5 + }] + }) + with open(path, 'w') as f: + json.dump(data, f, indent=2) + print(' ✓ Claude Code — added hook to ${settings_file}') +else: + print(' ✓ Claude Code — hook already configured') +" 2>/dev/null || echo " ⚠ Claude Code — could not update ${settings_file}" + else + echo " ⚠ Claude Code — exists but python3 not available to merge" + fi +} + +# Configure Copilot CLI preToolUse hook +configure_copilot_hook() { + local hooks_dir="${HOME}/.copilot/hooks" + local hooks_file="${hooks_dir}/devcontainer-guard.json" + + mkdir -p "$hooks_dir" + cat > "$hooks_file" << COPILOTEOF +{ + "version": 1, + "hooks": { + "preToolUse": [ + { + "type": "command", + "bash": "${HOOK_PATH}", + "timeoutSec": 5 + } + ] + } +} +COPILOTEOF + echo " ✓ Copilot CLI — created ${hooks_file}" +} + +echo "" +echo "==> Configuring agent host-protection hooks..." +configure_claude_hook +configure_copilot_hook + # Detect available backends echo "" echo "Backend CLIs detected (install as needed — MCP server gives helpful errors if missing):" diff --git a/skills/footer.md b/skills/footer.md index 285c808..24a0570 100644 --- a/skills/footer.md +++ b/skills/footer.md @@ -12,3 +12,5 @@ order: 90 - ✅ DO ask the user which account/machine type to use - ✅ DO use `devpod_ssh`, `devcontainer_exec`, or `codespaces_ssh` for everything - ✅ DO check `.devcontainer/devcontainer.json` first + +> **Note:** Host-protection hooks are installed for supported agent environments (Claude Code, GitHub Copilot CLI) that automatically block shell commands when a devcontainer is detected. If a command is blocked, use the appropriate MCP tool instead. From da4fe596706c51d119a21a219ae2a1ca4e819f0c Mon Sep 17 00:00:00 2001 From: Ani Date: Mon, 11 May 2026 07:27:38 +0000 Subject: [PATCH 08/19] Replace split_whitespace with shlex for shell-aware argument parsing - Replace shell-escape with shlex crate for both quoting and splitting - devcontainer_exec: always wrap in sh -c so agents can pass commands naturally (e.g. "cargo build", "cd /foo && npm test") without worrying about argument splitting - 5 other tools: use shlex::split() for CLI flag args to properly handle quoted values - file_ops: use shlex::try_quote() instead of shell_escape::escape() - Update test assertion to be quoting-strategy agnostic Fixes #10, fixes #15 --- Cargo.lock | 9 ++------- crates/devcontainer-mcp-core/Cargo.toml | 2 +- crates/devcontainer-mcp-core/src/file_ops.rs | 14 ++++++++------ crates/devcontainer-mcp/Cargo.toml | 1 + .../src/tools/devcontainer/build.rs | 7 ++++--- .../src/tools/devcontainer/exec.rs | 11 +++++------ .../devcontainer-mcp/src/tools/devcontainer/up.rs | 13 ++++++++++--- crates/devcontainer-mcp/src/tools/devpod/build.rs | 6 ++++-- .../devcontainer-mcp/src/tools/devpod/provider.rs | 7 ++++--- crates/devcontainer-mcp/src/tools/devpod/up.rs | 6 ++++-- 10 files changed, 43 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2987616..58fb079 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -304,6 +304,7 @@ dependencies = [ "schemars 1.2.1", "serde", "serde_json", + "shlex", "tokio", "tracing", "tracing-subscriber", @@ -319,7 +320,7 @@ dependencies = [ "futures-util", "serde", "serde_json", - "shell-escape", + "shlex", "thiserror", "tokio", "tracing", @@ -1189,12 +1190,6 @@ dependencies = [ "lazy_static", ] -[[package]] -name = "shell-escape" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45bb67a18fa91266cc7807181f62f9178a6873bfad7dc788c42e6430db40184f" - [[package]] name = "shlex" version = "1.3.0" diff --git a/crates/devcontainer-mcp-core/Cargo.toml b/crates/devcontainer-mcp-core/Cargo.toml index b737647..8c969ad 100644 --- a/crates/devcontainer-mcp-core/Cargo.toml +++ b/crates/devcontainer-mcp-core/Cargo.toml @@ -16,4 +16,4 @@ tracing = { workspace = true } futures-util = "0.3" async-trait = "0.1" base64 = "0.22" -shell-escape = "0.1" +shlex = "1" diff --git a/crates/devcontainer-mcp-core/src/file_ops.rs b/crates/devcontainer-mcp-core/src/file_ops.rs index c79ccee..93e98e4 100644 --- a/crates/devcontainer-mcp-core/src/file_ops.rs +++ b/crates/devcontainer-mcp-core/src/file_ops.rs @@ -4,10 +4,7 @@ //! formatting, and helpers to build shell commands for reading/writing files //! through any backend (DevPod SSH, devcontainer exec, Codespaces SSH). -use std::borrow::Cow; - use base64::{engine::general_purpose::STANDARD, Engine}; -use shell_escape::escape; use crate::error::{Error, Result}; @@ -57,7 +54,9 @@ pub fn apply_edit(content: &str, old_str: &str, new_str: &str) -> Result /// Shell-escape a string for safe embedding in a shell command. fn quote(s: &str) -> String { - escape(Cow::Borrowed(s)).into_owned() + shlex::try_quote(s) + .unwrap_or(std::borrow::Cow::Borrowed(s)) + .into_owned() } /// Build a shell command that reads a file via `cat`. @@ -133,8 +132,11 @@ mod tests { #[test] fn test_quote_path_with_single_quote() { let result = quote("it's"); - // Should not break when used in a shell command - assert!(!result.contains("it's") || result.contains("\\'")); + // Result should be shell-safe: either escaped or wrapped in quotes + assert!( + result != "it's", + "single quote must be escaped or quoted, got: {result}" + ); } #[test] diff --git a/crates/devcontainer-mcp/Cargo.toml b/crates/devcontainer-mcp/Cargo.toml index 48e283b..4770732 100644 --- a/crates/devcontainer-mcp/Cargo.toml +++ b/crates/devcontainer-mcp/Cargo.toml @@ -17,3 +17,4 @@ rmcp = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true } schemars = "1" +shlex = "1" diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/build.rs b/crates/devcontainer-mcp/src/tools/devcontainer/build.rs index caea2c3..b89cb60 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/build.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/build.rs @@ -25,12 +25,13 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - let extra: Vec<&str> = params + let extra: Vec = params .extra_args .as_deref() - .map(|a| a.split_whitespace().collect()) + .and_then(shlex::split) .unwrap_or_default(); - match devcontainer::build(¶ms.workspace_folder, &extra).await { + let extra_refs: Vec<&str> = extra.iter().map(|s| s.as_str()).collect(); + match devcontainer::build(¶ms.workspace_folder, &extra_refs).await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs index e97a521..763012b 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs @@ -25,12 +25,11 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - let cmd_args: Vec<&str> = params - .args - .as_deref() - .map(|a| a.split_whitespace().collect()) - .unwrap_or_default(); - match devcontainer::exec(¶ms.workspace_folder, ¶ms.command, &cmd_args).await { + let full_cmd = match ¶ms.args { + Some(a) => format!("{} {}", params.command, a), + None => params.command, + }; + match devcontainer::exec(¶ms.workspace_folder, "sh", &["-c", &full_cmd]).await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/up.rs b/crates/devcontainer-mcp/src/tools/devcontainer/up.rs index 7c9955d..b2b1b43 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/up.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/up.rs @@ -29,12 +29,19 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - let extra: Vec<&str> = params + let extra: Vec = params .extra_args .as_deref() - .map(|a| a.split_whitespace().collect()) + .and_then(shlex::split) .unwrap_or_default(); - match devcontainer::up(¶ms.workspace_folder, params.config.as_deref(), &extra).await { + let extra_refs: Vec<&str> = extra.iter().map(|s| s.as_str()).collect(); + match devcontainer::up( + ¶ms.workspace_folder, + params.config.as_deref(), + &extra_refs, + ) + .await + { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devpod/build.rs b/crates/devcontainer-mcp/src/tools/devpod/build.rs index 6ce4642..bdf8d83 100644 --- a/crates/devcontainer-mcp/src/tools/devpod/build.rs +++ b/crates/devcontainer-mcp/src/tools/devpod/build.rs @@ -19,8 +19,10 @@ impl DevContainerMcp { description = "Build a DevPod workspace image without starting it." )] async fn devpod_build(&self, Parameters(params): Parameters) -> String { - let parts: Vec<&str> = params.args.split_whitespace().collect(); - match devpod::build(&parts).await { + let parts: Vec = shlex::split(¶ms.args) + .unwrap_or_else(|| params.args.split_whitespace().map(String::from).collect()); + let part_refs: Vec<&str> = parts.iter().map(|s| s.as_str()).collect(); + match devpod::build(&part_refs).await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devpod/provider.rs b/crates/devcontainer-mcp/src/tools/devpod/provider.rs index 24ead82..e3ef756 100644 --- a/crates/devcontainer-mcp/src/tools/devpod/provider.rs +++ b/crates/devcontainer-mcp/src/tools/devpod/provider.rs @@ -37,12 +37,13 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - let opt_parts: Vec<&str> = params + let opt_parts: Vec = params .options .as_deref() - .map(|o| o.split_whitespace().collect()) + .and_then(shlex::split) .unwrap_or_default(); - match devpod::provider_add(¶ms.provider, &opt_parts).await { + let opt_refs: Vec<&str> = opt_parts.iter().map(|s| s.as_str()).collect(); + match devpod::provider_add(¶ms.provider, &opt_refs).await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devpod/up.rs b/crates/devcontainer-mcp/src/tools/devpod/up.rs index b0366c5..f854c1a 100644 --- a/crates/devcontainer-mcp/src/tools/devpod/up.rs +++ b/crates/devcontainer-mcp/src/tools/devpod/up.rs @@ -19,8 +19,10 @@ impl DevContainerMcp { description = "Create and start a DevPod workspace. Pass the source (git URL, local path, or image) and any flags as space-separated args. Returns full build output for self-healing." )] async fn devpod_up(&self, Parameters(params): Parameters) -> String { - let parts: Vec<&str> = params.args.split_whitespace().collect(); - match devpod::up(&parts).await { + let parts: Vec = shlex::split(¶ms.args) + .unwrap_or_else(|| params.args.split_whitespace().map(String::from).collect()); + let part_refs: Vec<&str> = parts.iter().map(|s| s.as_str()).collect(); + match devpod::up(&part_refs).await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } From b3ff47902aa73ff1bb89db18bcb79036383365b6 Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 14 May 2026 09:29:42 -0700 Subject: [PATCH 09/19] Add allowlist for host-safe commands in devcontainer guard hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook was blanket-blocking all bash/shell commands when a devcontainer was detected, including git operations that are inherently host-level. Add an allowlist of host-safe commands (git, gh) that are permitted even when a devcontainer exists. All commands in a chain (&&, ||, ;, |) must be on the allowlist — so 'git fetch && cargo build' is correctly blocked. Move hooks from hooks/ to .github/hooks/ so the project self-protects even without the MCP server installed. Commands like curl/wget are intentionally excluded since they can pipe to sh. The bypass (USER_CONFIRMED_HOST_OPERATION=1) still works for anything not on the allowlist. --- .github/hooks/devcontainer-guard.json | 2 +- .../hooks}/devcontainer-guard.sh | 59 ++++++++++++++++++- .../tests/hook_guard_test.rs | 2 +- install.ps1 | 2 +- install.sh | 2 +- 5 files changed, 62 insertions(+), 5 deletions(-) rename {hooks => .github/hooks}/devcontainer-guard.sh (57%) diff --git a/.github/hooks/devcontainer-guard.json b/.github/hooks/devcontainer-guard.json index 24c6fa2..a08ef1c 100644 --- a/.github/hooks/devcontainer-guard.json +++ b/.github/hooks/devcontainer-guard.json @@ -4,7 +4,7 @@ "preToolUse": [ { "type": "command", - "bash": "./hooks/devcontainer-guard.sh", + "bash": "./.github/hooks/devcontainer-guard.sh", "timeoutSec": 5 } ] diff --git a/hooks/devcontainer-guard.sh b/.github/hooks/devcontainer-guard.sh similarity index 57% rename from hooks/devcontainer-guard.sh rename to .github/hooks/devcontainer-guard.sh index fb78b19..430e308 100755 --- a/hooks/devcontainer-guard.sh +++ b/.github/hooks/devcontainer-guard.sh @@ -8,6 +8,9 @@ # Read-only tools (view, grep, glob) and file edits are allowed through — only # command execution is blocked. # +# Host-safe commands (git, gh, curl, etc.) are allowlisted and always permitted +# since they operate on the repo/host, not the project's build environment. +# # Bypass: include USER_CONFIRMED_HOST_OPERATION=1 in the command. # # Supports both agent payload formats: @@ -48,7 +51,61 @@ if [ ! -f "${CWD}/.devcontainer/devcontainer.json" ]; then exit 0 fi -# --- Devcontainer exists: block the tool call --- +# --- Devcontainer exists: check allowlist before blocking --- + +# Extract the command string from tool input (handles both formats) +COMMAND=$(echo "$INPUT" | jq -r '(.tool_input.command // .toolArgs.command // "") | tostring') + +# Commands that are safe to run on the host even when a devcontainer exists. +# These operate on the repo/host itself, not on the project's build environment. +ALLOWED_HOST_COMMANDS=( + git + gh +) + +# Extract all meaningful commands from a shell string, skipping env vars +# (KEY=VALUE) and cd/pushd/popd. Splits on &&, ||, ;, and | to catch every +# command in a chain or pipeline. +all_commands() { + local cmd="$1" + while IFS= read -r segment; do + segment="${segment#"${segment%%[![:space:]]*}"}" + [ -z "$segment" ] && continue + for token in $segment; do + if [[ "$token" == *=* && "$token" != -* ]]; then + continue + fi + case "$token" in + cd|pushd|popd) break ;; + esac + basename "$token" + break + done + done < <(echo "$cmd" | sed 's/ *&& */\n/g; s/ *|| */\n/g; s/ *; */\n/g; s/ *| */\n/g') +} + +# Every command in the chain must be on the allowlist +ALL_ALLOWED=true +while IFS= read -r cmd_name; do + [ -z "$cmd_name" ] && continue + FOUND=false + for allowed in "${ALLOWED_HOST_COMMANDS[@]}"; do + if [ "$cmd_name" = "$allowed" ]; then + FOUND=true + break + fi + done + if [ "$FOUND" = false ]; then + ALL_ALLOWED=false + break + fi +done < <(all_commands "$COMMAND") + +if [ "$ALL_ALLOWED" = true ] && [ -n "$(all_commands "$COMMAND")" ]; then + exit 0 +fi + +# --- Not on the allowlist: block the tool call --- DENY_REASON="Host execution blocked. This project has a devcontainer. Use devcontainer-mcp tools (devcontainer_exec, devpod_ssh, codespaces_ssh, and file operation tools) instead of running commands directly on the host." diff --git a/crates/devcontainer-mcp-core/tests/hook_guard_test.rs b/crates/devcontainer-mcp-core/tests/hook_guard_test.rs index cb30138..e277bf2 100644 --- a/crates/devcontainer-mcp-core/tests/hook_guard_test.rs +++ b/crates/devcontainer-mcp-core/tests/hook_guard_test.rs @@ -15,7 +15,7 @@ fn repo_root() -> std::path::PathBuf { } fn hook_path() -> std::path::PathBuf { - repo_root().join("hooks/devcontainer-guard.sh") + repo_root().join(".github/hooks/devcontainer-guard.sh") } /// Run the hook script with the given JSON input and return (stdout, exit_code). diff --git a/install.ps1 b/install.ps1 index 8e61b7c..8a0cf6e 100644 --- a/install.ps1 +++ b/install.ps1 @@ -129,7 +129,7 @@ foreach ($dir in $skillDirs) { Write-Step "Installing host-protection hook..." -$hookUrl = "https://raw.githubusercontent.com/$Repo/main/hooks/devcontainer-guard.sh" +$hookUrl = "https://raw.githubusercontent.com/$Repo/main/.github/hooks/devcontainer-guard.sh" $WslHookDir = "~/.local/share/devcontainer-mcp/hooks" $WslHookPath = "$WslHookDir/devcontainer-guard.sh" diff --git a/install.sh b/install.sh index 13d07ce..684e373 100755 --- a/install.sh +++ b/install.sh @@ -120,7 +120,7 @@ done # Install host-protection hooks # --------------------------------------------------------------------------- -HOOK_URL="https://raw.githubusercontent.com/${REPO}/main/hooks/devcontainer-guard.sh" +HOOK_URL="https://raw.githubusercontent.com/${REPO}/main/.github/hooks/devcontainer-guard.sh" HOOK_DIR="${HOME}/.local/share/devcontainer-mcp/hooks" HOOK_PATH="${HOOK_DIR}/devcontainer-guard.sh" From 09eba3ff7f70b82ddfdea29e2899753bf0ab0473 Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 14 May 2026 13:05:47 -0700 Subject: [PATCH 10/19] Add sessionStart hook to auto-inject SKILL.md on session start When a session starts in a directory with .devcontainer/devcontainer.json, the new devcontainer-skill-loader hook injects the SKILL.md content as additionalContext, making agents automatically aware of devcontainer-mcp tools without manual skill installation. - Add .github/hooks/devcontainer-skill-loader.sh and JSON config - Install SKILL.md to ~/.local/share/devcontainer-mcp/ for the loader - Configure sessionStart hooks for both Claude Code and Copilot CLI - Update install.sh and install.ps1 with the new hook Closes #19 --- .github/hooks/devcontainer-skill-loader.json | 12 +++ .github/hooks/devcontainer-skill-loader.sh | 49 +++++++++++ install.ps1 | 92 ++++++++++++++++---- install.sh | 92 +++++++++++++++++--- 4 files changed, 214 insertions(+), 31 deletions(-) create mode 100644 .github/hooks/devcontainer-skill-loader.json create mode 100755 .github/hooks/devcontainer-skill-loader.sh diff --git a/.github/hooks/devcontainer-skill-loader.json b/.github/hooks/devcontainer-skill-loader.json new file mode 100644 index 0000000..1a6a8ce --- /dev/null +++ b/.github/hooks/devcontainer-skill-loader.json @@ -0,0 +1,12 @@ +{ + "version": 1, + "hooks": { + "sessionStart": [ + { + "type": "command", + "bash": "./.github/hooks/devcontainer-skill-loader.sh", + "timeoutSec": 5 + } + ] + } +} diff --git a/.github/hooks/devcontainer-skill-loader.sh b/.github/hooks/devcontainer-skill-loader.sh new file mode 100755 index 0000000..5dcfbc9 --- /dev/null +++ b/.github/hooks/devcontainer-skill-loader.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# devcontainer-skill-loader.sh — SessionStart hook for Claude Code & Copilot CLI +# +# When a session starts in a directory with .devcontainer/devcontainer.json, +# injects the devcontainer-mcp SKILL.md content as additionalContext so the +# agent automatically knows how to use devcontainer-mcp tools. +# +# Supports both agent payload formats: +# Claude Code: { tool_name, tool_input, cwd, ... } +# Copilot CLI: { toolName, toolArgs, cwd, ... } + +set -euo pipefail + +INPUT=$(cat) + +# Extract working directory from the payload +CWD=$(echo "$INPUT" | jq -r '.cwd // empty') + +if [ -z "$CWD" ]; then + exit 0 +fi + +if [ ! -f "${CWD}/.devcontainer/devcontainer.json" ]; then + exit 0 +fi + +# Look for SKILL.md in order of preference +SKILL_PATH="" +SEARCH_PATHS=( + "${HOME}/.local/share/devcontainer-mcp/SKILL.md" + "${HOME}/.copilot/skills/devcontainer-mcp/SKILL.md" + "${HOME}/.claude/skills/devcontainer-mcp/SKILL.md" + "${HOME}/.agents/skills/devcontainer-mcp/SKILL.md" +) + +for p in "${SEARCH_PATHS[@]}"; do + if [ -f "$p" ]; then + SKILL_PATH="$p" + break + fi +done + +if [ -z "$SKILL_PATH" ]; then + exit 0 +fi + +SKILL_CONTENT=$(cat "$SKILL_PATH") + +jq -n --arg ctx "$SKILL_CONTENT" '{ "additionalContext": $ctx }' diff --git a/install.ps1 b/install.ps1 index 8a0cf6e..f71da2e 100644 --- a/install.ps1 +++ b/install.ps1 @@ -130,22 +130,35 @@ foreach ($dir in $skillDirs) { Write-Step "Installing host-protection hook..." $hookUrl = "https://raw.githubusercontent.com/$Repo/main/.github/hooks/devcontainer-guard.sh" +$loaderUrl = "https://raw.githubusercontent.com/$Repo/main/.github/hooks/devcontainer-skill-loader.sh" $WslHookDir = "~/.local/share/devcontainer-mcp/hooks" $WslHookPath = "$WslHookDir/devcontainer-guard.sh" +$WslLoaderPath = "$WslHookDir/devcontainer-skill-loader.sh" $hookResult = wsl -d $WslDistro bash -c "mkdir -p '$WslHookDir' && curl -fsSL -o '$WslHookPath' '$hookUrl' && chmod +x '$WslHookPath' && echo OK" 2>&1 if ($hookResult -match "OK") { - Write-Ok "Hook script installed in WSL at $WslHookPath" + Write-Ok "Guard hook installed in WSL at $WslHookPath" } else { - Write-Warn "Could not install hook script in WSL" + Write-Warn "Could not install guard hook in WSL" } -# Configure Claude Code PreToolUse hook (Windows-side) -Write-Step "Configuring agent host-protection hooks..." +$loaderResult = wsl -d $WslDistro bash -c "curl -fsSL -o '$WslLoaderPath' '$loaderUrl' && chmod +x '$WslLoaderPath' && echo OK" 2>&1 +if ($loaderResult -match "OK") { + Write-Ok "Skill-loader hook installed in WSL at $WslLoaderPath" +} else { + Write-Warn "Could not install skill-loader hook in WSL" +} + +# Install SKILL.md alongside hooks for the loader to find +$WslSkillDataPath = "~/.local/share/devcontainer-mcp/SKILL.md" +wsl -d $WslDistro bash -c "curl -fsSL -o '$WslSkillDataPath' '$skillUrl'" 2>&1 | Out-Null + +# Configure Claude Code PreToolUse + SessionStart hooks (Windows-side) +Write-Step "Configuring agent hooks..." $claudeSettings = "$env:USERPROFILE\.claude\settings.json" try { - $hookEntry = @{ + $guardEntry = @{ matcher = "Bash" hooks = @( @{ @@ -155,35 +168,64 @@ try { } ) } + $loaderEntry = @{ + hooks = @( + @{ + type = "command" + command = "wsl $WslLoaderPath" + timeout = 5 + } + ) + } if (Test-Path $claudeSettings) { $content = Get-Content -Raw $claudeSettings | ConvertFrom-Json if (-not $content.hooks) { $content | Add-Member -NotePropertyName "hooks" -NotePropertyValue ([PSCustomObject]@{}) } + + # PreToolUse: devcontainer-guard if (-not $content.hooks.PreToolUse) { $content.hooks | Add-Member -NotePropertyName "PreToolUse" -NotePropertyValue @() } - # Check if already configured - $already = $false + $alreadyGuard = $false foreach ($group in $content.hooks.PreToolUse) { foreach ($h in $group.hooks) { - if ($h.command -match "devcontainer-guard") { $already = $true; break } + if ($h.command -match "devcontainer-guard") { $alreadyGuard = $true; break } } } - if (-not $already) { - $content.hooks.PreToolUse += [PSCustomObject]$hookEntry - $content | ConvertTo-Json -Depth 10 | Set-Content $claudeSettings -Encoding UTF8 - Write-Ok "Claude Code — added hook to $claudeSettings" + if (-not $alreadyGuard) { + $content.hooks.PreToolUse += [PSCustomObject]$guardEntry + Write-Ok "Claude Code — added PreToolUse hook" } else { - Write-Ok "Claude Code — hook already configured" + Write-Ok "Claude Code — PreToolUse hook already configured" } + + # SessionStart: skill-loader + if (-not $content.hooks.SessionStart) { + $content.hooks | Add-Member -NotePropertyName "SessionStart" -NotePropertyValue @() + } + $alreadyLoader = $false + foreach ($group in $content.hooks.SessionStart) { + foreach ($h in $group.hooks) { + if ($h.command -match "skill-loader") { $alreadyLoader = $true; break } + } + } + if (-not $alreadyLoader) { + $content.hooks.SessionStart += [PSCustomObject]$loaderEntry + Write-Ok "Claude Code — added SessionStart hook" + } else { + Write-Ok "Claude Code — SessionStart hook already configured" + } + + $content | ConvertTo-Json -Depth 10 | Set-Content $claudeSettings -Encoding UTF8 } else { $dir = Split-Path $claudeSettings -Parent if ($dir) { New-Item -ItemType Directory -Path $dir -Force | Out-Null } $config = [PSCustomObject]@{ hooks = [PSCustomObject]@{ - PreToolUse = @([PSCustomObject]$hookEntry) + PreToolUse = @([PSCustomObject]$guardEntry) + SessionStart = @([PSCustomObject]$loaderEntry) } } $config | ConvertTo-Json -Depth 10 | Set-Content $claudeSettings -Encoding UTF8 @@ -193,11 +235,12 @@ try { Write-Warn "Claude Code — could not configure hooks" } -# Configure Copilot CLI preToolUse hook (Windows-side) +# Configure Copilot CLI preToolUse + sessionStart hooks (Windows-side) $copilotHooksDir = "$env:USERPROFILE\.copilot\hooks" try { New-Item -ItemType Directory -Path $copilotHooksDir -Force | Out-Null - $copilotHook = [PSCustomObject]@{ + + $copilotGuard = [PSCustomObject]@{ version = 1 hooks = [PSCustomObject]@{ preToolUse = @( @@ -209,8 +252,23 @@ try { ) } } - $copilotHook | ConvertTo-Json -Depth 10 | Set-Content "$copilotHooksDir\devcontainer-guard.json" -Encoding UTF8 + $copilotGuard | ConvertTo-Json -Depth 10 | Set-Content "$copilotHooksDir\devcontainer-guard.json" -Encoding UTF8 Write-Ok "Copilot CLI — created $copilotHooksDir\devcontainer-guard.json" + + $copilotLoader = [PSCustomObject]@{ + version = 1 + hooks = [PSCustomObject]@{ + sessionStart = @( + [PSCustomObject]@{ + type = "command" + bash = "wsl $WslLoaderPath" + timeoutSec = 5 + } + ) + } + } + $copilotLoader | ConvertTo-Json -Depth 10 | Set-Content "$copilotHooksDir\devcontainer-skill-loader.json" -Encoding UTF8 + Write-Ok "Copilot CLI — created $copilotHooksDir\devcontainer-skill-loader.json" } catch { Write-Warn "Copilot CLI — could not configure hooks" } diff --git a/install.sh b/install.sh index 684e373..d6f8fc2 100755 --- a/install.sh +++ b/install.sh @@ -121,8 +121,10 @@ done # --------------------------------------------------------------------------- HOOK_URL="https://raw.githubusercontent.com/${REPO}/main/.github/hooks/devcontainer-guard.sh" +LOADER_URL="https://raw.githubusercontent.com/${REPO}/main/.github/hooks/devcontainer-skill-loader.sh" HOOK_DIR="${HOME}/.local/share/devcontainer-mcp/hooks" HOOK_PATH="${HOOK_DIR}/devcontainer-guard.sh" +LOADER_PATH="${HOOK_DIR}/devcontainer-skill-loader.sh" echo "" echo "==> Installing host-protection hook..." @@ -131,10 +133,22 @@ curl -fsSL -o "$HOOK_PATH" "$HOOK_URL" 2>/dev/null && \ chmod +x "$HOOK_PATH" && \ echo " ${HOOK_PATH}" || echo " ⚠ Could not download hook script" -# Configure Claude Code PreToolUse hook +echo "" +echo "==> Installing skill-loader hook..." +curl -fsSL -o "$LOADER_PATH" "$LOADER_URL" 2>/dev/null && \ + chmod +x "$LOADER_PATH" && \ + echo " ${LOADER_PATH}" || echo " ⚠ Could not download skill-loader hook" + +# Install SKILL.md alongside hooks for the loader to find +SKILL_DATA_PATH="${HOME}/.local/share/devcontainer-mcp/SKILL.md" +curl -fsSL -o "$SKILL_DATA_PATH" "$SKILL_URL" 2>/dev/null && \ + echo " ${SKILL_DATA_PATH}" || true + +# Configure Claude Code PreToolUse + SessionStart hooks configure_claude_hook() { local settings_file="${HOME}/.claude/settings.json" local hook_command="$HOOK_PATH" + local loader_command="$LOADER_PATH" if [ ! -f "$settings_file" ]; then mkdir -p "$(dirname "$settings_file")" @@ -152,6 +166,17 @@ configure_claude_hook() { } ] } + ], + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "${loader_command}", + "timeout": 5 + } + ] + } ] } } @@ -163,22 +188,23 @@ import json, sys path = '${settings_file}' hook_cmd = '${hook_command}' +loader_cmd = '${loader_command}' with open(path) as f: data = json.load(f) hooks = data.setdefault('hooks', {}) -pre_tool = hooks.setdefault('PreToolUse', []) -# Check if devcontainer-guard is already configured -already = False +# --- PreToolUse: devcontainer-guard --- +pre_tool = hooks.setdefault('PreToolUse', []) +already_guard = False for group in pre_tool: for h in group.get('hooks', []): if 'devcontainer-guard' in h.get('command', ''): - already = True + already_guard = True break -if not already: +if not already_guard: pre_tool.append({ 'matcher': 'Bash', 'hooks': [{ @@ -187,24 +213,46 @@ if not already: 'timeout': 5 }] }) - with open(path, 'w') as f: - json.dump(data, f, indent=2) - print(' ✓ Claude Code — added hook to ${settings_file}') + print(' ✓ Claude Code — added PreToolUse hook to ${settings_file}') else: - print(' ✓ Claude Code — hook already configured') + print(' ✓ Claude Code — PreToolUse hook already configured') + +# --- SessionStart: skill-loader --- +session_start = hooks.setdefault('SessionStart', []) +already_loader = False +for group in session_start: + for h in group.get('hooks', []): + if 'skill-loader' in h.get('command', ''): + already_loader = True + break + +if not already_loader: + session_start.append({ + 'hooks': [{ + 'type': 'command', + 'command': loader_cmd, + 'timeout': 5 + }] + }) + print(' ✓ Claude Code — added SessionStart hook to ${settings_file}') +else: + print(' ✓ Claude Code — SessionStart hook already configured') + +with open(path, 'w') as f: + json.dump(data, f, indent=2) " 2>/dev/null || echo " ⚠ Claude Code — could not update ${settings_file}" else echo " ⚠ Claude Code — exists but python3 not available to merge" fi } -# Configure Copilot CLI preToolUse hook +# Configure Copilot CLI preToolUse + sessionStart hooks configure_copilot_hook() { local hooks_dir="${HOME}/.copilot/hooks" - local hooks_file="${hooks_dir}/devcontainer-guard.json" mkdir -p "$hooks_dir" - cat > "$hooks_file" << COPILOTEOF + + cat > "${hooks_dir}/devcontainer-guard.json" << COPILOTEOF { "version": 1, "hooks": { @@ -218,7 +266,23 @@ configure_copilot_hook() { } } COPILOTEOF - echo " ✓ Copilot CLI — created ${hooks_file}" + echo " ✓ Copilot CLI — created ${hooks_dir}/devcontainer-guard.json" + + cat > "${hooks_dir}/devcontainer-skill-loader.json" << COPILOTEOF +{ + "version": 1, + "hooks": { + "sessionStart": [ + { + "type": "command", + "bash": "${LOADER_PATH}", + "timeoutSec": 5 + } + ] + } +} +COPILOTEOF + echo " ✓ Copilot CLI — created ${hooks_dir}/devcontainer-skill-loader.json" } echo "" From 1a87eaae9ec7e6e1d35d8efe90f394a95814cf6b Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 22 May 2026 22:05:12 -0700 Subject: [PATCH 11/19] Add opencode skill + MCP server installation to installers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit opencode reads MCP configuration from ~/.config/opencode/opencode.json using a distinct schema: 'mcp' key (not 'mcpServers'), 'type: local', a single 'command' array, and explicit 'enabled: true'. Changes: - install.sh: add ~/.config/opencode/skills/devcontainer-mcp to SKILL_DIRS and write the opencode MCP entry via a dedicated configure_opencode_mcp function (preserves existing schema/model/sibling MCP entries). - install.ps1: mirror the skill directory addition and add Set-OpencodeMcpConfig that writes the wsl-bridge command array. - README.md: list opencode alongside the other supported clients. opencode has no documented PreToolUse/SessionStart hook system, so the host-protection guard and skill-loader hooks are not configured for it. Verified end-to-end inside the project's devcontainer: fresh install, idempotent rerun, and merge into a pre-existing config with sibling schema/model/MCP entries — all preserved. --- README.md | 2 +- install.ps1 | 43 +++++++++++++++++++++++++++++++++++++++++++ install.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8c63ed7..07ab0b4 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ devcontainer-mcp local Docker demo

-> Works with **GitHub Copilot**, **Claude**, **Cursor**, and any MCP-compatible client. +> Works with **GitHub Copilot**, **Claude**, **Cursor**, **opencode**, and any MCP-compatible client. ## The Problem diff --git a/install.ps1 b/install.ps1 index f71da2e..86cbdda 100644 --- a/install.ps1 +++ b/install.ps1 @@ -111,6 +111,7 @@ $skillDirs = @( "$env:USERPROFILE\.copilot\skills\devcontainer-mcp" "$env:USERPROFILE\.claude\skills\devcontainer-mcp" "$env:USERPROFILE\.agents\skills\devcontainer-mcp" + "$env:USERPROFILE\.config\opencode\skills\devcontainer-mcp" ) foreach ($dir in $skillDirs) { @@ -362,6 +363,48 @@ if (Test-Path "$env:USERPROFILE\.cursor") { Set-McpConfig "$env:USERPROFILE\.cursor\mcp.json" "Cursor" } +# opencode — uses a distinct schema (mcp key, type: "local", combined +# command array, enabled: true) and lives at ~/.config/opencode/opencode.json. +function Set-OpencodeMcpConfig { + $ConfigPath = "$env:USERPROFILE\.config\opencode\opencode.json" + $opencodeEntry = [PSCustomObject]@{ + type = "local" + command = @("wsl", $WslBinaryPath, "serve") + enabled = $true + } + + try { + if (Test-Path $ConfigPath) { + $content = Get-Content -Raw $ConfigPath | ConvertFrom-Json + if (-not $content.mcp) { + $content | Add-Member -NotePropertyName "mcp" -NotePropertyValue ([PSCustomObject]@{}) + } + if ($content.mcp.PSObject.Properties.Name -contains "devcontainer-mcp") { + Write-Ok "opencode — already configured" + return + } + $content.mcp | Add-Member -NotePropertyName "devcontainer-mcp" -NotePropertyValue $opencodeEntry + $content | ConvertTo-Json -Depth 10 | Set-Content $ConfigPath -Encoding UTF8 + Write-Ok "opencode — added to $ConfigPath" + } else { + $dir = Split-Path $ConfigPath -Parent + if ($dir) { New-Item -ItemType Directory -Path $dir -Force | Out-Null } + $config = [PSCustomObject]@{ + '$schema' = "https://opencode.ai/config.json" + mcp = [PSCustomObject]@{ + "devcontainer-mcp" = $opencodeEntry + } + } + $config | ConvertTo-Json -Depth 10 | Set-Content $ConfigPath -Encoding UTF8 + Write-Ok "opencode — created $ConfigPath" + } + } catch { + Write-Warn "opencode — could not update $ConfigPath" + } +} + +Set-OpencodeMcpConfig + # --------------------------------------------------------------------------- # Done # --------------------------------------------------------------------------- diff --git a/install.sh b/install.sh index d6f8fc2..da18c0f 100755 --- a/install.sh +++ b/install.sh @@ -106,6 +106,7 @@ SKILL_DIRS=( "${HOME}/.copilot/skills/devcontainer-mcp" "${HOME}/.claude/skills/devcontainer-mcp" "${HOME}/.agents/skills/devcontainer-mcp" + "${HOME}/.config/opencode/skills/devcontainer-mcp" ) echo "" @@ -346,6 +347,51 @@ else: fi } +# opencode uses a different config schema (mcp key, type: "local", combined +# command array, enabled: true) and a different file path. +configure_opencode_mcp() { + local config_file="${HOME}/.config/opencode/opencode.json" + local bin_path="${INSTALL_DIR}/devcontainer-mcp" + + if [ ! -f "$config_file" ]; then + mkdir -p "$(dirname "$config_file")" + cat > "$config_file" << OPENCODEEOF +{ + "\$schema": "https://opencode.ai/config.json", + "mcp": { + "devcontainer-mcp": { + "type": "local", + "command": ["${bin_path}", "serve"], + "enabled": true + } + } +} +OPENCODEEOF + echo " ✓ opencode — created ${config_file}" + elif command -v python3 >/dev/null 2>&1; then + python3 -c " +import json +path = '${config_file}' +with open(path) as f: + data = json.load(f) +servers = data.setdefault('mcp', {}) +if 'devcontainer-mcp' not in servers: + servers['devcontainer-mcp'] = { + 'type': 'local', + 'command': ['${bin_path}', 'serve'], + 'enabled': True, + } + with open(path, 'w') as f: + json.dump(data, f, indent=2) + print(' ✓ opencode — added to ${config_file}') +else: + print(' ✓ opencode — already configured') +" 2>/dev/null || echo " ⚠ opencode — could not update ${config_file}" + else + echo " ⚠ opencode — exists but python3 not available to merge" + fi +} + echo "" echo "==> Configuring MCP clients..." @@ -372,5 +418,8 @@ fi # Claude Code — ~/.claude.json configure_mcp_client "${HOME}/.claude.json" "Claude Code" +# opencode — ~/.config/opencode/opencode.json (uses different schema) +configure_opencode_mcp + echo "" echo "Done! devcontainer-mcp is ready to use." From e2da52fba0b31ff0d66ce5563516e202b06eb8f2 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 23 May 2026 13:56:00 -0700 Subject: [PATCH 12/19] devcontainer exec: stream output, support cancellation, reap container descendants Three defects in `devcontainer_exec` made the server look "dead" to clients running heavy parallel calls (e.g. concurrent `go test -race`, `go vet`, `go build`): 1. `cmd.output().await` blocked until the child exited, so the server sent zero bytes to the client until the work was done. Client-side idle/deadline watchdogs tripped during long execs, tore down the transport, and surfaced `MCP error -32000: Connection closed`. 2. There was no cancellation path. When rmcp's cancellation token fired (transport closed, peer cancelled), our handler kept running and so did the underlying `devcontainer exec` -- which meant any in-container work (a `go test -race` chewing on a core) ran forever. 3. The docker daemon reparents in-container processes under containerd-shim, so a /proc-based reap on the host can't see them. Killing our host-side `devcontainer exec` wrapper just closes the pipes; the workload survives. Changes: - `core::cli::run_cli_streaming`: replace `cmd.output()` with `cmd.spawn()` + concurrent line-by-line stdout/stderr capture. Accepts an optional `Arc` for live progress and a `CancellationToken`. On cancel, walks /proc descendants of the spawned child and SIGTERMs then SIGKILLs them (sufficient for host-side CLIs like `up`/`build`). - `core::process_tree::reap(pid, grace)`: new helper. BFS `/proc/*/status` for descendants, SIGTERM all, wait grace, SIGKILL survivors. Useful where the host kernel sees the full tree. - `core::exec_shim`: new module. `wrap()` returns a shell prelude that `setsid`s a backgrounded shell, emits `__DCMCP_PGID=__` on stderr, then `eval`s the user command read from env var `DCMCP_USER_CMD`. Delivering the user command via env var (not string interpolation) means commands containing single quotes, parens, or subshells survive the shim's own quoting. Backgrounding + `wait $!` is POSIX (BusyBox/dash compatible) -- `setsid -w` is not. - `core::devcontainer::exec_streaming`: rewritten to use the shim. The user command is passed inside the container via `devcontainer exec --remote-env DCMCP_USER_CMD=...`. The stderr reader captures the in-container PGID from the sentinel and scrubs the line from forwarded output. On cancel, sends `kill -TERM -` then `kill -KILL -` *inside the container* via bollard's `create_exec` / `start_exec` (detached); this reaches every process in the group, including grandchildren spawned by `go test`. `kill -- -N` is avoided because BusyBox `kill` rejects `--`. - `tools::devcontainer::exec`: handler extended with `CancellationToken`, `Peer`, `Meta` (extracted by rmcp's `tool_router` macro). When the client supplies a `progressToken` in `_meta`, every line of stdout/stderr is forwarded as a `notifications/progress` message with a strictly-increasing counter, keeping the wire warm during long execs. Verified end-to-end against a live devcontainer: - 3 parallel `go test -race -v -count=1` / `go vet` / `go build -a` calls complete correctly in 17s, server stays up, no protocol errors. - Progress notifications arrive live during a 3s exec (5 notifs spread across the call, not batched at the end). - A 60s `sleep` cancelled mid-call returns `Error: Operation cancelled by peer` within ~5s; server immediately responsive to the next call. - A nested in-container tree `(sleep 9999 & sleep 9999 & sleep 9999 & wait)` is fully reaped (5 -> 0 in-container processes) when the call is cancelled. DevPod and Codespaces `exec` paths still need the same treatment (separate commit) -- `exec_shim::wrap` is backend-agnostic; only the per-backend "send kill into the target" implementation differs. --- Cargo.lock | 4 + crates/devcontainer-mcp-core/Cargo.toml | 4 + crates/devcontainer-mcp-core/src/cli.rs | 172 +++++++++++- .../devcontainer-mcp-core/src/devcontainer.rs | 263 +++++++++++++++++- crates/devcontainer-mcp-core/src/error.rs | 3 + crates/devcontainer-mcp-core/src/exec_shim.rs | 166 +++++++++++ crates/devcontainer-mcp-core/src/file_ops.rs | 7 +- crates/devcontainer-mcp-core/src/lib.rs | 2 + .../devcontainer-mcp-core/src/process_tree.rs | 215 ++++++++++++++ crates/devcontainer-mcp/Cargo.toml | 2 + .../src/tools/devcontainer/exec.rs | 75 ++++- 11 files changed, 901 insertions(+), 12 deletions(-) create mode 100644 crates/devcontainer-mcp-core/src/exec_shim.rs create mode 100644 crates/devcontainer-mcp-core/src/process_tree.rs diff --git a/Cargo.lock b/Cargo.lock index 58fb079..7689f09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -298,6 +298,7 @@ name = "devcontainer-mcp" version = "0.1.0" dependencies = [ "anyhow", + "async-trait", "clap", "devcontainer-mcp-core", "rmcp", @@ -306,6 +307,7 @@ dependencies = [ "serde_json", "shlex", "tokio", + "tokio-util", "tracing", "tracing-subscriber", ] @@ -318,11 +320,13 @@ dependencies = [ "base64", "bollard", "futures-util", + "libc", "serde", "serde_json", "shlex", "thiserror", "tokio", + "tokio-util", "tracing", ] diff --git a/crates/devcontainer-mcp-core/Cargo.toml b/crates/devcontainer-mcp-core/Cargo.toml index 8c969ad..3c32dbc 100644 --- a/crates/devcontainer-mcp-core/Cargo.toml +++ b/crates/devcontainer-mcp-core/Cargo.toml @@ -8,6 +8,7 @@ repository.workspace = true [dependencies] tokio = { workspace = true } +tokio-util = "0.7" serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } @@ -17,3 +18,6 @@ futures-util = "0.3" async-trait = "0.1" base64 = "0.22" shlex = "1" + +[target.'cfg(unix)'.dependencies] +libc = "0.2" diff --git a/crates/devcontainer-mcp-core/src/cli.rs b/crates/devcontainer-mcp-core/src/cli.rs index cee82ec..7c007c1 100644 --- a/crates/devcontainer-mcp-core/src/cli.rs +++ b/crates/devcontainer-mcp-core/src/cli.rs @@ -1,7 +1,10 @@ use serde::Serialize; use std::collections::HashMap; use std::process::Stdio; +use std::sync::Arc; +use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::Command; +use tokio_util::sync::CancellationToken; use crate::error::{Error, Result}; @@ -57,8 +60,38 @@ impl CliBinary { } } +/// Which output stream a chunk came from. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OutputStream { + Stdout, + Stderr, +} + +/// A chunk of output from a streaming CLI invocation. +#[derive(Debug, Clone)] +pub struct OutputChunk { + pub stream: OutputStream, + /// One line of output (without trailing newline). + pub line: String, +} + +/// Sink for streamed output chunks. +/// +/// Implementations should be fast — they're awaited inline by the +/// background dispatcher and slow sinks will buffer up in the bounded +/// channel. Errors must be swallowed inside the sink (we don't want to +/// abort the underlying process because progress reporting failed). +#[async_trait::async_trait] +pub trait ChunkSink: Send + Sync + 'static { + async fn on_chunk(&self, chunk: OutputChunk); +} + /// Run a CLI command, capturing stdout/stderr/exit_code. /// If `parse_json` is true, attempts to parse stdout as JSON. +/// +/// Convenience wrapper around [`run_cli_streaming`] with no progress sink +/// and a fresh (never-fired) cancellation token. Callers that want +/// cancellation or progress should use [`run_cli_streaming`] directly. pub async fn run_cli(binary: &CliBinary, args: &[&str], parse_json: bool) -> Result { run_cli_with_env(binary, args, parse_json, None).await } @@ -69,9 +102,48 @@ pub async fn run_cli_with_env( args: &[&str], parse_json: bool, env: Option<&HashMap>, +) -> Result { + run_cli_streaming( + binary, + args, + parse_json, + env, + &CancellationToken::new(), + None, + ) + .await +} + +/// Run a CLI command with full control over cancellation and progress. +/// +/// - `cancel`: if cancelled while the child is running, the child is killed +/// (SIGTERM, then SIGKILL after a 2s grace period via `kill_on_drop`) +/// and [`Error::Cancelled`] is returned. Any output produced before +/// cancellation is forwarded to the sink but discarded from the return +/// value. +/// - `on_chunk`: optional sink invoked for every line of stdout/stderr as +/// it arrives. Useful for emitting MCP progress notifications. +/// +/// Regardless of streaming, the full stdout/stderr is also accumulated +/// and returned in [`CliOutput`] for callers that need the final result. +pub async fn run_cli_streaming( + binary: &CliBinary, + args: &[&str], + parse_json: bool, + env: Option<&HashMap>, + cancel: &CancellationToken, + on_chunk: Option>, ) -> Result { let mut cmd = Command::new(binary.command_name()); - cmd.args(args).stdout(Stdio::piped()).stderr(Stdio::piped()); + cmd.args(args) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + // Belt-and-braces: if this future is dropped (e.g. caller bails + // before we finish handling cancel), tokio will SIGKILL the + // child for us. Without this, a cancelled but un-awaited child + // would survive as a zombie / runaway process. + .kill_on_drop(true); if let Some(env_vars) = env { for (k, v) in env_vars { @@ -79,7 +151,7 @@ pub async fn run_cli_with_env( } } - let output = cmd.output().await.map_err(|e| { + let mut child = cmd.spawn().map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { binary.not_found_error() } else { @@ -87,20 +159,104 @@ pub async fn run_cli_with_env( } })?; - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - let exit_code = output.status.code().unwrap_or(-1); + let stdout = child.stdout.take().expect("stdout was piped"); + let stderr = child.stderr.take().expect("stderr was piped"); + + let stdout_task = tokio::spawn(drain_stream( + stdout, + OutputStream::Stdout, + on_chunk.clone(), + )); + let stderr_task = tokio::spawn(drain_stream( + stderr, + OutputStream::Stderr, + on_chunk.clone(), + )); + + // Wait for either: child exits, or cancellation fires. + let status = tokio::select! { + biased; + _ = cancel.cancelled() => { + tracing::warn!( + bin = binary.command_name(), + "cancellation received; reaping process tree" + ); + // SIGTERM the entire descendant tree (including any + // in-container processes spawned via `devcontainer exec` + // → `docker exec` → containerd-shim). Container PID + // namespacing hides PIDs from each other but not from + // the host, so /proc walking sees through it. + if let Some(root_pid) = child.id() { + crate::process_tree::reap(root_pid, std::time::Duration::from_secs(2)).await; + } + // The root child should now be dead; wait briefly to + // collect its exit status and let the reader tasks observe + // EOF on stdout/stderr. `kill_on_drop(true)` is our + // backstop if any of this fails. + let _ = tokio::time::timeout( + std::time::Duration::from_secs(1), + child.wait(), + ) + .await; + let _ = stdout_task.await; + let _ = stderr_task.await; + return Err(Error::Cancelled); + } + status = child.wait() => status?, + }; + + // Child exited; collect the accumulated output. + let stdout_buf = stdout_task.await.unwrap_or_default(); + let stderr_buf = stderr_task.await.unwrap_or_default(); + let exit_code = status.code().unwrap_or(-1); let json = if parse_json { - serde_json::from_str(&stdout).ok() + serde_json::from_str(&stdout_buf).ok() } else { None }; Ok(CliOutput { exit_code, - stdout, - stderr, + stdout: stdout_buf, + stderr: stderr_buf, json, }) } + +/// Read `reader` line-by-line, forwarding each line to `sink` (if set) +/// and accumulating into the returned buffer. EOF or error ends the loop. +async fn drain_stream( + reader: R, + stream: OutputStream, + sink: Option>, +) -> String { + let mut lines = BufReader::new(reader).lines(); + let mut buf = String::new(); + loop { + match lines.next_line().await { + Ok(Some(line)) => { + if !buf.is_empty() { + buf.push('\n'); + } + buf.push_str(&line); + if let Some(sink) = sink.as_ref() { + sink.on_chunk(OutputChunk { + stream, + line, + }) + .await; + } + } + Ok(None) => break, + Err(e) => { + tracing::debug!(%e, ?stream, "error reading child output stream"); + break; + } + } + } + if !buf.is_empty() { + buf.push('\n'); + } + buf +} diff --git a/crates/devcontainer-mcp-core/src/devcontainer.rs b/crates/devcontainer-mcp-core/src/devcontainer.rs index 167fc0d..cb0dd23 100644 --- a/crates/devcontainer-mcp-core/src/devcontainer.rs +++ b/crates/devcontainer-mcp-core/src/devcontainer.rs @@ -1,6 +1,8 @@ -use crate::cli::{run_cli, CliBinary, CliOutput}; +use crate::cli::{run_cli, ChunkSink, CliBinary, CliOutput}; use crate::docker; use crate::error::Result; +use std::sync::Arc; +use tokio_util::sync::CancellationToken; /// Run a `devcontainer` CLI command. async fn run_devcontainer(args: &[&str], parse_json: bool) -> Result { @@ -37,6 +39,265 @@ pub async fn exec( run_devcontainer(&args, false).await } +/// `devcontainer exec` — cancellable, streaming variant. +/// +/// `cancel` is honored at any point during the child's lifetime; if it +/// fires, every descendant inside the container is reaped via a docker +/// exec-side `kill -- -` against the process group the shim +/// established. `on_chunk`, if supplied, receives every line of +/// stdout/stderr as the child emits it — typically wired to an MCP +/// progress notification on the server side. +/// +/// This function differs from the generic [`crate::cli::run_cli_streaming`] +/// in one important way: container descendants are not in the host PID +/// namespace lineage of `devcontainer exec` (the docker daemon +/// reparents them under containerd-shim), so a `/proc` walk on the host +/// would miss them. We install a tiny `setsid` + sentinel shim around +/// the user command and use the captured PGID to reap them on cancel. +pub async fn exec_streaming( + workspace_folder: &str, + command: &str, + command_args: &[&str], + cancel: &CancellationToken, + on_chunk: Option>, +) -> Result { + use crate::cli::{OutputChunk, OutputStream}; + use std::process::Stdio; + use std::sync::Mutex; + use tokio::io::{AsyncBufReadExt, BufReader}; + use tokio::process::Command; + + // Build the user-side command string. The MCP handler invokes us + // as `("sh", &["-c", &full_cmd])`, which is the fast path: peel + // off the `-c` arg so the shim wraps the body directly instead of + // nesting an extra shell. For any other shape, fall back to a + // best-effort shell-quoted concatenation. + let user_cmd: String = if command == "sh" && command_args.len() == 2 && command_args[0] == "-c" + { + command_args[1].to_string() + } else { + let mut parts = vec![crate::file_ops::shell_quote(command)]; + for a in command_args { + parts.push(crate::file_ops::shell_quote(a)); + } + parts.join(" ") + }; + + let wrapped = crate::exec_shim::wrap(); + // The user command is passed to the shim via an env var so it + // doesn't need to be quoted into a nested `sh -c '...'`. We use + // `--remote-env NAME=VALUE` so the devcontainer CLI propagates + // it inside the container. + let remote_env_arg = format!("{}={}", crate::exec_shim::USER_CMD_ENV, user_cmd); + + // Spawn `devcontainer exec --workspace-folder --remote-env … sh -c `. + let mut cmd = Command::new(crate::cli::CliBinary::Devcontainer.command_name()); + cmd.args([ + "exec", + "--workspace-folder", + workspace_folder, + "--remote-env", + &remote_env_arg, + "sh", + "-c", + &wrapped, + ]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true); + + let mut child = cmd.spawn().map_err(|e| { + if e.kind() == std::io::ErrorKind::NotFound { + crate::error::Error::DevcontainerCliNotFound + } else { + crate::error::Error::Io(e) + } + })?; + + let stdout = child.stdout.take().expect("stdout piped"); + let stderr = child.stderr.take().expect("stderr piped"); + + // Shared captured PGID. The stderr reader fills this in as soon as + // the shim's sentinel line arrives; the cancel branch reads it. + let pgid: Arc>> = Arc::new(Mutex::new(None)); + + // stdout: forward verbatim (no sentinel to scrub). + let stdout_task = { + let sink = on_chunk.clone(); + tokio::spawn(async move { + let mut buf = String::new(); + let mut lines = BufReader::new(stdout).lines(); + while let Ok(Some(line)) = lines.next_line().await { + if !buf.is_empty() { + buf.push('\n'); + } + buf.push_str(&line); + if let Some(s) = sink.as_ref() { + s.on_chunk(OutputChunk { + stream: OutputStream::Stdout, + line, + }) + .await; + } + } + if !buf.is_empty() { + buf.push('\n'); + } + buf + }) + }; + + // stderr: scrub the sentinel line, otherwise forward verbatim. + let stderr_task = { + let sink = on_chunk.clone(); + let pgid = pgid.clone(); + tokio::spawn(async move { + let mut buf = String::new(); + let mut lines = BufReader::new(stderr).lines(); + while let Ok(Some(line)) = lines.next_line().await { + if let Some(found) = crate::exec_shim::try_parse_sentinel(&line) { + *pgid.lock().expect("pgid lock") = Some(found); + tracing::debug!(pgid = found, "captured in-container PGID"); + continue; // suppress sentinel from user-visible stderr + } + if !buf.is_empty() { + buf.push('\n'); + } + buf.push_str(&line); + if let Some(s) = sink.as_ref() { + s.on_chunk(OutputChunk { + stream: OutputStream::Stderr, + line, + }) + .await; + } + } + if !buf.is_empty() { + buf.push('\n'); + } + buf + }) + }; + + let status = tokio::select! { + biased; + _ = cancel.cancelled() => { + tracing::warn!("devcontainer exec: cancellation received"); + + // Kill the in-container process group first, then the host + // wrapper. Order matters: if we kill the host wrapper first, + // docker exec closes its stdio and we lose the channel — + // but the in-container processes keep running anyway. + let captured = *pgid.lock().expect("pgid lock"); + match captured { + Some(pgid) => { + if let Err(e) = kill_in_container_pgid(workspace_folder, pgid).await { + tracing::warn!(%e, "failed to kill in-container PGID"); + } + } + None => { + // Sentinel never arrived — either the shim hasn't + // emitted yet (very fast cancel) or `setsid` isn't + // available and the fallback path didn't emit + // before we cancelled. Either way, host-side reap + // is our only lever. + tracing::warn!( + "no in-container PGID captured; relying on host reap (may leak)" + ); + } + } + + // Now bring down the host wrapper and any of its host-side + // descendants we *can* see. + if let Some(root_pid) = child.id() { + crate::process_tree::reap(root_pid, std::time::Duration::from_secs(2)).await; + } + + let _ = tokio::time::timeout( + std::time::Duration::from_secs(1), + child.wait(), + ) + .await; + let _ = stdout_task.await; + let _ = stderr_task.await; + return Err(crate::error::Error::Cancelled); + } + status = child.wait() => status?, + }; + + let stdout_buf = stdout_task.await.unwrap_or_default(); + let stderr_buf = stderr_task.await.unwrap_or_default(); + + Ok(CliOutput { + exit_code: status.code().unwrap_or(-1), + stdout: stdout_buf, + stderr: stderr_buf, + json: None, + }) +} + +/// Send SIGTERM (then SIGKILL after a 2s grace) to the in-container +/// process group `pgid`. Uses bollard's exec API so it works against the +/// docker daemon without shelling out. +async fn kill_in_container_pgid(workspace_folder: &str, pgid: i32) -> Result<()> { + use bollard::exec::{CreateExecOptions, StartExecOptions}; + + let client = docker::connect()?; + let container = docker::find_container_by_local_folder(&client, workspace_folder) + .await? + .ok_or_else(|| { + crate::error::Error::Io(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("No devcontainer found for workspace: {workspace_folder}"), + )) + })?; + + // `kill - -` signals every process in the group. We + // deliberately omit the POSIX `--` argument separator because + // BusyBox/dash `kill` (common in slim container images) rejects + // it as "Illegal number". The form `kill -TERM -N` is accepted + // by every shell `kill` we care about. + let run = |sig: &'static str| -> futures_util::future::BoxFuture<'static, ()> { + let client = client.clone(); + let container_id = container.id.clone(); + let cmd = format!("kill -{sig} -{pgid} 2>/dev/null || true"); + Box::pin(async move { + let create = CreateExecOptions { + cmd: Some(vec!["sh".to_string(), "-c".to_string(), cmd]), + attach_stdout: Some(false), + attach_stderr: Some(false), + ..Default::default() + }; + match client.create_exec(&container_id, create).await { + Ok(res) => { + // `detach: true` makes start_exec return as soon as + // the docker daemon has spawned the kill — we don't + // need to stream its (empty) output. The signal will + // be delivered before we proceed to the next step. + let opts = StartExecOptions { + detach: true, + ..Default::default() + }; + if let Err(e) = client.start_exec(&res.id, Some(opts)).await { + tracing::debug!(%e, "start_exec for in-container kill failed"); + } + } + Err(e) => { + tracing::debug!(%e, "create_exec for in-container kill failed"); + } + } + }) + }; + + tracing::debug!(pgid, "in-container SIGTERM -"); + run("TERM").await; + tokio::time::sleep(std::time::Duration::from_secs(2)).await; + tracing::debug!(pgid, "in-container SIGKILL -"); + run("KILL").await; + Ok(()) +} + /// `devcontainer build` — build a dev container image. pub async fn build(workspace_folder: &str, extra_args: &[&str]) -> Result { let mut args = vec!["build", "--workspace-folder", workspace_folder]; diff --git a/crates/devcontainer-mcp-core/src/error.rs b/crates/devcontainer-mcp-core/src/error.rs index ec3105c..2392eab 100644 --- a/crates/devcontainer-mcp-core/src/error.rs +++ b/crates/devcontainer-mcp-core/src/error.rs @@ -36,6 +36,9 @@ pub enum Error { #[error("File edit error: {0}")] FileEdit(String), + #[error("Operation cancelled by peer")] + Cancelled, + #[error("IO error: {0}")] Io(#[from] std::io::Error), diff --git a/crates/devcontainer-mcp-core/src/exec_shim.rs b/crates/devcontainer-mcp-core/src/exec_shim.rs new file mode 100644 index 0000000..9c749e9 --- /dev/null +++ b/crates/devcontainer-mcp-core/src/exec_shim.rs @@ -0,0 +1,166 @@ +//! In-target process-group shim for cancellable cross-boundary execs. +//! +//! When we run a command across a process boundary (host → container via +//! `docker exec`, host → VM via SSH, …) we lose direct ancestry control: +//! the target-side processes do not appear as descendants of any host PID +//! we own. SIGTERM'ing our host-side wrapper closes the stdio pipes but +//! leaves the actual workload running, reparented to the target's init. +//! +//! To regain control we wrap the user's command with a tiny shim that: +//! 1. Runs the user command in a fresh session (`setsid`), so the +//! leaf shell's PID is also its process group ID (PGID), and the +//! PGID transitively addresses every descendant. +//! 2. Emits a recognizable sentinel on stderr containing that PGID, +//! so the host-side reader can capture it. +//! +//! On cancellation, the host sends `kill -- -` *inside the target* +//! (via the backend's exec channel — `docker exec`, `devpod ssh`, etc.), +//! which reaps the entire process group atomically. + +/// Marker that brackets the PGID line so we can find and strip it from +/// stderr without false positives from user output. +const SENTINEL_PREFIX: &str = "__DCMCP_PGID="; +const SENTINEL_SUFFIX: &str = "__"; + +/// Wrap a user-supplied shell command with the shim prelude. +/// +/// The returned string is intended to be passed as a single argument to +/// `sh -c` (or any POSIX shell) on the target. It expects to find the +/// user's command in the environment variable [`USER_CMD_ENV`] (see +/// [`USER_CMD_ENV`] for the name) — passing it that way avoids the +/// quoting hazards of nesting `sh -c '...'` inside another `sh -c '...'` +/// when the user command itself contains single quotes, parentheses, or +/// other shell metacharacters. +/// +/// The shim runs the user command via `eval`, in a fresh session +/// (`setsid`) so the leaf shell's PID is also its process group ID +/// (PGID), and prints `__DCMCP_PGID=__` to stderr once for the +/// host-side reader to capture. +pub fn wrap() -> String { + // We deliberately keep this string free of any user-supplied + // content. Every variable expansion comes from the target shell's + // environment, not from string interpolation on the host. + // + // We background the `setsid` child and `wait` on it instead of + // using `setsid -w` because the `-w` flag is util-linux–only: + // busybox/alpine images ship a `setsid` without it. Backgrounding + // + `wait` is POSIX and works on every shell we care about. + // + // The trailing `exit "$?"` propagates the inner program's exit + // code back up to the outer `sh -c` so `devcontainer exec` + // returns the correct status. + format!( + r#"if command -v setsid >/dev/null 2>&1; then + setsid sh -c 'printf "{prefix}%s{suffix}\n" "$$" 1>&2; eval "${env}"' & + __dcmcp_pid=$! + wait "$__dcmcp_pid" + exit "$?" +else + printf "{prefix}%s{suffix}\n" "$$" 1>&2 + eval "${env}" +fi"#, + prefix = SENTINEL_PREFIX, + suffix = SENTINEL_SUFFIX, + env = USER_CMD_ENV, + ) +} + +/// Environment variable through which the wrapped shim receives the +/// user command. Callers must set this in the spawned process's env +/// before invoking the shim string. +pub const USER_CMD_ENV: &str = "DCMCP_USER_CMD"; + +/// If `line` is the shim's sentinel, return the captured PGID. +/// Otherwise return `None`. The caller should suppress matching lines +/// from any downstream output. +pub fn try_parse_sentinel(line: &str) -> Option { + // Match `^\s*__DCMCP_PGID=(\d+)__\s*$`. A hand-rolled scan keeps + // the dependency footprint at zero. + let trimmed = line.trim(); + let rest = trimmed.strip_prefix(SENTINEL_PREFIX)?; + let num = rest.strip_suffix(SENTINEL_SUFFIX)?; + if num.is_empty() || !num.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + num.parse().ok() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn wrap_emits_sentinel_and_runs_user_cmd() { + // Smoke test: pass user cmd via env, confirm stdout has the + // user output and stderr has the sentinel. + let wrapped = wrap(); + let out = std::process::Command::new("sh") + .arg("-c") + .arg(&wrapped) + .env(USER_CMD_ENV, "echo hello") + .output() + .expect("spawn sh"); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stdout.contains("hello"), "stdout: {stdout:?}"); + assert!( + stderr.lines().any(|l| try_parse_sentinel(l).is_some()), + "no sentinel in stderr: {stderr:?}" + ); + } + + #[test] + fn wrap_handles_metacharacters_in_user_cmd() { + // The whole reason wrap() reads from env: user commands with + // single quotes, parens, subshells, etc. must not break the + // shim's own quoting. + let wrapped = wrap(); + let evil = r#"(echo "it's a (test)" & echo $((1+2)) ) | tr a-z A-Z"#; + let out = std::process::Command::new("sh") + .arg("-c") + .arg(&wrapped) + .env(USER_CMD_ENV, evil) + .output() + .expect("spawn sh"); + assert!(out.status.success(), "stderr: {:?}", String::from_utf8_lossy(&out.stderr)); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("IT'S A (TEST)"), "stdout: {stdout:?}"); + assert!(stdout.contains("3"), "stdout missing arithmetic: {stdout:?}"); + } + + #[test] + fn try_parse_sentinel_matches() { + assert_eq!(try_parse_sentinel("__DCMCP_PGID=42__"), Some(42)); + assert_eq!(try_parse_sentinel(" __DCMCP_PGID=12345__ "), Some(12345)); + } + + #[test] + fn try_parse_sentinel_rejects_garbage() { + assert_eq!(try_parse_sentinel("hello"), None); + assert_eq!(try_parse_sentinel("__DCMCP_PGID=abc__"), None); + assert_eq!(try_parse_sentinel("PGID=42"), None); + // Don't accept it embedded in other text: + assert_eq!(try_parse_sentinel("prefix __DCMCP_PGID=42__"), None); + } + + #[test] + fn sentinel_pgid_actually_addresses_descendants() { + // Run a sleep in the background under the shim and verify the + // emitted PGID > 0 (round-trip parse). We can't safely kill + // the group here without affecting the test runner, so the + // end-to-end kill is tested by the cli integration test. + let wrapped = wrap(); + let out = std::process::Command::new("sh") + .arg("-c") + .arg(&wrapped) + .env(USER_CMD_ENV, "sleep 0.2 & wait") + .output() + .expect("spawn"); + let stderr = String::from_utf8_lossy(&out.stderr); + let pgid = stderr + .lines() + .find_map(try_parse_sentinel) + .expect("sentinel emitted"); + assert!(pgid > 0); + } +} diff --git a/crates/devcontainer-mcp-core/src/file_ops.rs b/crates/devcontainer-mcp-core/src/file_ops.rs index 93e98e4..1f24740 100644 --- a/crates/devcontainer-mcp-core/src/file_ops.rs +++ b/crates/devcontainer-mcp-core/src/file_ops.rs @@ -53,12 +53,17 @@ pub fn apply_edit(content: &str, old_str: &str, new_str: &str) -> Result } /// Shell-escape a string for safe embedding in a shell command. -fn quote(s: &str) -> String { +pub fn shell_quote(s: &str) -> String { shlex::try_quote(s) .unwrap_or(std::borrow::Cow::Borrowed(s)) .into_owned() } +/// Convenience alias for module-local use. +fn quote(s: &str) -> String { + shell_quote(s) +} + /// Build a shell command that reads a file via `cat`. pub fn read_file_command(path: &str) -> String { format!("cat {}", quote(path)) diff --git a/crates/devcontainer-mcp-core/src/lib.rs b/crates/devcontainer-mcp-core/src/lib.rs index 3189d28..7c7bcf1 100644 --- a/crates/devcontainer-mcp-core/src/lib.rs +++ b/crates/devcontainer-mcp-core/src/lib.rs @@ -5,4 +5,6 @@ pub mod devcontainer; pub mod devpod; pub mod docker; pub mod error; +pub mod exec_shim; pub mod file_ops; +pub mod process_tree; diff --git a/crates/devcontainer-mcp-core/src/process_tree.rs b/crates/devcontainer-mcp-core/src/process_tree.rs new file mode 100644 index 0000000..0b46507 --- /dev/null +++ b/crates/devcontainer-mcp-core/src/process_tree.rs @@ -0,0 +1,215 @@ +//! Process-tree reaping. +//! +//! When a CLI we spawned crosses a process boundary (e.g. `devcontainer +//! exec` → `docker exec` → containerd-shim → user command inside the +//! container), simply SIGTERM'ing our immediate child does not kill the +//! actual work — the in-container processes are reparented to the +//! container's PID 1 and keep running. They consume CPU, hold locks, +//! and never exit. +//! +//! On Linux, every process in any namespace still has a host-visible PID +//! in the root PID namespace, and `/proc//status` exposes its parent +//! as `PPid:`. So from our root child PID we can BFS the full set of +//! descendants and terminate them ourselves — no in-container shim, no +//! cooperation from the backend CLI required. +//! +//! Non-Linux platforms get a no-op fallback that only signals the root +//! PID; this is fine because dev containers are a Linux-host feature. + +use std::time::Duration; + +/// Reap a process tree rooted at `root_pid`. +/// +/// 1. Discover every descendant of `root_pid` (including `root_pid` itself) +/// by walking `/proc/*/status`. +/// 2. Send `SIGTERM` to all of them. +/// 3. Wait up to `grace` for them to exit. +/// 4. Send `SIGKILL` to any survivors. +/// +/// Best-effort: any individual `/proc` read or `kill(2)` is allowed to +/// fail silently (the process may have already exited). The function +/// returns when the grace period is up; it does not guarantee every PID +/// is gone (a process can ignore SIGKILL only if it is uninterruptible +/// in the kernel, which is out of our control). +pub async fn reap(root_pid: u32, grace: Duration) { + #[cfg(target_os = "linux")] + { + reap_linux(root_pid, grace).await; + } + #[cfg(not(target_os = "linux"))] + { + // Fallback: signal the root only. Container reaping requires + // /proc-style introspection which we don't have here. + let _ = (root_pid, grace); + #[cfg(unix)] + unsafe { + libc::kill(root_pid as i32, libc::SIGTERM); + tokio::time::sleep(grace).await; + libc::kill(root_pid as i32, libc::SIGKILL); + } + } +} + +#[cfg(target_os = "linux")] +async fn reap_linux(root_pid: u32, grace: Duration) { + // Snapshot the full descendant set. We do this once, send SIGTERM, + // wait, then re-discover for SIGKILL (new grandchildren may have + // appeared during the grace period — e.g. a `go test` runner that + // spawned its own test binaries while we were waiting). + let initial = collect_descendants(root_pid); + tracing::debug!( + root = root_pid, + n = initial.len(), + "reap: SIGTERM descendants" + ); + signal_all(&initial, libc_signal::SIGTERM); + + // Wait for graceful exit. Short-circuit if everyone is already gone. + let deadline = std::time::Instant::now() + grace; + while std::time::Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(100)).await; + if !any_alive(&initial) { + tracing::debug!(root = root_pid, "reap: all descendants exited cleanly"); + return; + } + } + + // Re-discover and SIGKILL anything still around. We re-walk /proc + // because the surviving tree may have grown. + let survivors = collect_descendants(root_pid); + if !survivors.is_empty() { + tracing::warn!( + root = root_pid, + n = survivors.len(), + "reap: SIGKILL survivors" + ); + signal_all(&survivors, libc_signal::SIGKILL); + } +} + +#[cfg(target_os = "linux")] +fn collect_descendants(root: u32) -> Vec { + // Build a parent → children map by scanning /proc/*/status. This is + // O(N) where N is the number of processes on the host. For typical + // dev machines (a few hundred procs) this is sub-millisecond. + use std::collections::HashMap; + + let mut children_of: HashMap> = HashMap::new(); + let Ok(entries) = std::fs::read_dir("/proc") else { + return vec![root]; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(name_str) = name.to_str() else { continue }; + let Ok(pid) = name_str.parse::() else { continue }; + if let Some(ppid) = read_ppid(pid) { + children_of.entry(ppid).or_default().push(pid); + } + } + + // BFS from `root`. Include `root` itself so the caller can kill the + // whole tree in one pass. + let mut out = Vec::new(); + let mut stack = vec![root]; + while let Some(pid) = stack.pop() { + out.push(pid); + if let Some(kids) = children_of.get(&pid) { + stack.extend_from_slice(kids); + } + } + out +} + +#[cfg(target_os = "linux")] +fn read_ppid(pid: u32) -> Option { + // `/proc//status` is line-oriented. We only need the `PPid:` + // line, which appears near the top, so reading the whole file is + // wasteful but trivial in size. + let status = std::fs::read_to_string(format!("/proc/{pid}/status")).ok()?; + for line in status.lines() { + if let Some(rest) = line.strip_prefix("PPid:") { + return rest.trim().parse().ok(); + } + } + None +} + +#[cfg(unix)] +fn signal_all(pids: &[u32], sig: i32) { + for &pid in pids { + // kill(2) can fail with ESRCH if the process already exited; + // EPERM if we don't own it (shouldn't happen for our descendants + // unless they did a setuid). Either way, swallow. + unsafe { + libc::kill(pid as i32, sig); + } + } +} + +#[cfg(not(unix))] +fn signal_all(_: &[u32], _: i32) {} + +#[cfg(target_os = "linux")] +fn any_alive(pids: &[u32]) -> bool { + pids.iter().any(|&pid| { + // kill(pid, 0) probes existence + permission without sending a + // signal. Returns 0 if the process exists and we can signal it. + unsafe { libc::kill(pid as i32, 0) == 0 } + }) +} + +#[cfg(unix)] +mod libc_signal { + pub const SIGTERM: i32 = libc::SIGTERM; + pub const SIGKILL: i32 = libc::SIGKILL; +} + +#[cfg(not(unix))] +mod libc_signal { + pub const SIGTERM: i32 = 15; + pub const SIGKILL: i32 = 9; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(target_os = "linux")] + #[test] + fn read_ppid_for_self() { + let pid = std::process::id(); + let ppid = read_ppid(pid).expect("self has a PPid"); + // Whatever launched the test runner. + assert!(ppid > 0); + } + + #[cfg(target_os = "linux")] + #[test] + fn descendants_includes_root() { + let pid = std::process::id(); + let set = collect_descendants(pid); + assert!(set.contains(&pid)); + } + + #[cfg(target_os = "linux")] + #[tokio::test] + async fn reap_kills_a_child_tree() { + // Spawn `sh -c 'sleep 30 & sleep 30 & wait'` and reap it. + let mut child = tokio::process::Command::new("sh") + .args(["-c", "sleep 30 & sleep 30 & wait"]) + .kill_on_drop(true) + .spawn() + .expect("spawn"); + let root = child.id().expect("pid"); + // Let the grandchildren actually exist before we reap. + tokio::time::sleep(Duration::from_millis(200)).await; + let before = collect_descendants(root); + assert!(before.len() >= 3, "expected root + 2 sleeps, got {before:?}"); + + reap(root, Duration::from_secs(1)).await; + // Give the kernel a moment to actually reap zombies. + let _ = child.wait().await; + tokio::time::sleep(Duration::from_millis(100)).await; + assert!(!any_alive(&before), "some descendants survived: {before:?}"); + } +} diff --git a/crates/devcontainer-mcp/Cargo.toml b/crates/devcontainer-mcp/Cargo.toml index 4770732..46acee6 100644 --- a/crates/devcontainer-mcp/Cargo.toml +++ b/crates/devcontainer-mcp/Cargo.toml @@ -18,3 +18,5 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true } schemars = "1" shlex = "1" +async-trait = "0.1" +tokio-util = "0.7" diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs index 763012b..a381c99 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs @@ -1,6 +1,12 @@ +use std::sync::Arc; + +use devcontainer_mcp_core::cli::{ChunkSink, OutputChunk, OutputStream}; use devcontainer_mcp_core::devcontainer; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::{Meta, ProgressNotificationParam, ProgressToken}; +use rmcp::service::{Peer, RequestContext}; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; use crate::tools::common::format_output; use crate::tools::DevContainerMcp; @@ -15,6 +21,41 @@ struct DevcontainerExecParams { args: Option, } +/// Forwards every line of child output to the MCP peer as a +/// `notifications/progress` message. Used to keep the wire warm during +/// long-running execs (e.g. `go test -race`) so client-side watchdogs +/// don't tear down the transport. +struct ProgressChunkSink { + peer: Peer, + progress_token: ProgressToken, + /// Monotonic counter so each notification has a strictly-increasing + /// `progress` value, as required by the MCP spec. + counter: std::sync::atomic::AtomicU64, +} + +#[async_trait::async_trait] +impl ChunkSink for ProgressChunkSink { + async fn on_chunk(&self, chunk: OutputChunk) { + let n = self + .counter + .fetch_add(1, std::sync::atomic::Ordering::Relaxed) + + 1; + let prefix = match chunk.stream { + OutputStream::Stdout => "stdout", + OutputStream::Stderr => "stderr", + }; + let param = ProgressNotificationParam::new(self.progress_token.clone(), n as f64) + .with_message(format!("{prefix}: {}", chunk.line)); + if let Err(e) = self.peer.notify_progress(param).await { + // Sink errors are non-fatal: the peer may have disconnected + // or simply not subscribed to progress. We still want the + // child to keep running and produce a final response (which + // may or may not get delivered). + tracing::debug!(%e, "failed to send progress notification"); + } + } +} + #[tool_router(router = devcontainer_exec_router, vis = "pub(super)")] impl DevContainerMcp { #[tool( @@ -24,14 +65,44 @@ impl DevContainerMcp { async fn devcontainer_exec( &self, Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, ) -> String { let full_cmd = match ¶ms.args { Some(a) => format!("{} {}", params.command, a), None => params.command, }; - match devcontainer::exec(¶ms.workspace_folder, "sh", &["-c", &full_cmd]).await { + + // If the client supplied a progressToken in _meta, stream + // stdout/stderr to it as progress notifications. Otherwise just + // run with cancellation but no streaming. + let sink: Option> = meta.get_progress_token().map(|token| { + Arc::new(ProgressChunkSink { + peer: peer.clone(), + progress_token: token, + counter: std::sync::atomic::AtomicU64::new(0), + }) as Arc + }); + + match devcontainer::exec_streaming( + ¶ms.workspace_folder, + "sh", + &["-c", &full_cmd], + &ct, + sink, + ) + .await + { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } } } + +// `RequestContext` is not strictly required here — `CancellationToken`, +// `Peer`, and `Meta` are extracted directly by the tool_router +// macro via their respective `FromContextPart` impls — but importing it +// once keeps it discoverable for future tool authors. +#[allow(dead_code)] +fn _request_context_marker(_: RequestContext) {} From 89d8694d229f9cfb895adda152b700400ecc73c4 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 23 May 2026 14:10:29 -0700 Subject: [PATCH 13/19] devpod + codespaces ssh: stream output, support cancellation, reap remote PGID Extends the shim-based streaming + cancellation work to the two SSH-based exec backends. The mechanics are identical to the devcontainer backend (capture PGID via stderr sentinel, kill the remote process group on cancel) but two pieces had to change: 1. Neither `devpod ssh` nor `gh codespace ssh` has a `--remote-env` equivalent. We can't deliver `DCMCP_USER_CMD` through an env var. Added `exec_shim::wrap_self_contained(user_cmd)` which embeds the user command as a base64 blob inside the shell snippet itself. The wrapped string is shell-safe regardless of what's in the user command -- the encode/decode round-trip happens once, with no intermediate shell re-interpreting quoting. Verified against pathological inputs (single quotes, double quotes, backticks, subshells, arithmetic expansion, newlines). 2. Killing the remote PGID can't reuse the original SSH session (it's busy running the workload). We spawn a second short-lived `devpod ssh --command "kill -TERM -"` / `gh codespace ssh -c -- "kill -TERM -"` for each kill. Slower than the bollard path used by devcontainer (each kill is its own SSH handshake) but cancellation is rare and correctness wins over latency. Restructuring: - `cli::run_with_shim` -- new public helper that owns the spawn-stream-capture-cancel dance. Takes a `&dyn RemoteKiller` trait object so each backend supplies its own "send a kill into the target" implementation. Devcontainer, devpod, and codespaces all funnel through this one path; the only backend-specific code left is building the argv and constructing the killer. - `cli::RemoteKiller` -- new trait. Two methods isn't needed; one is enough (`async fn kill_pgid(&self, pgid: i32, signal: &str)`). Implementations: `DevcontainerKiller` (bollard create_exec), `DevpodKiller` (devpod ssh), `CodespaceKiller` (gh codespace ssh). - `devcontainer::exec_streaming` -- collapsed from ~200 lines of inline spawn/stream/cancel code to ~40 lines that build args + killer and call `run_with_shim`. Net diff is -130 lines because the shared helper replaces three copies of the same logic. New tests: - `exec_shim::wrap_self_contained_runs_user_cmd_with_no_env` -- smoke test: command body via PATH, no env at all. - `exec_shim::wrap_self_contained_preserves_arbitrary_bytes` -- pathological user command (single quotes, parens, subshells, arithmetic, newlines, backslashes) round-trips correctly. - `cli::shim_runner_tests::run_with_shim_invokes_killer_in_order_on_cancel` -- end-to-end through `run_with_shim` using a `devpod` PATH shim that proxies to local `sh`. Asserts: cancellation triggers a TERM then KILL on the same captured PGID, the call returns `Error::Cancelled`, the sentinel line is scrubbed from the sink. Regression matrix (all still passing after refactor): - devcontainer in-container nested-sleep tree is reaped (5 -> 0) - progress notifications arrive live during 3s exec (5 notifs) - cancellation returns `Error: Operation cancelled by peer` in ~5s - 3x parallel `go test -race -v` / `go vet` / `go build -a` in 19s - tools/list still registers devpod_ssh and codespaces_ssh with the correct public param schemas (CancellationToken/Peer/Meta hidden) DevPod and Codespaces backends were verified by: - unit-level via the PATH-shim end-to-end test (run_with_shim flow + RemoteKiller invocation order) - inspection: the only difference between the three backends is the kill transport, and all three follow the same shape. Live verification of DevPod and Codespaces cancellation against real targets is still pending -- needs accounts/setup -- but the shared helper path is the same code that already works against devcontainer. --- Cargo.lock | 253 +++++++++++++ crates/devcontainer-mcp-core/Cargo.toml | 3 + crates/devcontainer-mcp-core/src/cli.rs | 348 ++++++++++++++++++ .../devcontainer-mcp-core/src/codespaces.rs | 64 +++- .../devcontainer-mcp-core/src/devcontainer.rs | 300 +++++---------- crates/devcontainer-mcp-core/src/devpod.rs | 86 ++++- crates/devcontainer-mcp-core/src/exec_shim.rs | 70 ++++ .../src/tools/codespaces/ssh.rs | 62 +++- .../devcontainer-mcp/src/tools/devpod/ssh.rs | 61 ++- 9 files changed, 1027 insertions(+), 220 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7689f09..3d2552c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -324,6 +324,7 @@ dependencies = [ "serde", "serde_json", "shlex", + "tempfile", "thiserror", "tokio", "tokio-util", @@ -363,12 +364,24 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "fastrand" +version = "2.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" + [[package]] name = "find-msvc-tools" version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "form_urlencoded" version = "1.2.2" @@ -466,12 +479,34 @@ dependencies = [ "slab", ] +[[package]] +name = "getrandom" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0de51e6874e94e7bf76d726fc5d13ba782deca734ff60d5bb2fb2607c7406555" +dependencies = [ + "cfg-if", + "libc", + "r-efi", + "wasip2", + "wasip3", +] + [[package]] name = "hashbrown" version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +[[package]] +name = "hashbrown" +version = "0.15.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" +dependencies = [ + "foldhash", +] + [[package]] name = "hashbrown" version = "0.17.0" @@ -712,6 +747,12 @@ dependencies = [ "zerovec", ] +[[package]] +name = "id-arena" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d3067d79b975e8844ca9eb072e16b31c3c1c36928edf9c6789548c524d0d954" + [[package]] name = "ident_case" version = "1.0.1" @@ -790,12 +831,24 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" +[[package]] +name = "leb128fmt" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" + [[package]] name = "libc" version = "0.2.185" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "52ff2c0fe9bc6cb6b14a0592c2ff4fa9ceb83eea9db979b0487cd054946a2b8f" +[[package]] +name = "linux-raw-sys" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" + [[package]] name = "litemap" version = "0.8.2" @@ -935,6 +988,16 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" +[[package]] +name = "prettyplease" +version = "0.2.37" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "479ca8adacdd7ce8f1fb39ce9ecccbfe93a3f1344b3d0d97f20bc0196208f62b" +dependencies = [ + "proc-macro2", + "syn", +] + [[package]] name = "proc-macro2" version = "1.0.106" @@ -953,6 +1016,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r-efi" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" + [[package]] name = "redox_syscall" version = "0.5.18" @@ -1034,6 +1103,19 @@ dependencies = [ "syn", ] +[[package]] +name = "rustix" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys", +] + [[package]] name = "rustversion" version = "1.0.22" @@ -1090,6 +1172,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "semver" +version = "1.0.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a7852d02fc848982e0c167ef163aaff9cd91dc640ba85e263cb1ce46fae51cd" + [[package]] name = "serde" version = "1.0.228" @@ -1266,6 +1354,19 @@ dependencies = [ "syn", ] +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom", + "once_cell", + "rustix", + "windows-sys", +] + [[package]] name = "thiserror" version = "2.0.18" @@ -1456,6 +1557,12 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "unicode-xid" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" + [[package]] name = "url" version = "2.5.8" @@ -1501,6 +1608,24 @@ version = "0.11.1+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" +[[package]] +name = "wasip2" +version = "1.0.3+wasi-0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20064672db26d7cdc89c7798c48a0fdfac8213434a1186e5ef29fd560ae223d6" +dependencies = [ + "wit-bindgen 0.57.1", +] + +[[package]] +name = "wasip3" +version = "0.4.0+wasi-0.3.0-rc-2026-01-06" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5428f8bf88ea5ddc08faddef2ac4a67e390b88186c703ce6dbd955e1c145aca5" +dependencies = [ + "wit-bindgen 0.51.0", +] + [[package]] name = "wasm-bindgen" version = "0.2.118" @@ -1546,6 +1671,40 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "wasm-encoder" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "990065f2fe63003fe337b932cfb5e3b80e0b4d0f5ff650e6985b1048f62c8319" +dependencies = [ + "leb128fmt", + "wasmparser", +] + +[[package]] +name = "wasm-metadata" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb0e353e6a2fbdc176932bbaab493762eb1255a7900fe0fea1a2f96c296cc909" +dependencies = [ + "anyhow", + "indexmap 2.14.0", + "wasm-encoder", + "wasmparser", +] + +[[package]] +name = "wasmparser" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47b807c72e1bac69382b3a6fb3dbe8ea4c0ed87ff5629b8685ae6b9a611028fe" +dependencies = [ + "bitflags", + "hashbrown 0.15.5", + "indexmap 2.14.0", + "semver", +] + [[package]] name = "winapi" version = "0.3.9" @@ -1636,6 +1795,100 @@ dependencies = [ "windows-link", ] +[[package]] +name = "wit-bindgen" +version = "0.51.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7249219f66ced02969388cf2bb044a09756a083d0fab1e566056b04d9fbcaa5" +dependencies = [ + "wit-bindgen-rust-macro", +] + +[[package]] +name = "wit-bindgen" +version = "0.57.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" + +[[package]] +name = "wit-bindgen-core" +version = "0.51.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea61de684c3ea68cb082b7a88508a8b27fcc8b797d738bfc99a82facf1d752dc" +dependencies = [ + "anyhow", + "heck", + "wit-parser", +] + +[[package]] +name = "wit-bindgen-rust" +version = "0.51.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c566e0f4b284dd6561c786d9cb0142da491f46a9fbed79ea69cdad5db17f21" +dependencies = [ + "anyhow", + "heck", + "indexmap 2.14.0", + "prettyplease", + "syn", + "wasm-metadata", + "wit-bindgen-core", + "wit-component", +] + +[[package]] +name = "wit-bindgen-rust-macro" +version = "0.51.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c0f9bfd77e6a48eccf51359e3ae77140a7f50b1e2ebfe62422d8afdaffab17a" +dependencies = [ + "anyhow", + "prettyplease", + "proc-macro2", + "quote", + "syn", + "wit-bindgen-core", + "wit-bindgen-rust", +] + +[[package]] +name = "wit-component" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d66ea20e9553b30172b5e831994e35fbde2d165325bec84fc43dbf6f4eb9cb2" +dependencies = [ + "anyhow", + "bitflags", + "indexmap 2.14.0", + "log", + "serde", + "serde_derive", + "serde_json", + "wasm-encoder", + "wasm-metadata", + "wasmparser", + "wit-parser", +] + +[[package]] +name = "wit-parser" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ecc8ac4bc1dc3381b7f59c34f00b67e18f910c2c0f50015669dde7def656a736" +dependencies = [ + "anyhow", + "id-arena", + "indexmap 2.14.0", + "log", + "semver", + "serde", + "serde_derive", + "serde_json", + "unicode-xid", + "wasmparser", +] + [[package]] name = "writeable" version = "0.6.3" diff --git a/crates/devcontainer-mcp-core/Cargo.toml b/crates/devcontainer-mcp-core/Cargo.toml index 3c32dbc..9a02857 100644 --- a/crates/devcontainer-mcp-core/Cargo.toml +++ b/crates/devcontainer-mcp-core/Cargo.toml @@ -21,3 +21,6 @@ shlex = "1" [target.'cfg(unix)'.dependencies] libc = "0.2" + +[dev-dependencies] +tempfile = "3" diff --git a/crates/devcontainer-mcp-core/src/cli.rs b/crates/devcontainer-mcp-core/src/cli.rs index 7c007c1..18e72c8 100644 --- a/crates/devcontainer-mcp-core/src/cli.rs +++ b/crates/devcontainer-mcp-core/src/cli.rs @@ -260,3 +260,351 @@ async fn drain_stream( } buf } + +/// Drain a stream that may contain the [`crate::exec_shim`] sentinel. +/// +/// Lines matching `__DCMCP_PGID=N__` are intercepted: `N` is stored +/// into `pgid_out` and the line is suppressed from both the sink and +/// the accumulated buffer. All other lines flow through normally. +async fn drain_stream_with_sentinel( + reader: R, + stream: OutputStream, + sink: Option>, + pgid_out: Arc>>, +) -> String { + let mut lines = BufReader::new(reader).lines(); + let mut buf = String::new(); + loop { + match lines.next_line().await { + Ok(Some(line)) => { + if let Some(found) = crate::exec_shim::try_parse_sentinel(&line) { + *pgid_out.lock().expect("pgid lock") = Some(found); + tracing::debug!(pgid = found, ?stream, "captured remote PGID"); + continue; + } + if !buf.is_empty() { + buf.push('\n'); + } + buf.push_str(&line); + if let Some(sink) = sink.as_ref() { + sink.on_chunk(OutputChunk { stream, line }).await; + } + } + Ok(None) => break, + Err(e) => { + tracing::debug!(%e, ?stream, "error reading child output stream"); + break; + } + } + } + if !buf.is_empty() { + buf.push('\n'); + } + buf +} + +/// Trait for a backend-specific "send a kill into the remote target". +/// +/// Used by [`run_with_shim`] to deliver SIGTERM/SIGKILL to a remote +/// process group when cancellation fires. The runner calls this with +/// the PGID captured from the shim's sentinel; the implementor knows +/// how to reach into the target (docker exec, devpod ssh, gh ssh, …). +/// +/// Implementations should be *idempotent* and *forgiving*: a kill of +/// an already-exited process is fine, and the trait is invoked on a +/// best-effort basis. +#[async_trait::async_trait] +pub trait RemoteKiller: Send + Sync { + /// Send `signal` (a POSIX signal name like "TERM" or "KILL") to + /// the entire process group `pgid` on the remote target. + async fn kill_pgid(&self, pgid: i32, signal: &str); +} + +/// Run a CLI command whose remote target requires the [`crate::exec_shim`] +/// shim for cancellation. +/// +/// Differences from [`run_cli_streaming`]: +/// +/// - The stderr reader filters out the sentinel line emitted by the +/// shim and stores the captured remote PGID in shared state. +/// - On cancel, before reaping the host-side process tree, the helper +/// asks the supplied [`RemoteKiller`] to deliver SIGTERM then SIGKILL +/// to the captured PGID (with a 2-second grace between them). This +/// reaches workloads that have been reparented to the remote's init +/// (docker daemon's containerd-shim, sshd, …) and would otherwise +/// survive the death of our host-side wrapper. +/// +/// If the sentinel never arrives (extremely fast cancellation, or a +/// target without `setsid`/base64), the helper falls back to host-tree +/// reaping only — the remote process *may* leak in that narrow window. +pub async fn run_with_shim( + binary: &CliBinary, + args: &[&str], + env: Option<&HashMap>, + cancel: &CancellationToken, + on_chunk: Option>, + killer: Arc, +) -> Result { + let mut cmd = Command::new(binary.command_name()); + cmd.args(args) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true); + + if let Some(env_vars) = env { + for (k, v) in env_vars { + cmd.env(k, v); + } + } + + let mut child = cmd.spawn().map_err(|e| { + if e.kind() == std::io::ErrorKind::NotFound { + binary.not_found_error() + } else { + Error::Io(e) + } + })?; + + let stdout = child.stdout.take().expect("stdout was piped"); + let stderr = child.stderr.take().expect("stderr was piped"); + + // Shared captured PGID. Filled in by the stderr reader as soon as + // the shim's sentinel line arrives; consumed by the cancel branch. + let pgid: Arc>> = Arc::new(std::sync::Mutex::new(None)); + + // stdout: no sentinel to scrub (the shim writes it to stderr). + let stdout_task = tokio::spawn(drain_stream( + stdout, + OutputStream::Stdout, + on_chunk.clone(), + )); + // stderr: filter the sentinel line, forward everything else. + let stderr_task = tokio::spawn(drain_stream_with_sentinel( + stderr, + OutputStream::Stderr, + on_chunk.clone(), + pgid.clone(), + )); + + let status = tokio::select! { + biased; + _ = cancel.cancelled() => { + tracing::warn!( + bin = binary.command_name(), + "cancellation received; killing remote process group then host wrapper" + ); + + let captured = *pgid.lock().expect("pgid lock"); + match captured { + Some(pgid_val) => { + // Remote kill first: if we kill the host wrapper + // first, the transport to the target closes and + // the remote work keeps running. + killer.kill_pgid(pgid_val, "TERM").await; + tokio::time::sleep(std::time::Duration::from_secs(2)).await; + killer.kill_pgid(pgid_val, "KILL").await; + } + None => { + tracing::warn!( + bin = binary.command_name(), + "no remote PGID captured; relying on host reap (may leak)" + ); + } + } + + // Host-side reap of the wrapper and any host descendants + // we *can* see. For SSH-based backends this catches the + // local SSH client process. + if let Some(root_pid) = child.id() { + crate::process_tree::reap(root_pid, std::time::Duration::from_secs(2)).await; + } + + let _ = tokio::time::timeout( + std::time::Duration::from_secs(1), + child.wait(), + ) + .await; + let _ = stdout_task.await; + let _ = stderr_task.await; + return Err(Error::Cancelled); + } + status = child.wait() => status?, + }; + + let stdout_buf = stdout_task.await.unwrap_or_default(); + let stderr_buf = stderr_task.await.unwrap_or_default(); + + Ok(CliOutput { + exit_code: status.code().unwrap_or(-1), + stdout: stdout_buf, + stderr: stderr_buf, + json: None, + }) +} + +#[cfg(test)] +mod shim_runner_tests { + use super::*; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::sync::Mutex; + + /// Records every `kill_pgid` invocation so tests can assert on + /// the order and arguments. + struct RecordingKiller { + calls: Mutex>, + } + + #[async_trait::async_trait] + impl RemoteKiller for RecordingKiller { + async fn kill_pgid(&self, pgid: i32, signal: &str) { + self.calls.lock().unwrap().push((pgid, signal.to_string())); + } + } + + /// Sink that just counts received chunks. + struct CountingSink { + stdout_lines: AtomicU64, + stderr_lines: AtomicU64, + } + #[async_trait::async_trait] + impl ChunkSink for CountingSink { + async fn on_chunk(&self, chunk: OutputChunk) { + match chunk.stream { + OutputStream::Stdout => { + self.stdout_lines.fetch_add(1, Ordering::Relaxed); + } + OutputStream::Stderr => { + self.stderr_lines.fetch_add(1, Ordering::Relaxed); + } + } + } + } + + /// Create a `devpod` shim script in `dir` that ignores all args + /// except the value after `--command` and runs that under `sh -c`. + /// This lets us drive `run_with_shim` end-to-end without a real + /// DevPod install — `binary.command_name()` is `devpod`, and our + /// shim picks it up from `PATH`. + fn install_devpod_shim(dir: &std::path::Path) -> std::path::PathBuf { + let script = dir.join("devpod"); + // The fake devpod: parse out `--command ` and exec it + // via `sh -c`. Everything else (workspace name, --user, etc.) + // is discarded — we only care that the shim's body runs. + let body = r#"#!/bin/sh +cmd="" +while [ $# -gt 0 ]; do + case "$1" in + --command) + shift + cmd="$1" + shift + ;; + *) + shift + ;; + esac +done +exec sh -c "$cmd" +"#; + std::fs::write(&script, body).expect("write shim"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(&script).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&script, perms).unwrap(); + } + script + } + + /// End-to-end: shim wraps a sleep, runner captures PGID from + /// stderr, cancellation triggers RemoteKiller in the right order + /// (TERM then KILL with a 2s gap), and the call returns + /// Error::Cancelled. + /// + /// This exercises the full `run_with_shim` flow without docker + /// or any real backend — the `RemoteKiller` is a recording fake + /// and the "remote" is just a local shell invoked by a stub + /// `devpod` shim on PATH. + #[tokio::test] + async fn run_with_shim_invokes_killer_in_order_on_cancel() { + let tmp = tempfile::tempdir().expect("tmpdir"); + install_devpod_shim(tmp.path()); + let new_path = format!( + "{}:{}", + tmp.path().display(), + std::env::var("PATH").unwrap_or_default() + ); + // Safe because tests in this module are single-threaded by + // tokio::test's default runtime; if we move to multi_thread + // we'd need a process-wide PATH lock. + // SAFETY: see comment above. + unsafe { + std::env::set_var("PATH", &new_path); + } + + let killer = Arc::new(RecordingKiller { + calls: Mutex::new(Vec::new()), + }); + let sink = Arc::new(CountingSink { + stdout_lines: AtomicU64::new(0), + stderr_lines: AtomicU64::new(0), + }); + let cancel = CancellationToken::new(); + + // The "user command" — backgrounded sleeps + wait. The shim + // wraps this so the runner sees the sentinel on stderr. + let user_cmd = "sleep 30 & sleep 30 & wait"; + let wrapped = crate::exec_shim::wrap_self_contained(user_cmd); + + // Spawn the call and cancel it shortly after. + let killer_for_spawn: Arc = killer.clone(); + let sink_for_spawn: Arc = sink.clone(); + let cancel_for_spawn = cancel.clone(); + let handle = tokio::spawn(async move { + let args = vec!["ssh", "fake-ws", "--command", wrapped.as_str()]; + run_with_shim( + &CliBinary::DevPod, + &args, + None, + &cancel_for_spawn, + Some(sink_for_spawn), + killer_for_spawn, + ) + .await + }); + + // Give the shim time to emit the sentinel. + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + cancel.cancel(); + + let result = handle.await.expect("join"); + match result { + Err(Error::Cancelled) => { /* expected */ } + other => panic!("expected Error::Cancelled, got {:?}", other), + } + + // Killer should have been called once with TERM, then once + // with KILL, both targeting the same PGID (>0). The 2s gap + // is checked separately below. + let calls = killer.calls.lock().unwrap().clone(); + assert_eq!(calls.len(), 2, "expected exactly 2 kill calls, got {calls:?}"); + let (pgid1, sig1) = &calls[0]; + let (pgid2, sig2) = &calls[1]; + assert_eq!(sig1, "TERM"); + assert_eq!(sig2, "KILL"); + assert!(*pgid1 > 0, "captured PGID should be positive, got {pgid1}"); + assert_eq!(pgid1, pgid2, "both signals should target the same PGID"); + + // Sentinel must NOT have leaked into the sink's stderr count + // — if it had, sink.stderr_lines would include at least the + // sentinel line for every chunk. Concretely, the user_cmd + // produces no stderr at all, so stderr_lines should be 0. + assert_eq!( + sink.stderr_lines.load(Ordering::Relaxed), + 0, + "sentinel should be scrubbed from stderr forwarded to sink" + ); + } +} diff --git a/crates/devcontainer-mcp-core/src/codespaces.rs b/crates/devcontainer-mcp-core/src/codespaces.rs index 2324f2a..fbaf1f3 100644 --- a/crates/devcontainer-mcp-core/src/codespaces.rs +++ b/crates/devcontainer-mcp-core/src/codespaces.rs @@ -1,6 +1,10 @@ use std::collections::HashMap; +use std::sync::Arc; +use tokio_util::sync::CancellationToken; -use crate::cli::{run_cli_with_env, CliBinary, CliOutput}; +use crate::cli::{ + run_cli_with_env, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller, +}; use crate::error::Result; const LIST_FIELDS: &str = @@ -77,6 +81,64 @@ pub async fn ssh_exec( run_gh_cs(&args, false, Some(env)).await } +/// `gh codespace ssh` — cancellable, streaming variant. +/// +/// Same model as the DevPod backend: the user's command is wrapped +/// with [`crate::exec_shim::wrap_self_contained`] (the codespace +/// transport has no `--remote-env`), the captured PGID is stripped +/// from stderr, and on cancel a second `gh codespace ssh` delivers +/// `kill -TERM -` then `kill -KILL -` against the same +/// codespace. +pub async fn ssh_exec_streaming( + env: &HashMap, + codespace: &str, + command: &str, + cancel: &CancellationToken, + on_chunk: Option>, +) -> Result { + let wrapped = crate::exec_shim::wrap_self_contained(command); + // `gh codespace ssh -c -- ` — the `--` separates gh's + // own args from the remote command, so the wrapped shim lands as + // the single remote argument. + let full_args: Vec<&str> = vec!["codespace", "ssh", "-c", codespace, "--", &wrapped]; + + let killer: Arc = Arc::new(CodespaceKiller { + env: env.clone(), + codespace: codespace.to_string(), + }); + + run_with_shim( + &CliBinary::Gh, + &full_args, + Some(env), + cancel, + on_chunk, + killer, + ) + .await +} + +/// Delivers `kill - -` inside a Codespace by spawning a +/// fresh short-lived `gh codespace ssh`. Same trade-off as the DevPod +/// backend — every kill is its own SSH session, slow but reliable. +struct CodespaceKiller { + env: HashMap, + codespace: String, +} + +#[async_trait::async_trait] +impl RemoteKiller for CodespaceKiller { + async fn kill_pgid(&self, pgid: i32, signal: &str) { + let cmd = format!("kill -{signal} -{pgid} 2>/dev/null || true"); + let args = vec!["codespace", "ssh", "-c", &self.codespace, "--", &cmd]; + if let Err(e) = + run_cli_with_env(&CliBinary::Gh, &args, false, Some(&self.env)).await + { + tracing::debug!(%e, pgid, signal, "gh codespace ssh kill failed"); + } + } +} + /// `gh codespace stop` — stop a running codespace. pub async fn stop(env: &HashMap, codespace: &str) -> Result { let args = vec!["stop", "-c", codespace]; diff --git a/crates/devcontainer-mcp-core/src/devcontainer.rs b/crates/devcontainer-mcp-core/src/devcontainer.rs index cb0dd23..e110cc0 100644 --- a/crates/devcontainer-mcp-core/src/devcontainer.rs +++ b/crates/devcontainer-mcp-core/src/devcontainer.rs @@ -1,4 +1,4 @@ -use crate::cli::{run_cli, ChunkSink, CliBinary, CliOutput}; +use crate::cli::{run_cli, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller}; use crate::docker; use crate::error::Result; use std::sync::Arc; @@ -42,18 +42,18 @@ pub async fn exec( /// `devcontainer exec` — cancellable, streaming variant. /// /// `cancel` is honored at any point during the child's lifetime; if it -/// fires, every descendant inside the container is reaped via a docker -/// exec-side `kill -- -` against the process group the shim -/// established. `on_chunk`, if supplied, receives every line of -/// stdout/stderr as the child emits it — typically wired to an MCP -/// progress notification on the server side. +/// fires, every descendant inside the container is reaped via a +/// bollard `create_exec` + `start_exec` of `kill - -` +/// against the process group the shim established. `on_chunk`, if +/// supplied, receives every line of stdout/stderr as the child emits +/// it — typically wired to an MCP progress notification on the server +/// side. /// -/// This function differs from the generic [`crate::cli::run_cli_streaming`] -/// in one important way: container descendants are not in the host PID -/// namespace lineage of `devcontainer exec` (the docker daemon -/// reparents them under containerd-shim), so a `/proc` walk on the host -/// would miss them. We install a tiny `setsid` + sentinel shim around -/// the user command and use the captured PGID to reap them on cancel. +/// Container descendants are not in the host PID namespace lineage of +/// `devcontainer exec` (the docker daemon reparents them under +/// containerd-shim), so a `/proc` walk on the host would miss them. +/// We install a tiny `setsid` + sentinel shim around the user command +/// and use the captured PGID to reap them on cancel. pub async fn exec_streaming( workspace_folder: &str, command: &str, @@ -61,12 +61,6 @@ pub async fn exec_streaming( cancel: &CancellationToken, on_chunk: Option>, ) -> Result { - use crate::cli::{OutputChunk, OutputStream}; - use std::process::Stdio; - use std::sync::Mutex; - use tokio::io::{AsyncBufReadExt, BufReader}; - use tokio::process::Command; - // Build the user-side command string. The MCP handler invokes us // as `("sh", &["-c", &full_cmd])`, which is the fast path: peel // off the `-c` arg so the shim wraps the body directly instead of @@ -87,12 +81,12 @@ pub async fn exec_streaming( // The user command is passed to the shim via an env var so it // doesn't need to be quoted into a nested `sh -c '...'`. We use // `--remote-env NAME=VALUE` so the devcontainer CLI propagates - // it inside the container. + // it inside the container. This is the env-var path; the + // self-contained `wrap_self_contained` variant is for backends + // (DevPod, Codespaces) that can't pass remote env vars. let remote_env_arg = format!("{}={}", crate::exec_shim::USER_CMD_ENV, user_cmd); - // Spawn `devcontainer exec --workspace-folder --remote-env … sh -c `. - let mut cmd = Command::new(crate::cli::CliBinary::Devcontainer.command_name()); - cmd.args([ + let args: [&str; 7] = [ "exec", "--workspace-folder", workspace_folder, @@ -100,202 +94,88 @@ pub async fn exec_streaming( &remote_env_arg, "sh", "-c", - &wrapped, - ]) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .kill_on_drop(true); - - let mut child = cmd.spawn().map_err(|e| { - if e.kind() == std::io::ErrorKind::NotFound { - crate::error::Error::DevcontainerCliNotFound - } else { - crate::error::Error::Io(e) - } - })?; + ]; + // run_with_shim takes a slice of &str; build a Vec to add the wrapped tail. + let mut all_args: Vec<&str> = args.to_vec(); + all_args.push(&wrapped); + + let killer: Arc = Arc::new(DevcontainerKiller { + workspace_folder: workspace_folder.to_string(), + }); + + run_with_shim( + &CliBinary::Devcontainer, + &all_args, + None, + cancel, + on_chunk, + killer, + ) + .await +} - let stdout = child.stdout.take().expect("stdout piped"); - let stderr = child.stderr.take().expect("stderr piped"); +/// Delivers `kill - -` inside the devcontainer associated +/// with a workspace folder, using bollard's exec API. +struct DevcontainerKiller { + workspace_folder: String, +} - // Shared captured PGID. The stderr reader fills this in as soon as - // the shim's sentinel line arrives; the cancel branch reads it. - let pgid: Arc>> = Arc::new(Mutex::new(None)); +#[async_trait::async_trait] +impl RemoteKiller for DevcontainerKiller { + async fn kill_pgid(&self, pgid: i32, signal: &str) { + use bollard::exec::{CreateExecOptions, StartExecOptions}; - // stdout: forward verbatim (no sentinel to scrub). - let stdout_task = { - let sink = on_chunk.clone(); - tokio::spawn(async move { - let mut buf = String::new(); - let mut lines = BufReader::new(stdout).lines(); - while let Ok(Some(line)) = lines.next_line().await { - if !buf.is_empty() { - buf.push('\n'); - } - buf.push_str(&line); - if let Some(s) = sink.as_ref() { - s.on_chunk(OutputChunk { - stream: OutputStream::Stdout, - line, - }) - .await; - } + let client = match docker::connect() { + Ok(c) => c, + Err(e) => { + tracing::warn!(%e, "failed to connect to docker for in-container kill"); + return; } - if !buf.is_empty() { - buf.push('\n'); + }; + let container = match docker::find_container_by_local_folder(&client, &self.workspace_folder).await { + Ok(Some(c)) => c, + Ok(None) => { + tracing::warn!( + workspace = %self.workspace_folder, + "no devcontainer found for in-container kill" + ); + return; } - buf - }) - }; - - // stderr: scrub the sentinel line, otherwise forward verbatim. - let stderr_task = { - let sink = on_chunk.clone(); - let pgid = pgid.clone(); - tokio::spawn(async move { - let mut buf = String::new(); - let mut lines = BufReader::new(stderr).lines(); - while let Ok(Some(line)) = lines.next_line().await { - if let Some(found) = crate::exec_shim::try_parse_sentinel(&line) { - *pgid.lock().expect("pgid lock") = Some(found); - tracing::debug!(pgid = found, "captured in-container PGID"); - continue; // suppress sentinel from user-visible stderr - } - if !buf.is_empty() { - buf.push('\n'); - } - buf.push_str(&line); - if let Some(s) = sink.as_ref() { - s.on_chunk(OutputChunk { - stream: OutputStream::Stderr, - line, - }) - .await; - } + Err(e) => { + tracing::warn!(%e, "container lookup failed during in-container kill"); + return; } - if !buf.is_empty() { - buf.push('\n'); + }; + + // `kill - -` signals every process in the group. + // The POSIX `--` argument separator is deliberately omitted + // because BusyBox/dash `kill` (common in slim container + // images) rejects it as "Illegal number". + let cmd = format!("kill -{signal} -{pgid} 2>/dev/null || true"); + let create = CreateExecOptions { + cmd: Some(vec!["sh".to_string(), "-c".to_string(), cmd]), + attach_stdout: Some(false), + attach_stderr: Some(false), + ..Default::default() + }; + let res = match client.create_exec(&container.id, create).await { + Ok(r) => r, + Err(e) => { + tracing::debug!(%e, "create_exec for in-container kill failed"); + return; } - buf - }) - }; - - let status = tokio::select! { - biased; - _ = cancel.cancelled() => { - tracing::warn!("devcontainer exec: cancellation received"); - - // Kill the in-container process group first, then the host - // wrapper. Order matters: if we kill the host wrapper first, - // docker exec closes its stdio and we lose the channel — - // but the in-container processes keep running anyway. - let captured = *pgid.lock().expect("pgid lock"); - match captured { - Some(pgid) => { - if let Err(e) = kill_in_container_pgid(workspace_folder, pgid).await { - tracing::warn!(%e, "failed to kill in-container PGID"); - } - } - None => { - // Sentinel never arrived — either the shim hasn't - // emitted yet (very fast cancel) or `setsid` isn't - // available and the fallback path didn't emit - // before we cancelled. Either way, host-side reap - // is our only lever. - tracing::warn!( - "no in-container PGID captured; relying on host reap (may leak)" - ); - } - } - - // Now bring down the host wrapper and any of its host-side - // descendants we *can* see. - if let Some(root_pid) = child.id() { - crate::process_tree::reap(root_pid, std::time::Duration::from_secs(2)).await; - } - - let _ = tokio::time::timeout( - std::time::Duration::from_secs(1), - child.wait(), - ) - .await; - let _ = stdout_task.await; - let _ = stderr_task.await; - return Err(crate::error::Error::Cancelled); + }; + // `detach: true` makes start_exec return as soon as the docker + // daemon has spawned the kill; we don't need to stream its + // (empty) output. + let opts = StartExecOptions { + detach: true, + ..Default::default() + }; + if let Err(e) = client.start_exec(&res.id, Some(opts)).await { + tracing::debug!(%e, "start_exec for in-container kill failed"); } - status = child.wait() => status?, - }; - - let stdout_buf = stdout_task.await.unwrap_or_default(); - let stderr_buf = stderr_task.await.unwrap_or_default(); - - Ok(CliOutput { - exit_code: status.code().unwrap_or(-1), - stdout: stdout_buf, - stderr: stderr_buf, - json: None, - }) -} - -/// Send SIGTERM (then SIGKILL after a 2s grace) to the in-container -/// process group `pgid`. Uses bollard's exec API so it works against the -/// docker daemon without shelling out. -async fn kill_in_container_pgid(workspace_folder: &str, pgid: i32) -> Result<()> { - use bollard::exec::{CreateExecOptions, StartExecOptions}; - - let client = docker::connect()?; - let container = docker::find_container_by_local_folder(&client, workspace_folder) - .await? - .ok_or_else(|| { - crate::error::Error::Io(std::io::Error::new( - std::io::ErrorKind::NotFound, - format!("No devcontainer found for workspace: {workspace_folder}"), - )) - })?; - - // `kill - -` signals every process in the group. We - // deliberately omit the POSIX `--` argument separator because - // BusyBox/dash `kill` (common in slim container images) rejects - // it as "Illegal number". The form `kill -TERM -N` is accepted - // by every shell `kill` we care about. - let run = |sig: &'static str| -> futures_util::future::BoxFuture<'static, ()> { - let client = client.clone(); - let container_id = container.id.clone(); - let cmd = format!("kill -{sig} -{pgid} 2>/dev/null || true"); - Box::pin(async move { - let create = CreateExecOptions { - cmd: Some(vec!["sh".to_string(), "-c".to_string(), cmd]), - attach_stdout: Some(false), - attach_stderr: Some(false), - ..Default::default() - }; - match client.create_exec(&container_id, create).await { - Ok(res) => { - // `detach: true` makes start_exec return as soon as - // the docker daemon has spawned the kill — we don't - // need to stream its (empty) output. The signal will - // be delivered before we proceed to the next step. - let opts = StartExecOptions { - detach: true, - ..Default::default() - }; - if let Err(e) = client.start_exec(&res.id, Some(opts)).await { - tracing::debug!(%e, "start_exec for in-container kill failed"); - } - } - Err(e) => { - tracing::debug!(%e, "create_exec for in-container kill failed"); - } - } - }) - }; - - tracing::debug!(pgid, "in-container SIGTERM -"); - run("TERM").await; - tokio::time::sleep(std::time::Duration::from_secs(2)).await; - tracing::debug!(pgid, "in-container SIGKILL -"); - run("KILL").await; - Ok(()) + } } /// `devcontainer build` — build a dev container image. diff --git a/crates/devcontainer-mcp-core/src/devpod.rs b/crates/devcontainer-mcp-core/src/devpod.rs index 20667d2..c24ee63 100644 --- a/crates/devcontainer-mcp-core/src/devpod.rs +++ b/crates/devcontainer-mcp-core/src/devpod.rs @@ -1,6 +1,8 @@ use serde::{Deserialize, Serialize}; +use std::sync::Arc; +use tokio_util::sync::CancellationToken; -use crate::cli::{run_cli, CliBinary, CliOutput}; +use crate::cli::{run_cli, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller}; use crate::error::{Error, Result}; /// Run a devpod CLI command with the given args. @@ -108,6 +110,88 @@ pub async fn ssh_exec( run_devpod(&args, false).await } +/// `devpod ssh --command` — cancellable, streaming variant. +/// +/// Same shim treatment as [`crate::devcontainer::exec_streaming`]: the +/// user's command is wrapped with [`crate::exec_shim::wrap_self_contained`] +/// so it survives the SSH transport without quoting hazards, the +/// captured remote PGID is stripped from stderr, and on cancel a +/// second `devpod ssh --command` delivers `kill -TERM -` then +/// `kill -KILL -` to the same workspace. +/// +/// DevPod (unlike the devcontainer CLI) has no `--remote-env` flag, +/// hence the self-contained shim variant which embeds the user +/// command as a base64 blob inside the shell snippet. +pub async fn ssh_exec_streaming( + workspace: &str, + command: &str, + user: Option<&str>, + workdir: Option<&str>, + cancel: &CancellationToken, + on_chunk: Option>, +) -> Result { + let wrapped = crate::exec_shim::wrap_self_contained(command); + let mut args = vec!["ssh", workspace, "--command", &wrapped]; + if let Some(u) = user { + args.push("--user"); + args.push(u); + } + if let Some(w) = workdir { + args.push("--workdir"); + args.push(w); + } + + let killer: Arc = Arc::new(DevpodKiller { + workspace: workspace.to_string(), + user: user.map(str::to_string), + }); + + run_with_shim( + &CliBinary::DevPod, + &args, + None, + cancel, + on_chunk, + killer, + ) + .await +} + +/// Delivers `kill - -` inside a DevPod workspace by +/// spawning a fresh short-lived `devpod ssh --command "kill ..."`. +/// +/// We can't reuse the original SSH session (it's busy running the +/// workload that we're trying to interrupt), so each kill is its own +/// `devpod ssh` round trip. This is slower than the docker exec path +/// — SSH auth handshake plus DevPod's own session setup — but cancel +/// is a rare path and we'd rather pay 1–2 seconds of latency than +/// leak the workload. +struct DevpodKiller { + workspace: String, + user: Option, +} + +#[async_trait::async_trait] +impl RemoteKiller for DevpodKiller { + async fn kill_pgid(&self, pgid: i32, signal: &str) { + // `kill - -` with the same BusyBox-friendly form + // we use for the devcontainer backend (no `--`). + let cmd = format!("kill -{signal} -{pgid} 2>/dev/null || true"); + let mut args = vec!["ssh", &self.workspace, "--command", &cmd]; + if let Some(u) = self.user.as_deref() { + args.push("--user"); + args.push(u); + } + // Use the plain (non-streaming, non-cancellable) runner so we + // don't recursively pull in the whole shim machinery for a + // 50-byte one-shot. We swallow errors: a kill that fails + // because the target has already exited is fine. + if let Err(e) = run_cli(&CliBinary::DevPod, &args, false).await { + tracing::debug!(%e, pgid, signal, "devpod ssh kill failed"); + } + } +} + // --------------------------------------------------------------------------- // Logs // --------------------------------------------------------------------------- diff --git a/crates/devcontainer-mcp-core/src/exec_shim.rs b/crates/devcontainer-mcp-core/src/exec_shim.rs index 9c749e9..afdf57c 100644 --- a/crates/devcontainer-mcp-core/src/exec_shim.rs +++ b/crates/devcontainer-mcp-core/src/exec_shim.rs @@ -70,6 +70,39 @@ fi"#, /// before invoking the shim string. pub const USER_CMD_ENV: &str = "DCMCP_USER_CMD"; +/// Self-contained variant of [`wrap`] that embeds the user command +/// directly in the returned shell string as a base64 blob. +/// +/// Use this when the backend transport can carry a shell command but +/// cannot propagate environment variables — e.g. `devpod ssh +/// --command ` and `gh codespace ssh -- `, neither of which +/// has a `--remote-env`-style flag. +/// +/// The returned string is shell-safe regardless of what's in +/// `user_cmd`: the user command is encoded once on the host and +/// decoded once on the target, with no intermediate shell layers +/// re-interpreting its quoting. +pub fn wrap_self_contained(user_cmd: &str) -> String { + use base64::{engine::general_purpose::STANDARD, Engine}; + + let encoded = STANDARD.encode(user_cmd.as_bytes()); + // The shell decodes the blob into `DCMCP_USER_CMD` and then runs + // the standard shim, which already knows how to eval that env + // var. This keeps the two shim flavors structurally identical and + // means a single sentinel parser handles both. + // + // We require `base64` on the target. It is present in every + // container image we've encountered (coreutils, busybox, alpine) + // because the file-write path already depends on it. + let shim = wrap(); + format!( + "{env}=$(printf '%s' '{encoded}' | base64 -d) && export {env} && {shim}", + env = USER_CMD_ENV, + encoded = encoded, + shim = shim, + ) +} + /// If `line` is the shim's sentinel, return the captured PGID. /// Otherwise return `None`. The caller should suppress matching lines /// from any downstream output. @@ -163,4 +196,41 @@ mod tests { .expect("sentinel emitted"); assert!(pgid > 0); } + + #[test] + fn wrap_self_contained_runs_user_cmd_with_no_env() { + // The whole point of the self-contained variant: no env var + // setup, command flows through the shell string itself. + let wrapped = wrap_self_contained(r#"echo "it's a (self-contained) test""#); + let out = std::process::Command::new("sh") + .arg("-c") + .arg(&wrapped) + .output() + .expect("spawn"); + assert!(out.status.success(), "stderr: {}", String::from_utf8_lossy(&out.stderr)); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stdout.contains("it's a (self-contained) test"), "stdout: {stdout:?}"); + assert!( + stderr.lines().any(|l| try_parse_sentinel(l).is_some()), + "no sentinel in stderr: {stderr:?}" + ); + } + + #[test] + fn wrap_self_contained_preserves_arbitrary_bytes() { + // Pathological user command: single quotes, double quotes, + // backticks, dollar signs, backslashes, newlines, parens. + let user_cmd = "echo 'a' \"b\" `echo c` $((1+1)) \\\\ \n echo done"; + let wrapped = wrap_self_contained(user_cmd); + let out = std::process::Command::new("sh") + .arg("-c") + .arg(&wrapped) + .output() + .expect("spawn"); + assert!(out.status.success(), "stderr: {}", String::from_utf8_lossy(&out.stderr)); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("a b c 2"), "stdout: {stdout:?}"); + assert!(stdout.contains("done"), "stdout: {stdout:?}"); + } } diff --git a/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs b/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs index 6ff581f..0a9fdff 100644 --- a/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs +++ b/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs @@ -1,6 +1,12 @@ +use std::sync::Arc; + +use devcontainer_mcp_core::cli::{ChunkSink, OutputChunk, OutputStream}; use devcontainer_mcp_core::{auth, codespaces}; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::{Meta, ProgressNotificationParam, ProgressToken}; +use rmcp::service::Peer; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; use crate::tools::common::format_output; use crate::tools::DevContainerMcp; @@ -15,18 +21,68 @@ struct CodespacesSshParams { command: String, } +/// See the devcontainer/exec.rs version for full commentary; this is a +/// near-identical copy specialized to the Codespaces backend. +struct ProgressChunkSink { + peer: Peer, + progress_token: ProgressToken, + counter: std::sync::atomic::AtomicU64, +} + +#[async_trait::async_trait] +impl ChunkSink for ProgressChunkSink { + async fn on_chunk(&self, chunk: OutputChunk) { + let n = self + .counter + .fetch_add(1, std::sync::atomic::Ordering::Relaxed) + + 1; + let prefix = match chunk.stream { + OutputStream::Stdout => "stdout", + OutputStream::Stderr => "stderr", + }; + let param = ProgressNotificationParam::new(self.progress_token.clone(), n as f64) + .with_message(format!("{prefix}: {}", chunk.line)); + if let Err(e) = self.peer.notify_progress(param).await { + tracing::debug!(%e, "failed to send progress notification"); + } + } +} + #[tool_router(router = codespaces_ssh_router, vis = "pub(super)")] impl DevContainerMcp { #[tool( name = "codespaces_ssh", description = "Execute a command inside a GitHub Codespace via SSH. Requires a GitHub auth handle." )] - async fn codespaces_ssh(&self, Parameters(params): Parameters) -> String { + async fn codespaces_ssh( + &self, + Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, + ) -> String { let env = match auth::resolve_handle_env(¶ms.auth).await { Ok(e) => e, Err(e) => return format!("Auth error: {e}"), }; - match codespaces::ssh_exec(&env, ¶ms.codespace, ¶ms.command).await { + + let sink: Option> = meta.get_progress_token().map(|token| { + Arc::new(ProgressChunkSink { + peer: peer.clone(), + progress_token: token, + counter: std::sync::atomic::AtomicU64::new(0), + }) as Arc + }); + + match codespaces::ssh_exec_streaming( + &env, + ¶ms.codespace, + ¶ms.command, + &ct, + sink, + ) + .await + { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devpod/ssh.rs b/crates/devcontainer-mcp/src/tools/devpod/ssh.rs index f1cf55e..256ca21 100644 --- a/crates/devcontainer-mcp/src/tools/devpod/ssh.rs +++ b/crates/devcontainer-mcp/src/tools/devpod/ssh.rs @@ -1,8 +1,15 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; +use std::sync::Arc; + +use devcontainer_mcp_core::cli::{ChunkSink, OutputChunk, OutputStream}; use devcontainer_mcp_core::devpod; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::{Meta, ProgressNotificationParam, ProgressToken}; +use rmcp::service::Peer; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; + +use crate::tools::common::format_output; +use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] struct DevpodSshParams { @@ -18,18 +25,62 @@ struct DevpodSshParams { workdir: Option, } +/// Forwards every line of child output to the MCP peer as a +/// `notifications/progress` message, keeping the transport warm during +/// long-running execs. +struct ProgressChunkSink { + peer: Peer, + progress_token: ProgressToken, + counter: std::sync::atomic::AtomicU64, +} + +#[async_trait::async_trait] +impl ChunkSink for ProgressChunkSink { + async fn on_chunk(&self, chunk: OutputChunk) { + let n = self + .counter + .fetch_add(1, std::sync::atomic::Ordering::Relaxed) + + 1; + let prefix = match chunk.stream { + OutputStream::Stdout => "stdout", + OutputStream::Stderr => "stderr", + }; + let param = ProgressNotificationParam::new(self.progress_token.clone(), n as f64) + .with_message(format!("{prefix}: {}", chunk.line)); + if let Err(e) = self.peer.notify_progress(param).await { + tracing::debug!(%e, "failed to send progress notification"); + } + } +} + #[tool_router(router = devpod_ssh_router, vis = "pub(super)")] impl DevContainerMcp { #[tool( name = "devpod_ssh", description = "Execute a command inside a DevPod workspace via SSH. Returns stdout, stderr, and exit code." )] - async fn devpod_ssh(&self, Parameters(params): Parameters) -> String { - match devpod::ssh_exec( + async fn devpod_ssh( + &self, + Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, + ) -> String { + let sink: Option> = meta.get_progress_token().map(|token| { + Arc::new(ProgressChunkSink { + peer: peer.clone(), + progress_token: token, + counter: std::sync::atomic::AtomicU64::new(0), + }) as Arc + }); + + match devpod::ssh_exec_streaming( ¶ms.workspace, ¶ms.command, params.user.as_deref(), params.workdir.as_deref(), + &ct, + sink, ) .await { From 48e8eab98177ee2638d65650bdcce50e7b8d3bf7 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 23 May 2026 19:10:52 -0700 Subject: [PATCH 14/19] lifecycle tools: stream + honor cancellation; sentinel parser accepts decorated lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap that bit us when `devpod_up` ran past opencode's 60-second client-side request timeout: the client sent `notifications/cancelled`, our handler ignored it, and the `devpod` subprocess kept right on provisioning a GCE VM that ran up billing until I noticed it. Same defect existed in `devcontainer_up`, `devcontainer_build`, `devpod_build`, and `codespaces_create`. For each of those five tools: - New `*_streaming` variant in core that takes a `&CancellationToken` and an optional `ChunkSink`, routing through `run_cli_streaming`. No `RemoteKiller` needed — these all run host-side; `process_tree::reap` on the host suffices. Devpod/devcontainer's own CLI rollback logic handles partial cloud/docker state when we kill the host process. - Tool handlers gain the standard `CancellationToken`/`Peer`/`Meta` extractors and call into the new `_streaming` core fn. Shared scaffolding factored into `tools/common.rs`: - `ProgressChunkSink` and `progress_sink_from_meta(&meta, &peer)`, used by all eight long-running tools (3 exec/ssh + 5 lifecycle). The three existing exec/ssh handlers (devcontainer_exec, devpod_ssh, codespaces_ssh) lose ~30 lines each of inline ProgressChunkSink code and now share the helper. Five new handlers gain it for free. Sentinel parser fix discovered during live DevPod verification: - `try_parse_sentinel` now finds the sentinel anywhere within a line, not just as a trimmed standalone match. `devpod ssh` prefixes every line of remote stderr with ANSI color codes, a timestamp, and an `info`/`error` log level, e.g. `\x1b[0;1;36minfo \x1b[0m__DCMCP_PGID=385__` The old strict parser missed this entirely, so the in-target PGID was never captured and the remote tree leaked on cancel (the case my first devpod_ssh end-to-end run exposed). Verified the relaxed parser still rejects substrings without the proper `__` terminator, missing digits, and non-numeric PGIDs; added `try_parse_sentinel_accepts_decorated_lines` covering both the real `devpod ssh` shape and a plainer `[time] info ...` shape, plus tightened the `rejects_garbage` test. Live verification matrix: devcontainer_exec (local docker): - 3x parallel `go test -race`/`vet`/`build` still completes in ~17s with all responses correctly correlated. (unchanged) - In-container nested-sleep tree (5 procs) reaped on cancel: 5 -> 0. (unchanged) - Progress notifications stream live: 5 notifs over a 3s exec. codespaces_ssh (live Codespace on basicLinux32gb): - 5 progress notifications during a 5s exec. - Remote PGID captured via the `gh codespace ssh` stderr, `kill -TERM -` then `kill -KILL -` via fresh `gh codespace ssh` sessions, 3 nested sleeps reaped. - Server immediately responsive to next call. devpod_ssh (live GCE workspace on gcloud provider): - 7 progress notifications during a 5s exec. - Remote PGID extracted from devpod's ANSI-decorated stderr, `kill -TERM -` then `kill -KILL -` via fresh `devpod ssh --command` calls, 3 nested sleeps reaped. - Server immediately responsive to next call. devpod_up (live GCE provisioning): - 114 progress notifications over 45s of GCE VM + image build. - Cancellation 45s in returned `Error: Operation cancelled by peer` in 2.04s; `devpod` host process tree reaped (n=2 TERM, n=1 KILL survivor). - Explicit `devpod delete --force` after the cancel left zero GCE instances. devpod's own rollback handles the partial machine state correctly. cargo test: 21 unit tests in core (was 20; +1 for the decorated-line parser) + 10 hook-guard, all passing. Known follow-up (not addressed here): devpod_ssh kill-on-cancel takes ~25s end-to-end because each `devpod ssh --command "kill ..."` pays a fresh SSH handshake and we do it twice (TERM then KILL with a 2s gap). Could collapse both signals into one session: `kill -TERM -N; sleep 2; kill -KILL -N`. Would cut cancel latency for the devpod and codespaces backends to ~3-5s. Tracked as a separate item. --- .../devcontainer-mcp-core/src/codespaces.rs | 49 ++++++++++++++- .../devcontainer-mcp-core/src/devcontainer.rs | 57 ++++++++++++++++- crates/devcontainer-mcp-core/src/devpod.rs | 62 ++++++++++++++++++- crates/devcontainer-mcp-core/src/exec_shim.rs | 57 ++++++++++++++--- .../src/tools/codespaces/create.rs | 15 ++++- .../src/tools/codespaces/ssh.rs | 52 ++-------------- crates/devcontainer-mcp/src/tools/common.rs | 61 +++++++++++++++++- .../src/tools/devcontainer/build.rs | 14 ++++- .../src/tools/devcontainer/exec.rs | 62 ++----------------- .../src/tools/devcontainer/up.rs | 15 ++++- .../src/tools/devpod/build.rs | 21 +++++-- .../devcontainer-mcp/src/tools/devpod/ssh.rs | 43 +------------ .../devcontainer-mcp/src/tools/devpod/up.rs | 21 +++++-- 13 files changed, 353 insertions(+), 176 deletions(-) diff --git a/crates/devcontainer-mcp-core/src/codespaces.rs b/crates/devcontainer-mcp-core/src/codespaces.rs index fbaf1f3..4f1d574 100644 --- a/crates/devcontainer-mcp-core/src/codespaces.rs +++ b/crates/devcontainer-mcp-core/src/codespaces.rs @@ -3,7 +3,8 @@ use std::sync::Arc; use tokio_util::sync::CancellationToken; use crate::cli::{ - run_cli_with_env, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller, + run_cli_streaming, run_cli_with_env, run_with_shim, ChunkSink, CliBinary, CliOutput, + RemoteKiller, }; use crate::error::Result; @@ -61,6 +62,52 @@ pub async fn create( run_gh_cs(&args, false, Some(env)).await } +/// `gh codespace create` — cancellable, streaming variant. +/// +/// Codespace creation takes 3–5 minutes (image pull, devcontainer +/// build, postCreate, …). Streams `gh codespace create` output as +/// progress notifications and honors cancellation — when the client +/// times out we kill the host `gh` process. Note: this only reaps +/// the host wrapper; if the GitHub API has already started +/// provisioning the codespace remotely, the codespace itself will +/// continue to come up and bill until the caller explicitly deletes +/// it via `codespaces_delete`. The wire-level cancel is the most we +/// can do — there's no `gh codespace cancel-create` API. +pub async fn create_streaming( + env: &HashMap, + repo: &str, + branch: Option<&str>, + machine: Option<&str>, + devcontainer_path: Option<&str>, + display_name: Option<&str>, + idle_timeout: Option<&str>, + cancel: &CancellationToken, + on_chunk: Option>, +) -> Result { + let mut args = vec!["codespace", "create", "--repo", repo]; + if let Some(b) = branch { + args.push("--branch"); + args.push(b); + } + if let Some(m) = machine { + args.push("--machine"); + args.push(m); + } + if let Some(d) = devcontainer_path { + args.push("--devcontainer-path"); + args.push(d); + } + if let Some(n) = display_name { + args.push("--display-name"); + args.push(n); + } + if let Some(t) = idle_timeout { + args.push("--idle-timeout"); + args.push(t); + } + run_cli_streaming(&CliBinary::Gh, &args, false, Some(env), cancel, on_chunk).await +} + /// `gh codespace list` — list codespaces. pub async fn list(env: &HashMap, repo: Option<&str>) -> Result { let mut args = vec!["list", "--json", LIST_FIELDS]; diff --git a/crates/devcontainer-mcp-core/src/devcontainer.rs b/crates/devcontainer-mcp-core/src/devcontainer.rs index e110cc0..34e07fc 100644 --- a/crates/devcontainer-mcp-core/src/devcontainer.rs +++ b/crates/devcontainer-mcp-core/src/devcontainer.rs @@ -1,4 +1,6 @@ -use crate::cli::{run_cli, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller}; +use crate::cli::{ + run_cli, run_cli_streaming, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller, +}; use crate::docker; use crate::error::Result; use std::sync::Arc; @@ -28,6 +30,38 @@ pub async fn up( run_devcontainer(&args, true).await } +/// `devcontainer up` — cancellable, streaming variant. +/// +/// `devcontainer up` does a docker build + container create + the +/// devcontainer lifecycle commands (postCreate, postStart, etc.) all +/// in one invocation. That can easily take several minutes on a +/// cold image. Same rationale as `crate::devpod::up_streaming`: +/// cancellation prevents leaked partial-up containers, and progress +/// streaming keeps client transports warm. +pub async fn up_streaming( + workspace_folder: &str, + config: Option<&str>, + extra_args: &[&str], + cancel: &CancellationToken, + on_chunk: Option>, +) -> Result { + let mut args = vec!["up", "--workspace-folder", workspace_folder]; + if let Some(c) = config { + args.push("--config"); + args.push(c); + } + args.extend_from_slice(extra_args); + run_cli_streaming( + &CliBinary::Devcontainer, + &args, + true, + None, + cancel, + on_chunk, + ) + .await +} + /// `devcontainer exec` — execute a command in a running dev container. pub async fn exec( workspace_folder: &str, @@ -185,6 +219,27 @@ pub async fn build(workspace_folder: &str, extra_args: &[&str]) -> Result>, +) -> Result { + let mut args = vec!["build", "--workspace-folder", workspace_folder]; + args.extend_from_slice(extra_args); + run_cli_streaming( + &CliBinary::Devcontainer, + &args, + true, + None, + cancel, + on_chunk, + ) + .await +} + /// `devcontainer read-configuration` — read devcontainer config as JSON. pub async fn read_configuration(workspace_folder: &str, config: Option<&str>) -> Result { let mut args = vec!["read-configuration", "--workspace-folder", workspace_folder]; diff --git a/crates/devcontainer-mcp-core/src/devpod.rs b/crates/devcontainer-mcp-core/src/devpod.rs index c24ee63..99b5958 100644 --- a/crates/devcontainer-mcp-core/src/devpod.rs +++ b/crates/devcontainer-mcp-core/src/devpod.rs @@ -2,7 +2,9 @@ use serde::{Deserialize, Serialize}; use std::sync::Arc; use tokio_util::sync::CancellationToken; -use crate::cli::{run_cli, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller}; +use crate::cli::{ + run_cli, run_cli_streaming, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller, +}; use crate::error::{Error, Result}; /// Run a devpod CLI command with the given args. @@ -31,6 +33,45 @@ pub async fn up(args: &[&str]) -> Result { run_devpod(&cmd_args, false).await } +/// `devpod up` — cancellable, streaming variant. +/// +/// Provisioning a cloud workspace (especially the GCP/AWS/Azure +/// providers) can take 2–10 minutes. Wrapping `devpod up` through +/// [`run_cli_streaming`] gives us two things: +/// +/// 1. **Cancellation**: when the client times out and sends +/// `notifications/cancelled`, our handler's [`CancellationToken`] +/// fires and we reap the host process tree (the `devpod` CLI and +/// its descendants). Devpod's own cleanup logic then unwinds any +/// partial cloud resources (it has rollback semantics for failed +/// `up`). +/// +/// 2. **Progress notifications**: every line of `devpod up` output +/// ("Creating devcontainer...", "npm install...", …) is forwarded +/// as an MCP progress notification, keeping the wire warm so +/// idle-based client timeouts don't trip. +/// +/// Unlike `ssh_exec_streaming` this path uses [`run_cli_streaming`] +/// directly — no shim is needed because `devpod up` runs entirely on +/// the host (it makes GCP API calls; no remote process to reap). +pub async fn up_streaming( + args: &[&str], + cancel: &CancellationToken, + on_chunk: Option>, +) -> Result { + let mut cmd_args = vec!["up", "--open-ide=false"]; + cmd_args.extend_from_slice(args); + run_cli_streaming( + &CliBinary::DevPod, + &cmd_args, + false, + None, + cancel, + on_chunk, + ) + .await +} + /// `devpod stop` — stop a workspace. pub async fn stop(workspace: &str) -> Result { run_devpod(&["stop", workspace], false).await @@ -52,6 +93,25 @@ pub async fn build(args: &[&str]) -> Result { run_devpod(&cmd_args, false).await } +/// `devpod build` — cancellable, streaming variant. See [`up_streaming`]. +pub async fn build_streaming( + args: &[&str], + cancel: &CancellationToken, + on_chunk: Option>, +) -> Result { + let mut cmd_args = vec!["build"]; + cmd_args.extend_from_slice(args); + run_cli_streaming( + &CliBinary::DevPod, + &cmd_args, + false, + None, + cancel, + on_chunk, + ) + .await +} + // --------------------------------------------------------------------------- // Workspace queries // --------------------------------------------------------------------------- diff --git a/crates/devcontainer-mcp-core/src/exec_shim.rs b/crates/devcontainer-mcp-core/src/exec_shim.rs index afdf57c..405e4fb 100644 --- a/crates/devcontainer-mcp-core/src/exec_shim.rs +++ b/crates/devcontainer-mcp-core/src/exec_shim.rs @@ -103,15 +103,33 @@ pub fn wrap_self_contained(user_cmd: &str) -> String { ) } -/// If `line` is the shim's sentinel, return the captured PGID. +/// If `line` contains the shim's sentinel, return the captured PGID. /// Otherwise return `None`. The caller should suppress matching lines /// from any downstream output. +/// +/// We accept the sentinel anywhere within the line, not just as a +/// standalone match, because intermediate transports decorate remote +/// stderr with prefixes that vary by backend: +/// - `gh codespace ssh`: passes lines through unmodified. +/// - `devpod ssh`: prefixes each line with ANSI color codes, a +/// timestamp, and an `info`/`error` log-level tag. +/// - Future backends may layer on their own prefixes. +/// +/// The sentinel itself (`__DCMCP_PGID=NNN__`) is intentionally +/// distinctive — both delimiters are `__DCMCP_*__` — so the chance +/// of a false positive from user output containing that exact byte +/// sequence is vanishingly small. Bounding the digit count to 31 +/// bits keeps us safe against malformed input that would otherwise +/// trip `parse`'s overflow check. pub fn try_parse_sentinel(line: &str) -> Option { - // Match `^\s*__DCMCP_PGID=(\d+)__\s*$`. A hand-rolled scan keeps - // the dependency footprint at zero. - let trimmed = line.trim(); - let rest = trimmed.strip_prefix(SENTINEL_PREFIX)?; - let num = rest.strip_suffix(SENTINEL_SUFFIX)?; + // Locate the prefix; bail if absent. + let after_prefix = line.find(SENTINEL_PREFIX) + .map(|i| &line[i + SENTINEL_PREFIX.len()..])?; + // The PGID runs from here to the next `__` (the suffix). Look + // forward for the suffix; if not found, this isn't our sentinel + // (could be a substring collision in user output). + let end = after_prefix.find(SENTINEL_SUFFIX)?; + let num = &after_prefix[..end]; if num.is_empty() || !num.bytes().all(|b| b.is_ascii_digit()) { return None; } @@ -167,13 +185,36 @@ mod tests { assert_eq!(try_parse_sentinel(" __DCMCP_PGID=12345__ "), Some(12345)); } + #[test] + fn try_parse_sentinel_accepts_decorated_lines() { + // Real-world cases observed against backend transports that + // decorate remote stderr with their own prefixes: + // + // - devpod ssh prepends ANSI color codes + timestamp + log level. + // - Some shells may inject ESC sequences from PROMPT_COMMAND + // into stderr on first connection. + // + // The sentinel itself is distinctive enough that "anywhere in + // the line" is safe in practice. + assert_eq!( + try_parse_sentinel("\u{1b}[0;1;36minfo \u{1b}[0m__DCMCP_PGID=385__"), + Some(385), + ); + assert_eq!( + try_parse_sentinel("[19:05:32] info __DCMCP_PGID=12345__"), + Some(12345), + ); + } + #[test] fn try_parse_sentinel_rejects_garbage() { assert_eq!(try_parse_sentinel("hello"), None); assert_eq!(try_parse_sentinel("__DCMCP_PGID=abc__"), None); assert_eq!(try_parse_sentinel("PGID=42"), None); - // Don't accept it embedded in other text: - assert_eq!(try_parse_sentinel("prefix __DCMCP_PGID=42__"), None); + // Truncated (no terminator) — must not match. + assert_eq!(try_parse_sentinel("__DCMCP_PGID=42"), None); + // Missing PGID number. + assert_eq!(try_parse_sentinel("__DCMCP_PGID=__"), None); } #[test] diff --git a/crates/devcontainer-mcp/src/tools/codespaces/create.rs b/crates/devcontainer-mcp/src/tools/codespaces/create.rs index 5d94fc6..b3ee212 100644 --- a/crates/devcontainer-mcp/src/tools/codespaces/create.rs +++ b/crates/devcontainer-mcp/src/tools/codespaces/create.rs @@ -1,8 +1,11 @@ use devcontainer_mcp_core::{auth, codespaces}; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::Meta; +use rmcp::service::Peer; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; -use crate::tools::common::format_output; +use crate::tools::common::{format_output, progress_sink_from_meta}; use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] @@ -36,12 +39,16 @@ impl DevContainerMcp { async fn codespaces_create( &self, Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, ) -> String { let env = match auth::resolve_handle_env(¶ms.auth).await { Ok(e) => e, Err(e) => return format!("Auth error: {e}"), }; - match codespaces::create( + let sink = progress_sink_from_meta(&meta, &peer); + match codespaces::create_streaming( &env, ¶ms.repo, params.branch.as_deref(), @@ -49,6 +56,8 @@ impl DevContainerMcp { params.devcontainer_path.as_deref(), params.display_name.as_deref(), params.idle_timeout.as_deref(), + &ct, + sink, ) .await { diff --git a/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs b/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs index 0a9fdff..db29880 100644 --- a/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs +++ b/crates/devcontainer-mcp/src/tools/codespaces/ssh.rs @@ -1,14 +1,11 @@ -use std::sync::Arc; - -use devcontainer_mcp_core::cli::{ChunkSink, OutputChunk, OutputStream}; use devcontainer_mcp_core::{auth, codespaces}; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{Meta, ProgressNotificationParam, ProgressToken}; +use rmcp::model::Meta; use rmcp::service::Peer; use rmcp::{tool, tool_router, RoleServer}; use tokio_util::sync::CancellationToken; -use crate::tools::common::format_output; +use crate::tools::common::{format_output, progress_sink_from_meta}; use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] @@ -21,33 +18,6 @@ struct CodespacesSshParams { command: String, } -/// See the devcontainer/exec.rs version for full commentary; this is a -/// near-identical copy specialized to the Codespaces backend. -struct ProgressChunkSink { - peer: Peer, - progress_token: ProgressToken, - counter: std::sync::atomic::AtomicU64, -} - -#[async_trait::async_trait] -impl ChunkSink for ProgressChunkSink { - async fn on_chunk(&self, chunk: OutputChunk) { - let n = self - .counter - .fetch_add(1, std::sync::atomic::Ordering::Relaxed) - + 1; - let prefix = match chunk.stream { - OutputStream::Stdout => "stdout", - OutputStream::Stderr => "stderr", - }; - let param = ProgressNotificationParam::new(self.progress_token.clone(), n as f64) - .with_message(format!("{prefix}: {}", chunk.line)); - if let Err(e) = self.peer.notify_progress(param).await { - tracing::debug!(%e, "failed to send progress notification"); - } - } -} - #[tool_router(router = codespaces_ssh_router, vis = "pub(super)")] impl DevContainerMcp { #[tool( @@ -66,22 +36,10 @@ impl DevContainerMcp { Err(e) => return format!("Auth error: {e}"), }; - let sink: Option> = meta.get_progress_token().map(|token| { - Arc::new(ProgressChunkSink { - peer: peer.clone(), - progress_token: token, - counter: std::sync::atomic::AtomicU64::new(0), - }) as Arc - }); + let sink = progress_sink_from_meta(&meta, &peer); - match codespaces::ssh_exec_streaming( - &env, - ¶ms.codespace, - ¶ms.command, - &ct, - sink, - ) - .await + match codespaces::ssh_exec_streaming(&env, ¶ms.codespace, ¶ms.command, &ct, sink) + .await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), diff --git a/crates/devcontainer-mcp/src/tools/common.rs b/crates/devcontainer-mcp/src/tools/common.rs index 9ef4902..7770619 100644 --- a/crates/devcontainer-mcp/src/tools/common.rs +++ b/crates/devcontainer-mcp/src/tools/common.rs @@ -1,4 +1,10 @@ -use devcontainer_mcp_core::cli::CliOutput; +use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; + +use devcontainer_mcp_core::cli::{ChunkSink, CliOutput, OutputChunk, OutputStream}; +use rmcp::model::{Meta, ProgressNotificationParam, ProgressToken}; +use rmcp::service::Peer; +use rmcp::RoleServer; /// Format a CliOutput as a JSON string for MCP responses. pub fn format_output(output: &CliOutput) -> String { @@ -10,3 +16,56 @@ pub fn format_output(output: &CliOutput) -> String { }) .to_string() } + +/// Forwards every line of child output to the MCP peer as a +/// `notifications/progress` message. Used by all long-running tools +/// (`*_exec`, `*_ssh`, `*_up`, `*_build`, `codespaces_create`) so +/// clients can render progress and so idle-based client timeouts +/// don't trip on multi-minute operations. +/// +/// The MCP spec requires a strictly-increasing `progress` value on +/// every notification with the same token, hence the per-sink +/// atomic counter. +struct ProgressChunkSink { + peer: Peer, + progress_token: ProgressToken, + counter: AtomicU64, +} + +#[async_trait::async_trait] +impl ChunkSink for ProgressChunkSink { + async fn on_chunk(&self, chunk: OutputChunk) { + let n = self.counter.fetch_add(1, Ordering::Relaxed) + 1; + let prefix = match chunk.stream { + OutputStream::Stdout => "stdout", + OutputStream::Stderr => "stderr", + }; + let param = ProgressNotificationParam::new(self.progress_token.clone(), n as f64) + .with_message(format!("{prefix}: {}", chunk.line)); + if let Err(e) = self.peer.notify_progress(param).await { + // Sink errors are non-fatal: the peer may have + // disconnected or simply not subscribed to progress. + // We still want the child to keep running and produce + // a final response (which may or may not get delivered). + tracing::debug!(%e, "failed to send progress notification"); + } + } +} + +/// Build a [`ChunkSink`] from a request's `_meta`. +/// +/// Returns `Some(Arc)` if the client supplied a +/// `progressToken`, `None` otherwise. Tool handlers should pass the +/// returned sink straight into `*_streaming` core functions. +pub fn progress_sink_from_meta( + meta: &Meta, + peer: &Peer, +) -> Option> { + meta.get_progress_token().map(|token| { + Arc::new(ProgressChunkSink { + peer: peer.clone(), + progress_token: token, + counter: AtomicU64::new(0), + }) as Arc + }) +} diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/build.rs b/crates/devcontainer-mcp/src/tools/devcontainer/build.rs index b89cb60..82b3bcd 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/build.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/build.rs @@ -1,8 +1,11 @@ use devcontainer_mcp_core::devcontainer; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::Meta; +use rmcp::service::Peer; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; -use crate::tools::common::format_output; +use crate::tools::common::{format_output, progress_sink_from_meta}; use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] @@ -24,6 +27,9 @@ impl DevContainerMcp { async fn devcontainer_build( &self, Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, ) -> String { let extra: Vec = params .extra_args @@ -31,7 +37,9 @@ impl DevContainerMcp { .and_then(shlex::split) .unwrap_or_default(); let extra_refs: Vec<&str> = extra.iter().map(|s| s.as_str()).collect(); - match devcontainer::build(¶ms.workspace_folder, &extra_refs).await { + let sink = progress_sink_from_meta(&meta, &peer); + match devcontainer::build_streaming(¶ms.workspace_folder, &extra_refs, &ct, sink).await + { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs index a381c99..0a4cb86 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs @@ -1,14 +1,11 @@ -use std::sync::Arc; - -use devcontainer_mcp_core::cli::{ChunkSink, OutputChunk, OutputStream}; use devcontainer_mcp_core::devcontainer; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{Meta, ProgressNotificationParam, ProgressToken}; -use rmcp::service::{Peer, RequestContext}; +use rmcp::model::Meta; +use rmcp::service::Peer; use rmcp::{tool, tool_router, RoleServer}; use tokio_util::sync::CancellationToken; -use crate::tools::common::format_output; +use crate::tools::common::{format_output, progress_sink_from_meta}; use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] @@ -21,41 +18,6 @@ struct DevcontainerExecParams { args: Option, } -/// Forwards every line of child output to the MCP peer as a -/// `notifications/progress` message. Used to keep the wire warm during -/// long-running execs (e.g. `go test -race`) so client-side watchdogs -/// don't tear down the transport. -struct ProgressChunkSink { - peer: Peer, - progress_token: ProgressToken, - /// Monotonic counter so each notification has a strictly-increasing - /// `progress` value, as required by the MCP spec. - counter: std::sync::atomic::AtomicU64, -} - -#[async_trait::async_trait] -impl ChunkSink for ProgressChunkSink { - async fn on_chunk(&self, chunk: OutputChunk) { - let n = self - .counter - .fetch_add(1, std::sync::atomic::Ordering::Relaxed) - + 1; - let prefix = match chunk.stream { - OutputStream::Stdout => "stdout", - OutputStream::Stderr => "stderr", - }; - let param = ProgressNotificationParam::new(self.progress_token.clone(), n as f64) - .with_message(format!("{prefix}: {}", chunk.line)); - if let Err(e) = self.peer.notify_progress(param).await { - // Sink errors are non-fatal: the peer may have disconnected - // or simply not subscribed to progress. We still want the - // child to keep running and produce a final response (which - // may or may not get delivered). - tracing::debug!(%e, "failed to send progress notification"); - } - } -} - #[tool_router(router = devcontainer_exec_router, vis = "pub(super)")] impl DevContainerMcp { #[tool( @@ -74,16 +36,7 @@ impl DevContainerMcp { None => params.command, }; - // If the client supplied a progressToken in _meta, stream - // stdout/stderr to it as progress notifications. Otherwise just - // run with cancellation but no streaming. - let sink: Option> = meta.get_progress_token().map(|token| { - Arc::new(ProgressChunkSink { - peer: peer.clone(), - progress_token: token, - counter: std::sync::atomic::AtomicU64::new(0), - }) as Arc - }); + let sink = progress_sink_from_meta(&meta, &peer); match devcontainer::exec_streaming( ¶ms.workspace_folder, @@ -99,10 +52,3 @@ impl DevContainerMcp { } } } - -// `RequestContext` is not strictly required here — `CancellationToken`, -// `Peer`, and `Meta` are extracted directly by the tool_router -// macro via their respective `FromContextPart` impls — but importing it -// once keeps it discoverable for future tool authors. -#[allow(dead_code)] -fn _request_context_marker(_: RequestContext) {} diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/up.rs b/crates/devcontainer-mcp/src/tools/devcontainer/up.rs index b2b1b43..9924a0c 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/up.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/up.rs @@ -1,8 +1,11 @@ use devcontainer_mcp_core::devcontainer; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::Meta; +use rmcp::service::Peer; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; -use crate::tools::common::format_output; +use crate::tools::common::{format_output, progress_sink_from_meta}; use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] @@ -28,6 +31,9 @@ impl DevContainerMcp { async fn devcontainer_up( &self, Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, ) -> String { let extra: Vec = params .extra_args @@ -35,10 +41,13 @@ impl DevContainerMcp { .and_then(shlex::split) .unwrap_or_default(); let extra_refs: Vec<&str> = extra.iter().map(|s| s.as_str()).collect(); - match devcontainer::up( + let sink = progress_sink_from_meta(&meta, &peer); + match devcontainer::up_streaming( ¶ms.workspace_folder, params.config.as_deref(), &extra_refs, + &ct, + sink, ) .await { diff --git a/crates/devcontainer-mcp/src/tools/devpod/build.rs b/crates/devcontainer-mcp/src/tools/devpod/build.rs index bdf8d83..eccd638 100644 --- a/crates/devcontainer-mcp/src/tools/devpod/build.rs +++ b/crates/devcontainer-mcp/src/tools/devpod/build.rs @@ -1,8 +1,12 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; use devcontainer_mcp_core::devpod; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::Meta; +use rmcp::service::Peer; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; + +use crate::tools::common::{format_output, progress_sink_from_meta}; +use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] struct DevpodBuildParams { @@ -18,11 +22,18 @@ impl DevContainerMcp { name = "devpod_build", description = "Build a DevPod workspace image without starting it." )] - async fn devpod_build(&self, Parameters(params): Parameters) -> String { + async fn devpod_build( + &self, + Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, + ) -> String { let parts: Vec = shlex::split(¶ms.args) .unwrap_or_else(|| params.args.split_whitespace().map(String::from).collect()); let part_refs: Vec<&str> = parts.iter().map(|s| s.as_str()).collect(); - match devpod::build(&part_refs).await { + let sink = progress_sink_from_meta(&meta, &peer); + match devpod::build_streaming(&part_refs, &ct, sink).await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devpod/ssh.rs b/crates/devcontainer-mcp/src/tools/devpod/ssh.rs index 256ca21..9dedc95 100644 --- a/crates/devcontainer-mcp/src/tools/devpod/ssh.rs +++ b/crates/devcontainer-mcp/src/tools/devpod/ssh.rs @@ -1,14 +1,11 @@ -use std::sync::Arc; - -use devcontainer_mcp_core::cli::{ChunkSink, OutputChunk, OutputStream}; use devcontainer_mcp_core::devpod; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{Meta, ProgressNotificationParam, ProgressToken}; +use rmcp::model::Meta; use rmcp::service::Peer; use rmcp::{tool, tool_router, RoleServer}; use tokio_util::sync::CancellationToken; -use crate::tools::common::format_output; +use crate::tools::common::{format_output, progress_sink_from_meta}; use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] @@ -25,34 +22,6 @@ struct DevpodSshParams { workdir: Option, } -/// Forwards every line of child output to the MCP peer as a -/// `notifications/progress` message, keeping the transport warm during -/// long-running execs. -struct ProgressChunkSink { - peer: Peer, - progress_token: ProgressToken, - counter: std::sync::atomic::AtomicU64, -} - -#[async_trait::async_trait] -impl ChunkSink for ProgressChunkSink { - async fn on_chunk(&self, chunk: OutputChunk) { - let n = self - .counter - .fetch_add(1, std::sync::atomic::Ordering::Relaxed) - + 1; - let prefix = match chunk.stream { - OutputStream::Stdout => "stdout", - OutputStream::Stderr => "stderr", - }; - let param = ProgressNotificationParam::new(self.progress_token.clone(), n as f64) - .with_message(format!("{prefix}: {}", chunk.line)); - if let Err(e) = self.peer.notify_progress(param).await { - tracing::debug!(%e, "failed to send progress notification"); - } - } -} - #[tool_router(router = devpod_ssh_router, vis = "pub(super)")] impl DevContainerMcp { #[tool( @@ -66,13 +35,7 @@ impl DevContainerMcp { peer: Peer, meta: Meta, ) -> String { - let sink: Option> = meta.get_progress_token().map(|token| { - Arc::new(ProgressChunkSink { - peer: peer.clone(), - progress_token: token, - counter: std::sync::atomic::AtomicU64::new(0), - }) as Arc - }); + let sink = progress_sink_from_meta(&meta, &peer); match devpod::ssh_exec_streaming( ¶ms.workspace, diff --git a/crates/devcontainer-mcp/src/tools/devpod/up.rs b/crates/devcontainer-mcp/src/tools/devpod/up.rs index f854c1a..ebf0ad3 100644 --- a/crates/devcontainer-mcp/src/tools/devpod/up.rs +++ b/crates/devcontainer-mcp/src/tools/devpod/up.rs @@ -1,8 +1,12 @@ -use crate::tools::common::format_output; -use crate::tools::DevContainerMcp; use devcontainer_mcp_core::devpod; use rmcp::handler::server::wrapper::Parameters; -use rmcp::{tool, tool_router}; +use rmcp::model::Meta; +use rmcp::service::Peer; +use rmcp::{tool, tool_router, RoleServer}; +use tokio_util::sync::CancellationToken; + +use crate::tools::common::{format_output, progress_sink_from_meta}; +use crate::tools::DevContainerMcp; #[derive(serde::Deserialize, schemars::JsonSchema)] struct DevpodUpParams { @@ -18,11 +22,18 @@ impl DevContainerMcp { name = "devpod_up", description = "Create and start a DevPod workspace. Pass the source (git URL, local path, or image) and any flags as space-separated args. Returns full build output for self-healing." )] - async fn devpod_up(&self, Parameters(params): Parameters) -> String { + async fn devpod_up( + &self, + Parameters(params): Parameters, + ct: CancellationToken, + peer: Peer, + meta: Meta, + ) -> String { let parts: Vec = shlex::split(¶ms.args) .unwrap_or_else(|| params.args.split_whitespace().map(String::from).collect()); let part_refs: Vec<&str> = parts.iter().map(|s| s.as_str()).collect(); - match devpod::up(&part_refs).await { + let sink = progress_sink_from_meta(&meta, &peer); + match devpod::up_streaming(&part_refs, &ct, sink).await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), } From 3f78eca555f94d4bdef919ce59dca3bbf6b537ec Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 23 May 2026 19:34:11 -0700 Subject: [PATCH 15/19] clippy: allow too_many_arguments on codespaces::create_streaming Each parameter mirrors a distinct gh codespace create flag and the non-streaming sibling create() has the same shape minus the cancel + on_chunk pair. Bundling into a struct would obscure the call site for no real readability win. --- crates/devcontainer-mcp-core/src/codespaces.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/devcontainer-mcp-core/src/codespaces.rs b/crates/devcontainer-mcp-core/src/codespaces.rs index 4f1d574..4c6f92f 100644 --- a/crates/devcontainer-mcp-core/src/codespaces.rs +++ b/crates/devcontainer-mcp-core/src/codespaces.rs @@ -73,6 +73,12 @@ pub async fn create( /// continue to come up and bill until the caller explicitly deletes /// it via `codespaces_delete`. The wire-level cancel is the most we /// can do — there's no `gh codespace cancel-create` API. +// +// `clippy::too_many_arguments` — each parameter mirrors a distinct +// `gh codespace create` flag, and the non-streaming sibling +// [`create`] has the same shape with one fewer pair. Bundling into +// a struct would obscure the call site for no real readability win. +#[allow(clippy::too_many_arguments)] pub async fn create_streaming( env: &HashMap, repo: &str, From 98fceadb744fdd73aee5d2a7e0f26aa1a6b6c7d3 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Wed, 27 May 2026 11:36:29 -0700 Subject: [PATCH 16/19] Add multi-container devcontainer workspace support Support the VS Code multi-container pattern where a workspace has multiple .devcontainer//devcontainer.json files referencing shared services in a single docker-compose.yml. Core changes: - New devcontainer_config module: discovers all configs in a workspace (root + .devcontainer/devcontainer.json + .devcontainer/*/devcontainer.json), parses them via jsonc-parser (handles comments + trailing commas per the devcontainer.json spec), and resolves abs paths + compose service + dockerComposeFile paths relative to each config's directory. - Reliable container lookup in docker.rs: matches compose configs via com.docker.compose.service + com.docker.compose.project.config_files rather than the devcontainer.local_folder label, which the CLI only stamps on the first container of a compose stack. The no-config path unions devcontainer.local_folder and com.docker.compose.project.working_dir matches so sibling containers are visible. Canonicalizes paths to survive macOS /private/var/... aliasing. - All devcontainer tools (up, exec, build, stop, remove, status, read_config, file_*) now accept an optional config parameter. Single- container workflows are unchanged. - exec passes the resolved container id to the CLI via --container-id, which lets it reach sibling compose containers that the CLI's own workspace-folder lookup can't see. - Ambiguity is surfaced as structured output: status returns {state:'Ambiguous',candidates:[...],hint:'...'} and lookup-style tools return errors listing every candidate so the agent can retry with the right config. - New devcontainer_list_configs MCP tool exposes the discovery output. Drive-by: narrow the cfg gate on process_tree.rs signal_all, any_alive, and libc_signal from cfg(unix) to cfg(target_os = 'linux') to match their only call site (reap_linux). The cfg(not(unix)) stubs were unreachable and silenced spurious dead_code warnings on macOS. README: new Multi-container workspaces section + tool count 45->46. --- Cargo.lock | 10 + README.md | 14 +- crates/devcontainer-mcp-core/Cargo.toml | 1 + .../devcontainer-mcp-core/src/devcontainer.rs | 273 ++++++++--- .../src/devcontainer_config.rs | 445 ++++++++++++++++++ crates/devcontainer-mcp-core/src/docker.rs | 184 +++++++- crates/devcontainer-mcp-core/src/lib.rs | 1 + .../devcontainer-mcp-core/src/process_tree.rs | 13 +- .../src/tools/devcontainer/build.rs | 13 +- .../src/tools/devcontainer/exec.rs | 5 + .../src/tools/devcontainer/files.rs | 37 +- .../src/tools/devcontainer/list_configs.rs | 30 ++ .../src/tools/devcontainer/mod.rs | 2 + .../src/tools/devcontainer/remove.rs | 12 +- .../src/tools/devcontainer/status.rs | 40 +- .../src/tools/devcontainer/stop.rs | 6 +- .../src/tools/devcontainer/up.rs | 4 +- 17 files changed, 982 insertions(+), 108 deletions(-) create mode 100644 crates/devcontainer-mcp-core/src/devcontainer_config.rs create mode 100644 crates/devcontainer-mcp/src/tools/devcontainer/list_configs.rs diff --git a/Cargo.lock b/Cargo.lock index 3d2552c..06dd48c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -320,6 +320,7 @@ dependencies = [ "base64", "bollard", "futures-util", + "jsonc-parser", "libc", "serde", "serde_json", @@ -825,6 +826,15 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "jsonc-parser" +version = "0.26.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d6d80e6d70e7911a29f3cf3f44f452df85d06f73572b494ca99a2cad3fcf8f4" +dependencies = [ + "serde_json", +] + [[package]] name = "lazy_static" version = "1.5.0" diff --git a/README.md b/README.md index 07ab0b4..e920864 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ Codespaces tools require an auth handle (e.g. `"github-aniongithub"`). The MCP s Supported providers: **GitHub**, **AWS**, **Azure**, **GCP**, **Kubernetes** -## MCP Tools (45 total) +## MCP Tools (46 total) ### Auth (4 tools) @@ -149,7 +149,7 @@ Supported providers: **GitHub**, **AWS**, **Azure**, **GCP**, **Kubernetes** | `devpod_file_edit` | Surgical string replacement — old_str → new_str | | `devpod_file_list` | List directory contents (non-hidden, 2 levels deep) | -### devcontainer CLI (11 tools) +### devcontainer CLI (12 tools) | Tool | Description | |------|-------------| @@ -157,6 +157,7 @@ Supported providers: **GitHub**, **AWS**, **Azure**, **GCP**, **Kubernetes** | `devcontainer_exec` | Execute a command inside a running dev container | | `devcontainer_build` | Build a dev container image | | `devcontainer_read_config` | Read merged devcontainer configuration as JSON | +| `devcontainer_list_configs` | Discover all devcontainer.json files in a workspace (single + multi-container) | | `devcontainer_stop` | Stop a dev container (via Docker API) | | `devcontainer_remove` | Remove a dev container and its resources | | `devcontainer_status` | Get dev container state by workspace folder | @@ -221,6 +222,15 @@ Install backend CLIs as needed — the MCP server detects them at runtime and re When `devcontainer_up`, `devpod_up`, or `codespaces_create` fails, the full build output (including errors) is returned to the agent. The agent can read the error, fix the `Dockerfile` or `devcontainer.json`, and retry — making the dev environment a **dynamic, agent-managed asset** rather than a static prerequisite. +## Multi-container workspaces + +The devcontainer spec supports [connecting to multiple containers](https://code.visualstudio.com/remote/advancedcontainers/connect-multiple-containers) in one workspace by placing per-service configs at `.devcontainer//devcontainer.json`, each pointing at a shared `docker-compose.yml`. `devcontainer-mcp` supports this pattern end-to-end: + +- **Discovery** — `devcontainer_list_configs` returns every config it finds (root `.devcontainer.json`, `.devcontainer/devcontainer.json`, and each `.devcontainer/*/devcontainer.json`) with its kind (`image` / `dockerfile` / `compose`), service name, and absolute path. +- **Targeting** — Every devcontainer tool (`up`, `exec`, `build`, `stop`, `remove`, `status`, `read_config`, `file_*`) accepts an optional `config` parameter pointing at a specific `devcontainer.json`. Single-container workflows continue to work unchanged — `config` defaults to whatever the devcontainer CLI auto-detects. +- **Ambiguity handling** — When a workspace has multiple configs and no `config` is provided, lookup-style tools (`status`, `exec`, `stop`, `remove`, `file_*`) return a structured `Ambiguous` result listing every matching container so the agent can pick the right one. `status` reports this as `{"state":"Ambiguous","candidates":[...],"hint":"..."}`. +- **Robust container matching** — Sibling compose containers are identified via `com.docker.compose.service` + `com.docker.compose.project.config_files` (not the unreliable `devcontainer.local_folder` label, which is only stamped on the first container). + ## Development This project eats its own dogfood — development happens inside its own devcontainer. diff --git a/crates/devcontainer-mcp-core/Cargo.toml b/crates/devcontainer-mcp-core/Cargo.toml index 9a02857..18c6f2e 100644 --- a/crates/devcontainer-mcp-core/Cargo.toml +++ b/crates/devcontainer-mcp-core/Cargo.toml @@ -18,6 +18,7 @@ futures-util = "0.3" async-trait = "0.1" base64 = "0.22" shlex = "1" +jsonc-parser = { version = "0.26", features = ["serde"] } [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/crates/devcontainer-mcp-core/src/devcontainer.rs b/crates/devcontainer-mcp-core/src/devcontainer.rs index 34e07fc..ca037c9 100644 --- a/crates/devcontainer-mcp-core/src/devcontainer.rs +++ b/crates/devcontainer-mcp-core/src/devcontainer.rs @@ -1,7 +1,8 @@ use crate::cli::{ run_cli, run_cli_streaming, run_with_shim, ChunkSink, CliBinary, CliOutput, RemoteKiller, }; -use crate::docker; +use crate::devcontainer_config::{resolve_config, ResolvedConfig}; +use crate::docker::{self, DevcontainerLookup}; use crate::error::Result; use std::sync::Arc; use tokio_util::sync::CancellationToken; @@ -11,6 +12,63 @@ async fn run_devcontainer(args: &[&str], parse_json: bool) -> Result run_cli(&CliBinary::Devcontainer, args, parse_json).await } +/// Best-effort resolve of `config` against `workspace_folder`. Returns +/// `None` if `config` is `None` *or* if parsing fails (we log + fall back +/// to no-config matching so a malformed config file never blocks lookups +/// the agent could otherwise satisfy). +fn try_resolve(workspace_folder: &str, config: Option<&str>) -> Option { + let cfg = config?; + match resolve_config(workspace_folder, cfg) { + Ok(r) => Some(r), + Err(e) => { + tracing::warn!(%e, config = %cfg, "failed to resolve devcontainer config; lookup will fall back to no-config matching"); + None + } + } +} + +/// Make `config` absolute against `workspace_folder` so the devcontainer +/// CLI resolves it correctly regardless of the MCP server's CWD. If +/// already absolute, returned unchanged. Falls back to the input when +/// canonicalize fails (e.g. file doesn't exist yet). +fn abs_config(workspace_folder: &str, config: &str) -> String { + let p = std::path::Path::new(config); + let joined = if p.is_absolute() { + p.to_path_buf() + } else { + std::path::Path::new(workspace_folder).join(p) + }; + std::fs::canonicalize(&joined) + .map(|c| c.to_string_lossy().into_owned()) + .unwrap_or_else(|_| joined.to_string_lossy().into_owned()) +} + +/// Build a uniform "ambiguous match" error string for stop/remove/status +/// callers when multiple containers match `workspace_folder` and no +/// `config` was supplied. Lists each candidate's id, name, compose service, +/// and devcontainer.config_file label so the agent has everything it needs +/// to retry with the right `config`. +fn ambiguous_error(workspace_folder: &str, candidates: &[docker::ContainerInfo]) -> String { + let lines: Vec = candidates + .iter() + .map(|c| { + let svc = c.compose_service().unwrap_or("(none)"); + let cfg = c.devcontainer_config_file().unwrap_or("(unlabeled)"); + format!( + " - id={short} name={name} service={svc} config_file={cfg}", + short = c.id.chars().take(12).collect::(), + name = c.name, + ) + }) + .collect(); + format!( + "Multiple containers match workspace `{workspace_folder}`. \ + Re-run with `config` set to the devcontainer.json path of the one you want. \ + Use `devcontainer_list_configs` to enumerate. Candidates:\n{}", + lines.join("\n") + ) +} + // --------------------------------------------------------------------------- // Lifecycle // --------------------------------------------------------------------------- @@ -22,7 +80,8 @@ pub async fn up( extra_args: &[&str], ) -> Result { let mut args = vec!["up", "--workspace-folder", workspace_folder]; - if let Some(c) = config { + let cfg_abs = config.map(|c| abs_config(workspace_folder, c)); + if let Some(c) = cfg_abs.as_deref() { args.push("--config"); args.push(c); } @@ -46,7 +105,8 @@ pub async fn up_streaming( on_chunk: Option>, ) -> Result { let mut args = vec!["up", "--workspace-folder", workspace_folder]; - if let Some(c) = config { + let cfg_abs = config.map(|c| abs_config(workspace_folder, c)); + if let Some(c) = cfg_abs.as_deref() { args.push("--config"); args.push(c); } @@ -63,12 +123,32 @@ pub async fn up_streaming( } /// `devcontainer exec` — execute a command in a running dev container. +/// +/// Resolves the target container via our reliable label-based lookup and +/// passes its id to the CLI via `--container-id`. This works for sibling +/// compose containers that the devcontainer CLI's own workspace-folder +/// lookup can't see (because only the first container in a compose stack +/// gets stamped with `devcontainer.*` labels). pub async fn exec( workspace_folder: &str, + config: Option<&str>, command: &str, command_args: &[&str], ) -> Result { - let mut args = vec!["exec", "--workspace-folder", workspace_folder, command]; + let container = lookup_one_or_err(workspace_folder, config).await?; + let mut args = vec![ + "exec", + "--container-id", + &container.id, + "--workspace-folder", + workspace_folder, + ]; + let cfg_abs = config.map(|c| abs_config(workspace_folder, c)); + if let Some(c) = cfg_abs.as_deref() { + args.push("--config"); + args.push(c); + } + args.push(command); args.extend_from_slice(command_args); run_devcontainer(&args, false).await } @@ -90,6 +170,7 @@ pub async fn exec( /// and use the captured PGID to reap them on cancel. pub async fn exec_streaming( workspace_folder: &str, + config: Option<&str>, command: &str, command_args: &[&str], cancel: &CancellationToken, @@ -120,21 +201,27 @@ pub async fn exec_streaming( // (DevPod, Codespaces) that can't pass remote env vars. let remote_env_arg = format!("{}={}", crate::exec_shim::USER_CMD_ENV, user_cmd); - let args: [&str; 7] = [ + // Resolve target container up-front (see `exec` docs for rationale). + let container = lookup_one_or_err(workspace_folder, config).await?; + + let mut all_args: Vec<&str> = vec![ "exec", + "--container-id", + &container.id, "--workspace-folder", workspace_folder, - "--remote-env", - &remote_env_arg, - "sh", - "-c", ]; - // run_with_shim takes a slice of &str; build a Vec to add the wrapped tail. - let mut all_args: Vec<&str> = args.to_vec(); + let cfg_abs = config.map(|c| abs_config(workspace_folder, c)); + if let Some(c) = cfg_abs.as_deref() { + all_args.push("--config"); + all_args.push(c); + } + all_args.extend_from_slice(&["--remote-env", &remote_env_arg, "sh", "-c"]); all_args.push(&wrapped); let killer: Arc = Arc::new(DevcontainerKiller { workspace_folder: workspace_folder.to_string(), + config: config.map(str::to_string), }); run_with_shim( @@ -149,9 +236,12 @@ pub async fn exec_streaming( } /// Delivers `kill - -` inside the devcontainer associated -/// with a workspace folder, using bollard's exec API. +/// with a workspace folder, using bollard's exec API. When `config` is +/// provided, the target container is disambiguated using the same +/// resolution as the rest of the lifecycle tools. struct DevcontainerKiller { workspace_folder: String, + config: Option, } #[async_trait::async_trait] @@ -166,20 +256,34 @@ impl RemoteKiller for DevcontainerKiller { return; } }; - let container = match docker::find_container_by_local_folder(&client, &self.workspace_folder).await { - Ok(Some(c)) => c, - Ok(None) => { - tracing::warn!( - workspace = %self.workspace_folder, - "no devcontainer found for in-container kill" - ); - return; - } - Err(e) => { - tracing::warn!(%e, "container lookup failed during in-container kill"); - return; - } - }; + let resolved = try_resolve(&self.workspace_folder, self.config.as_deref()); + let container = + match docker::find_devcontainer(&client, &self.workspace_folder, resolved.as_ref()) + .await + { + Ok(DevcontainerLookup::One(c)) => c, + Ok(DevcontainerLookup::Many(candidates)) => { + // Best effort: pick the first; we may be racing the + // user's own teardown, so an imperfect kill is fine. + tracing::warn!( + workspace = %self.workspace_folder, + count = candidates.len(), + "ambiguous container lookup during in-container kill; picking first" + ); + candidates.into_iter().next().unwrap() + } + Ok(DevcontainerLookup::None) => { + tracing::warn!( + workspace = %self.workspace_folder, + "no devcontainer found for in-container kill" + ); + return; + } + Err(e) => { + tracing::warn!(%e, "container lookup failed during in-container kill"); + return; + } + }; // `kill - -` signals every process in the group. // The POSIX `--` argument separator is deliberately omitted @@ -213,8 +317,17 @@ impl RemoteKiller for DevcontainerKiller { } /// `devcontainer build` — build a dev container image. -pub async fn build(workspace_folder: &str, extra_args: &[&str]) -> Result { +pub async fn build( + workspace_folder: &str, + config: Option<&str>, + extra_args: &[&str], +) -> Result { let mut args = vec!["build", "--workspace-folder", workspace_folder]; + let cfg_abs = config.map(|c| abs_config(workspace_folder, c)); + if let Some(c) = cfg_abs.as_deref() { + args.push("--config"); + args.push(c); + } args.extend_from_slice(extra_args); run_devcontainer(&args, true).await } @@ -223,11 +336,17 @@ pub async fn build(workspace_folder: &str, extra_args: &[&str]) -> Result, extra_args: &[&str], cancel: &CancellationToken, on_chunk: Option>, ) -> Result { let mut args = vec!["build", "--workspace-folder", workspace_folder]; + let cfg_abs = config.map(|c| abs_config(workspace_folder, c)); + if let Some(c) = cfg_abs.as_deref() { + args.push("--config"); + args.push(c); + } args.extend_from_slice(extra_args); run_cli_streaming( &CliBinary::Devcontainer, @@ -243,7 +362,8 @@ pub async fn build_streaming( /// `devcontainer read-configuration` — read devcontainer config as JSON. pub async fn read_configuration(workspace_folder: &str, config: Option<&str>) -> Result { let mut args = vec!["read-configuration", "--workspace-folder", workspace_folder]; - if let Some(c) = config { + let cfg_abs = config.map(|c| abs_config(workspace_folder, c)); + if let Some(c) = cfg_abs.as_deref() { args.push("--config"); args.push(c); } @@ -255,40 +375,65 @@ pub async fn read_configuration(workspace_folder: &str, config: Option<&str>) -> // Lifecycle via bollard (devcontainer CLI has no stop/down) // --------------------------------------------------------------------------- +/// Look up the single container for `workspace_folder` + `config`, +/// returning a structured error for `None` / `Many`. Used by stop/remove, +/// which refuse to act on ambiguous matches. +async fn lookup_one_or_err( + workspace_folder: &str, + config: Option<&str>, +) -> Result { + let client = docker::connect()?; + let resolved = try_resolve(workspace_folder, config); + match docker::find_devcontainer(&client, workspace_folder, resolved.as_ref()).await? { + DevcontainerLookup::One(c) => Ok(c), + DevcontainerLookup::None => Err(crate::error::Error::Io(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("No devcontainer found for workspace: {workspace_folder}"), + ))), + DevcontainerLookup::Many(candidates) => Err(crate::error::Error::Io( + std::io::Error::other(ambiguous_error(workspace_folder, &candidates)), + )), + } +} + /// Stop a dev container found by its workspace folder label. -pub async fn stop(workspace_folder: &str) -> Result { +pub async fn stop(workspace_folder: &str, config: Option<&str>) -> Result { let client = docker::connect()?; - let container = docker::find_container_by_local_folder(&client, workspace_folder) - .await? - .ok_or_else(|| { - crate::error::Error::Io(std::io::Error::new( - std::io::ErrorKind::NotFound, - format!("No devcontainer found for workspace: {workspace_folder}"), - )) - })?; + let container = lookup_one_or_err(workspace_folder, config).await?; docker::stop_container(&client, &container.id).await?; Ok(format!("Stopped container {}", container.name)) } /// Remove a dev container found by its workspace folder label. -pub async fn remove(workspace_folder: &str, force: bool) -> Result { +pub async fn remove(workspace_folder: &str, config: Option<&str>, force: bool) -> Result { let client = docker::connect()?; - let container = docker::find_container_by_local_folder(&client, workspace_folder) - .await? - .ok_or_else(|| { - crate::error::Error::Io(std::io::Error::new( - std::io::ErrorKind::NotFound, - format!("No devcontainer found for workspace: {workspace_folder}"), - )) - })?; + let container = lookup_one_or_err(workspace_folder, config).await?; docker::remove_container(&client, &container.id, force).await?; Ok(format!("Removed container {}", container.name)) } -/// Get status of a dev container by workspace folder label. -pub async fn status(workspace_folder: &str) -> Result> { +/// Status outcome for a devcontainer lookup. `Ambiguous` is the +/// multi-container case where no `config` was supplied to disambiguate; +/// callers should surface the candidates and point the agent at +/// `devcontainer_list_configs`. +#[derive(Debug, Clone)] +pub enum StatusOutcome { + NotFound, + Found(docker::ContainerInfo), + Ambiguous(Vec), +} + +/// Get status of a dev container by workspace folder + optional config. +pub async fn status(workspace_folder: &str, config: Option<&str>) -> Result { let client = docker::connect()?; - docker::find_container_by_local_folder(&client, workspace_folder).await + let resolved = try_resolve(workspace_folder, config); + Ok( + match docker::find_devcontainer(&client, workspace_folder, resolved.as_ref()).await? { + DevcontainerLookup::None => StatusOutcome::NotFound, + DevcontainerLookup::One(c) => StatusOutcome::Found(c), + DevcontainerLookup::Many(v) => StatusOutcome::Ambiguous(v), + }, + ) } // --------------------------------------------------------------------------- @@ -296,25 +441,35 @@ pub async fn status(workspace_folder: &str) -> Result Result { +pub async fn file_read( + workspace_folder: &str, + config: Option<&str>, + path: &str, +) -> Result { let cmd = crate::file_ops::read_file_command(path); - exec(workspace_folder, "sh", &["-c", &cmd]).await + exec(workspace_folder, config, "sh", &["-c", &cmd]).await } /// Write (create or overwrite) a file in a dev container. -pub async fn file_write(workspace_folder: &str, path: &str, content: &str) -> Result { +pub async fn file_write( + workspace_folder: &str, + config: Option<&str>, + path: &str, + content: &str, +) -> Result { let cmd = crate::file_ops::write_file_command(path, content); - exec(workspace_folder, "sh", &["-c", &cmd]).await + exec(workspace_folder, config, "sh", &["-c", &cmd]).await } /// Surgical edit: replace exactly one occurrence of `old_str` with `new_str`. pub async fn file_edit( workspace_folder: &str, + config: Option<&str>, path: &str, old_str: &str, new_str: &str, ) -> Result { - let read_output = file_read(workspace_folder, path).await?; + let read_output = file_read(workspace_folder, config, path).await?; if read_output.exit_code != 0 { return Err(crate::error::Error::FileRead(format!( "Failed to read {path}: {}", @@ -324,7 +479,7 @@ pub async fn file_edit( let modified = crate::file_ops::apply_edit(&read_output.stdout, old_str, new_str)?; - let write_output = file_write(workspace_folder, path, &modified).await?; + let write_output = file_write(workspace_folder, config, path, &modified).await?; if write_output.exit_code != 0 { return Err(crate::error::Error::FileEdit(format!( "Failed to write {path}: {}", @@ -336,7 +491,11 @@ pub async fn file_edit( } /// List directory contents in a dev container. -pub async fn file_list(workspace_folder: &str, path: &str) -> Result { +pub async fn file_list( + workspace_folder: &str, + config: Option<&str>, + path: &str, +) -> Result { let cmd = crate::file_ops::list_dir_command(path); - exec(workspace_folder, "sh", &["-c", &cmd]).await + exec(workspace_folder, config, "sh", &["-c", &cmd]).await } diff --git a/crates/devcontainer-mcp-core/src/devcontainer_config.rs b/crates/devcontainer-mcp-core/src/devcontainer_config.rs new file mode 100644 index 0000000..e7b8a7d --- /dev/null +++ b/crates/devcontainer-mcp-core/src/devcontainer_config.rs @@ -0,0 +1,445 @@ +//! Discovery and parsing of `devcontainer.json` files within a workspace. +//! +//! Implements the VS Code multi-container workspace pattern: a single repo +//! can contain several `.devcontainer//devcontainer.json` files, +//! typically referencing different services of a shared `docker-compose.yml`. +//! +//! Used by the `devcontainer_list_configs` MCP tool and as the basis for +//! disambiguating Docker container lookups when an operation targets a +//! specific config in a multi-container workspace. + +use crate::error::Result; +use jsonc_parser::ParseOptions; +use serde::Serialize; +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +/// One field of a `dockerComposeFile` — either a single path or an array. +fn json_to_string_list(value: &serde_json::Value) -> Vec { + match value { + serde_json::Value::String(s) => vec![s.clone()], + serde_json::Value::Array(items) => items + .iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect(), + _ => Vec::new(), + } +} + +/// Classification of a parsed devcontainer.json based on which top-level +/// fields are present. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum ConfigKind { + /// `dockerComposeFile` + `service` — multi-container compose project. + Compose, + /// `build.dockerfile` or top-level `dockerFile` — image is built locally. + Dockerfile, + /// `image` — pre-built image is used directly. + Image, + /// None of the above could be determined. + Unknown, +} + +/// One entry returned by [`list_configs`]: a discovered devcontainer.json +/// with a best-effort summary of its top-level fields. `path` is always +/// **relative** to the workspace folder so it can be fed straight back +/// into another tool's `config` parameter. +#[derive(Debug, Clone, Serialize)] +pub struct DiscoveredConfig { + pub path: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub image: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub service: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub docker_compose_file: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub workspace_folder: Option, + pub kind: ConfigKind, + /// When parsing fails, the entry still appears with the error message + /// here and all other fields empty — so an agent can see *that* the + /// config exists even if it can't be parsed. + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +/// Resolved view of a devcontainer.json used by the Docker container lookup +/// to disambiguate multi-container workspaces. Paths are absolute and +/// canonicalized so substring comparisons against container labels are +/// reliable across `/var/folders/...` ↔ `/private/var/folders/...` aliases +/// (macOS) and similar. +#[derive(Debug, Clone)] +pub struct ResolvedConfig { + pub abs_path: PathBuf, + pub service: Option, + /// Absolute, canonicalized paths of every `dockerComposeFile` entry. + pub compose_files_abs: Vec, + pub kind: ConfigKind, +} + +/// Parse `text` as JSONC (JSON with comments + trailing commas, per the +/// devcontainer spec) into a `serde_json::Value`. +fn parse_jsonc(text: &str) -> std::result::Result { + let parsed = jsonc_parser::parse_to_serde_value(text, &ParseOptions::default()) + .map_err(|e| e.to_string())? + .ok_or_else(|| "empty document".to_string())?; + Ok(parsed) +} + +/// Determine the search paths for devcontainer configs under `workspace`. +/// Returns absolute paths in priority order: root `.devcontainer.json`, +/// `.devcontainer/devcontainer.json`, then every +/// `.devcontainer/*/devcontainer.json`. +fn candidate_paths(workspace: &Path) -> Vec { + let mut out = Vec::new(); + let root_dotfile = workspace.join(".devcontainer.json"); + if root_dotfile.is_file() { + out.push(root_dotfile); + } + let dc_dir = workspace.join(".devcontainer"); + let root_in_dir = dc_dir.join("devcontainer.json"); + if root_in_dir.is_file() { + out.push(root_in_dir); + } + if let Ok(entries) = std::fs::read_dir(&dc_dir) { + // Sort sub-folder configs deterministically so the agent sees a + // stable order across calls. + let mut subdirs: Vec = entries + .filter_map(|e| e.ok()) + .map(|e| e.path()) + .filter(|p| p.is_dir()) + .collect(); + subdirs.sort(); + for sub in subdirs { + let candidate = sub.join("devcontainer.json"); + if candidate.is_file() { + out.push(candidate); + } + } + } + out +} + +/// Convert `abs` to a path relative to `base`, falling back to the absolute +/// path's string form if it isn't actually inside `base`. +fn rel_to(abs: &Path, base: &Path) -> String { + abs.strip_prefix(base) + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_else(|_| abs.to_string_lossy().into_owned()) +} + +/// Best-effort canonicalization that falls back to the input on error. +/// Used because compose files referenced from a not-yet-built devcontainer +/// might not exist at scan time, but we still want a stable absolute path. +fn canonicalize_or_keep(p: &Path) -> PathBuf { + std::fs::canonicalize(p).unwrap_or_else(|_| p.to_path_buf()) +} + +fn classify(obj: &serde_json::Map) -> ConfigKind { + if obj.get("dockerComposeFile").is_some() { + return ConfigKind::Compose; + } + let has_dockerfile = obj.get("dockerFile").is_some() + || obj + .get("build") + .and_then(|b| b.as_object()) + .is_some_and(|b| b.contains_key("dockerfile") || b.contains_key("dockerFile")); + if has_dockerfile { + return ConfigKind::Dockerfile; + } + if obj.get("image").is_some() { + return ConfigKind::Image; + } + ConfigKind::Unknown +} + +/// Build a [`DiscoveredConfig`] from a parsed JSON value. Top-level keys are +/// best-effort: missing or wrong-typed fields are skipped, not errors. +fn build_discovered(rel_path: String, value: &serde_json::Value) -> DiscoveredConfig { + let Some(obj) = value.as_object() else { + return DiscoveredConfig { + path: rel_path, + error: Some("top-level value is not an object".into()), + kind: ConfigKind::Unknown, + name: None, + image: None, + service: None, + docker_compose_file: Vec::new(), + workspace_folder: None, + }; + }; + DiscoveredConfig { + path: rel_path, + name: obj.get("name").and_then(|v| v.as_str()).map(str::to_string), + image: obj + .get("image") + .and_then(|v| v.as_str()) + .map(str::to_string), + service: obj + .get("service") + .and_then(|v| v.as_str()) + .map(str::to_string), + docker_compose_file: obj + .get("dockerComposeFile") + .map(json_to_string_list) + .unwrap_or_default(), + workspace_folder: obj + .get("workspaceFolder") + .and_then(|v| v.as_str()) + .map(str::to_string), + kind: classify(obj), + error: None, + } +} + +/// Discover every devcontainer.json under `workspace_folder` and return a +/// best-effort summary of each. Parse errors are reported per-entry; the +/// call itself only fails on IO problems with `workspace_folder`. +pub fn list_configs(workspace_folder: &str) -> Result> { + let workspace = PathBuf::from(workspace_folder); + let workspace_abs = canonicalize_or_keep(&workspace); + let mut out = Vec::new(); + for abs_path in candidate_paths(&workspace_abs) { + let rel = rel_to(&abs_path, &workspace_abs); + let entry = match std::fs::read_to_string(&abs_path) { + Ok(text) => match parse_jsonc(&text) { + Ok(v) => build_discovered(rel, &v), + Err(e) => DiscoveredConfig { + path: rel, + error: Some(format!("JSONC parse error: {e}")), + kind: ConfigKind::Unknown, + name: None, + image: None, + service: None, + docker_compose_file: Vec::new(), + workspace_folder: None, + }, + }, + Err(e) => DiscoveredConfig { + path: rel, + error: Some(format!("read error: {e}")), + kind: ConfigKind::Unknown, + name: None, + image: None, + service: None, + docker_compose_file: Vec::new(), + workspace_folder: None, + }, + }; + out.push(entry); + } + Ok(out) +} + +/// Resolve `config` (a path to a devcontainer.json, either absolute or +/// relative to `workspace_folder`) into a [`ResolvedConfig`] used by the +/// Docker container lookup. +/// +/// `dockerComposeFile` entries are resolved against the *config file's* +/// directory (matching the devcontainer CLI's behavior) and canonicalized. +pub fn resolve_config(workspace_folder: &str, config: &str) -> Result { + let workspace = canonicalize_or_keep(&PathBuf::from(workspace_folder)); + let config_path = { + let p = PathBuf::from(config); + if p.is_absolute() { + p + } else { + workspace.join(p) + } + }; + let abs_path = canonicalize_or_keep(&config_path); + let text = std::fs::read_to_string(&abs_path)?; + let value = parse_jsonc(&text).map_err(|e| { + crate::error::Error::Io(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("JSONC parse error in {}: {e}", abs_path.display()), + )) + })?; + let obj = value.as_object().ok_or_else(|| { + crate::error::Error::Io(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("top-level value is not an object in {}", abs_path.display()), + )) + })?; + + let kind = classify(obj); + let service = obj + .get("service") + .and_then(|v| v.as_str()) + .map(str::to_string); + + // `dockerComposeFile` paths are resolved against the directory + // containing the devcontainer.json, per the devcontainer spec. + let config_dir = abs_path.parent().unwrap_or(&workspace).to_path_buf(); + let compose_files_abs: Vec = obj + .get("dockerComposeFile") + .map(json_to_string_list) + .unwrap_or_default() + .into_iter() + .map(|entry| { + let p = PathBuf::from(&entry); + let abs = if p.is_absolute() { + p + } else { + config_dir.join(p) + }; + canonicalize_or_keep(&abs) + }) + .collect(); + + Ok(ResolvedConfig { + abs_path, + service, + compose_files_abs, + kind, + }) +} + +/// Helper for callers that want a `BTreeMap` of common +/// label filters derived from a [`ResolvedConfig`]. Not used directly by +/// the docker layer (which builds its own filters), but handy for tests +/// and tracing. +pub fn expected_labels(resolved: &ResolvedConfig) -> BTreeMap { + let mut out = BTreeMap::new(); + if let Some(svc) = &resolved.service { + out.insert("com.docker.compose.service".into(), svc.clone()); + } + out.insert( + "devcontainer.config_file".into(), + resolved.abs_path.to_string_lossy().into_owned(), + ); + out +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + fn write(p: &Path, contents: &str) { + if let Some(parent) = p.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(p, contents).unwrap(); + } + + #[test] + fn discovers_root_devcontainer_in_subdir() { + let dir = tempdir().unwrap(); + write( + &dir.path().join(".devcontainer/devcontainer.json"), + r#"{ "name": "Root", "image": "alpine:3.20" }"#, + ); + let out = list_configs(dir.path().to_str().unwrap()).unwrap(); + assert_eq!(out.len(), 1); + assert_eq!(out[0].path, ".devcontainer/devcontainer.json"); + assert_eq!(out[0].name.as_deref(), Some("Root")); + assert_eq!(out[0].kind, ConfigKind::Image); + } + + #[test] + fn discovers_root_dotfile() { + let dir = tempdir().unwrap(); + write( + &dir.path().join(".devcontainer.json"), + r#"{ "image": "alpine:3.20" }"#, + ); + let out = list_configs(dir.path().to_str().unwrap()).unwrap(); + assert_eq!(out.len(), 1); + assert_eq!(out[0].path, ".devcontainer.json"); + } + + #[test] + fn discovers_multi_subfolder_configs_sorted() { + let dir = tempdir().unwrap(); + write( + &dir.path().join(".devcontainer/python/devcontainer.json"), + r#"// python config + { + "name": "Python", + "dockerComposeFile": ["../../docker-compose.yml"], + "service": "python-api", + "workspaceFolder": "/workspace/py", + }"#, + ); + write( + &dir.path().join(".devcontainer/node/devcontainer.json"), + r#"{ + "name": "Node", + "dockerComposeFile": "../../docker-compose.yml", + "service": "node-app" + }"#, + ); + let out = list_configs(dir.path().to_str().unwrap()).unwrap(); + assert_eq!(out.len(), 2); + // Sorted by directory name: node, python + assert_eq!(out[0].service.as_deref(), Some("node-app")); + assert_eq!(out[1].service.as_deref(), Some("python-api")); + assert_eq!(out[0].kind, ConfigKind::Compose); + assert_eq!(out[1].kind, ConfigKind::Compose); + // Single-string dockerComposeFile is normalized to a one-element list. + assert_eq!(out[0].docker_compose_file.len(), 1); + // Trailing comma + comments parsed by JSONC. + assert_eq!(out[1].workspace_folder.as_deref(), Some("/workspace/py")); + } + + #[test] + fn malformed_config_reports_per_entry_error() { + let dir = tempdir().unwrap(); + write( + &dir.path().join(".devcontainer/broken/devcontainer.json"), + r#"{ "name": "broken" "#, + ); + let out = list_configs(dir.path().to_str().unwrap()).unwrap(); + assert_eq!(out.len(), 1); + assert!(out[0].error.is_some()); + } + + #[test] + fn classifies_dockerfile_build() { + let dir = tempdir().unwrap(); + write( + &dir.path().join(".devcontainer/devcontainer.json"), + r#"{ "build": { "dockerfile": "Dockerfile" } }"#, + ); + let out = list_configs(dir.path().to_str().unwrap()).unwrap(); + assert_eq!(out[0].kind, ConfigKind::Dockerfile); + } + + #[test] + fn resolve_config_canonicalizes_compose_files() { + let dir = tempdir().unwrap(); + write( + &dir.path().join("docker-compose.yml"), + "services:\n a:\n image: alpine\n", + ); + write( + &dir.path().join(".devcontainer/a/devcontainer.json"), + r#"{ + "dockerComposeFile": ["../../docker-compose.yml"], + "service": "a" + }"#, + ); + let resolved = resolve_config( + dir.path().to_str().unwrap(), + ".devcontainer/a/devcontainer.json", + ) + .unwrap(); + assert_eq!(resolved.service.as_deref(), Some("a")); + assert_eq!(resolved.kind, ConfigKind::Compose); + assert_eq!(resolved.compose_files_abs.len(), 1); + let compose = &resolved.compose_files_abs[0]; + assert!(compose.is_absolute()); + assert!(compose.ends_with("docker-compose.yml")); + } + + #[test] + fn list_returns_empty_when_no_configs() { + let dir = tempdir().unwrap(); + let out = list_configs(dir.path().to_str().unwrap()).unwrap(); + assert!(out.is_empty()); + } +} diff --git a/crates/devcontainer-mcp-core/src/docker.rs b/crates/devcontainer-mcp-core/src/docker.rs index 48fc878..34ca335 100644 --- a/crates/devcontainer-mcp-core/src/docker.rs +++ b/crates/devcontainer-mcp-core/src/docker.rs @@ -6,6 +6,7 @@ use futures_util::StreamExt; use serde::Serialize; use std::collections::HashMap; +use crate::devcontainer_config::{ConfigKind, ResolvedConfig}; use crate::error::Result; /// Summary of a container's state. @@ -18,45 +19,178 @@ pub struct ContainerInfo { pub labels: HashMap, } +impl ContainerInfo { + /// `com.docker.compose.service` label, if present — used for ambiguity + /// diagnostics in multi-container workspaces. + pub fn compose_service(&self) -> Option<&str> { + self.labels + .get("com.docker.compose.service") + .map(String::as_str) + } + + /// `devcontainer.config_file` label, if present. + pub fn devcontainer_config_file(&self) -> Option<&str> { + self.labels + .get("devcontainer.config_file") + .map(String::as_str) + } +} + +/// Outcome of [`find_devcontainer`]. `Many` is surfaced to callers so they +/// can either pick deterministically (e.g. status returning the first + +/// flagging `multipleMatches`) or refuse and ask for a `config` (e.g. stop +/// / remove). +#[derive(Debug, Clone)] +pub enum DevcontainerLookup { + None, + One(ContainerInfo), + Many(Vec), +} + +impl DevcontainerLookup { + pub fn into_one(self) -> Option { + match self { + DevcontainerLookup::One(c) => Some(c), + _ => None, + } + } + + /// Convenience: collapse `One`/`Many` to "any of them" for diagnostics. + pub fn candidates(&self) -> &[ContainerInfo] { + match self { + DevcontainerLookup::None => &[], + DevcontainerLookup::One(c) => std::slice::from_ref(c), + DevcontainerLookup::Many(v) => v.as_slice(), + } + } +} + /// Create a Docker client connected to the local socket. pub fn connect() -> Result { Ok(Docker::connect_with_local_defaults()?) } -/// Find a container by the standard `devcontainer.local_folder` label. -pub async fn find_container_by_local_folder( - docker: &Docker, - local_folder: &str, -) -> Result> { +/// Internal helper: list containers matching one label filter. +async fn list_by_label(docker: &Docker, label_eq: &str) -> Result> { let mut filters = HashMap::new(); - filters.insert( - "label".to_string(), - vec![format!("devcontainer.local_folder={local_folder}")], - ); - + filters.insert("label".to_string(), vec![label_eq.to_string()]); let options = ListContainersOptions { all: true, filters, ..Default::default() }; - let containers = docker.list_containers(Some(options)).await?; + Ok(containers.into_iter().map(container_to_info).collect()) +} - Ok(containers.into_iter().next().map(|c| { - let labels = c.labels.unwrap_or_default(); - ContainerInfo { - id: c.id.unwrap_or_default(), - name: c - .names - .and_then(|n| n.first().cloned()) - .unwrap_or_default() - .trim_start_matches('/') - .to_string(), - image: c.image.unwrap_or_default(), - state: c.state.unwrap_or_default(), - labels, +fn container_to_info(c: bollard::models::ContainerSummary) -> ContainerInfo { + let labels = c.labels.unwrap_or_default(); + ContainerInfo { + id: c.id.unwrap_or_default(), + name: c + .names + .and_then(|n| n.first().cloned()) + .unwrap_or_default() + .trim_start_matches('/') + .to_string(), + image: c.image.unwrap_or_default(), + state: c.state.unwrap_or_default(), + labels, + } +} + +/// Find devcontainer-managed container(s) for `workspace_folder`, optionally +/// scoped to a specific resolved config. +/// +/// Strategy: +/// +/// 1. **Compose config provided** — filter by `com.docker.compose.service` +/// and verify the container's `com.docker.compose.project.config_files` +/// label contains one of the resolved compose-file paths. This is the +/// only reliable path for sibling containers in a multi-service +/// workspace, because the devcontainer CLI only stamps `devcontainer.*` +/// labels on the first container of the compose project. +/// 2. **Image / Dockerfile config provided** — filter by +/// `devcontainer.config_file=`. The CLI labels these +/// consistently. +/// 3. **No config** — start with `devcontainer.local_folder=`. +/// If that misses, fall back to +/// `com.docker.compose.project.working_dir=` to catch the +/// multi-container case where sibling containers lack devcontainer.* +/// labels. +pub async fn find_devcontainer( + docker: &Docker, + workspace_folder: &str, + config: Option<&ResolvedConfig>, +) -> Result { + // Canonicalize the workspace path the same way the CLI does so + // label substring/equality checks survive macOS `/private/var/...` + // aliasing and trailing-slash variations. + let workspace_abs = std::fs::canonicalize(workspace_folder) + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_else(|_| workspace_folder.to_string()); + + let candidates: Vec = match config { + Some(cfg) if cfg.kind == ConfigKind::Compose => { + if let Some(service) = cfg.service.as_deref() { + let service_filter = format!("com.docker.compose.service={service}"); + let by_service = list_by_label(docker, &service_filter).await?; + let expected: Vec = cfg + .compose_files_abs + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect(); + by_service + .into_iter() + .filter(|c| { + let Some(cfg_files) = + c.labels.get("com.docker.compose.project.config_files") + else { + return false; + }; + expected.iter().any(|exp| cfg_files.contains(exp.as_str())) + }) + .collect() + } else { + // Compose config without a service field — fall back to + // the no-config matching logic inline (avoids async + // recursion). + lookup_without_config(docker, &workspace_abs).await? + } } - })) + Some(cfg) => { + let cfg_str = cfg.abs_path.to_string_lossy(); + let filter = format!("devcontainer.config_file={cfg_str}"); + list_by_label(docker, &filter).await? + } + None => lookup_without_config(docker, &workspace_abs).await?, + }; + + Ok(match candidates.len() { + 0 => DevcontainerLookup::None, + 1 => DevcontainerLookup::One(candidates.into_iter().next().unwrap()), + _ => DevcontainerLookup::Many(candidates), + }) +} + +/// No-config matching: union of `devcontainer.local_folder` and +/// `com.docker.compose.project.working_dir` matches (deduped by container +/// id). Single-container workspaces yield exactly the primary container; +/// multi-container compose workspaces yield every sibling, even those +/// that don't carry `devcontainer.*` labels (the CLI only stamps them on +/// the first container of the compose project). +async fn lookup_without_config(docker: &Docker, workspace_abs: &str) -> Result> { + let primary = format!("devcontainer.local_folder={workspace_abs}"); + let mut hits = list_by_label(docker, &primary).await?; + let fallback = format!("com.docker.compose.project.working_dir={workspace_abs}"); + let fallback_hits = list_by_label(docker, &fallback).await?; + let seen: std::collections::HashSet = hits.iter().map(|c| c.id.clone()).collect(); + for c in fallback_hits { + if !seen.contains(&c.id) { + hits.push(c); + } + } + Ok(hits) } /// Inspect a container by name or ID. diff --git a/crates/devcontainer-mcp-core/src/lib.rs b/crates/devcontainer-mcp-core/src/lib.rs index 7c7bcf1..a54d566 100644 --- a/crates/devcontainer-mcp-core/src/lib.rs +++ b/crates/devcontainer-mcp-core/src/lib.rs @@ -2,6 +2,7 @@ pub mod auth; pub mod cli; pub mod codespaces; pub mod devcontainer; +pub mod devcontainer_config; pub mod devpod; pub mod docker; pub mod error; diff --git a/crates/devcontainer-mcp-core/src/process_tree.rs b/crates/devcontainer-mcp-core/src/process_tree.rs index 0b46507..50f43d2 100644 --- a/crates/devcontainer-mcp-core/src/process_tree.rs +++ b/crates/devcontainer-mcp-core/src/process_tree.rs @@ -134,7 +134,7 @@ fn read_ppid(pid: u32) -> Option { None } -#[cfg(unix)] +#[cfg(target_os = "linux")] fn signal_all(pids: &[u32], sig: i32) { for &pid in pids { // kill(2) can fail with ESRCH if the process already exited; @@ -146,9 +146,6 @@ fn signal_all(pids: &[u32], sig: i32) { } } -#[cfg(not(unix))] -fn signal_all(_: &[u32], _: i32) {} - #[cfg(target_os = "linux")] fn any_alive(pids: &[u32]) -> bool { pids.iter().any(|&pid| { @@ -158,18 +155,12 @@ fn any_alive(pids: &[u32]) -> bool { }) } -#[cfg(unix)] +#[cfg(target_os = "linux")] mod libc_signal { pub const SIGTERM: i32 = libc::SIGTERM; pub const SIGKILL: i32 = libc::SIGKILL; } -#[cfg(not(unix))] -mod libc_signal { - pub const SIGTERM: i32 = 15; - pub const SIGKILL: i32 = 9; -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/build.rs b/crates/devcontainer-mcp/src/tools/devcontainer/build.rs index 82b3bcd..790fdf3 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/build.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/build.rs @@ -12,6 +12,10 @@ use crate::tools::DevContainerMcp; struct DevcontainerBuildParams { #[schemars(description = "Path to the workspace folder")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, #[schemars( description = "Additional flags as space-separated args, e.g. '--no-cache --image-name my-image'" )] @@ -38,7 +42,14 @@ impl DevContainerMcp { .unwrap_or_default(); let extra_refs: Vec<&str> = extra.iter().map(|s| s.as_str()).collect(); let sink = progress_sink_from_meta(&meta, &peer); - match devcontainer::build_streaming(¶ms.workspace_folder, &extra_refs, &ct, sink).await + match devcontainer::build_streaming( + ¶ms.workspace_folder, + params.config.as_deref(), + &extra_refs, + &ct, + sink, + ) + .await { Ok(output) => format_output(&output), Err(e) => format!("Error: {e}"), diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs index 0a4cb86..54775ee 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/exec.rs @@ -12,6 +12,10 @@ use crate::tools::DevContainerMcp; struct DevcontainerExecParams { #[schemars(description = "Path to the workspace folder")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, #[schemars(description = "Command to execute inside the container")] command: String, #[schemars(description = "Arguments for the command as a space-separated string")] @@ -40,6 +44,7 @@ impl DevContainerMcp { match devcontainer::exec_streaming( ¶ms.workspace_folder, + params.config.as_deref(), "sh", &["-c", &full_cmd], &ct, diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/files.rs b/crates/devcontainer-mcp/src/tools/devcontainer/files.rs index d22ba25..fc98e1b 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/files.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/files.rs @@ -8,6 +8,10 @@ use crate::tools::DevContainerMcp; struct DevcontainerFileReadParams { #[schemars(description = "Path to the workspace folder")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, #[schemars(description = "Path to the file inside the container")] path: String, #[schemars(description = "Start line number (1-based, inclusive)")] @@ -22,6 +26,10 @@ struct DevcontainerFileReadParams { struct DevcontainerFileWriteParams { #[schemars(description = "Path to the workspace folder")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, #[schemars(description = "Path to the file inside the container")] path: String, #[schemars(description = "File content to write")] @@ -32,6 +40,10 @@ struct DevcontainerFileWriteParams { struct DevcontainerFileEditParams { #[schemars(description = "Path to the workspace folder")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, #[schemars(description = "Path to the file inside the container")] path: String, #[schemars(description = "The exact string in the file to replace. Must match exactly once.")] @@ -44,6 +56,10 @@ struct DevcontainerFileEditParams { struct DevcontainerFileListParams { #[schemars(description = "Path to the workspace folder")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, #[schemars(description = "Path to the directory inside the container (defaults to '.')")] path: Option, } @@ -58,7 +74,13 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - match devcontainer::file_read(¶ms.workspace_folder, ¶ms.path).await { + match devcontainer::file_read( + ¶ms.workspace_folder, + params.config.as_deref(), + ¶ms.path, + ) + .await + { Ok(output) => { if output.exit_code != 0 { return format!( @@ -84,8 +106,13 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - match devcontainer::file_write(¶ms.workspace_folder, ¶ms.path, ¶ms.content) - .await + match devcontainer::file_write( + ¶ms.workspace_folder, + params.config.as_deref(), + ¶ms.path, + ¶ms.content, + ) + .await { Ok(output) => { if output.exit_code != 0 { @@ -112,6 +139,7 @@ impl DevContainerMcp { ) -> String { match devcontainer::file_edit( ¶ms.workspace_folder, + params.config.as_deref(), ¶ms.path, ¶ms.old_str, ¶ms.new_str, @@ -132,7 +160,8 @@ impl DevContainerMcp { Parameters(params): Parameters, ) -> String { let dir = params.path.as_deref().unwrap_or("."); - match devcontainer::file_list(¶ms.workspace_folder, dir).await { + match devcontainer::file_list(¶ms.workspace_folder, params.config.as_deref(), dir).await + { Ok(output) => { if output.exit_code != 0 { format!( diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/list_configs.rs b/crates/devcontainer-mcp/src/tools/devcontainer/list_configs.rs new file mode 100644 index 0000000..5f22358 --- /dev/null +++ b/crates/devcontainer-mcp/src/tools/devcontainer/list_configs.rs @@ -0,0 +1,30 @@ +use devcontainer_mcp_core::devcontainer_config; +use rmcp::handler::server::wrapper::Parameters; +use rmcp::{tool, tool_router}; + +use crate::tools::DevContainerMcp; + +#[derive(serde::Deserialize, schemars::JsonSchema)] +struct DevcontainerListConfigsParams { + #[schemars(description = "Path to the workspace folder to scan for devcontainer.json files")] + workspace_folder: String, +} + +#[tool_router(router = devcontainer_list_configs_router, vis = "pub(super)")] +impl DevContainerMcp { + #[tool( + name = "devcontainer_list_configs", + description = "Enumerate devcontainer.json files in a workspace. Returns each discovered config (root `.devcontainer.json`, `.devcontainer/devcontainer.json`, and `.devcontainer/*/devcontainer.json`) with its parsed name, image, service, dockerComposeFile, workspaceFolder, and kind (compose | image | dockerfile | unknown). Use the returned `path` directly as the `config` parameter to other devcontainer tools to target a specific container in a multi-container workspace." + )] + async fn devcontainer_list_configs( + &self, + Parameters(params): Parameters, + ) -> String { + match devcontainer_config::list_configs(¶ms.workspace_folder) { + Ok(entries) => { + serde_json::to_string(&entries).unwrap_or_else(|e| format!("Error: {e}")) + } + Err(e) => format!("Error: {e}"), + } + } +} diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/mod.rs b/crates/devcontainer-mcp/src/tools/devcontainer/mod.rs index 08de29b..3ba0406 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/mod.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/mod.rs @@ -2,6 +2,7 @@ mod build; mod config; mod exec; mod files; +mod list_configs; mod remove; mod status; mod stop; @@ -21,5 +22,6 @@ impl DevContainerMcp { + Self::devcontainer_remove_router() + Self::devcontainer_status_router() + Self::devcontainer_files_router() + + Self::devcontainer_list_configs_router() } } diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/remove.rs b/crates/devcontainer-mcp/src/tools/devcontainer/remove.rs index dabfe53..72638dc 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/remove.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/remove.rs @@ -8,6 +8,10 @@ use crate::tools::DevContainerMcp; struct DevcontainerRemoveParams { #[schemars(description = "Path to the workspace folder (used to find the container by label)")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, #[schemars(description = "Force removal even if the container is running")] force: Option, } @@ -22,7 +26,13 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - match devcontainer::remove(¶ms.workspace_folder, params.force.unwrap_or(false)).await { + match devcontainer::remove( + ¶ms.workspace_folder, + params.config.as_deref(), + params.force.unwrap_or(false), + ) + .await + { Ok(msg) => msg, Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/status.rs b/crates/devcontainer-mcp/src/tools/devcontainer/status.rs index c8de498..c35e530 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/status.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/status.rs @@ -1,6 +1,7 @@ -use devcontainer_mcp_core::devcontainer; +use devcontainer_mcp_core::devcontainer::{self, StatusOutcome}; use rmcp::handler::server::wrapper::Parameters; use rmcp::{tool, tool_router}; +use serde_json::json; use crate::tools::DevContainerMcp; @@ -8,23 +9,52 @@ use crate::tools::DevContainerMcp; struct DevcontainerStatusParams { #[schemars(description = "Path to the workspace folder")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, } #[tool_router(router = devcontainer_status_router, vis = "pub(super)")] impl DevContainerMcp { #[tool( name = "devcontainer_status", - description = "Get the status of a local dev container. Returns container info (state, image, labels) or null if not found." + description = "Get the status of a local dev container. Returns container info (state, image, labels) for a single match, `{\"state\":\"NotFound\"}` if nothing matches, or `{\"state\":\"Ambiguous\",\"candidates\":[...]}` if multiple containers match the workspace and no `config` was supplied to disambiguate." )] async fn devcontainer_status( &self, Parameters(params): Parameters, ) -> String { - match devcontainer::status(¶ms.workspace_folder).await { - Ok(Some(info)) => { + match devcontainer::status(¶ms.workspace_folder, params.config.as_deref()).await { + Ok(StatusOutcome::Found(info)) => { serde_json::to_string(&info).unwrap_or_else(|e| format!("Error: {e}")) } - Ok(None) => r#"{"state":"NotFound"}"#.to_string(), + Ok(StatusOutcome::NotFound) => r#"{"state":"NotFound"}"#.to_string(), + Ok(StatusOutcome::Ambiguous(candidates)) => { + // Surface every candidate's identity, service, and + // config-file label so the agent has enough info to + // pick the right `config` and retry without further + // calls. + let entries: Vec<_> = candidates + .iter() + .map(|c| { + json!({ + "id": c.id, + "name": c.name, + "image": c.image, + "state": c.state, + "composeService": c.compose_service(), + "configFile": c.devcontainer_config_file(), + }) + }) + .collect(); + json!({ + "state": "Ambiguous", + "candidates": entries, + "hint": "Multiple containers match this workspace. Call devcontainer_list_configs and re-run with the `config` parameter set to the desired devcontainer.json.", + }) + .to_string() + } Err(e) => format!("Error: {e}"), } } diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/stop.rs b/crates/devcontainer-mcp/src/tools/devcontainer/stop.rs index 38acf79..78c30c0 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/stop.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/stop.rs @@ -8,6 +8,10 @@ use crate::tools::DevContainerMcp; struct DevcontainerStopParams { #[schemars(description = "Path to the workspace folder (used to find the container by label)")] workspace_folder: String, + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] + config: Option, } #[tool_router(router = devcontainer_stop_router, vis = "pub(super)")] @@ -20,7 +24,7 @@ impl DevContainerMcp { &self, Parameters(params): Parameters, ) -> String { - match devcontainer::stop(¶ms.workspace_folder).await { + match devcontainer::stop(¶ms.workspace_folder, params.config.as_deref()).await { Ok(msg) => msg, Err(e) => format!("Error: {e}"), } diff --git a/crates/devcontainer-mcp/src/tools/devcontainer/up.rs b/crates/devcontainer-mcp/src/tools/devcontainer/up.rs index 9924a0c..9c7dd79 100644 --- a/crates/devcontainer-mcp/src/tools/devcontainer/up.rs +++ b/crates/devcontainer-mcp/src/tools/devcontainer/up.rs @@ -14,7 +14,9 @@ struct DevcontainerUpParams { description = "Path to the workspace folder containing .devcontainer/devcontainer.json" )] workspace_folder: String, - #[schemars(description = "Path to a specific devcontainer.json (overrides auto-detection)")] + #[schemars( + description = "Path to a specific devcontainer.json (use to disambiguate multi-container workspaces)" + )] config: Option, #[schemars( description = "Additional flags as space-separated args, e.g. '--remove-existing-container --build-no-cache'" From ba2ceaba98661deb78e93f810af42c06af3e41ec Mon Sep 17 00:00:00 2001 From: aniongithub Date: Wed, 27 May 2026 23:13:38 -0700 Subject: [PATCH 17/19] install.sh: auto-install devcontainer CLI via standalone bundle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the devcontainer CLI is missing, the installer now bootstraps it via the official @devcontainers/cli standalone installer, which bundles its own Node.js runtime — so no system Node is required. - Installs into ~/.local/share/devcontainer-mcp/devcontainers/ (a folder we own, kept separate from the user's other tools). - Symlinks the resulting binary into INSTALL_DIR/devcontainer so it sits on the same PATH line the installer already tells users to add. - Pre-creates the parent dir so the upstream installer's parent-writable check passes on fresh systems. - Failure is non-fatal: prints a pointer to the upstream install script and continues. devpod and gh remain detect-only (out of scope). install.ps1 delegates to install.sh inside WSL, so Windows users get the auto-install for free. --- install.sh | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/install.sh b/install.sh index da18c0f..4407da6 100755 --- a/install.sh +++ b/install.sh @@ -3,8 +3,12 @@ set -euo pipefail # devcontainer-mcp installer # Downloads the latest release binary. -# Backend CLIs (devpod, devcontainer, gh) are detected at runtime — -# if missing, the MCP server returns a helpful error message. +# Backend CLIs: +# - devcontainer (@devcontainers/cli) is auto-installed via the official +# standalone installer (bundles its own Node.js runtime) into +# ~/.local/share/devcontainer-mcp/devcontainers/ when missing. +# - devpod and gh are detected only; if missing, the MCP server returns +# a helpful error message and the installer prints install URLs. # # Usage: # curl -fsSL https://raw.githubusercontent.com/aniongithub/devcontainer-mcp/main/install.sh | bash @@ -295,7 +299,35 @@ configure_copilot_hook echo "" echo "Backend CLIs detected (install as needed — MCP server gives helpful errors if missing):" command -v devpod >/dev/null 2>&1 && echo " ✓ devpod" || echo " ✗ devpod — https://devpod.sh/docs/getting-started/install" -command -v devcontainer >/dev/null 2>&1 && echo " ✓ devcontainer" || echo " ✗ devcontainer — https://github.com/devcontainers/cli#install-script" + +# devcontainer CLI: auto-install via the official standalone installer +# (bundles its own Node.js runtime — no system Node required) into a +# folder we own, then symlink the binary into INSTALL_DIR so it picks +# up the same PATH guidance the script gives for the main binary. +install_devcontainer_cli() { + local cli_prefix="${HOME}/.local/share/devcontainer-mcp/devcontainers" + local installer_url="https://raw.githubusercontent.com/devcontainers/cli/main/scripts/install.sh" + echo " … devcontainer CLI not found — installing standalone bundle into ${cli_prefix}" + mkdir -p "$(dirname "$cli_prefix")" + if ! curl -fsSL "$installer_url" | sh -s -- --prefix "$cli_prefix" >/dev/null 2>&1; then + echo " ✗ devcontainer — standalone install failed; see https://github.com/devcontainers/cli#install-script" + return 1 + fi + local shim="${cli_prefix}/bin/devcontainer" + if [ ! -x "$shim" ]; then + echo " ✗ devcontainer — install completed but ${shim} is missing" + return 1 + fi + ln -sf "$shim" "${INSTALL_DIR}/devcontainer" + echo " ✓ devcontainer (auto-installed → ${INSTALL_DIR}/devcontainer)" +} + +if command -v devcontainer >/dev/null 2>&1; then + echo " ✓ devcontainer" +else + install_devcontainer_cli || true +fi + command -v gh >/dev/null 2>&1 && echo " ✓ gh (codespaces)" || echo " ✗ gh (codespaces) — https://cli.github.com/" # --------------------------------------------------------------------------- From 61a58ef40895474d4dbdd4df3ae63dede8b0abe5 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 6 Jun 2026 00:06:01 -0700 Subject: [PATCH 18/19] Add MCP Registry publishing on release - Create server.json with MCPB package entries for all 4 platforms - Add publish-mcp-registry job to release workflow (OIDC auth) - Move permissions from workflow-level to per-job - Downloads release artifacts, computes SHA-256, patches server.json - Uses platform-matching jq selects for robust patching --- .github/workflows/release.yml | 55 +++++++++++++++++++++++++++++++++-- server.json | 37 +++++++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 server.json diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a4eea12..461d9f2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,13 +4,12 @@ on: release: types: [created] -permissions: - contents: write - jobs: build: name: Build release binaries runs-on: ubuntu-latest + permissions: + contents: write steps: - uses: actions/checkout@v5 @@ -49,6 +48,8 @@ jobs: build-macos: name: Build ${{ matrix.artifact }} runs-on: macos-latest + permissions: + contents: write strategy: matrix: include: @@ -80,6 +81,8 @@ jobs: upload-install-script: name: Upload install scripts runs-on: ubuntu-latest + permissions: + contents: write steps: - uses: actions/checkout@v5 - name: Upload install.sh and install.ps1 @@ -88,3 +91,49 @@ jobs: files: | install.sh install.ps1 + + publish-mcp-registry: + name: Publish to MCP Registry + runs-on: ubuntu-latest + needs: [build, build-macos] + permissions: + id-token: write + contents: read + steps: + - uses: actions/checkout@v5 + + - name: Install mcp-publisher + run: | + curl -fsSL "https://github.com/modelcontextprotocol/registry/releases/latest/download/mcp-publisher_linux_amd64.tar.gz" \ + | tar xz mcp-publisher + + - name: Authenticate to MCP Registry (OIDC) + run: ./mcp-publisher login github-oidc + + - name: Patch server.json with version, URLs, and SHA-256 hashes + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + + TAG="${{ github.event.release.tag_name }}" + VERSION="${TAG#v}" + BASE="https://github.com/aniongithub/devcontainer-mcp/releases/download/${TAG}" + + # Set top-level version + jq --arg v "$VERSION" '.version = $v' server.json > server.tmp && mv server.tmp server.json + + # Download each platform artifact, compute SHA-256, patch identifier + hash + PLATFORMS=("linux-x64" "linux-arm64" "darwin-x64" "darwin-arm64") + for platform in "${PLATFORMS[@]}"; do + FILE="devcontainer-mcp-${platform}.tar.gz" + curl -fsSL --retry 5 --retry-delay 3 -o "$FILE" "${BASE}/${FILE}" + SHA="$(sha256sum "$FILE" | awk '{print $1}')" + URL="${BASE}/${FILE}" + jq --arg platform "$platform" --arg url "$URL" --arg sha "$SHA" \ + '(.packages[] | select(.identifier | contains($platform))) |= (.identifier = $url | .fileSha256 = $sha)' \ + server.json > server.tmp && mv server.tmp server.json + done + + - name: Publish to MCP Registry + run: ./mcp-publisher publish diff --git a/server.json b/server.json new file mode 100644 index 0000000..6b09a4a --- /dev/null +++ b/server.json @@ -0,0 +1,37 @@ +{ + "$schema": "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json", + "name": "io.github.aniongithub/devcontainer-mcp", + "title": "devcontainer-mcp", + "description": "MCP server that lets AI agents create, manage, and work inside dev containers across Docker, DevPod, and GitHub Codespaces.", + "repository": { + "url": "https://github.com/aniongithub/devcontainer-mcp", + "source": "github" + }, + "version": "0.0.0", + "packages": [ + { + "registryType": "mcpb", + "identifier": "https://github.com/aniongithub/devcontainer-mcp/releases/download/v0.0.0/devcontainer-mcp-linux-x64.tar.gz", + "fileSha256": "PLACEHOLDER", + "transport": { "type": "stdio" } + }, + { + "registryType": "mcpb", + "identifier": "https://github.com/aniongithub/devcontainer-mcp/releases/download/v0.0.0/devcontainer-mcp-linux-arm64.tar.gz", + "fileSha256": "PLACEHOLDER", + "transport": { "type": "stdio" } + }, + { + "registryType": "mcpb", + "identifier": "https://github.com/aniongithub/devcontainer-mcp/releases/download/v0.0.0/devcontainer-mcp-darwin-x64.tar.gz", + "fileSha256": "PLACEHOLDER", + "transport": { "type": "stdio" } + }, + { + "registryType": "mcpb", + "identifier": "https://github.com/aniongithub/devcontainer-mcp/releases/download/v0.0.0/devcontainer-mcp-darwin-arm64.tar.gz", + "fileSha256": "PLACEHOLDER", + "transport": { "type": "stdio" } + } + ] +} From 8e58cc061396d03db5a7598c91533948840a1c19 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sat, 6 Jun 2026 00:19:56 -0700 Subject: [PATCH 19/19] Fix server.json description to meet 100-char limit --- server.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server.json b/server.json index 6b09a4a..116fe95 100644 --- a/server.json +++ b/server.json @@ -2,7 +2,7 @@ "$schema": "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json", "name": "io.github.aniongithub/devcontainer-mcp", "title": "devcontainer-mcp", - "description": "MCP server that lets AI agents create, manage, and work inside dev containers across Docker, DevPod, and GitHub Codespaces.", + "description": "Manage dev container environments via MCP (Docker, DevPod, Codespaces).", "repository": { "url": "https://github.com/aniongithub/devcontainer-mcp", "source": "github"