From 703bca439be545273f43d8b3b678957433691433 Mon Sep 17 00:00:00 2001 From: Gabriel Lipnik Date: Thu, 27 Aug 2026 21:36:13 +0200 Subject: [PATCH 1/5] feat: add ACP mode for editor integration Run any supported agent as an Agent Client Protocol adapter, so ACP-capable editors (e.g. Zed) can embed VibePod containers directly in their AI panel. Container isolation, profiles, overlays, the MITM proxy and local metric collection stay active. --- README.md | 63 ++++++--- docs/acp.md | 81 ++++++++++++ docs/configuration.md | 128 +++++++++--------- docs/index.md | 33 ++--- mkdocs.yml | 11 +- src/vibepod/cli.py | 16 +++ src/vibepod/commands/run.py | 229 ++++++++++++++++++++++++++------ src/vibepod/core/agents.py | 8 ++ src/vibepod/core/docker.py | 191 +++++++++++++++++++++++++-- src/vibepod/core/launch.py | 4 +- src/vibepod/utils/console.py | 12 ++ tests/test_agents.py | 15 +++ tests/test_cli.py | 19 +++ tests/test_docker.py | 244 +++++++++++++++++++++++++++++++++++ tests/test_run.py | 190 +++++++++++++++++++++++++++ 15 files changed, 1088 insertions(+), 156 deletions(-) create mode 100644 docs/acp.md diff --git a/README.md b/README.md index bb91f2e..6335143 100644 --- a/README.md +++ b/README.md @@ -75,22 +75,43 @@ vp run -- Use `--ikwid` to append each agent's auto-approval / permission-skip flag when supported. -| Agent | `--ikwid` appended args | -|---|---| -| `claude` | `--dangerously-skip-permissions` | -| `gemini` | `--approval-mode=yolo` | -| `devstral` (`vibe`) | `--auto-approve` | -| `copilot` | `--yolo` | -| `codex` | `--dangerously-bypass-approvals-and-sandbox` | -| `pi` | `--approve` | -| `agy` | `--dangerously-skip-permissions` | -| `opencode` | Not supported | -| `auggie` | Not supported | -| `tau` | Not supported | -| `jcode` | Not supported | -| `freebuff` | Not supported | -| `qwen` | `--approval-mode=yolo` | -| `dsh` | Not supported | +| Agent | `--ikwid` appended args | +| ------------------- | -------------------------------------------- | +| `claude` | `--dangerously-skip-permissions` | +| `gemini` | `--approval-mode=yolo` | +| `devstral` (`vibe`) | `--auto-approve` | +| `copilot` | `--yolo` | +| `codex` | `--dangerously-bypass-approvals-and-sandbox` | +| `pi` | `--approve` | +| `agy` | `--dangerously-skip-permissions` | +| `opencode` | Not supported | +| `auggie` | Not supported | +| `tau` | Not supported | +| `jcode` | Not supported | +| `freebuff` | Not supported | +| `qwen` | `--approval-mode=yolo` | +| `dsh` | Not supported | + +## Editor integration (`--acp`) + +`vp run --acp` turns VibePod into an [Agent Client Protocol](https://agentclientprotocol.com/) adapter, so the containerized agent appears directly in the AI panel of any editor with ACP support (e.g. [Zed](https://zed.dev/docs/ai/external-agents)) — with isolation, profiles, overlays and proxy metrics intact. Supported out of the box: `claude`, `gemini`, `qwen`, `codex`. + +Register `vp` as a custom/external agent server in your editor. Zed example (`settings.json`): + +```json +{ + "agent_servers": { + "VibePod Claude": { + "type": "custom", + "command": "vp", + "args": ["run", "claude", "--acp"], + "env": {} + } + } +} +``` + +Run `vp config allow-dir /path/to/project` once first (the editor's stdin is a pipe, so the interactive prompt cannot run). See the [ACP docs](docs/acp.md) for details and limitations. ![VibePod CLI preview](https://raw.githubusercontent.com/VibePod/vibepod-cli/main/docs/assets/preview.png) @@ -132,11 +153,11 @@ a built-in dashboard. ![VibePod Analytics Dashboard](https://raw.githubusercontent.com/VibePod/vibepod-cli/main/docs/assets/dashboard.png) -| Command | Description | -|------------------|----------------------------------------------------| -| `vp logs start` | Start or resume dashboard for collected metrics | -| `vp logs stop` | Stop the dashboard container | -| `vp logs status` | Show dashboard container status | +| Command | Description | +| ---------------- | ----------------------------------------------- | +| `vp logs start` | Start or resume dashboard for collected metrics | +| `vp logs stop` | Stop the dashboard container | +| `vp logs status` | Show dashboard container status | The dashboard shows per-agent HTTP traffic, usage over time, and Claude token metrics. It also lets you compare agents side-by-side. All data stays on your diff --git a/docs/acp.md b/docs/acp.md new file mode 100644 index 0000000..83878a3 --- /dev/null +++ b/docs/acp.md @@ -0,0 +1,81 @@ +# Editor integration (ACP mode) + +VibePod can act as an [Agent Client Protocol](https://agentclientprotocol.com/) +(ACP) adapter. ACP is an editor-agnostic, JSON-RPC-based protocol: any editor +or client with ACP support can launch `vp run --acp` as a subprocess +and embed the containerized agent directly in its AI panel — instead of only +the integrated terminal. Everything that makes a VibePod run what it is — +container isolation, profiles, project overlays, the MITM proxy and local +metric collection — stays active. + +## Supported agents + +`claude`, `gemini`, `qwen` and `codex` ship an ACP adapter command. Other +agents abort with an error listing the supported agents (you can still provide +your own adapter via `agents..acp_command` in the config). + +## Setup + +Register `vp` as an external/custom agent server in your editor. The exact +location and format depend on the editor; the general shape is a command plus +arguments. For example, in [Zed](https://zed.dev/docs/ai/external-agents) +(`settings.json`): + +```json +{ + "agent_servers": { + "VibePod Claude": { + "type": "custom", + "command": "vp", + "args": ["run", "claude", "--acp"], + "env": {} + } + } +} +``` + +Repeat the block for `gemini`, `qwen` or `codex` (adjust the `run` argument) +if you want more than one. Make sure `vp` is on the `PATH` your editor +inherits (or use an absolute path). + +Then allow your project directory once — under an editor, stdin is a pipe, so +the interactive allow prompt cannot run: + +```bash +vp config allow-dir /path/to/your/project +``` + +Open the AI/agent panel in your editor, pick the VibePod thread type, and +start a thread inside your project. + +## How it works + +`vp run --acp` starts the container **without a TTY**, attaches before +the entrypoint runs (so the first JSON-RPC frames are not lost), and +demultiplexes the Docker stream: container stdout carries only the +newline-delimited JSON-RPC stream, all VibePod messages go to stderr. The +workspace is mounted a second time onto its own host path so absolute paths +from the editor (session cwd, @-mentions, diffs) resolve identically inside +the container. + +When you close the thread, the editor kills the `vp` process; the container +sees stdin EOF, the adapter exits, and `auto_remove` cleans up — no orphaned +containers. + +## Limitations + +- **Windows hosts are not supported**: the path-parity mount requires the host + workspace path to be a valid container path. +- The workspace host path must not collide with container-reserved paths + (`/workspace`, `/config`, `/claude`, `/qwen`, `/etc`, `/usr`, + `/tmp/.X11-unix` and the agent's config mount); `--acp` aborts with a clear + message if it does. +- `--acp` cannot be combined with `--detach`; the ACP client owns the process + lifetime. +- `--ikwid` is ignored — permissions are negotiated by the editor over ACP. + +## Debugging + +VibePod's diagnostics (e.g. a missing `vp config allow-dir`) go to stderr and +show up in the editor's ACP logs. Zed, for example, exposes them via +`dev: open acp logs` from the command palette. diff --git a/docs/configuration.md b/docs/configuration.md index ee95bdf..f1fd22d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -48,12 +48,18 @@ agents: claude: enabled: true image: vibepod/claude:latest - auto_pull: null # Per-agent override: true/false, or null to use global auto_pull - env: {} # Extra environment variables passed to the container - volumes: [] # Reserved for future use - ports: [] # Ports to publish on the host, `docker run -p` syntax - init: [] # Optional shell commands run before agent startup + auto_pull: null # Per-agent override: true/false, or null to use global auto_pull + env: {} # Extra environment variables passed to the container + volumes: [] # Reserved for future use + ports: [] # Ports to publish on the host, `docker run -p` syntax + init: [] # Optional shell commands run before agent startup overlay: true # Set false to ignore the project's .vibepod/overlay/ (see Project overlays) + # acp_command: ACP adapter command for `vp run --acp` (editor + # integration via the Agent Client Protocol). Defaults exist for claude, + # gemini, qwen and codex; override with a list (or a string, which is + # shell-split). Not present by default — just add the key to override. + # just add the key to override. + # acp_command: ["npx", "-y", "@agentclientprotocol/claude-agent-acp"] gemini: enabled: true @@ -156,21 +162,21 @@ agents: env: {} volumes: [] ports: - - "127.0.0.1:3080:3081" # Web UI; container side must stay 3081 + - "127.0.0.1:3080:3081" # Web UI; container side must stay 3081 init: [] # Connect agents to a local or remote LLM server (Ollama, vLLM, etc.) llm: enabled: false - base_url: "" # Server endpoint URL - api_key: "" # Auth token (set to "ollama" for Ollama) - model: "" # Model name passed to the agent + base_url: "" # Server endpoint URL + api_key: "" # Auth token (set to "ollama" for Ollama) + model: "" # Model name passed to the agent logging: enabled: true image: vibepod/datasette:latest db_path: ~/.config/vibepod/logs.db - ui_port: 8001 # Port for the Datasette UI + ui_port: 8001 # Port for the Datasette UI proxy: enabled: true @@ -179,7 +185,7 @@ proxy: ca_dir: ~/.config/vibepod/proxy/mitmproxy ca_path: ~/.config/vibepod/proxy/mitmproxy/mitmproxy-ca-cert.pem filter: - mode: open # open | allow | deny + mode: open # open | allow | deny allow: [] deny: [] ``` @@ -188,22 +194,22 @@ proxy: These variables override the corresponding config keys without editing any file: -| Variable | Config key | Example | -|---|---|---| -| `VP_DEFAULT_AGENT` | `default_agent` | `VP_DEFAULT_AGENT=vibe` | -| `VP_AUTO_PULL` | `auto_pull` | `VP_AUTO_PULL=true` | -| `VP_AUTO_CLEAN` | `auto_clean` | `VP_AUTO_CLEAN=false` | -| `VP_LOG_LEVEL` | `log_level` | `VP_LOG_LEVEL=debug` | -| `VP_NO_COLOR` | `no_color` | `VP_NO_COLOR=true` | -| `VP_DATASETTE_PORT` | `logging.ui_port` | `VP_DATASETTE_PORT=9001` | -| `VP_PROXY_ENABLED` | `proxy.enabled` | `VP_PROXY_ENABLED=false` | -| `VP_PROXY_FILTER_MODE` | `proxy.filter.mode` | `VP_PROXY_FILTER_MODE=allow` | -| `VP_LLM_ENABLED` | `llm.enabled` | `VP_LLM_ENABLED=true` | -| `VP_LLM_BASE_URL` | `llm.base_url` | `VP_LLM_BASE_URL=http://localhost:11434` | -| `VP_LLM_API_KEY` | `llm.api_key` | `VP_LLM_API_KEY=ollama` | -| `VP_LLM_MODEL` | `llm.model` | `VP_LLM_MODEL=qwen3:14b` | -| `VP_CONFIG_DIR` | *(config root)* | `VP_CONFIG_DIR=/custom/path` | -| `VP_PROFILE` | `profile` | `VP_PROFILE=work` | +| Variable | Config key | Example | +| ---------------------- | ------------------- | ---------------------------------------- | +| `VP_DEFAULT_AGENT` | `default_agent` | `VP_DEFAULT_AGENT=vibe` | +| `VP_AUTO_PULL` | `auto_pull` | `VP_AUTO_PULL=true` | +| `VP_AUTO_CLEAN` | `auto_clean` | `VP_AUTO_CLEAN=false` | +| `VP_LOG_LEVEL` | `log_level` | `VP_LOG_LEVEL=debug` | +| `VP_NO_COLOR` | `no_color` | `VP_NO_COLOR=true` | +| `VP_DATASETTE_PORT` | `logging.ui_port` | `VP_DATASETTE_PORT=9001` | +| `VP_PROXY_ENABLED` | `proxy.enabled` | `VP_PROXY_ENABLED=false` | +| `VP_PROXY_FILTER_MODE` | `proxy.filter.mode` | `VP_PROXY_FILTER_MODE=allow` | +| `VP_LLM_ENABLED` | `llm.enabled` | `VP_LLM_ENABLED=true` | +| `VP_LLM_BASE_URL` | `llm.base_url` | `VP_LLM_BASE_URL=http://localhost:11434` | +| `VP_LLM_API_KEY` | `llm.api_key` | `VP_LLM_API_KEY=ollama` | +| `VP_LLM_MODEL` | `llm.model` | `VP_LLM_MODEL=qwen3:14b` | +| `VP_CONFIG_DIR` | _(config root)_ | `VP_CONFIG_DIR=/custom/path` | +| `VP_PROFILE` | `profile` | `VP_PROFILE=work` | The `profile` key selects the active [credential profile](profiles.md); the `--profile` flag on `vp run`, `vp task create`, and `vp doctor claude` takes @@ -213,24 +219,24 @@ precedence over both the variable and the config key. Each agent image can be overridden individually: -| Variable | Agent | -|---|---| -| `VP_IMAGE_CLAUDE` | claude | -| `VP_IMAGE_GEMINI` | gemini | -| `VP_IMAGE_OPENCODE` | opencode | -| `VP_IMAGE_DEVSTRAL` | devstral | -| `VP_IMAGE_AUGGIE` | auggie | -| `VP_IMAGE_COPILOT` | copilot | -| `VP_IMAGE_CODEX` | codex | -| `VP_IMAGE_PI` | pi | -| `VP_IMAGE_AGY` | agy | -| `VP_IMAGE_TAU` | tau | -| `VP_IMAGE_JCODE` | jcode | -| `VP_IMAGE_FREEBUFF` | freebuff | -| `VP_IMAGE_QWEN` | qwen | -| `VP_IMAGE_DSH` | dsh | -| `VP_DATASETTE_IMAGE` | datasette (logs UI) | -| `VP_PROXY_IMAGE` | proxy | +| Variable | Agent | +| ------------------------ | ----------------------------------- | +| `VP_IMAGE_CLAUDE` | claude | +| `VP_IMAGE_GEMINI` | gemini | +| `VP_IMAGE_OPENCODE` | opencode | +| `VP_IMAGE_DEVSTRAL` | devstral | +| `VP_IMAGE_AUGGIE` | auggie | +| `VP_IMAGE_COPILOT` | copilot | +| `VP_IMAGE_CODEX` | codex | +| `VP_IMAGE_PI` | pi | +| `VP_IMAGE_AGY` | agy | +| `VP_IMAGE_TAU` | tau | +| `VP_IMAGE_JCODE` | jcode | +| `VP_IMAGE_FREEBUFF` | freebuff | +| `VP_IMAGE_QWEN` | qwen | +| `VP_IMAGE_DSH` | dsh | +| `VP_DATASETTE_IMAGE` | datasette (logs UI) | +| `VP_PROXY_IMAGE` | proxy | | `VP_SKILLS_ENGINE_IMAGE` | skills-engine (used by `vp skills`) | Set `VP_IMAGE_NAMESPACE` to change the prefix for all default images at once: @@ -293,17 +299,17 @@ By default no user-defined ports are published, so anything the agent starts ins agents: claude: ports: - - "8000:8000" # host:container - - "127.0.0.1:9229:9229" # bind to a specific host interface - - "6000:6000/udp" # UDP - - "3000" # container port on a random host port + - "8000:8000" # host:container + - "127.0.0.1:9229:9229" # bind to a specific host interface + - "6000:6000/udp" # UDP + - "3000" # container port on a random host port ``` !!! warning "Always quote port entries" - YAML 1.1 parses unquoted `host:container` pairs as base-60 numbers: - `- 22:22` loads as the integer `1342` and would publish the wrong port. - Out-of-range results (e.g. `3000:30` → `180030`) are rejected at startup, - but in-range ones are not detectable — quote every entry. +YAML 1.1 parses unquoted `host:container` pairs as base-60 numbers: +`- 22:22` loads as the integer `1342` and would publish the wrong port. +Out-of-range results (e.g. `3000:30` → `180030`) are rejected at startup, +but in-range ones are not detectable — quote every entry. Like other agent keys, a project-level `ports` list replaces the global one for that agent. The list applies to both `vp run` and `vp task` containers; note that two containers cannot publish the same host port at the same time, so a fixed host port limits you to one such container per agent. @@ -333,10 +339,10 @@ VP_PROXY_ENABLED=false vp run claude ``` !!! note "Podman users" - The proxy relies on container-name DNS resolution over the `vibepod-network` - network. If you use Podman with the CNI backend, install the `dnsname` - plugin (e.g. `podman-plugins` on Fedora, `golang-github-containernetworking-plugin-dnsname` - on Debian/Ubuntu) and recreate the network. See [Quickstart — Using Podman](quickstart.md#using-podman-instead-of-docker) for full instructions. +The proxy relies on container-name DNS resolution over the `vibepod-network` +network. If you use Podman with the CNI backend, install the `dnsname` +plugin (e.g. `podman-plugins` on Fedora, `golang-github-containernetworking-plugin-dnsname` +on Debian/Ubuntu) and recreate the network. See [Quickstart — Using Podman](quickstart.md#using-podman-instead-of-docker) for full instructions. ### Allow/deny filtering @@ -396,7 +402,7 @@ launching a proxied agent. An image with a missing or different schema label is rejected with an upgrade error. !!! warning "Filtering is not a network sandbox" - The filter controls requests that use the injected proxy. A container can - override its proxy environment or attempt a direct connection unless a - separate network-control layer prevents that. VibePod warns when explicit - `HTTP_PROXY` or `HTTPS_PROXY` values bypass its identified proxy URL. +The filter controls requests that use the injected proxy. A container can +override its proxy environment or attempt a direct connection unless a +separate network-control layer prevents that. VibePod warns when explicit +`HTTP_PROXY` or `HTTPS_PROXY` values bypass its identified proxy URL. diff --git a/docs/index.md b/docs/index.md index 2a453b8..d5da761 100644 --- a/docs/index.md +++ b/docs/index.md @@ -13,22 +13,22 @@ VibePod (`vp`) lets you run any supported AI coding agent in an isolated Docker ## Supported Agents -| Agent | Provider | Shortcut | -|-------|----------|----------| -| `claude` | Anthropic | `vp c` | -| `gemini` | Google | `vp g` | -| `opencode` | OpenAI | `vp o` | -| `devstral` (alias: `vibe`) | Mistral | `vp d` | -| `auggie` | Augment Code | `vp a` | -| `copilot` | GitHub | `vp p` | -| `codex` | OpenAI | `vp x` | -| `pi` | Earendil | `vp pi` | -| `agy` (Antigravity) | Google | `vp n` | -| `tau` | Hugging Face | `vp t` | -| `jcode` | 1jehuang | `vp j` | -| `freebuff` | CodebuffAI | `vp fb` | -| `qwen` | Qwen (Alibaba) | `vp q` | -| `dsh` (DeepSeek Harness) | DeepSeek | `vp ds` | +| Agent | Provider | Shortcut | +| -------------------------- | -------------- | -------- | +| `claude` | Anthropic | `vp c` | +| `gemini` | Google | `vp g` | +| `opencode` | OpenAI | `vp o` | +| `devstral` (alias: `vibe`) | Mistral | `vp d` | +| `auggie` | Augment Code | `vp a` | +| `copilot` | GitHub | `vp p` | +| `codex` | OpenAI | `vp x` | +| `pi` | Earendil | `vp pi` | +| `agy` (Antigravity) | Google | `vp n` | +| `tau` | Hugging Face | `vp t` | +| `jcode` | 1jehuang | `vp j` | +| `freebuff` | CodebuffAI | `vp fb` | +| `qwen` | Qwen (Alibaba) | `vp q` | +| `dsh` (DeepSeek Harness) | DeepSeek | `vp ds` | ## Next Steps @@ -36,4 +36,5 @@ VibePod (`vp`) lets you run any supported AI coding agent in an isolated Docker - [**Development**](development.md) — local setup, tests, and docs workflow. - [**Agents**](agents/index.md) — per-agent setup and credential instructions. - [**Configuration**](configuration.md) — full reference for global and project-level config. +- [**Editor integration (ACP)**](acp.md) — run VibePod agents in ACP-capable editors (Zed & co.). - [**CLI Reference**](cli-reference.md) — every command and flag. diff --git a/mkdocs.yml b/mkdocs.yml index 3f565ed..3633cbc 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -54,13 +54,14 @@ nav: - Agents: agents/index.md - Profiles: profiles.md - Skills: - - Overview: skills/index.md - - Locator format: skills/locators.md - - Authoring: skills/authoring.md + - Overview: skills/index.md + - Locator format: skills/locators.md + - Authoring: skills/authoring.md - Project overlays: - - Overview: overlays/index.md - - Recipes: overlays/recipes.md + - Overview: overlays/index.md + - Recipes: overlays/recipes.md - Herdr integration: herdr.md + - Editor integration (ACP): acp.md - Configuration: configuration.md - LLM Integration: llm.md - CLI Reference: cli-reference.md diff --git a/src/vibepod/cli.py b/src/vibepod/cli.py index be6f775..cbeb749 100644 --- a/src/vibepod/cli.py +++ b/src/vibepod/cli.py @@ -96,6 +96,13 @@ def run_command( help="I Know What I'm Doing: enable auto-approval / skip permission prompts", ), ] = False, + acp: Annotated[ + bool, + typer.Option( + "--acp", + help="Run as an Agent Client Protocol (ACP) adapter for ACP-capable editors", + ), + ] = False, profile: Annotated[ str | None, typer.Option("--profile", help="Credential profile to use (see `vp profile list`)"), @@ -116,6 +123,7 @@ def run_command( network=network, paste_images=paste_images, ikwid=ikwid, + acp=acp, profile=profile, passthrough_args=_context_args(ctx), ) @@ -200,6 +208,13 @@ def _alias( help="I Know What I'm Doing: enable auto-approval / skip permission prompts", ), ] = False, + acp: Annotated[ + bool, + typer.Option( + "--acp", + help="Run as an Agent Client Protocol (ACP) adapter for ACP-capable editors", + ), + ] = False, profile: Annotated[ str | None, typer.Option("--profile", help="Credential profile to use (see `vp profile list`)"), @@ -219,6 +234,7 @@ def _alias( network=network, paste_images=paste_images, ikwid=ikwid, + acp=acp, profile=profile, passthrough_args=_context_args(ctx), ) diff --git a/src/vibepod/commands/run.py b/src/vibepod/commands/run.py index 4b60cef..441b7d9 100644 --- a/src/vibepod/commands/run.py +++ b/src/vibepod/commands/run.py @@ -16,6 +16,7 @@ from vibepod import __version__ from vibepod.constants import EXIT_DOCKER_NOT_RUNNING, SUPPORTED_AGENTS from vibepod.core.agents import ( + AGENT_SPECS, agent_config_dir, effective_agent_image, get_agent_shortcut, @@ -104,10 +105,69 @@ from vibepod.core.proxy_filter import remove_container_policy from vibepod.core.resume import show_resume_hint from vibepod.core.session_logger import SessionLogger -from vibepod.utils.console import error, info, success, warning +from vibepod.utils.console import error, info, route_to_stderr, success, warning _SAFE_SKILL_ID_RE = re.compile(r"^[a-z0-9][a-z0-9_-]*$") +# Container paths that must never be shadowed by the ACP path-parity mount of +# the workspace onto its own host path. +_ACP_RESERVED_CONTAINER_PATHS = ( + "/workspace", + "/config", + "/claude", + "/qwen", + "/etc", + "/usr", + "/tmp/.X11-unix", +) + + +def _resolve_acp_command(spec: Any, agent_cfg: dict[str, Any]) -> list[str] | None: + """Resolve the ACP adapter command: config override wins over the spec.""" + override = agent_cfg.get("acp_command") + if override is None: + default = spec.acp_command + return list(default) if default is not None else None + if isinstance(override, str): + return override.split() + return [str(part) for part in override] + + +def _acp_workspace_mount_path(workspace_path: Path, spec: Any) -> str: + """Return the host path the workspace is bound to for ACP path parity. + + ACP clients (editors like Zed) send absolute host paths (session cwd, + @-mentions) and expect absolute host paths back in diffs. Binding the + workspace onto its own host path (in addition to /workspace) makes both + sides agree. Aborts when the host path would shadow a container-reserved + path or is not a POSIX path (Windows hosts are not supported in ACP mode). + """ + host_path = str(workspace_path) + if not host_path.startswith("/"): + error( + f"--acp requires a POSIX workspace path; '{host_path}' is not " + "usable as a container path (Windows hosts are not supported " + "in ACP mode).", + ) + raise typer.Exit(1) + + reserved = {*_ACP_RESERVED_CONTAINER_PATHS, spec.config_mount_path} + for reserved_path in sorted(reserved): + shadows_reserved = host_path == reserved_path or host_path.startswith( + reserved_path + "/", + ) + # Also guard the reverse: binding /tmp over the container /tmp would + # shadow the /tmp/.X11-unix mount target inside it. + shadowed_by_mount = reserved_path.startswith(host_path + "/") + if shadows_reserved or shadowed_by_mount: + error( + f"--acp cannot mount the workspace at '{host_path}': it collides " + f"with the container-reserved path '{reserved_path}'. " + "Move the project to a non-reserved host path.", + ) + raise typer.Exit(1) + return host_path + def _is_safe_skill_id(skill_id: str) -> bool: """Return True for skill IDs safe to use as one container path segment.""" @@ -371,6 +431,13 @@ def run( help="I Know What I'm Doing: enable auto-approval / skip permission prompts", ), ] = False, + acp: Annotated[ + bool, + typer.Option( + "--acp", + help="Run as an Agent Client Protocol (ACP) adapter for ACP-capable editors", + ), + ] = False, profile: Annotated[ str | None, typer.Option("--profile", help="Credential profile to use (see `vp profile list`)"), @@ -384,6 +451,13 @@ def run( be parsed as VibePod flags. """ passthrough_args = passthrough_args or [] + if acp and detach: + error("--acp cannot be combined with --detach: the ACP client owns the process lifetime.") + raise typer.Exit(1) + if acp: + # Must happen before any output: stdout carries only the ACP + # JSON-RPC stream, so all console output is rerouted to stderr. + route_to_stderr() config = get_config() try: active_profile = resolve_profile(profile, config) @@ -402,7 +476,7 @@ def run( error(f"Unknown agent '{selected_agent_input}'. Supported: {', '.join(supported_labels)}") raise typer.Exit(1) - _reexec_with_herdr_hint(selected_agent, config, no_herdr=no_herdr) + _reexec_with_herdr_hint(selected_agent, config, no_herdr=no_herdr or acp) workspace_path = workspace.expanduser().resolve() if not workspace_path.exists() or not workspace_path.is_dir(): @@ -436,6 +510,22 @@ def run( agent_cfg = config.get("agents", {}).get(selected_agent, {}) spec = get_agent_spec(selected_agent) + + acp_workspace_mount: str | None = None + acp_command: list[str] | None = None + if acp: + acp_command = _resolve_acp_command(spec, agent_cfg) + if acp_command is None: + acp_capable = [ + name for name, agent_spec in AGENT_SPECS.items() if agent_spec.acp_command + ] + error( + f"Agent '{selected_agent}' has no ACP adapter. " + f"Supported: {', '.join(sorted(acp_capable))}. " + "Set agents..acp_command in the config to provide one.", + ) + raise typer.Exit(1) + acp_workspace_mount = _acp_workspace_mount_path(workspace_path, spec) if spec.preview: warning( f"{selected_agent} is a developer preview; upstream warns of " @@ -554,6 +644,8 @@ def run( ) command = spec.command + if acp: + command = list(acp_command or []) entrypoint: list[str] | None = None if init_commands: info(f"Applying {len(init_commands)} init command(s) before startup") @@ -565,7 +657,12 @@ def run( entrypoint = _init_entrypoint(init_commands) if ikwid: - if spec.ikwid_args: + if acp: + warning( + "--ikwid ignored in ACP mode: permissions are negotiated by " + "the ACP client (i.e. the editor).", + ) + elif spec.ikwid_args: if command is None: try: command = manager.resolve_launch_command(image=image, command=spec.command) @@ -577,7 +674,7 @@ def run( else: warning(f"IKWID mode not supported for agent '{selected_agent}', ignoring") - if llm_command_extra: + if llm_command_extra and not acp: command = list(command or []) + llm_command_extra if passthrough_args: @@ -613,7 +710,7 @@ def run( selected_agent, config_dir, config, - no_herdr=no_herdr, + no_herdr=no_herdr or acp, ) herdr_labels = ( {_HERDR_PANE_LABEL: os.environ["HERDR_PANE_ID"]} @@ -713,6 +810,9 @@ def run( entrypoint=entrypoint, userns_mode=agent_userns_mode, extra_labels=launch_labels, + workspace_mount_path=acp_workspace_mount, + start=not acp, + tty=not acp, ) except Exception: if proxy_policy_id is not None: @@ -720,50 +820,87 @@ def run( raise container.reload() - if container.status != "running": + if container.status != "running" and not (acp and container.status == "created"): recent = container.logs(tail=50).decode("utf-8", errors="replace") error("Container exited immediately after start.") if recent.strip(): - print(recent) + if acp: + # stdout is the ACP JSON-RPC stream; keep diagnostics on stderr. + sys.stderr.write(recent) + sys.stderr.flush() + else: + print(recent) if herdr_volumes: _release_herdr_agent(selected_agent) _clear_herdr_metadata(selected_agent) raise typer.Exit(1) - # Prefer the inspected bindings (they resolve ephemeral 0-port publishes to - # the daemon-assigned port); fall back to the requested bindings when the - # inspect payload has no Ports section. - inspected_ports = (container.attrs.get("NetworkSettings") or {}).get("Ports") or None - web_url = _web_ui_url(spec.web_container_port, inspected_ports or agent_ports) - if web_url: - success(f"{selected_agent} Web UI → {web_url}") - info("Sessions persist in the agent config dir.") - - if extra_network and extra_network != network_name: - try: - manager.connect_network(container, extra_network) - info(f"Connected to additional network: {extra_network}") - except DockerClientError as exc: - warning(str(exc)) - - if proxy_db_path is not None: - container_ip = _get_container_ip(container, network_name) - if container_ip: - mapping_path = proxy_db_path.parent / "containers.json" - mapping_updated = _update_container_mapping( - mapping_path, - container_ip, - container.id, - container.name, - selected_agent, - policy_id=proxy_policy_id, - profile=active_profile, - ) - if not mapping_updated: - warning( - f"Could not write proxy container mapping at {mapping_path}. " - "Fix proxy directory permissions to restore container attribution.", + def _finish_acp_launch() -> None: + """Start the container after the attach socket is open, then wire up + everything that needs a started container (extra network, proxy + attribution). Called from attach_stdio before the first byte flows.""" + container.start() + if extra_network and extra_network != network_name: + try: + manager.connect_network(container, extra_network) + info(f"Connected to additional network: {extra_network}") + except DockerClientError as exc: + warning(str(exc)) + if proxy_db_path is not None: + container.reload() + container_ip = _get_container_ip(container, network_name) + if container_ip: + mapping_path = proxy_db_path.parent / "containers.json" + mapping_updated = _update_container_mapping( + mapping_path, + container_ip, + container.id, + container.name, + selected_agent, + policy_id=proxy_policy_id, + profile=active_profile, ) + if not mapping_updated: + warning( + f"Could not write proxy container mapping at {mapping_path}. " + "Fix proxy directory permissions to restore container attribution.", + ) + + if not acp: + # Prefer the inspected bindings (they resolve ephemeral 0-port publishes to + # the daemon-assigned port); fall back to the requested bindings when the + # inspect payload has no Ports section. + inspected_ports = (container.attrs.get("NetworkSettings") or {}).get("Ports") or None + web_url = _web_ui_url(spec.web_container_port, inspected_ports or agent_ports) + if web_url: + success(f"{selected_agent} Web UI → {web_url}") + info("Sessions persist in the agent config dir.") + + if extra_network and extra_network != network_name: + try: + manager.connect_network(container, extra_network) + info(f"Connected to additional network: {extra_network}") + except DockerClientError as exc: + warning(str(exc)) + + if proxy_db_path is not None: + container_ip = _get_container_ip(container, network_name) + if container_ip: + mapping_path = proxy_db_path.parent / "containers.json" + mapping_updated = _update_container_mapping( + mapping_path, + container_ip, + container.id, + container.name, + selected_agent, + policy_id=proxy_policy_id, + profile=active_profile, + ) + if not mapping_updated: + warning( + f"Could not write proxy container mapping at {mapping_path}. " + "Fix proxy directory permissions to restore container attribution.", + ) if detach: if selected_agent == "claude" and "setup-token" in passthrough_args: @@ -797,9 +934,15 @@ def run( exit_reason = "normal" output_tail = b"" - warning("Attached to container. Use Ctrl+C to stop.") + if not acp: + warning("Attached to container. Use Ctrl+C to stop.") try: - output_tail = manager.attach_interactive(container, logger=logger) + if acp: + # No SessionLogger frames here: JSON-RPC payloads embed whole file + # contents, and the ACP client owns the transcript. + manager.attach_stdio(container, on_attached=_finish_acp_launch) + else: + output_tail = manager.attach_interactive(container, logger=logger) except KeyboardInterrupt: exit_reason = "keyboard_interrupt" info("Stopping container...") @@ -817,7 +960,7 @@ def run( if selected_agent == "claude" and "setup-token" in passthrough_args and exit_reason == "normal": _capture_claude_setup_token(config_dir) - if exit_reason == "normal": + if exit_reason == "normal" and not acp: show_resume_hint(selected_agent, output_tail) diff --git a/src/vibepod/core/agents.py b/src/vibepod/core/agents.py index 3f57d6d..9346753 100644 --- a/src/vibepod/core/agents.py +++ b/src/vibepod/core/agents.py @@ -34,6 +34,10 @@ class AgentSpec: headless_command: list[str] | None = None preview: bool = False web_container_port: int | None = None + # acp_command replaces `command` entirely for `vp run --acp` (Zed Agent + # Panel via the Agent Client Protocol). None means the agent does not ship + # an ACP adapter and `--acp` aborts with the list of supported agents. + acp_command: list[str] | None = None AGENT_SPECS: dict[str, AgentSpec] = { @@ -58,6 +62,7 @@ class AgentSpec: }, llm_model_args=["--model"], headless_prefix=["-p"], + acp_command=["npx", "-y", "@agentclientprotocol/claude-agent-acp"], ), "gemini": AgentSpec( "gemini", @@ -70,6 +75,7 @@ class AgentSpec: "/config", {"HOME": "/config"}, ikwid_args=["--approval-mode=yolo"], + acp_command=["gemini", "--experimental-acp"], ), "opencode": AgentSpec( "opencode", @@ -133,6 +139,7 @@ class AgentSpec: }, llm_model_args=["--oss", "-m"], headless_prefix=["exec"], + acp_command=["npx", "-y", "@agentclientprotocol/codex-acp"], ), "pi": AgentSpec( "pi", @@ -205,6 +212,7 @@ class AgentSpec: {"QWEN_CONFIG_DIR": "/qwen"}, ikwid_args=["--approval-mode=yolo"], headless_prefix=["-p"], + acp_command=["qwen", "--experimental-acp"], ), "dsh": AgentSpec( "dsh", diff --git a/src/vibepod/core/docker.py b/src/vibepod/core/docker.py index 9d16949..a58b0d5 100644 --- a/src/vibepod/core/docker.py +++ b/src/vibepod/core/docker.py @@ -6,9 +6,11 @@ import select import shutil import signal +import socket import subprocess import sys import threading +import time from pathlib import Path from typing import Any from uuid import uuid4 @@ -609,6 +611,9 @@ def run_agent( entrypoint: list[str] | None = None, userns_mode: str | None = None, extra_labels: dict[str, str] | None = None, + workspace_mount_path: str | None = None, + start: bool = True, + tty: bool = True, ) -> Any: container_name = name or f"vibepod-{agent}-{uuid4().hex[:8]}" @@ -626,11 +631,20 @@ def run_agent( f"{workspace}:/workspace:rw", f"{config_dir}:{config_mount_path}:rw", ] + if workspace_mount_path: + # ACP path parity: bind the workspace a second time onto its own + # host path so host-side absolute paths (ACP session cwd, @-mentions, + # diffs) resolve identically inside the container. + volumes.insert(1, f"{workspace}:{workspace_mount_path}:rw") if extra_volumes: volumes.extend(f"{host}:{bind}:{mode}" for host, bind, mode in extra_volumes) try: - if userns_mode is not None: + if userns_mode is not None or not start: + # Low-level create path: needed for Podman's `keep-id` (docker-py + # rejects it) and for ACP's create-without-start ordering, and + # because the high-level containers.create/run does not accept + # `stdin_once` (the stdin-EOF lifecycle hook). host_config = self.client.api.create_host_config( binds=volumes, auto_remove=auto_remove, @@ -641,19 +655,23 @@ def run_agent( # docker-py validates userns_mode against Docker's enum and rejects # Podman's `keep-id`, so set the Docker-compatible HostConfig field # directly for Podman engines. - host_config["UsernsMode"] = userns_mode + if userns_mode is not None: + host_config["UsernsMode"] = userns_mode create_kwargs: dict[str, Any] = { "image": image, "name": container_name, "command": command, - "tty": True, + "tty": tty, "stdin_open": True, "labels": labels, "environment": environment, - "working_dir": "/workspace", + "working_dir": workspace_mount_path or "/workspace", "host_config": host_config, } + # Note: docker-py derives StdinOnce=true itself from + # detach=False + stdin_open=True (ContainerConfig), which is + # the ACP lifecycle hook — no explicit flag exists. if ports: # (port, proto) tuples: raw "1456/tcp" keys would be # re-suffixed by docker-py's exposed-port normalization @@ -668,7 +686,8 @@ def run_agent( created = self.client.api.create_container(**create_kwargs) container_id = created["Id"] - self.client.api.start(container_id) + if start: + self.client.api.start(container_id) return self.client.containers.get(container_id) run_kwargs: dict[str, Any] = { @@ -676,13 +695,13 @@ def run_agent( "name": container_name, "command": command, "detach": True, - "tty": True, + "tty": tty, "stdin_open": True, "auto_remove": auto_remove, "labels": labels, "environment": environment, "volumes": volumes, - "working_dir": "/workspace", + "working_dir": workspace_mount_path or "/workspace", "network": network, # Docker Desktop resolves host.docker.internal natively, but Docker # Engine on Linux and Podman only do so with an explicit @@ -838,6 +857,28 @@ def find_proxy(self) -> Any | None: ) return containers[0] if containers else None + def remove_proxy(self, existing: Any, timeout: float = 15.0) -> None: + """Force-remove a non-running proxy container. + + Concurrent launches (e.g. an editor spawning `vp run` twice) can race + on the removal; Docker then answers 409 "removal ... is already in + progress". Treat that as success and wait until the container is gone + so the caller can create its replacement. + """ + try: + existing.remove(force=True) + except APIError as exc: + if "already in progress" not in str(exc): + raise + deadline = time.time() + timeout + while time.time() < deadline: + if self.find_proxy() is None: + return + time.sleep(0.2) + raise DockerClientError( + "Timed out waiting for the previous vibepod-proxy container to be removed.", + ) + def ensure_proxy( self, image: str, @@ -852,7 +893,7 @@ def ensure_proxy( if policy_schema is not None: self.require_proxy_policy_schema(existing.image, policy_schema) return existing - existing.remove(force=True) + self.remove_proxy(existing) if hasattr(self.client, "images"): try: @@ -992,3 +1033,137 @@ def _on_winch(signum: int, frame: Any) -> None: if stdin_fd is not None and old_tty is not None and termios is not None: termios.tcsetattr(stdin_fd, termios.TCSADRAIN, old_tty) return bytes(output_tail) + + def attach_stdio( + self, + container: Any, + logger: Any = None, + on_attached: Any = None, + ) -> int: + """Attach local stdin/stdout to a container without a TTY (ACP mode). + + Unlike ``attach_interactive`` this performs no raw-mode switching, no + resize handling and no ``logs`` replay; instead it demultiplexes the + Docker stream frame protocol so container stdout goes to our stdout + and stderr to our stderr, keeping the JSON-RPC stream clean. + Returns the container's exit code (0 when the container is stopped by + signal handling rather than observed exiting). + """ + try: + sock_wrapper = self.client.api.attach_socket( + container.id, + params={ + "stdin": 1, + "stdout": 1, + "stderr": 1, + "stream": 1, + }, + ) + except Exception as exc: # pragma: no cover - runtime Docker behavior + raise DockerClientError(f"Failed to attach to container: {exc}") from exc + + sock = getattr(sock_wrapper, "_sock", sock_wrapper) + if on_attached is not None: + # Create-attach-start ordering: the ACP adapter emits its first + # JSON-RPC frames immediately once the entrypoint runs, so the + # caller starts the container only after the attach is in place. + on_attached() + stdin_fd = None + if sys.stdin is not None and not sys.stdin.closed: + try: + stdin_fd = sys.stdin.fileno() + except (OSError, ValueError): + stdin_fd = None + + exit_code = 0 + stop_requested = False + old_sigterm = None + old_sigint = None + + def _request_stop(signum: int, frame: Any) -> None: + del signum, frame + nonlocal stop_requested + stop_requested = True + try: + container.stop(timeout=5) + except Exception: # pragma: no cover - best effort + pass + + try: + old_sigterm = signal.getsignal(signal.SIGTERM) + signal.signal(signal.SIGTERM, _request_stop) + old_sigint = signal.getsignal(signal.SIGINT) + signal.signal(signal.SIGINT, _request_stop) + except ValueError: # pragma: no cover - not in main thread (tests) + old_sigterm = old_sigint = None + + buffer = b"" + try: + while True: + if stop_requested: + break + readers: list[Any] = [sock] + if stdin_fd is not None: + readers.append(sys.stdin) + ready, _, _ = select.select(readers, [], []) + + if sock in ready: + data = sock.recv(65536) + if not data: + break + buffer += data + # Docker (no TTY) frames each stream chunk with an 8-byte + # header: stream byte, 3 padding bytes, 4-byte big-endian + # payload length. Frames can split across recv() bounds, + # so buffer until header and payload are complete. + while len(buffer) >= 8: + stream_type = buffer[0] + length = int.from_bytes(buffer[4:8], "big") + if len(buffer) < 8 + length: + break + payload = buffer[8 : 8 + length] + buffer = buffer[8 + length :] + if stream_type == 1: + sys.stdout.buffer.write(payload) + sys.stdout.buffer.flush() + elif stream_type == 2: + sys.stderr.buffer.write(payload) + sys.stderr.buffer.flush() + if logger is not None: + logger.log_output(payload) + + if stdin_fd is not None and sys.stdin in ready: + try: + user_data = os.read(stdin_fd, 65536) + except OSError: + user_data = b"" + if not user_data: + # stdin EOF (ACP client went away): close our write + # side so the adapter sees EOF and exits, but keep + # draining the container stream until it ends. + try: + sock.shutdown(socket.SHUT_WR) + except OSError: # pragma: no cover - already closed + pass + stdin_fd = None + else: + try: + sock.sendall(user_data) + except OSError: + stdin_fd = None + finally: + try: + sock_wrapper.close() + except Exception: # pragma: no cover - best effort + pass + if old_sigterm is not None: + signal.signal(signal.SIGTERM, old_sigterm) + if old_sigint is not None: + signal.signal(signal.SIGINT, old_sigint) + + try: + container.reload() + exit_code = int(container.attrs.get("State", {}).get("ExitCode", 0) or 0) + except Exception: # pragma: no cover - auto-removed containers + exit_code = 0 + return exit_code diff --git a/src/vibepod/core/launch.py b/src/vibepod/core/launch.py index 2df25af..01d5f6e 100644 --- a/src/vibepod/core/launch.py +++ b/src/vibepod/core/launch.py @@ -373,8 +373,8 @@ def provision_proxy( if manager.pull_if_newer(image, auto_clean=False): manager.require_proxy_policy_schema(image, policy_schema) existing = manager.find_proxy() - if existing: - existing.remove(force=True) + if existing and existing.status != "running": + manager.remove_proxy(existing) container = manager.ensure_proxy( image=image, db_path=db_path, diff --git a/src/vibepod/utils/console.py b/src/vibepod/utils/console.py index 01e72d3..dd0c7ee 100644 --- a/src/vibepod/utils/console.py +++ b/src/vibepod/utils/console.py @@ -7,6 +7,18 @@ console = Console() +def route_to_stderr() -> None: + """Redirect all console output to stderr. + + Used by ACP mode: stdout must carry only the JSON-RPC stream, so every + info()/error() line is routed to stderr instead. Callers import the + info/error helpers (not ``console`` itself), so swapping the module-global + console reroutes output project-wide. + """ + global console + console = Console(stderr=True) + + def info(message: str) -> None: console.print(f"[cyan]{message}[/cyan]") diff --git a/tests/test_agents.py b/tests/test_agents.py index 9077016..23c8c87 100644 --- a/tests/test_agents.py +++ b/tests/test_agents.py @@ -255,6 +255,21 @@ def test_agents_without_llm_env_map() -> None: assert spec.llm_env_map is None, f"{agent} should not have llm_env_map" +def test_acp_commands_match_contract() -> None: + expected = { + "claude": ["npx", "-y", "@agentclientprotocol/claude-agent-acp"], + "gemini": ["gemini", "--experimental-acp"], + "qwen": ["qwen", "--experimental-acp"], + "codex": ["npx", "-y", "@agentclientprotocol/codex-acp"], + } + for agent in SUPPORTED_AGENTS: + spec = get_agent_spec(agent) + if agent in expected: + assert spec.acp_command == expected[agent] + else: + assert spec.acp_command is None, f"{agent} should not have acp_command" + + def test_opencode_spec_matches_container_contract() -> None: spec = get_agent_spec("opencode") assert spec.id == "opencode" diff --git a/tests/test_cli.py b/tests/test_cli.py index 48042f8..ce52b65 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -308,6 +308,25 @@ def _fake_run(agent=None, **kwargs) -> None: # noqa: ANN001, ANN003, ARG001 assert called["passthrough"] == ["--model", "sonnet", "hello"] +def test_run_command_forwards_acp_flag(monkeypatch) -> None: + called: dict[str, object] = {} + + def _fake_run(agent=None, **kwargs) -> None: # noqa: ANN001, ANN003, ARG001 + called["agent"] = agent + called["acp"] = kwargs.get("acp") + + monkeypatch.setattr(run_cmd, "run", _fake_run) + + result = runner.invoke(app, ["run", "claude", "--acp"]) + assert result.exit_code == 0 + assert called["acp"] is True + + result = runner.invoke(app, ["claude", "--acp"]) + assert result.exit_code == 0 + assert called["agent"] == "claude" + assert called["acp"] is True + + def test_run_command_forwards_overlay_flags(monkeypatch) -> None: called: dict[str, object] = {} diff --git a/tests/test_docker.py b/tests/test_docker.py index b91696a..e6e87d0 100644 --- a/tests/test_docker.py +++ b/tests/test_docker.py @@ -8,6 +8,7 @@ import tempfile from collections.abc import Iterator from pathlib import Path +from typing import Any from unittest.mock import MagicMock, patch import pytest @@ -28,6 +29,249 @@ ) +# --------------------------------------------------------------------------- +# ACP support: run_agent(start=False/tty=False) and attach_stdio +# --------------------------------------------------------------------------- + + +class _AcpLowLevelApi: + def __init__(self) -> None: + self.create_kwargs: dict | None = None + self.host_config_kwargs: dict | None = None + self.started: str | None = None + + def create_host_config(self, **kwargs): + self.host_config_kwargs = kwargs + return {"Binds": kwargs["binds"], "AutoRemove": kwargs["auto_remove"]} + + def create_container(self, **kwargs): + self.create_kwargs = kwargs + return {"Id": "created"} + + def start(self, container_id: str) -> None: + self.started = container_id + + +class _AcpLowLevelClient: + def __init__(self) -> None: + self.api = _AcpLowLevelApi() + + class _Containers: + def get(self, container_id: str): + return {"id": container_id} + + self.containers = _Containers() + + +def _run_acp_agent(manager: DockerManager, tmp_path: Path, **overrides): + kwargs = dict( + agent="claude", + image="vibepod/claude:latest", + workspace=tmp_path / "workspace", + config_dir=tmp_path / "agents" / "claude", + config_mount_path="/claude", + env={}, + command=["npx", "-y", "@agentclientprotocol/claude-agent-acp"], + auto_remove=True, + name=None, + version="test", + ) + kwargs.update(overrides) + return manager.run_agent(**kwargs) + + +def test_run_agent_start_false_creates_without_starting(tmp_path: Path) -> None: + client = _AcpLowLevelClient() + manager = object.__new__(DockerManager) + manager.client = client # type: ignore[assignment] + + (tmp_path / "workspace").mkdir() + (tmp_path / "agents" / "claude").mkdir(parents=True) + + container = _run_acp_agent( + manager, + tmp_path, + workspace_mount_path=str(tmp_path / "workspace"), + start=False, + tty=False, + ) + + assert container == {"id": "created"} + assert client.api.started is None + kwargs = client.api.create_kwargs + assert kwargs is not None + assert kwargs["tty"] is False + assert kwargs["stdin_open"] is True + assert kwargs["working_dir"] == str(tmp_path / "workspace") + binds = client.api.host_config_kwargs["binds"] + assert f"{tmp_path / 'workspace'}:/workspace:rw" in binds + assert f"{tmp_path / 'workspace'}:{tmp_path / 'workspace'}:rw" in binds + + +def test_run_agent_workspace_mount_path_sets_working_dir(tmp_path: Path) -> None: + client = _AcpLowLevelClient() + manager = object.__new__(DockerManager) + manager.client = client # type: ignore[assignment] + + (tmp_path / "workspace").mkdir() + (tmp_path / "agents" / "claude").mkdir(parents=True) + + _run_acp_agent( + manager, + tmp_path, + workspace_mount_path=str(tmp_path / "workspace"), + start=False, + ) + + kwargs = client.api.create_kwargs + assert kwargs is not None + assert kwargs["working_dir"] == str(tmp_path / "workspace") + assert kwargs["tty"] is True + assert "UsernsMode" not in client.api.create_kwargs["host_config"] + + +def test_run_agent_podman_branch_honors_start_false(tmp_path: Path) -> None: + client = _AcpLowLevelClient() + manager = object.__new__(DockerManager) + manager.client = client # type: ignore[assignment] + + (tmp_path / "workspace").mkdir() + (tmp_path / "agents" / "claude").mkdir(parents=True) + + _run_acp_agent( + manager, + tmp_path, + userns_mode="keep-id", + workspace_mount_path=str(tmp_path / "workspace"), + start=False, + tty=False, + ) + + assert client.api.started is None + kwargs = client.api.create_kwargs + assert kwargs is not None + assert kwargs["tty"] is False + assert kwargs["working_dir"] == str(tmp_path / "workspace") + binds = client.api.host_config_kwargs["binds"] + assert f"{tmp_path / 'workspace'}:{tmp_path / 'workspace'}:rw" in binds + + +class _FakeStreamSocket: + def __init__(self, chunks: list[bytes]) -> None: + self._chunks = list(chunks) + self.shutdown_calls: list[int] = [] + + def recv(self, _size: int) -> bytes: + if self._chunks: + return self._chunks.pop(0) + return b"" + + def shutdown(self, how: int) -> None: + self.shutdown_calls.append(how) + + def sendall(self, data: bytes) -> None: + del data + + +class _FakeSocketWrapper: + def __init__(self, sock: _FakeStreamSocket) -> None: + self._sock = sock + + def close(self) -> None: + pass + + +class _AttachClient: + def __init__(self, chunks: list[bytes]) -> None: + self.wrapper = _FakeSocketWrapper(_FakeStreamSocket(chunks)) + + @property + def api(self): + return self + + def attach_socket(self, _id: str, params: dict) -> _FakeSocketWrapper: + assert params == {"stdin": 1, "stdout": 1, "stderr": 1, "stream": 1} + return self.wrapper + + +class _AttachContainer: + id = "abc123" + attrs = {"State": {"ExitCode": 0}} + + def reload(self) -> None: + pass + + def stop(self, timeout: int = 0) -> None: + pass + + +def _run_attach_stdio(monkeypatch, chunks: list[bytes]) -> tuple[bytes, bytes, int]: + import types + + from vibepod.core import docker as docker_mod + + out = io.BytesIO() + err = io.BytesIO() + fake_stdout = types.SimpleNamespace(buffer=out) + fake_stderr = types.SimpleNamespace(buffer=err) + fake_stdin = types.SimpleNamespace(closed=False) + + def _no_fileno() -> int: + raise ValueError("no fd in test") + + fake_stdin.fileno = _no_fileno + + client = _AttachClient(chunks) + manager = object.__new__(DockerManager) + manager.client = client # type: ignore[assignment] + + select_calls: list[list[Any]] = [] + + def _fake_select(readers, writ, exc): + select_calls.append(list(readers)) + # Always report the stream socket ready; recv() returns b"" once the + # scripted chunks are drained, which ends the attach loop. + return ([readers[0]], [], []) + + monkeypatch.setattr(docker_mod.select, "select", _fake_select) + monkeypatch.setattr(docker_mod.sys, "stdout", fake_stdout) + monkeypatch.setattr(docker_mod.sys, "stderr", fake_stderr) + monkeypatch.setattr(docker_mod.sys, "stdin", fake_stdin) + + exit_code = manager.attach_stdio(_AttachContainer()) + return out.getvalue(), err.getvalue(), exit_code + + +def test_attach_stdio_demuxes_split_frames(monkeypatch) -> None: + header_out = b"\x01\x00\x00\x00" + len(b'{"jsonrpc": "2.0"}\n').to_bytes(4, "big") + header_err = b"\x02\x00\x00\x00" + len(b"boom\n").to_bytes(4, "big") + # The stdout frame is split across recv() boundaries. + chunks = [ + header_out[:5], + header_out[5:] + b'{"jsonr', + b'pc": "2.0"}\n' + header_err + b"boom\n", + ] + + out, err, exit_code = _run_attach_stdio(monkeypatch, chunks) + + assert out == b'{"jsonrpc": "2.0"}\n' + assert err == b"boom\n" + assert exit_code == 0 + + +def test_attach_stdio_preserves_null_bytes_and_routes_stderr(monkeypatch) -> None: + payload = b"a\x00b" + chunks = [ + b"\x01\x00\x00\x00" + len(payload).to_bytes(4, "big") + payload, + b"\x02\x00\x00\x00\x00\x00\x00\x00", + ] + + out, err, _ = _run_attach_stdio(monkeypatch, chunks) + + assert out == payload + assert err == b"" + + @pytest.fixture() def socket_dir() -> Iterator[Path]: """Short-path directory for binding AF_UNIX sockets. diff --git a/tests/test_run.py b/tests/test_run.py index ebe5330..3b3e6a2 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -19,6 +19,7 @@ from vibepod.commands import run as run_cmd from vibepod.constants import EXIT_DOCKER_NOT_RUNNING, SUPPORTED_AGENTS from vibepod.core import launch, skills_engine +from vibepod.core.agents import get_agent_spec from vibepod.core.docker import DockerClientError, DockerManager # --------------------------------------------------------------------------- @@ -2670,6 +2671,8 @@ def test_run_recreates_proxy_when_image_updated(monkeypatch, tmp_path: Path) -> events: list[str] = [] class _OldProxyContainer: + status = "exited" + def remove(self, force: bool = False) -> None: events.append("proxy.remove") @@ -2696,6 +2699,9 @@ def clean_untagged_images(self) -> int: def find_proxy(self) -> object: return _OldProxyContainer() + def remove_proxy(self, existing: object) -> None: + events.append("proxy.remove") + def ensure_proxy(self, **kwargs) -> None: # type: ignore[no-untyped-def] events.append("ensure_proxy") @@ -2727,3 +2733,187 @@ def run_agent(self, **kwargs) -> object: # type: ignore[no-untyped-def] run_cmd.run(agent="claude", workspace=tmp_path, detach=True) assert events == ["pull_if_newer", "proxy.remove", "ensure_proxy"] + + +# --------------------------------------------------------------------------- +# ACP mode (--acp) +# --------------------------------------------------------------------------- + + +class _AcpContainer: + def __init__(self) -> None: + self.id = "acp123" + self.name = "vibepod-claude-acp" + self.status = "created" + self.attrs: dict = {"NetworkSettings": {"Ports": {}}} + self.started = False + + def reload(self) -> None: + pass + + def start(self) -> None: + self.started = True + self.status = "running" + + def stop(self, timeout: int = 0) -> None: + pass + + +def _make_acp_manager(captured: dict): + class _AcpCapturingDockerManager: + def ensure_network(self, name: str) -> None: + pass + + def networks_with_running_containers(self) -> list[str]: + return [] + + def pull_image(self, image: str, auto_clean: bool = False) -> None: + pass + + def resolve_launch_command(self, image: str, command: list[str] | None) -> list[str]: + return command or ["noop"] + + def run_agent(self, **kwargs) -> object: # type: ignore[no-untyped-def] + captured.update(kwargs) + return _AcpContainer() + + def attach_stdio(self, container, logger=None, on_attached=None) -> int: # type: ignore[no-untyped-def] + captured["attach_stdio_called"] = True + if on_attached is not None: + on_attached() + captured["container_started_by_attach"] = container.started + return 0 + + return _AcpCapturingDockerManager + + +class _FakeSessionLogger: + def __init__(self, *_args, **_kwargs) -> None: + pass + + def open_session(self, **_kwargs) -> None: + pass + + def close_session(self, *_args, **_kwargs) -> None: + pass + + +@pytest.fixture() +def _acp_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path: + """Common stubs for ACP run tests; returns the workspace dir.""" + monkeypatch.setenv("VP_CONFIG_DIR", str(tmp_path / "config")) + monkeypatch.setattr(run_cmd, "SessionLogger", _FakeSessionLogger) + workspace = tmp_path / "workspace" + workspace.mkdir() + return workspace + + +def test_acp_uses_acp_command_and_stdio_container(monkeypatch, _acp_env) -> None: + captured: dict = {} + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + + assert captured["command"] == ["npx", "-y", "@agentclientprotocol/claude-agent-acp"] + assert captured["start"] is False + assert captured["tty"] is False + assert captured["workspace_mount_path"] == str(_acp_env) + assert captured["attach_stdio_called"] is True + assert captured["container_started_by_attach"] is True + + +def test_acp_config_override_wins_over_default(monkeypatch, _acp_env) -> None: + captured: dict = {} + config = _make_config() + config["agents"] = { + "claude": {"env": {}, "init": [], "acp_command": ["custom-acp", "--serve"]}, + } + monkeypatch.setattr(run_cmd, "get_config", lambda: config) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + + assert captured["command"] == ["custom-acp", "--serve"] + + +def test_acp_unsupported_agent_aborts(monkeypatch, _acp_env) -> None: + captured: dict = {} + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + with pytest.raises(typer.Exit) as exc: + run_cmd.run(agent="pi", workspace=_acp_env, acp=True) + + assert exc.value.exit_code == 1 + assert "command" not in captured + + +def test_acp_detach_aborts(monkeypatch, _acp_env) -> None: + captured: dict = {} + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + with pytest.raises(typer.Exit) as exc: + run_cmd.run(agent="claude", workspace=_acp_env, acp=True, detach=True) + + assert exc.value.exit_code == 1 + assert "command" not in captured + + +def test_acp_skips_herdr_hint(monkeypatch, _acp_env) -> None: + captured: dict = {} + calls: dict = {} + + def _fake_hint(agent, config, no_herdr=False): + calls["no_herdr"] = no_herdr + + monkeypatch.setattr(run_cmd, "_reexec_with_herdr_hint", _fake_hint) + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + + assert calls["no_herdr"] is True + + +def test_acp_ignores_ikwid_with_warning(monkeypatch, _acp_env) -> None: + captured: dict = {} + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + run_cmd.run(agent="claude", workspace=_acp_env, acp=True, ikwid=True) + + assert captured["command"] == ["npx", "-y", "@agentclientprotocol/claude-agent-acp"] + + +def test_acp_workspace_mount_path_guard(tmp_path: Path) -> None: + spec = get_agent_spec("claude") + good = run_cmd._acp_workspace_mount_path(tmp_path / "proj", spec) + assert good == str(tmp_path / "proj") + + for reserved in ("/workspace", "/config", "/etc/vibepod", "/usr/local", "/claude"): + with pytest.raises(typer.Exit): + run_cmd._acp_workspace_mount_path(Path(reserved), spec) + + # A host path containing a reserved mount target also collides. + with pytest.raises(typer.Exit): + run_cmd._acp_workspace_mount_path(Path("/tmp"), spec) + + # Non-POSIX paths (Windows hosts) are rejected. + with pytest.raises(typer.Exit): + run_cmd._acp_workspace_mount_path(Path("C:\\projects\\demo"), spec) + + +def test_acp_routes_console_to_stderr(monkeypatch, _acp_env) -> None: + from vibepod.utils import console as console_mod + + captured: dict = {} + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + original = console_mod.console + try: + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + assert console_mod.console.stderr is True + finally: + console_mod.console = original From 8442e23b76291966139f6034cb0adb9ca2757428 Mon Sep 17 00:00:00 2001 From: Harald Nezbeda Date: Fri, 28 Aug 2026 16:09:44 +0000 Subject: [PATCH 2/5] feat: extend ACP mode to five more agents opencode, copilot, auggie, jcode and devstral ship an ACP server in the image, so they start without an adapter download. gemini's acp_command respelled the binary as a bare `gemini`, dropping the node/HOME launcher f5e6f0f added for the Alpine shebang; a new test pins the rule that an ACP command extends `command`. auggie and copilot bind their credential dir at the home their entrypoint creates for an unknown host uid (macOS 501): `su` resets HOME, and an ACP session has no TTY for a login flow. --- README.md | 2 +- docs/acp.md | 44 ++++++++++++++++++++++++++++++++------ docs/configuration.md | 6 +++--- src/vibepod/core/agents.py | 22 ++++++++++++++++++- src/vibepod/core/launch.py | 7 ++++++ tests/test_agents.py | 34 ++++++++++++++++++++++++++++- tests/test_run.py | 2 ++ 7 files changed, 105 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 6335143..bff8d24 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,7 @@ Use `--ikwid` to append each agent's auto-approval / permission-skip flag when s ## Editor integration (`--acp`) -`vp run --acp` turns VibePod into an [Agent Client Protocol](https://agentclientprotocol.com/) adapter, so the containerized agent appears directly in the AI panel of any editor with ACP support (e.g. [Zed](https://zed.dev/docs/ai/external-agents)) — with isolation, profiles, overlays and proxy metrics intact. Supported out of the box: `claude`, `gemini`, `qwen`, `codex`. +`vp run --acp` turns VibePod into an [Agent Client Protocol](https://agentclientprotocol.com/) adapter, so the containerized agent appears directly in the AI panel of any editor with ACP support (e.g. [Zed](https://zed.dev/docs/ai/external-agents)) — with isolation, profiles, overlays and proxy metrics intact. Supported out of the box: `claude`, `gemini`, `qwen`, `codex`, `opencode`, `copilot`, `auggie`, `jcode` and `devstral`. Register `vp` as a custom/external agent server in your editor. Zed example (`settings.json`): diff --git a/docs/acp.md b/docs/acp.md index 83878a3..33f5d8b 100644 --- a/docs/acp.md +++ b/docs/acp.md @@ -10,9 +10,29 @@ metric collection — stays active. ## Supported agents -`claude`, `gemini`, `qwen` and `codex` ship an ACP adapter command. Other -agents abort with an error listing the supported agents (you can still provide -your own adapter via `agents..acp_command` in the config). +Nine agents ship an ACP adapter command. They split into two kinds, which +differ in what has to happen before the first JSON-RPC frame: + +| Agent | Adapter | +| ---------- | ------------------------------------------- | +| `opencode` | `opencode acp` — built into the CLI | +| `copilot` | `copilot --acp --stdio` — built into the CLI | +| `auggie` | `auggie --acp` — built into the CLI | +| `jcode` | `jcode acp` — built into the CLI | +| `gemini` | `gemini --experimental-acp` — built in | +| `qwen` | `qwen --experimental-acp` — built in | +| `devstral` | `vibe-acp` — separate binary in the image | +| `claude` | `npx @agentclientprotocol/claude-agent-acp` | +| `codex` | `npx @agentclientprotocol/codex-acp` | + +The first seven run a binary that is already in the image, so they start +offline and immediately. `claude` and `codex` fetch their adapter over the +network on every launch, which adds startup latency and needs the package +registry reachable through the proxy filter. + +Other agents abort with an error listing the supported agents (you can still +provide your own adapter via `agents..acp_command` in the config — +that is also how you pin an `npx` adapter to a version). ## Setup @@ -34,9 +54,9 @@ arguments. For example, in [Zed](https://zed.dev/docs/ai/external-agents) } ``` -Repeat the block for `gemini`, `qwen` or `codex` (adjust the `run` argument) -if you want more than one. Make sure `vp` is on the `PATH` your editor -inherits (or use an absolute path). +Repeat the block for any other supported agent (adjust the `run` argument) if +you want more than one. Make sure `vp` is on the `PATH` your editor inherits +(or use an absolute path). Then allow your project directory once — under an editor, stdin is a pipe, so the interactive allow prompt cannot run: @@ -45,6 +65,18 @@ the interactive allow prompt cannot run: vp config allow-dir /path/to/your/project ``` +Authenticate the agent once interactively too, before the first ACP session: + +```bash +vp run # sign in, then quit +``` + +An ACP session cannot log you in. Most adapters expose no authentication +method to the editor at all, and the ones that do point it at a command inside +the container that the editor would run on your host. Credentials persist in +the agent's config dir, so this is a one-time step per agent and profile; skip +it and the thread starts cleanly and then fails on the first prompt. + Open the AI/agent panel in your editor, pick the VibePod thread type, and start a thread inside your project. diff --git a/docs/configuration.md b/docs/configuration.md index f1fd22d..dfad89d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -56,9 +56,9 @@ agents: overlay: true # Set false to ignore the project's .vibepod/overlay/ (see Project overlays) # acp_command: ACP adapter command for `vp run --acp` (editor # integration via the Agent Client Protocol). Defaults exist for claude, - # gemini, qwen and codex; override with a list (or a string, which is - # shell-split). Not present by default — just add the key to override. - # just add the key to override. + # gemini, qwen, codex, opencode, copilot, auggie, jcode and devstral; + # override with a list (or a string, split on whitespace). Not present by + # default — just add the key to override. # acp_command: ["npx", "-y", "@agentclientprotocol/claude-agent-acp"] gemini: diff --git a/src/vibepod/core/agents.py b/src/vibepod/core/agents.py index 9346753..f9248da 100644 --- a/src/vibepod/core/agents.py +++ b/src/vibepod/core/agents.py @@ -75,7 +75,17 @@ class AgentSpec: "/config", {"HOME": "/config"}, ikwid_args=["--approval-mode=yolo"], - acp_command=["gemini", "--experimental-acp"], + # Same launcher as `command`: --acp replaces it wholesale, so the + # shebang/HOME workaround above has to be repeated here. Keep + # --experimental-acp (the `--acp` alias only exists in gemini-cli + # >= 0.33; the image tracks upstream and is not pinned). + acp_command=[ + "env", + "HOME=/config", + "node", + "/usr/local/bin/gemini", + "--experimental-acp", + ], ), "opencode": AgentSpec( "opencode", @@ -92,6 +102,7 @@ class AgentSpec: "XDG_STATE_HOME": "/config/.local/state", "XDG_CACHE_HOME": "/config/.cache", }, + acp_command=["opencode", "acp"], ), "devstral": AgentSpec( "devstral", @@ -104,6 +115,9 @@ class AgentSpec: platform="linux/amd64", run_as_host_user=True, ikwid_args=["--auto-approve"], + # Separate console script shipped by the same mistral-vibe package, + # not a flag on `devstral`. + acp_command=["vibe-acp"], ), "auggie": AgentSpec( "auggie", @@ -114,6 +128,7 @@ class AgentSpec: "/config", {"HOME": "/config"}, headless_prefix=["--print"], + acp_command=["auggie", "--acp"], ), "copilot": AgentSpec( "copilot", @@ -124,6 +139,10 @@ class AgentSpec: "/config", {"HOME": "/config"}, ikwid_args=["--yolo"], + # --stdio is the default transport, but it is mutually exclusive with + # --port: pass it so an inherited config cannot move the adapter onto + # a socket the attach stream never sees. + acp_command=["copilot", "--acp", "--stdio"], ), "codex": AgentSpec( "codex", @@ -187,6 +206,7 @@ class AgentSpec: # so pointing HOME at the persisted /config mount covers both. {"HOME": "/config", "JCODE_NO_AUTO_UPDATE": "1"}, headless_prefix=["run"], + acp_command=["jcode", "acp"], ), "freebuff": AgentSpec( "freebuff", diff --git a/src/vibepod/core/launch.py b/src/vibepod/core/launch.py index 01d5f6e..7587352 100644 --- a/src/vibepod/core/launch.py +++ b/src/vibepod/core/launch.py @@ -219,11 +219,17 @@ def init_entrypoint(init_commands: list[str]) -> list[str]: def agent_extra_volumes(agent: str, config_dir: Path) -> list[tuple[str, str, str]]: """Return agent-specific bind mounts as (host_path, container_path, mode).""" + # These images drop privileges with `su`, which resets HOME to the runtime + # user's passwd entry — so the credential dir has to be bound at every home + # the entrypoint can land on, including the one it creates for a host uid it + # does not already know (macOS Docker Desktop's 501, for one). ACP mode + # cannot recover from a miss: editors have no TTY for a login flow. if agent == "auggie": host = str(config_dir / ".augment") return [ (host, "/root/.augment", "rw"), (host, "/home/node/.augment", "rw"), + (host, "/home/auggie/.augment", "rw"), ] if agent == "copilot": host = str(config_dir / ".copilot") @@ -231,6 +237,7 @@ def agent_extra_volumes(agent: str, config_dir: Path) -> list[tuple[str, str, st (host, "/root/.copilot", "rw"), (host, "/home/node/.copilot", "rw"), (host, "/home/coder/.copilot", "rw"), + (host, "/home/copilot/.copilot", "rw"), ] if agent == "opencode": xdg_config = config_dir / ".config" / "opencode" diff --git a/tests/test_agents.py b/tests/test_agents.py index 23c8c87..124e05a 100644 --- a/tests/test_agents.py +++ b/tests/test_agents.py @@ -258,9 +258,20 @@ def test_agents_without_llm_env_map() -> None: def test_acp_commands_match_contract() -> None: expected = { "claude": ["npx", "-y", "@agentclientprotocol/claude-agent-acp"], - "gemini": ["gemini", "--experimental-acp"], + "gemini": [ + "env", + "HOME=/config", + "node", + "/usr/local/bin/gemini", + "--experimental-acp", + ], "qwen": ["qwen", "--experimental-acp"], "codex": ["npx", "-y", "@agentclientprotocol/codex-acp"], + "opencode": ["opencode", "acp"], + "copilot": ["copilot", "--acp", "--stdio"], + "auggie": ["auggie", "--acp"], + "jcode": ["jcode", "acp"], + "devstral": ["vibe-acp"], } for agent in SUPPORTED_AGENTS: spec = get_agent_spec(agent) @@ -270,6 +281,27 @@ def test_acp_commands_match_contract() -> None: assert spec.acp_command is None, f"{agent} should not have acp_command" +def test_in_image_acp_commands_extend_the_launch_command() -> None: + """Agents whose ACP mode is their own binary must reuse their launch argv. + + ``--acp`` replaces ``spec.command`` wholesale, so an ACP command that spells + the binary differently silently drops launcher workarounds (gemini's + ``env HOME=... node /usr/local/bin/gemini`` shebang bypass, for one). + Agents driven by an external adapter (npx packages, devstral's ``vibe-acp`` + console script) are exempt. + """ + external_adapters = {"claude", "codex", "devstral"} + for agent in SUPPORTED_AGENTS: + spec = get_agent_spec(agent) + if spec.acp_command is None or agent in external_adapters: + continue + assert spec.command is not None, f"{agent} needs a command to extend" + prefix = spec.acp_command[: len(spec.command)] + assert prefix == spec.command, ( + f"{agent} acp_command must start with {spec.command}, got {spec.acp_command}" + ) + + def test_opencode_spec_matches_container_contract() -> None: spec = get_agent_spec("opencode") assert spec.id == "opencode" diff --git a/tests/test_run.py b/tests/test_run.py index 3b3e6a2..217b89e 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -83,6 +83,7 @@ def test_agent_extra_volumes_for_auggie(tmp_path: Path) -> None: assert run_cmd._agent_extra_volumes("auggie", config_dir) == [ (str(augment_dir), "/root/.augment", "rw"), (str(augment_dir), "/home/node/.augment", "rw"), + (str(augment_dir), "/home/auggie/.augment", "rw"), ] @@ -113,6 +114,7 @@ def test_agent_extra_volumes_for_copilot(tmp_path: Path) -> None: (str(config_host), "/root/.copilot", "rw"), (str(config_host), "/home/node/.copilot", "rw"), (str(config_host), "/home/coder/.copilot", "rw"), + (str(config_host), "/home/copilot/.copilot", "rw"), ] From 03f65968ff5e228198128cc3ef5dca8c0ea96dea Mon Sep 17 00:00:00 2001 From: Harald Nezbeda Date: Fri, 28 Aug 2026 16:43:32 +0000 Subject: [PATCH 3/5] fix: replace a running proxy when a newer image is pulled --- src/vibepod/core/docker.py | 17 ++++++++--- src/vibepod/core/launch.py | 6 +++- tests/test_docker.py | 56 +++++++++++++++++++++++++++++++++++ tests/test_provision_proxy.py | 3 ++ tests/test_proxy_cmd.py | 3 ++ 5 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/vibepod/core/docker.py b/src/vibepod/core/docker.py index a58b0d5..33b274a 100644 --- a/src/vibepod/core/docker.py +++ b/src/vibepod/core/docker.py @@ -858,21 +858,30 @@ def find_proxy(self) -> Any | None: return containers[0] if containers else None def remove_proxy(self, existing: Any, timeout: float = 15.0) -> None: - """Force-remove a non-running proxy container. + """Force-remove a proxy container and wait for it to disappear. Concurrent launches (e.g. an editor spawning `vp run` twice) can race on the removal; Docker then answers 409 "removal ... is already in - progress". Treat that as success and wait until the container is gone - so the caller can create its replacement. + progress", or 404 if the peer already finished. Treat both as success + and wait until the container is gone so the caller can create its + replacement. + + The wait tracks *this* container, not any proxy: a peer's replacement + carries the same labels, so waiting for `find_proxy()` to go empty + would never be satisfied once one exists. """ + target_id = getattr(existing, "id", None) try: existing.remove(force=True) + except NotFound: + return except APIError as exc: if "already in progress" not in str(exc): raise deadline = time.time() + timeout while time.time() < deadline: - if self.find_proxy() is None: + current = self.find_proxy() + if current is None or getattr(current, "id", None) != target_id: return time.sleep(0.2) raise DockerClientError( diff --git a/src/vibepod/core/launch.py b/src/vibepod/core/launch.py index 7587352..5f68072 100644 --- a/src/vibepod/core/launch.py +++ b/src/vibepod/core/launch.py @@ -380,7 +380,11 @@ def provision_proxy( if manager.pull_if_newer(image, auto_clean=False): manager.require_proxy_policy_schema(image, policy_schema) existing = manager.find_proxy() - if existing and existing.status != "running": + if existing: + # Including a *running* proxy: ensure_proxy returns one as-is, + # so leaving it would keep serving the superseded image for the + # life of the container. remove_proxy tolerates a concurrent + # launch removing it first. manager.remove_proxy(existing) container = manager.ensure_proxy( image=image, diff --git a/tests/test_docker.py b/tests/test_docker.py index e6e87d0..ffd6614 100644 --- a/tests/test_docker.py +++ b/tests/test_docker.py @@ -703,6 +703,62 @@ def test_ensure_proxy_rejects_incompatible_running_container(mock_docker, tmp_pa ) +@patch("vibepod.core.docker.docker") +def test_remove_proxy_returns_once_the_target_container_is_gone(mock_docker) -> None: + mock_client = MagicMock() + mock_docker.from_env.return_value = mock_client + existing = MagicMock(id="old") + mock_client.containers.list.return_value = [] + + manager = DockerManager() + manager.remove_proxy(existing, timeout=1.0) + + existing.remove.assert_called_once_with(force=True) + + +@patch("vibepod.core.docker.docker") +def test_remove_proxy_returns_when_a_peer_already_created_the_replacement(mock_docker) -> None: + """A racing launch's new proxy carries the same labels as the old one. + + Waiting for `find_proxy()` to go empty would then never be satisfied, and + the second launch would abort even though a healthy proxy is up. + """ + mock_client = MagicMock() + mock_docker.from_env.return_value = mock_client + existing = MagicMock(id="old") + mock_client.containers.list.return_value = [MagicMock(id="new")] + + manager = DockerManager() + manager.remove_proxy(existing, timeout=1.0) + + +@patch("vibepod.core.docker.docker") +def test_remove_proxy_tolerates_a_peer_winning_the_removal(mock_docker) -> None: + mock_client = MagicMock() + mock_docker.from_env.return_value = mock_client + mock_client.containers.list.return_value = [] + + for error in ( + APIError("409 Client Error: removal of container abc is already in progress"), + NotFound("404 Client Error: No such container: abc"), + ): + existing = MagicMock(id="old") + existing.remove.side_effect = error + DockerManager().remove_proxy(existing, timeout=1.0) + + +@patch("vibepod.core.docker.docker") +def test_remove_proxy_times_out_while_the_target_lingers(mock_docker) -> None: + mock_client = MagicMock() + mock_docker.from_env.return_value = mock_client + existing = MagicMock(id="old") + mock_client.containers.list.return_value = [existing] + + manager = DockerManager() + with pytest.raises(DockerClientError, match="Timed out"): + manager.remove_proxy(existing, timeout=0.5) + + def test_discover_podman_socket_skipped_when_docker_host_set(monkeypatch) -> None: monkeypatch.setenv("DOCKER_HOST", "unix:///var/run/docker.sock") assert _discover_podman_socket() is None diff --git a/tests/test_provision_proxy.py b/tests/test_provision_proxy.py index 7311ec4..a8c8bd3 100644 --- a/tests/test_provision_proxy.py +++ b/tests/test_provision_proxy.py @@ -33,6 +33,9 @@ def require_proxy_policy_schema(self, image: str, required: str) -> None: def find_proxy(self) -> object: return self.running + def remove_proxy(self, existing: object, timeout: float = 15.0) -> None: + existing.remove(force=True) # type: ignore[attr-defined] + def ensure_proxy(self, **kwargs: object) -> str: self.ensured = True return "proxy-container" diff --git a/tests/test_proxy_cmd.py b/tests/test_proxy_cmd.py index ccbe563..d42517c 100644 --- a/tests/test_proxy_cmd.py +++ b/tests/test_proxy_cmd.py @@ -33,6 +33,9 @@ def require_proxy_policy_schema(self, image: object, required: str = "2") -> Non def find_proxy(self): return self._container + def remove_proxy(self, existing, timeout: float = 15.0) -> None: + existing.remove(force=True) + def clean_untagged_images(self) -> int: self._events.append("clean_untagged_images") return 0 From baaac10772903108bd89e7c6b3690edb3693fd46 Mon Sep 17 00:00:00 2001 From: Harald Nezbeda Date: Fri, 28 Aug 2026 18:16:19 +0000 Subject: [PATCH 4/5] fix: scope ACP Windows support to WSL2 --- docs/acp.md | 42 ++++++++++++++++++++++++++++++++++-- src/vibepod/commands/run.py | 9 ++++---- tests/test_run.py | 43 ++++++++++++++++++++++++++++++++++--- 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/docs/acp.md b/docs/acp.md index 33f5d8b..24d355a 100644 --- a/docs/acp.md +++ b/docs/acp.md @@ -94,10 +94,48 @@ When you close the thread, the editor kills the `vp` process; the container sees stdin EOF, the adapter exits, and `auto_remove` cleans up — no orphaned containers. +## Windows: run it from WSL2 + +ACP itself is platform-neutral, and your editor's ACP support is not the +problem — the path-parity mount is. A Linux container's bind *target* has to be +a Linux path, so `C:\Users\you\proj` cannot be mounted onto itself and `--acp` +refuses a Windows workspace path. + +WSL2 works today, with no special flags: put the project on the WSL filesystem, +enable Docker Desktop's WSL integration, install `vp` in the distro, and open +the project as a **remote WSL project** in your editor (in Zed: +`projects: open folder in wsl`). The editor then spawns `vp` inside the distro, +every path on both sides is POSIX, and the parity mount lines up. + +```json +{ + "agent_servers": { + "VibePod Claude": { + "type": "custom", + "command": "/home/you/.local/bin/vp", + "args": ["run", "claude", "--acp", "-w", "/home/you/proj"], + "env": {} + } + } +} +``` + +Use an absolute path to `vp` (the spawn environment is not a login shell) and +pass `-w` explicitly: VibePod picks the workspace from `--workspace` at launch +and never reads the cwd the ACP client sends, and the spawn cwd is not +guaranteed to be your project. + +!!! warning "Do not bridge a Windows-side project through `wsl.exe`" + + Running a Windows-native editor against a Windows-side project with + `"command": "wsl.exe"` looks like it works and then silently misbehaves: + the editor sends `C:\dev\proj` while VibePod mounts `/mnt/c/dev/proj`, and + nothing translates between them. The path starts with `/`, so the guard + above does not catch it. Keep the project, the editor's remote server and + `vp` all on the Linux side. + ## Limitations -- **Windows hosts are not supported**: the path-parity mount requires the host - workspace path to be a valid container path. - The workspace host path must not collide with container-reserved paths (`/workspace`, `/config`, `/claude`, `/qwen`, `/etc`, `/usr`, `/tmp/.X11-unix` and the agent's config mount); `--acp` aborts with a clear diff --git a/src/vibepod/commands/run.py b/src/vibepod/commands/run.py index 441b7d9..a1a2ff0 100644 --- a/src/vibepod/commands/run.py +++ b/src/vibepod/commands/run.py @@ -140,14 +140,15 @@ def _acp_workspace_mount_path(workspace_path: Path, spec: Any) -> str: @-mentions) and expect absolute host paths back in diffs. Binding the workspace onto its own host path (in addition to /workspace) makes both sides agree. Aborts when the host path would shadow a container-reserved - path or is not a POSIX path (Windows hosts are not supported in ACP mode). + path, or is not a POSIX path: a Linux container's bind target cannot be a + Windows path, so native Windows hosts run ACP mode from WSL2 instead. """ host_path = str(workspace_path) if not host_path.startswith("/"): error( - f"--acp requires a POSIX workspace path; '{host_path}' is not " - "usable as a container path (Windows hosts are not supported " - "in ACP mode).", + f"--acp requires a POSIX workspace path; '{host_path}' cannot be a " + "container path. On Windows, run the editor and vp inside WSL2 " + "(see the ACP docs).", ) raise typer.Exit(1) diff --git a/tests/test_run.py b/tests/test_run.py index 217b89e..dd4257b 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -9,7 +9,7 @@ import re import subprocess import sys -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest import typer @@ -2673,7 +2673,7 @@ def test_run_recreates_proxy_when_image_updated(monkeypatch, tmp_path: Path) -> events: list[str] = [] class _OldProxyContainer: - status = "exited" + status = "running" def remove(self, force: bool = False) -> None: events.append("proxy.remove") @@ -2800,6 +2800,17 @@ def close_session(self, *_args, **_kwargs) -> None: pass +# `--acp` binds the workspace onto its own host path, which a Linux container +# cannot do with a Windows path: run() aborts at _acp_workspace_mount_path +# before any container is created. Windows users run ACP mode from WSL2, where +# the workspace path is POSIX — so these tests exercise the supported host, and +# only native-Windows interpreters skip. +_requires_posix_workspace = pytest.mark.skipif( + os.name == "nt", + reason="ACP mode runs from WSL2 on Windows, where workspace paths are POSIX", +) + + @pytest.fixture() def _acp_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path: """Common stubs for ACP run tests; returns the workspace dir.""" @@ -2810,6 +2821,7 @@ def _acp_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path: return workspace +@_requires_posix_workspace def test_acp_uses_acp_command_and_stdio_container(monkeypatch, _acp_env) -> None: captured: dict = {} monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) @@ -2825,6 +2837,7 @@ def test_acp_uses_acp_command_and_stdio_container(monkeypatch, _acp_env) -> None assert captured["container_started_by_attach"] is True +@_requires_posix_workspace def test_acp_config_override_wins_over_default(monkeypatch, _acp_env) -> None: captured: dict = {} config = _make_config() @@ -2863,6 +2876,7 @@ def test_acp_detach_aborts(monkeypatch, _acp_env) -> None: assert "command" not in captured +@_requires_posix_workspace def test_acp_skips_herdr_hint(monkeypatch, _acp_env) -> None: captured: dict = {} calls: dict = {} @@ -2879,6 +2893,7 @@ def _fake_hint(agent, config, no_herdr=False): assert calls["no_herdr"] is True +@_requires_posix_workspace def test_acp_ignores_ikwid_with_warning(monkeypatch, _acp_env) -> None: captured: dict = {} monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) @@ -2889,11 +2904,32 @@ def test_acp_ignores_ikwid_with_warning(monkeypatch, _acp_env) -> None: assert captured["command"] == ["npx", "-y", "@agentclientprotocol/claude-agent-acp"] -def test_acp_workspace_mount_path_guard(tmp_path: Path) -> None: +@_requires_posix_workspace +def test_acp_workspace_mount_path_accepts_a_posix_workspace(tmp_path: Path) -> None: spec = get_agent_spec("claude") good = run_cmd._acp_workspace_mount_path(tmp_path / "proj", spec) assert good == str(tmp_path / "proj") + +def test_acp_workspace_mount_path_accepts_wsl_paths() -> None: + """WSL2 is the supported Windows route, so its paths must pass the guard. + + Both shapes matter: a project on the distro filesystem, and one on a + Windows drive reached through /mnt (which works for the mount, but is the + setup that silently mismatches paths — see docs/acp.md). + """ + spec = get_agent_spec("claude") + # PurePosixPath, not Path: a native-Windows interpreter would stringify + # these with backslashes and the assertion would test nothing. + for wsl_path in ("/home/you/proj", "/mnt/c/dev/proj"): + assert run_cmd._acp_workspace_mount_path(PurePosixPath(wsl_path), spec) == wsl_path + + +def test_acp_workspace_mount_path_guard(tmp_path: Path) -> None: + spec = get_agent_spec("claude") + + # On Windows these are rejected by the POSIX check rather than the + # reserved-path check — a different reason, but the same right outcome. for reserved in ("/workspace", "/config", "/etc/vibepod", "/usr/local", "/claude"): with pytest.raises(typer.Exit): run_cmd._acp_workspace_mount_path(Path(reserved), spec) @@ -2907,6 +2943,7 @@ def test_acp_workspace_mount_path_guard(tmp_path: Path) -> None: run_cmd._acp_workspace_mount_path(Path("C:\\projects\\demo"), spec) +@_requires_posix_workspace def test_acp_routes_console_to_stderr(monkeypatch, _acp_env) -> None: from vibepod.utils import console as console_mod From 5cf901680d12ab09994e56dc099b6fcf24809301 Mon Sep 17 00:00:00 2001 From: Harald Nezbeda Date: Tue, 1 Sep 2026 19:15:13 +0000 Subject: [PATCH 5/5] fix: ACP exit codes, init-command override, attach cleanup and stdout hygiene --- AGENTS.md | 3 +- docs/acp.md | 8 +- docs/configuration.md | 28 +++---- src/vibepod/commands/run.py | 129 +++++++++++++++++++----------- src/vibepod/core/docker.py | 54 ++++++++++--- src/vibepod/utils/console.py | 10 +-- tests/conftest.py | 16 ++++ tests/test_docker.py | 89 ++++++++++++++++++++- tests/test_run.py | 147 +++++++++++++++++++++++++++++++++-- 9 files changed, 396 insertions(+), 88 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ab7779a..6803891 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,8 @@ To add a new agent, mirror the `qwen` (or `freebuff`) entry end-to-end: `SUPPORTED_AGENTS`/`AGENT_SHORTCUTS`/`AGENT_ALIASES`/`IMAGE_OVERRIDE_ENV_KEYS` and `get_default_images()` in `src/vibepod/constants.py`, the `AGENT_SPECS` entry in `src/vibepod/core/agents.py` (mount contract must match the -vibepod-agents image entrypoint), the per-agent defaults in +vibepod-agents image entrypoint; set `acp_command` when the agent ships an +ACP adapter, see `docs/acp.md`), the per-agent defaults in `src/vibepod/core/config.py`, skills paths in `src/vibepod/commands/run.py::_agent_skill_paths`, and the docs + tests (`docs/agents/index.md`, `README.md`, `docs/quickstart.md`, `docs/index.md`, diff --git a/docs/acp.md b/docs/acp.md index 24d355a..bf470c5 100644 --- a/docs/acp.md +++ b/docs/acp.md @@ -88,11 +88,15 @@ demultiplexes the Docker stream: container stdout carries only the newline-delimited JSON-RPC stream, all VibePod messages go to stderr. The workspace is mounted a second time onto its own host path so absolute paths from the editor (session cwd, @-mentions, diffs) resolve identically inside -the container. +the container. If that path goes through a symlink (macOS `/tmp`, a linked +`~/code`), the unresolved spelling is bound as well, so the path the editor +sends and the resolved one both exist in the container. When you close the thread, the editor kills the `vp` process; the container sees stdin EOF, the adapter exits, and `auto_remove` cleans up — no orphaned -containers. +containers. `vp` exits with the adapter's exit code, so a crashed adapter +shows up in the editor as a failure rather than a clean exit, and a container +whose attach failed before it ever started is removed rather than left behind. ## Windows: run it from WSL2 diff --git a/docs/configuration.md b/docs/configuration.md index dfad89d..9ffa946 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -57,8 +57,8 @@ agents: # acp_command: ACP adapter command for `vp run --acp` (editor # integration via the Agent Client Protocol). Defaults exist for claude, # gemini, qwen, codex, opencode, copilot, auggie, jcode and devstral; - # override with a list (or a string, split on whitespace). Not present by - # default — just add the key to override. + # override with a list, or a string parsed with shell quoting rules. Not + # present by default — just add the key to override. # acp_command: ["npx", "-y", "@agentclientprotocol/claude-agent-acp"] gemini: @@ -306,10 +306,10 @@ agents: ``` !!! warning "Always quote port entries" -YAML 1.1 parses unquoted `host:container` pairs as base-60 numbers: -`- 22:22` loads as the integer `1342` and would publish the wrong port. -Out-of-range results (e.g. `3000:30` → `180030`) are rejected at startup, -but in-range ones are not detectable — quote every entry. + YAML 1.1 parses unquoted `host:container` pairs as base-60 numbers: + `- 22:22` loads as the integer `1342` and would publish the wrong port. + Out-of-range results (e.g. `3000:30` → `180030`) are rejected at startup, + but in-range ones are not detectable — quote every entry. Like other agent keys, a project-level `ports` list replaces the global one for that agent. The list applies to both `vp run` and `vp task` containers; note that two containers cannot publish the same host port at the same time, so a fixed host port limits you to one such container per agent. @@ -339,10 +339,10 @@ VP_PROXY_ENABLED=false vp run claude ``` !!! note "Podman users" -The proxy relies on container-name DNS resolution over the `vibepod-network` -network. If you use Podman with the CNI backend, install the `dnsname` -plugin (e.g. `podman-plugins` on Fedora, `golang-github-containernetworking-plugin-dnsname` -on Debian/Ubuntu) and recreate the network. See [Quickstart — Using Podman](quickstart.md#using-podman-instead-of-docker) for full instructions. + The proxy relies on container-name DNS resolution over the `vibepod-network` + network. If you use Podman with the CNI backend, install the `dnsname` + plugin (e.g. `podman-plugins` on Fedora, `golang-github-containernetworking-plugin-dnsname` + on Debian/Ubuntu) and recreate the network. See [Quickstart — Using Podman](quickstart.md#using-podman-instead-of-docker) for full instructions. ### Allow/deny filtering @@ -402,7 +402,7 @@ launching a proxied agent. An image with a missing or different schema label is rejected with an upgrade error. !!! warning "Filtering is not a network sandbox" -The filter controls requests that use the injected proxy. A container can -override its proxy environment or attempt a direct connection unless a -separate network-control layer prevents that. VibePod warns when explicit -`HTTP_PROXY` or `HTTPS_PROXY` values bypass its identified proxy URL. + The filter controls requests that use the injected proxy. A container can + override its proxy environment or attempt a direct connection unless a + separate network-control layer prevents that. VibePod warns when explicit + `HTTP_PROXY` or `HTTPS_PROXY` values bypass its identified proxy URL. diff --git a/src/vibepod/commands/run.py b/src/vibepod/commands/run.py index a1a2ff0..22947d8 100644 --- a/src/vibepod/commands/run.py +++ b/src/vibepod/commands/run.py @@ -5,9 +5,10 @@ import json import os import re +import shlex import sys import time -from pathlib import Path +from pathlib import Path, PurePath from typing import Annotated, Any import typer @@ -17,6 +18,7 @@ from vibepod.constants import EXIT_DOCKER_NOT_RUNNING, SUPPORTED_AGENTS from vibepod.core.agents import ( AGENT_SPECS, + AgentSpec, agent_config_dir, effective_agent_image, get_agent_shortcut, @@ -122,18 +124,33 @@ ) -def _resolve_acp_command(spec: Any, agent_cfg: dict[str, Any]) -> list[str] | None: - """Resolve the ACP adapter command: config override wins over the spec.""" +def _resolve_acp_command(spec: AgentSpec, agent_cfg: dict[str, Any]) -> list[str] | None: + """Resolve the ACP adapter command: config override wins over the spec. + + A string override is parsed with shell quoting rules, so a quoted argument + containing spaces stays one argv element. An empty override means "no + adapter", the same as an agent without a default. + """ override = agent_cfg.get("acp_command") if override is None: default = spec.acp_command return list(default) if default is not None else None if isinstance(override, str): - return override.split() - return [str(part) for part in override] + parts = shlex.split(override) + else: + parts = [str(part) for part in override] + return parts or None + + +def _remove_container_quietly(container: Any) -> None: + """Force-remove *container*, tolerating one that is already gone.""" + try: + container.remove(force=True) + except Exception: + pass -def _acp_workspace_mount_path(workspace_path: Path, spec: Any) -> str: +def _acp_workspace_mount_path(workspace_path: PurePath, spec: AgentSpec) -> str: """Return the host path the workspace is bound to for ACP path parity. ACP clients (editors like Zed) send absolute host paths (session cwd, @@ -452,13 +469,14 @@ def run( be parsed as VibePod flags. """ passthrough_args = passthrough_args or [] + if acp: + # Must happen before any output, including the --detach conflict + # below: stdout carries only the ACP JSON-RPC stream, so all console + # output is rerouted to stderr. + route_to_stderr() if acp and detach: error("--acp cannot be combined with --detach: the ACP client owns the process lifetime.") raise typer.Exit(1) - if acp: - # Must happen before any output: stdout carries only the ACP - # JSON-RPC stream, so all console output is rerouted to stderr. - route_to_stderr() config = get_config() try: active_profile = resolve_profile(profile, config) @@ -513,6 +531,7 @@ def run( spec = get_agent_spec(selected_agent) acp_workspace_mount: str | None = None + acp_workspace_alias: str | None = None acp_command: list[str] | None = None if acp: acp_command = _resolve_acp_command(spec, agent_cfg) @@ -527,6 +546,12 @@ def run( ) raise typer.Exit(1) acp_workspace_mount = _acp_workspace_mount_path(workspace_path, spec) + # resolve() above followed symlinks, but the editor keeps sending the + # path as it was opened (macOS /tmp -> /private/tmp, ~/code -> another + # volume). Bind that spelling too so either form works in the container. + logical_workspace = Path(os.path.normpath(workspace.expanduser().absolute())) + if str(logical_workspace) != str(workspace_path): + acp_workspace_alias = _acp_workspace_mount_path(logical_workspace, spec) if spec.preview: warning( f"{selected_agent} is a developer preview; upstream warns of " @@ -651,7 +676,13 @@ def run( if init_commands: info(f"Applying {len(init_commands)} init command(s) before startup") try: - command = manager.resolve_launch_command(image=image, command=spec.command) + # The init wrapper replaces the image entrypoint, so the launch + # argv has to be made explicit. In ACP mode that argv is the + # adapter command, not the interactive one. + command = manager.resolve_launch_command( + image=image, + command=acp_command if acp else spec.command, + ) except DockerClientError as exc: error(str(exc)) raise typer.Exit(1) from exc @@ -706,6 +737,8 @@ def run( Path(host_path).mkdir(parents=True, exist_ok=True) extra_volumes.extend(_skills_mounts_for_agent(selected_agent, workspace_path)) + if acp_workspace_alias is not None: + extra_volumes.append((str(workspace_path), acp_workspace_alias, "rw")) herdr_volumes, herdr_env = _apply_herdr_if_enabled( selected_agent, @@ -791,6 +824,7 @@ def run( launch_labels["vibepod.profile"] = active_profile if proxy_policy_id is not None: launch_labels["vibepod.proxy-policy"] = proxy_policy_id + auto_remove = bool(config.get("auto_remove", True)) try: container = manager.run_agent( agent=selected_agent, @@ -800,7 +834,7 @@ def run( config_mount_path=spec.config_mount_path, env=merged_env, command=command, - auto_remove=bool(config.get("auto_remove", True)), + auto_remove=auto_remove, name=name, version=__version__, network=network_name, @@ -836,11 +870,11 @@ def run( _clear_herdr_metadata(selected_agent) raise typer.Exit(1) - def _finish_acp_launch() -> None: - """Start the container after the attach socket is open, then wire up - everything that needs a started container (extra network, proxy - attribution). Called from attach_stdio before the first byte flows.""" - container.start() + def _wire_started_container() -> None: + """Connect the extra network and record proxy attribution. + + Needs a running container: the IP is only known after start. + """ if extra_network and extra_network != network_name: try: manager.connect_network(container, extra_network) @@ -848,7 +882,6 @@ def _finish_acp_launch() -> None: except DockerClientError as exc: warning(str(exc)) if proxy_db_path is not None: - container.reload() container_ip = _get_container_ip(container, network_name) if container_ip: mapping_path = proxy_db_path.parent / "containers.json" @@ -867,6 +900,20 @@ def _finish_acp_launch() -> None: "Fix proxy directory permissions to restore container attribution.", ) + acp_started = False + + def _finish_acp_launch() -> None: + """Start the container once the attach socket is open, then wire it up. + + Called from attach_stdio before the first byte flows. + """ + nonlocal acp_started + container.start() + acp_started = True + # The pre-start inspect carries no network settings yet. + container.reload() + _wire_started_container() + if not acp: # Prefer the inspected bindings (they resolve ephemeral 0-port publishes to # the daemon-assigned port); fall back to the requested bindings when the @@ -876,32 +923,7 @@ def _finish_acp_launch() -> None: if web_url: success(f"{selected_agent} Web UI → {web_url}") info("Sessions persist in the agent config dir.") - - if extra_network and extra_network != network_name: - try: - manager.connect_network(container, extra_network) - info(f"Connected to additional network: {extra_network}") - except DockerClientError as exc: - warning(str(exc)) - - if proxy_db_path is not None: - container_ip = _get_container_ip(container, network_name) - if container_ip: - mapping_path = proxy_db_path.parent / "containers.json" - mapping_updated = _update_container_mapping( - mapping_path, - container_ip, - container.id, - container.name, - selected_agent, - policy_id=proxy_policy_id, - profile=active_profile, - ) - if not mapping_updated: - warning( - f"Could not write proxy container mapping at {mapping_path}. " - "Fix proxy directory permissions to restore container attribution.", - ) + _wire_started_container() if detach: if selected_agent == "claude" and "setup-token" in passthrough_args: @@ -935,13 +957,18 @@ def _finish_acp_launch() -> None: exit_reason = "normal" output_tail = b"" + exit_code = 0 if not acp: warning("Attached to container. Use Ctrl+C to stop.") try: if acp: # No SessionLogger frames here: JSON-RPC payloads embed whole file # contents, and the ACP client owns the transcript. - manager.attach_stdio(container, on_attached=_finish_acp_launch) + exit_code = manager.attach_stdio( + container, + on_attached=_finish_acp_launch, + auto_remove=auto_remove, + ) else: output_tail = manager.attach_interactive(container, logger=logger) except KeyboardInterrupt: @@ -951,6 +978,14 @@ def _finish_acp_launch() -> None: success("Stopped") except Exception: exit_reason = "error" + if acp: + # AutoRemove only fires on exit, so a container that never started + # (the attach failed) would otherwise linger in `created`; a + # started one has just lost its only client. Drop it either way, + # plus the launch policy of a container that never ran. + _remove_container_quietly(container) + if proxy_policy_id is not None and not acp_started: + remove_container_policy(config, proxy_policy_id) raise finally: logger.close_session(exit_reason) @@ -964,6 +999,10 @@ def _finish_acp_launch() -> None: if exit_reason == "normal" and not acp: show_resume_hint(selected_agent, output_tail) + if acp and exit_code != 0: + # Surface adapter failures to the ACP client instead of exiting 0. + raise typer.Exit(exit_code) + def _read_masked_line(prompt: str) -> str: """Read a line from stdin echoing '*' for each character. diff --git a/src/vibepod/core/docker.py b/src/vibepod/core/docker.py index 33b274a..13aaa14 100644 --- a/src/vibepod/core/docker.py +++ b/src/vibepod/core/docker.py @@ -462,7 +462,11 @@ def build_image( raise DockerClientError(f"Failed to build image {tag}: {chunk['error']}") stream = chunk.get("stream") if stream and stream.strip(): - print(stream, end="" if stream.endswith("\n") else "\n") + # Through the shared console so ACP mode's stderr routing + # keeps build logs off the JSON-RPC stream on stdout. + from vibepod.utils.console import console + + console.out(stream, end="" if stream.endswith("\n") else "\n", highlight=False) except DockerClientError: raise except (APIError, DockerException) as exc: @@ -1048,6 +1052,7 @@ def attach_stdio( container: Any, logger: Any = None, on_attached: Any = None, + auto_remove: bool = False, ) -> int: """Attach local stdin/stdout to a container without a TTY (ACP mode). @@ -1055,8 +1060,9 @@ def attach_stdio( resize handling and no ``logs`` replay; instead it demultiplexes the Docker stream frame protocol so container stdout goes to our stdout and stderr to our stderr, keeping the JSON-RPC stream clean. - Returns the container's exit code (0 when the container is stopped by - signal handling rather than observed exiting). + Returns the container's exit code, also when the container was + stopped by the SIGINT/SIGTERM handling. ``auto_remove`` must match the + container's AutoRemove setting so the exit code survives its removal. """ try: sock_wrapper = self.client.api.attach_socket( @@ -1072,6 +1078,29 @@ def attach_stdio( raise DockerClientError(f"Failed to attach to container: {exc}") from exc sock = getattr(sock_wrapper, "_sock", sock_wrapper) + + # Register the exit-code wait before the container starts. With + # AutoRemove the container can be gone by the time the stream closes, + # so inspecting it afterwards races the daemon; a wait opened up front + # is answered with the status code even once the container is removed + # (docker's own CLI waits for "removed" under --rm for the same reason). + wait_result: dict[str, Any] = {} + + def _wait_for_exit() -> None: + try: + result = self.client.api.wait( + container.id, + timeout=None, + condition="removed" if auto_remove else "next-exit", + ) + except Exception: + return + if isinstance(result, dict): + wait_result.update(result) + + waiter = threading.Thread(target=_wait_for_exit, name="vibepod-acp-wait", daemon=True) + waiter.start() + if on_attached is not None: # Create-attach-start ordering: the ACP adapter emits its first # JSON-RPC frames immediately once the entrypoint runs, so the @@ -1084,7 +1113,6 @@ def attach_stdio( except (OSError, ValueError): stdin_fd = None - exit_code = 0 stop_requested = False old_sigterm = None old_sigint = None @@ -1170,9 +1198,15 @@ def _request_stop(signum: int, frame: Any) -> None: if old_sigint is not None: signal.signal(signal.SIGINT, old_sigint) - try: - container.reload() - exit_code = int(container.attrs.get("State", {}).get("ExitCode", 0) or 0) - except Exception: # pragma: no cover - auto-removed containers - exit_code = 0 - return exit_code + waiter.join(timeout=10) + status = wait_result.get("StatusCode") + if status is None: + # No wait answer (daemon without wait conditions, or the stream + # ended without an exit): fall back to an inspect, which only + # works while the container still exists. + try: + container.reload() + status = container.attrs.get("State", {}).get("ExitCode", 0) + except Exception: # pragma: no cover - auto-removed containers + status = 0 + return int(status or 0) diff --git a/src/vibepod/utils/console.py b/src/vibepod/utils/console.py index dd0c7ee..b00d06f 100644 --- a/src/vibepod/utils/console.py +++ b/src/vibepod/utils/console.py @@ -11,12 +11,12 @@ def route_to_stderr() -> None: """Redirect all console output to stderr. Used by ACP mode: stdout must carry only the JSON-RPC stream, so every - info()/error() line is routed to stderr instead. Callers import the - info/error helpers (not ``console`` itself), so swapping the module-global - console reroutes output project-wide. + info()/error() line is routed to stderr instead. The shared ``console`` + object is flipped in place rather than rebound: several modules bind the + object itself at import time (``from vibepod.utils.console import + console``), and a rebound module global would never reach those. """ - global console - console = Console(stderr=True) + console.stderr = True def info(message: str) -> None: diff --git a/tests/conftest.py b/tests/conftest.py index 224749e..9b36ce4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,6 +15,22 @@ sys.path.insert(0, str(SRC)) +@pytest.fixture(autouse=True) +def _reset_console_routing(): + """Undo `route_to_stderr()` (flipped by `vp run --acp`) after each test. + + The console is one shared module object; without this, stdout assertions + in tests that run after an ACP test become order-dependent. + """ + from vibepod.utils import console as console_mod + + console = console_mod.console + was_stderr = console.stderr + yield + console_mod.console = console + console.stderr = was_stderr + + @pytest.fixture(autouse=True) def _no_ambient_herdr_env(monkeypatch): """Strip herdr pane env so tests are hermetic. diff --git a/tests/test_docker.py b/tests/test_docker.py index ffd6614..ece6539 100644 --- a/tests/test_docker.py +++ b/tests/test_docker.py @@ -182,8 +182,10 @@ def close(self) -> None: class _AttachClient: - def __init__(self, chunks: list[bytes]) -> None: + def __init__(self, chunks: list[bytes], wait_result: dict | None = None) -> None: self.wrapper = _FakeSocketWrapper(_FakeStreamSocket(chunks)) + self.wait_result = wait_result + self.wait_conditions: list[str | None] = [] @property def api(self): @@ -193,6 +195,12 @@ def attach_socket(self, _id: str, params: dict) -> _FakeSocketWrapper: assert params == {"stdin": 1, "stdout": 1, "stderr": 1, "stream": 1} return self.wrapper + def wait(self, _id: str, timeout: float | None = None, condition: str | None = None) -> dict: + self.wait_conditions.append(condition) + if self.wait_result is None: + raise RuntimeError("wait not scripted for this fake") + return dict(self.wait_result) + class _AttachContainer: id = "abc123" @@ -205,7 +213,14 @@ def stop(self, timeout: int = 0) -> None: pass -def _run_attach_stdio(monkeypatch, chunks: list[bytes]) -> tuple[bytes, bytes, int]: +def _run_attach_stdio( + monkeypatch, + chunks: list[bytes], + *, + client: _AttachClient | None = None, + container: Any = None, + auto_remove: bool = False, +) -> tuple[bytes, bytes, int]: import types from vibepod.core import docker as docker_mod @@ -221,7 +236,8 @@ def _no_fileno() -> int: fake_stdin.fileno = _no_fileno - client = _AttachClient(chunks) + if client is None: + client = _AttachClient(chunks) manager = object.__new__(DockerManager) manager.client = client # type: ignore[assignment] @@ -238,7 +254,10 @@ def _fake_select(readers, writ, exc): monkeypatch.setattr(docker_mod.sys, "stderr", fake_stderr) monkeypatch.setattr(docker_mod.sys, "stdin", fake_stdin) - exit_code = manager.attach_stdio(_AttachContainer()) + exit_code = manager.attach_stdio( + container if container is not None else _AttachContainer(), + auto_remove=auto_remove, + ) return out.getvalue(), err.getvalue(), exit_code @@ -272,6 +291,68 @@ def test_attach_stdio_preserves_null_bytes_and_routes_stderr(monkeypatch) -> Non assert err == b"" +def test_attach_stdio_takes_the_exit_code_from_a_wait_opened_before_start(monkeypatch) -> None: + """With AutoRemove the container is gone once the stream closes, so an + inspect afterwards cannot see the exit code; the wait opened up front can.""" + + class _GoneContainer(_AttachContainer): + def reload(self) -> None: + raise NotFound("auto-removed") + + client = _AttachClient([], wait_result={"StatusCode": 3}) + + _, _, exit_code = _run_attach_stdio( + monkeypatch, + [], + client=client, + container=_GoneContainer(), + auto_remove=True, + ) + + assert exit_code == 3 + assert client.wait_conditions == ["removed"] + + +def test_attach_stdio_waits_for_the_next_exit_without_auto_remove(monkeypatch) -> None: + client = _AttachClient([], wait_result={"StatusCode": 0}) + + _run_attach_stdio(monkeypatch, [], client=client) + + assert client.wait_conditions == ["next-exit"] + + +def test_attach_stdio_falls_back_to_inspect_without_a_wait_answer(monkeypatch) -> None: + class _ExitedContainer(_AttachContainer): + attrs = {"State": {"ExitCode": 5}} + + _, _, exit_code = _run_attach_stdio(monkeypatch, [], container=_ExitedContainer()) + + assert exit_code == 5 + + +def test_build_image_streams_output_through_the_console(capsys) -> None: + """ACP mode reroutes the console to stderr; build logs have to follow it.""" + from vibepod.utils.console import route_to_stderr + + class _BuildApi: + def build(self, **_kwargs): + return iter([{"stream": "Step 1/2 : FROM base\n"}, {"stream": " ---> abc\n"}]) + + class _BuildClient: + api = _BuildApi() + + manager = object.__new__(DockerManager) + manager.client = _BuildClient() # type: ignore[assignment] + route_to_stderr() + + manager.build_image(io.BytesIO(b""), "vibepod/overlay:test", {}) + + out, err = capsys.readouterr() + assert out == "" + assert "Step 1/2 : FROM base" in err + assert "---> abc" in err + + @pytest.fixture() def socket_dir() -> Iterator[Path]: """Short-path directory for binding AF_UNIX sockets. diff --git a/tests/test_run.py b/tests/test_run.py index dd4257b..c4eef5f 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -2749,6 +2749,7 @@ def __init__(self) -> None: self.status = "created" self.attrs: dict = {"NetworkSettings": {"Ports": {}}} self.started = False + self.removed: list[bool] = [] def reload(self) -> None: pass @@ -2760,6 +2761,9 @@ def start(self) -> None: def stop(self, timeout: int = 0) -> None: pass + def remove(self, force: bool = False) -> None: + self.removed.append(force) + def _make_acp_manager(captured: dict): class _AcpCapturingDockerManager: @@ -2777,10 +2781,19 @@ def resolve_launch_command(self, image: str, command: list[str] | None) -> list[ def run_agent(self, **kwargs) -> object: # type: ignore[no-untyped-def] captured.update(kwargs) - return _AcpContainer() + container = _AcpContainer() + captured["container"] = container + return container - def attach_stdio(self, container, logger=None, on_attached=None) -> int: # type: ignore[no-untyped-def] + def attach_stdio( # type: ignore[no-untyped-def] + self, + container, + logger=None, + on_attached=None, + auto_remove=False, + ) -> int: captured["attach_stdio_called"] = True + captured["attach_auto_remove"] = auto_remove if on_attached is not None: on_attached() captured["container_started_by_attach"] = container.started @@ -2835,6 +2848,8 @@ def test_acp_uses_acp_command_and_stdio_container(monkeypatch, _acp_env) -> None assert captured["workspace_mount_path"] == str(_acp_env) assert captured["attach_stdio_called"] is True assert captured["container_started_by_attach"] is True + # The attach needs the AutoRemove setting to pick the right wait condition. + assert captured["attach_auto_remove"] is True @_requires_posix_workspace @@ -2950,9 +2965,127 @@ def test_acp_routes_console_to_stderr(monkeypatch, _acp_env) -> None: captured: dict = {} monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) - original = console_mod.console - try: + + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + + # The conftest fixture flips the shared console back after the test. + assert console_mod.console.stderr is True + + +def test_acp_detach_conflict_is_reported_on_stderr(monkeypatch, _acp_env, capsys) -> None: + """Even the earliest --acp error must stay off the JSON-RPC stream.""" + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + + with pytest.raises(typer.Exit): + run_cmd.run(agent="claude", workspace=_acp_env, acp=True, detach=True) + + out, err = capsys.readouterr() + assert out == "" + assert "--detach" in err + + +@_requires_posix_workspace +def test_acp_init_commands_keep_the_acp_command(monkeypatch, _acp_env) -> None: + """The init wrapper needs an explicit argv; in ACP mode that is the adapter.""" + captured: dict = {} + config = _make_config() + config["agents"]["claude"]["init"] = ["echo hi"] + monkeypatch.setattr(run_cmd, "get_config", lambda: config) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + + assert captured["command"] == ["npx", "-y", "@agentclientprotocol/claude-agent-acp"] + assert captured["entrypoint"] == run_cmd._init_entrypoint(["echo hi"]) + + +@_requires_posix_workspace +def test_acp_adapter_exit_code_propagates(monkeypatch, _acp_env) -> None: + captured: dict = {} + + class _CrashingAdapterManager(_make_acp_manager(captured)): + def attach_stdio( # type: ignore[no-untyped-def] + self, + container, + logger=None, + on_attached=None, + auto_remove=False, + ) -> int: + if on_attached is not None: + on_attached() + return 7 + + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _CrashingAdapterManager) + + with pytest.raises(typer.Exit) as exc: run_cmd.run(agent="claude", workspace=_acp_env, acp=True) - assert console_mod.console.stderr is True - finally: - console_mod.console = original + + assert exc.value.exit_code == 7 + + +@_requires_posix_workspace +def test_acp_attach_failure_removes_the_created_container(monkeypatch, _acp_env) -> None: + """AutoRemove never fires for a container that was created but not started.""" + captured: dict = {} + + class _AttachFailsManager(_make_acp_manager(captured)): + def attach_stdio( # type: ignore[no-untyped-def] + self, + container, + logger=None, + on_attached=None, + auto_remove=False, + ) -> int: + raise DockerClientError("attach failed") + + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _AttachFailsManager) + + with pytest.raises(DockerClientError): + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + + container = captured["container"] + assert container.started is False + assert container.removed == [True] + + +@_requires_posix_workspace +def test_acp_mounts_the_unresolved_workspace_path_too(monkeypatch, _acp_env) -> None: + """The editor sends the path as opened, while resolve() follows symlinks.""" + captured: dict = {} + link = _acp_env.parent / "link" + link.symlink_to(_acp_env, target_is_directory=True) + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + run_cmd.run(agent="claude", workspace=link, acp=True) + + assert captured["workspace_mount_path"] == str(_acp_env) + assert (str(_acp_env), str(link), "rw") in captured["extra_volumes"] + + +@_requires_posix_workspace +def test_acp_skips_the_alias_mount_for_a_plain_path(monkeypatch, _acp_env) -> None: + captured: dict = {} + monkeypatch.setattr(run_cmd, "get_config", lambda: _make_config()) + monkeypatch.setattr(run_cmd, "DockerManager", _make_acp_manager(captured)) + + run_cmd.run(agent="claude", workspace=_acp_env, acp=True) + + assert all(bind != str(_acp_env) for _, bind, _ in captured["extra_volumes"]) + + +def test_resolve_acp_command_parses_string_overrides_like_a_shell() -> None: + spec = get_agent_spec("claude") + quoted = {"acp_command": "my-acp --config '/p/with space.json'"} + + assert run_cmd._resolve_acp_command(spec, quoted) == [ + "my-acp", + "--config", + "/p/with space.json", + ] + assert run_cmd._resolve_acp_command(spec, {}) == spec.acp_command + # An empty override is "no adapter", not "run the image default". + assert run_cmd._resolve_acp_command(spec, {"acp_command": ""}) is None + assert run_cmd._resolve_acp_command(spec, {"acp_command": []}) is None