diff --git a/README.md b/README.md index e00e0eb..9f1e5e7 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,10 @@ pier skills # install task skills for your agent - `--agent` installs a [supported coding agent](https://github.com/laude-institute/harbor) and registers skills from `skills_dir` in task.toml. Optional — without it you get a plain workspace. To install multiple agents, run `pier start --agent ` again from the workspace. - `--image` accepts any Docker image — a stock OS, a project image with tools pre-installed, etc. - `--no-mount` keeps files inside the container only (no bind-mount to host). `pier stop` copies files back. -- `-f` / `--force` allows starting in a non-empty directory. +- **`--delete`** stops the container (if any), removes the entire workspace directory at `-d`, and starts fresh — use when you want to discard a previous pier session there. +- **`--ae` / `--agent-env`**, **`-e`**, and **`--env-file`**: variables persisted for **`pier exec`** and agent CLIs run that way (e.g. API keys for Claude). Prefer **`--env-file`** for secrets so values stay off the shell argv. +- **`--ee` / `--environment-env`**: variables injected into the **Docker Compose service environment** when the container is created — use when something must see the var at **container startup** (entrypoint, install hooks, or any process that does not go through `pier exec`). Example: `DEBIAN_FRONTEND=noninteractive` for apt during image bring-up; use **`-e` / `--ae`** for keys only needed when you run the agent via **`pier exec`**. +- `--exec ""` runs a command in the container immediately after a successful container start (not supported with `--host`). ### Work in the workspace @@ -140,6 +143,9 @@ pier start https://github.com/org/repo#tasks/my-task # Manage existing workspace pier start --agent claude-code # install agent in current workspace pier start # restart a stopped container +pier start ./tasks/my-task -d ./ws --delete # remove prior session at ./ws, then start +pier start ./tasks/my-task -d ./ws --exec "claude --help" +pier start ./tasks/my-task -d ./my-workspace --env-file ./.env # secrets off argv (shell wrappers, CI) ``` - `task_path` can be a local directory or a remote git reference (`URL#path`). Optional — omit for task-free mode. @@ -148,9 +154,11 @@ pier start # restart a stopped container - `--ports` exposes container ports to the host (e.g., `--ports 8888`). - `--mounts-json` adds volume mounts as a JSON array (e.g., `--mounts-json '["./skills:/opt/skills:ro"]'`). - `-e` passes container-mode environment variables in `KEY=VALUE` format (repeatable). Stored in the session and forwarded on every `pier exec`. -- `--env-file` loads container-mode environment variables from a `.env` file. Same behavior as `-e` for each line. +- `--env-file` loads container-mode environment variables from a `.env` file. Same behavior as `-e` for each line. Prefer this over many `-e` / `--ae` flags when forwarding secrets so values are not visible on the process command line. - `--no-mount` keeps files inside the container only (no bind-mount to host). `pier stop` copies files back. -- `-f` / `--force` allows starting in a non-empty directory. +- **`--delete`** removes any existing session at `-d` (stop container, delete workspace tree) before creating a new one. +- **`-e` / `--env-file` / `--ae` / `--agent-env`**: exec-time / session env (forwarded on `pier exec`; `--ae` overrides host `*_API_KEY` for the same key). **`--ee` / `--environment-env`**: compose service env at **container start** — use when the variable must exist before or outside `pier exec` (see quick start above). +- `--exec` runs one command in the container right after start (container mode only). - `--host` skips the container (workspace only). - `--agent` installs a coding agent. To install additional agents, run `pier start --agent ` again from the workspace. When `task_path` is omitted, it operates on the current workspace. @@ -164,7 +172,7 @@ pier exec claude pier exec -d -- quarto preview --port 8888 --host 0.0.0.0 --no-browse ``` -- **Container mode**: delegates to `docker exec` in the container's working directory +- **Container mode**: delegates to `docker exec` in the container's working directory. Host `*_API_KEY` environment variables are forwarded unless the session has the same key from `--ae` / `--agent-env` (session wins). - **Host mode**: runs the command directly with `TASK_WORKSPACE` set - `-d` / `--detach`: runs in the background (useful for servers like quarto preview) @@ -183,6 +191,13 @@ pier verify Stop the Docker container for the current workspace (container mode only). The workspace directory is preserved. Restart later with `pier start` (no arguments, from inside the workspace). +```bash +pier stop # stop the current workspace's container +pier stop --all # stop every container-mode workspace pier knows about +``` + +If Harbor cannot tear down the environment (for example the task directory was removed), pier falls back to `docker rm -f` and clears the local session. + ### `pier list` Show active workspaces. diff --git a/pier/cli.py b/pier/cli.py index 36b9c94..6def090 100644 --- a/pier/cli.py +++ b/pier/cli.py @@ -22,6 +22,7 @@ import logging import os import re +import shlex import shutil import subprocess import tomllib @@ -265,6 +266,28 @@ def _validate_env_kv(entry: str, source: str) -> str: return f"{key}={value}" +def _validate_env_vars(env_list: tuple[str, ...], label: str) -> None: + for entry in env_list: + if "=" not in entry or entry.startswith("="): + raise click.ClickException( + f"Invalid {label} format: {entry!r}. Use KEY=VALUE." + ) + + +def _merge_env_lists( + old: list[str] | None, new: tuple[str, ...] | list[str] +) -> list[str]: + """Merge new env vars into old list, overriding by key.""" + merged: dict[str, str] = {} + for entry in old or []: + k, _, v = entry.partition("=") + merged[k] = v + for entry in new: + k, _, v = entry.partition("=") + merged[k] = v + return [f"{k}={v}" for k, v in merged.items()] + + def _resolve_restart_ports( requested_ports: tuple[int, ...] | list[int] | None, existing_sess: dict ) -> list[int]: @@ -416,6 +439,57 @@ def _resolve_task_path(task_path: str) -> Path: return task_dir +def _force_teardown(workspace: Path) -> None: + """Remove an existing workspace directory and stop its container (if any).""" + if _session_json_path(workspace).exists(): + try: + sess = _load_session(workspace) + if sess.get("mode") == "container": + hsid = _get_hsid(sess, workspace) + harbor_trial_dir = _harbor_trial_dir(workspace) + try: + harbor_bridge.stop_environment( + Path(sess["task_dir"]), + hsid, + harbor_trial_dir, + ) + click.echo( + f"Stopped existing container for {_workspace_label(workspace)!r}." + ) + except Exception as e: + click.echo( + f"Warning: could not stop container cleanly: {e}", + err=True, + ) + _force_remove_container(hsid, workspace) + except Exception as e: + click.echo(f"Warning: could not load session: {e}", err=True) + + index = _index_load() + ws_str = str(workspace.resolve()) + if ws_str in index: + index.remove(ws_str) + _index_save(index) + + if workspace.exists(): + shutil.rmtree(workspace) + click.echo(f"Removed existing workspace at {workspace}.") + + +def _force_remove_container(hsid: str, workspace: Path) -> None: + """Fallback: remove the container via Docker and clean up the session.""" + container = harbor_bridge.get_container_name(hsid) + subprocess.run(["docker", "rm", "-f", container], capture_output=True) + sp = _session_json_path(workspace) + if sp.exists(): + sp.unlink() + index = _index_load() + ws_str = str(workspace.resolve()) + if ws_str in index: + index.remove(ws_str) + _index_save(index) + + def _print_reward(reward: dict) -> None: """Print reward value and any extra details from a reward dict.""" click.echo(f"Reward: {reward.get('reward', 'N/A')}") @@ -541,11 +615,31 @@ def cli(): help="Don't bind-mount the workspace into the container. Files stay inside the container only.", ) @click.option( - "-f", - "--force", + "--delete", + "delete_workspace", is_flag=True, default=False, - help="Allow starting in a non-empty directory.", + help="Remove any existing pier session at -d (stop container, delete workspace tree) before starting.", +) +@click.option( + "--ae", + "--agent-env", + "agent_env", + multiple=True, + help="Agent/session env (KEY=VALUE). Persisted and passed to pier exec; overrides host *_API_KEY for the same name.", +) +@click.option( + "--ee", + "--environment-env", + "environment_env", + multiple=True, + help="Compose service env at container start (KEY=VALUE). For exec-time vars use -e, --env-file, or --ae.", +) +@click.option( + "--exec", + "exec_cmd_str", + default=None, + help="Run a command in the container after start (container mode only).", ) def start( task_path: str | None, @@ -558,7 +652,10 @@ def start( extra_env_cli: tuple[str, ...], env_file: str | None, no_mount: bool, - force: bool, + delete_workspace: bool, + agent_env: tuple[str, ...], + environment_env: tuple[str, ...], + exec_cmd_str: str | None, ) -> None: """Launch a workspace, or install an agent into an existing one. @@ -581,6 +678,13 @@ def start( """ extra_mounts = _parse_mounts_json(mounts_json) + if agent_env: + _validate_env_vars(agent_env, "--ae/--agent-env") + if environment_env: + _validate_env_vars(environment_env, "--ee/--environment-env") + if exec_cmd_str and host: + raise click.ClickException("--exec is not supported with --host.") + if no_mount and host: raise click.ClickException("--no-mount and --host are mutually exclusive.") if host and extra_env_cli: @@ -620,13 +724,27 @@ def start( extra_mounts=extra_mounts, extra_env=extra_env_list, no_mount=no_mount, - force=force, + delete_workspace=delete_workspace, + agent_env=list(agent_env), + environment_env=list(environment_env), ) + if exec_cmd_str: + sess, ws = _resolve_workspace(str(workspace)) + try: + _exec_container(sess, ws, shlex.split(exec_cmd_str)) + except SystemExit as e: + if e.code: + click.echo(f"--exec command exited with code {e.code}.", err=True) + raise return # No task_path, no image → operate on existing workspace from cwd if task_path is None: - _start_existing(agent=agent) + _start_existing( + agent=agent, + agent_env=agent_env, + environment_env=environment_env, + ) return if image: @@ -649,6 +767,10 @@ def start( "e.g. pier start ./tasks/my-task -d ./my-workspace" ) + workspace = workspace.resolve() + if delete_workspace and _session_json_path(workspace).exists(): + _force_teardown(workspace) + # Collision check — existing session in this workspace if _session_json_path(workspace).exists(): existing_sess = _load_session(workspace) @@ -658,7 +780,13 @@ def start( hsid = _get_hsid(existing_sess, workspace) if harbor_bridge.is_environment_running(hsid): if agent: - _install_agents_into_running(existing_sess, workspace, [agent]) + _install_agents_into_running( + existing_sess, + workspace, + [agent], + agent_env=agent_env, + environment_env=environment_env, + ) return click.echo("Container is already running.") return @@ -675,6 +803,18 @@ def start( resolved_extra_mounts = _resolve_restart_mounts( extra_mounts, existing_sess ) + merged_ae = ( + _merge_env_lists(existing_sess.get("agent_env"), agent_env) + if agent_env + else list(existing_sess.get("agent_env", [])) + ) + merged_ee = ( + _merge_env_lists( + existing_sess.get("environment_env"), environment_env + ) + if environment_env + else list(existing_sess.get("environment_env", [])) + ) _start_container( task_dir, workspace, @@ -683,6 +823,8 @@ def start( extra_mounts=resolved_extra_mounts, extra_env=extra_env_list or existing_sess.get("extra_env", []), no_mount=existing_sess.get("no_mount", False), + agent_env=merged_ae, + environment_env=merged_ee, ) return else: @@ -690,11 +832,8 @@ def start( click.echo(f"Workspace already exists at {workspace}.") return - # Safety check: don't write into a non-empty directory by accident. - if not force and workspace.exists() and any(workspace.iterdir()): - raise click.ClickException( - f"Directory {workspace} is not empty. Use -f/--force to proceed anyway." - ) + if workspace.exists() and any(workspace.iterdir()): + click.echo(f"Note: directory {workspace} is not empty.", err=True) workspace.mkdir(parents=True, exist_ok=True) # Seed workspace with the same files the container WORKDIR would have. @@ -714,10 +853,24 @@ def start( extra_mounts=extra_mounts, extra_env=extra_env_list, no_mount=no_mount, + agent_env=list(agent_env), + environment_env=list(environment_env), ) + if exec_cmd_str: + sess, ws = _resolve_workspace(str(workspace)) + try: + _exec_container(sess, ws, shlex.split(exec_cmd_str)) + except SystemExit as e: + if e.code: + click.echo(f"--exec command exited with code {e.code}.", err=True) + raise -def _start_existing(agent: str | None) -> None: +def _start_existing( + agent: str | None, + agent_env: tuple[str, ...] = (), + environment_env: tuple[str, ...] = (), +) -> None: """Operate on an existing workspace (no task_path given). Without --agent: restart a stopped container. @@ -751,6 +904,16 @@ def _start_existing(agent: str | None) -> None: if not harbor_bridge.is_environment_running(hsid): # Restart the stopped container (reinstalls all agents) agents = _add_agent(sess.get("agents", []), agent) + merged_ae = ( + _merge_env_lists(sess.get("agent_env"), agent_env) + if agent_env + else list(sess.get("agent_env", [])) + ) + merged_ee = ( + _merge_env_lists(sess.get("environment_env"), environment_env) + if environment_env + else list(sess.get("environment_env", [])) + ) _start_container( Path(sess["task_dir"]), ws, @@ -759,11 +922,19 @@ def _start_existing(agent: str | None) -> None: extra_mounts=_resolve_restart_mounts(None, sess), extra_env=sess.get("extra_env", []), no_mount=sess.get("no_mount", False), + agent_env=merged_ae, + environment_env=merged_ee, ) return if agent: - _install_agents_into_running(sess, ws, [agent]) + _install_agents_into_running( + sess, + ws, + [agent], + agent_env=agent_env, + environment_env=environment_env, + ) else: click.echo("Container is already running.") @@ -786,6 +957,32 @@ def _start_host(task_dir: Path, workspace: Path) -> None: click.echo("Task instruction is at .task/instruction.md.") +_AGENT_BINARY: dict[str, str] = { + "claude-code": "claude", + "codex": "codex", + "goose": "goose", +} + + +def _is_agent_installed(harbor_session_id: str, agent: str) -> bool: + """Probe the container to check if the agent binary is already on PATH.""" + binary = _AGENT_BINARY.get(agent) + if not binary: + return False + _, path_prefix = harbor_bridge.get_agent_exec_env(agent) + container = harbor_bridge.get_container_name(harbor_session_id) + if path_prefix: + cmd = ["docker", "exec", container, "sh", "-c", + f"export PATH={path_prefix}:$PATH && which {binary}"] + else: + cmd = ["docker", "exec", container, "which", binary] + try: + result = subprocess.run(cmd, capture_output=True, timeout=5) + return result.returncode == 0 + except Exception: + return False + + def _install_agent( task_dir: Path, harbor_session_id: str, @@ -795,10 +992,17 @@ def _install_agent( """Validate and install a Harbor agent into a running container.""" if not harbor_bridge.is_valid_agent(agent): raise click.ClickException(f"Unknown agent {agent!r}.") - click.echo(f"Installing {agent} in the container...") + already = _is_agent_installed(harbor_session_id, agent) + if already: + click.echo(f"{agent} already installed, skipping install step...") + else: + click.echo(f"Installing {agent} in the container...") try: - harbor_bridge.setup_agent(task_dir, harbor_session_id, trial_dir, agent) - click.echo(f"{agent} installed.") + harbor_bridge.setup_agent( + task_dir, harbor_session_id, trial_dir, agent, + skip_install=already, + ) + click.echo(f"{agent} ready.") except Exception as e: raise click.ClickException(f"Agent setup failed: {e}") @@ -807,6 +1011,9 @@ def _install_agents_into_running( sess: dict, workspace: Path, agents: list[str], + *, + agent_env: tuple[str, ...] = (), + environment_env: tuple[str, ...] = (), ) -> None: """Install one or more agents into an already-running container and update session.""" task_dir = Path(sess["task_dir"]) @@ -818,6 +1025,12 @@ def _install_agents_into_running( for agent in agents: current = _add_agent(current, agent) sess["agents"] = current + if agent_env: + sess["agent_env"] = _merge_env_lists(sess.get("agent_env"), agent_env) + if environment_env: + sess["environment_env"] = _merge_env_lists( + sess.get("environment_env"), environment_env + ) _save_session(workspace, sess) @@ -829,6 +1042,8 @@ def _start_container( extra_mounts: list[str] | None = None, extra_env: list[str] | None = None, no_mount: bool = False, + agent_env: list[str] | None = None, + environment_env: list[str] | None = None, ) -> None: hsid = _harbor_session_id(workspace) @@ -844,6 +1059,7 @@ def _start_container( workspace_dir=None if no_mount else workspace, ports=ports, extra_mounts=extra_mounts, + environment_env=environment_env or [], ) except Exception as e: raise click.ClickException(f"Failed to start container: {e}") @@ -879,21 +1095,23 @@ def _start_container( for agent in agents or []: _install_agent(task_dir, hsid, harbor_trial_dir, agent) - _save_session( - workspace, - { - "mode": "container", - "task_dir": str(task_dir), - "task_ref": task_dir.name, - "harbor_session_id": hsid, - "agents": agents or [], - "ports": ports or [], - "extra_mounts": extra_mounts or [], - "extra_env": extra_env or [], - "no_mount": no_mount, - "started_at": datetime.now(timezone.utc).isoformat(), - }, - ) + session_data: dict = { + "mode": "container", + "task_dir": str(task_dir), + "task_ref": task_dir.name, + "harbor_session_id": hsid, + "agents": agents or [], + "ports": ports or [], + "extra_mounts": extra_mounts or [], + "extra_env": extra_env or [], + "no_mount": no_mount, + "started_at": datetime.now(timezone.utc).isoformat(), + } + if agent_env: + session_data["agent_env"] = agent_env + if environment_env: + session_data["environment_env"] = environment_env + _save_session(workspace, session_data) click.echo(f"\nWorkspace {label!r} ready.") click.echo(f" cd {workspace}") @@ -918,7 +1136,9 @@ def _start_task_free( extra_mounts: list[str] | None = None, extra_env: list[str] | None = None, no_mount: bool = False, - force: bool = False, + delete_workspace: bool = False, + agent_env: list[str] | None = None, + environment_env: list[str] | None = None, ) -> None: """Start a task-free container from a base image. @@ -926,6 +1146,11 @@ def _start_task_free( so we can reuse Harbor's standard environment machinery instead of maintaining a separate code path. """ + ae_list = agent_env or [] + ee_list = environment_env or [] + + if delete_workspace and _session_json_path(workspace).exists(): + _force_teardown(workspace) # If workspace already has a session, handle like _start_existing if _session_json_path(workspace).exists(): @@ -934,7 +1159,13 @@ def _start_task_free( hsid = _get_hsid(existing_sess, workspace) if harbor_bridge.is_environment_running(hsid): if agents: - _install_agents_into_running(existing_sess, workspace, agents) + _install_agents_into_running( + existing_sess, + workspace, + agents, + agent_env=tuple(ae_list), + environment_env=tuple(ee_list), + ) else: click.echo("Container is already running.") return @@ -948,6 +1179,18 @@ def _start_task_free( resolved_extra_mounts = _resolve_restart_mounts( extra_mounts, existing_sess ) + merged_ae = ( + _merge_env_lists(existing_sess.get("agent_env"), tuple(ae_list)) + if ae_list + else list(existing_sess.get("agent_env", [])) + ) + merged_ee = ( + _merge_env_lists( + existing_sess.get("environment_env"), tuple(ee_list) + ) + if ee_list + else list(existing_sess.get("environment_env", [])) + ) _start_container( harbor_bridge.create_synthetic_task_dir( image, _pier_dir(workspace) @@ -958,17 +1201,16 @@ def _start_task_free( extra_mounts=resolved_extra_mounts, extra_env=extra_env or existing_sess.get("extra_env", []), no_mount=existing_sess.get("no_mount", False), + agent_env=merged_ae, + environment_env=merged_ee, ) sess = _load_session(workspace) sess["image"] = image _save_session(workspace, sess) return - # Safety check for non-empty directory. - if not force and workspace.exists() and any(workspace.iterdir()): - raise click.ClickException( - f"Directory {workspace} is not empty. Use -f/--force to proceed anyway." - ) + if workspace.exists() and any(workspace.iterdir()): + click.echo(f"Note: directory {workspace} is not empty.", err=True) workspace.mkdir(parents=True, exist_ok=True) # Create synthetic task so Harbor's environment machinery works @@ -985,6 +1227,8 @@ def _start_task_free( extra_mounts=extra_mounts, extra_env=extra_env, no_mount=no_mount, + agent_env=ae_list, + environment_env=ee_list, ) # Update session with task-free metadata (overwrite what _start_container wrote) @@ -1043,10 +1287,17 @@ def _exec_container( task_dir = Path(sess["task_dir"]) env: dict[str, str] = {} env.update(harbor_bridge.resolve_task_env(task_dir)) + for var, val in os.environ.items(): + if var.endswith("_API_KEY") and var not in env: + env[var] = val for kv in sess.get("extra_env", []): key, _, val = kv.partition("=") if key: env[key] = val + for entry in sess.get("agent_env", []): + k, _, v = entry.partition("=") + if k: + env[k] = v # Merge agent env vars and PATH prefixes. path_prefixes: list[str] = [] @@ -1297,16 +1548,46 @@ def _verify_host_in_container( type=click.Path(), help="Workspace directory.", ) -def stop(workspace_dir: str | None) -> None: +@click.option( + "--all", + "stop_all", + is_flag=True, + default=False, + help="Stop all active container-mode workspaces.", +) +def stop(workspace_dir: str | None, stop_all: bool) -> None: """Stop the container for a workspace.""" + if stop_all: + workspaces = _all_workspaces() + stopped = 0 + for s, ws in workspaces: + if s.get("mode") != "container": + continue + try: + _stop_one_workspace(s, ws) + click.echo(f"Stopped {_workspace_label(ws)!r}.") + stopped += 1 + except click.ClickException as e: + click.echo( + f"Warning: could not stop {_workspace_label(ws)!r}: {e}", + err=True, + ) + if stopped == 0: + click.echo("No container-mode workspaces to stop.") + return + sess, workspace = _resolve_workspace(workspace_dir) if sess.get("mode") != "container": raise click.ClickException("No container to stop (host-mode workspace).") + _stop_one_workspace(sess, workspace) + click.echo(f"Container for {_workspace_label(workspace)!r} stopped.") + + +def _stop_one_workspace(sess: dict, workspace: Path) -> None: + """Stop container for one workspace (no success message).""" if sess.get("no_mount"): - # Copy workspace files from container to host before stopping, - # excluding .pier/ to avoid overwriting session data. hsid = _get_hsid(sess, workspace) container = harbor_bridge.get_container_name(hsid) workdir = harbor_bridge.get_container_workdir(Path(sess["task_dir"])) @@ -1315,7 +1596,6 @@ def stop(workspace_dir: str | None) -> None: _tar_copy_from_container(container, workdir, workspace) _stop_container_env(sess, workspace) - click.echo(f"Container for {_workspace_label(workspace)!r} stopped.") def _stop_container_env(sess: dict, workspace: Path) -> None: @@ -1330,8 +1610,8 @@ def _stop_container_env(sess: dict, workspace: Path) -> None: hsid, harbor_trial_dir, ) - except Exception as e: - raise click.ClickException(f"Failed to stop container: {e}") + except Exception: + _force_remove_container(hsid, workspace) @cli.command("list") @@ -1693,6 +1973,23 @@ def view(path: str | None, port: str, bind_host: str) -> None: harbor_bridge.run_view_command(folder=pier_dir, port=port, host=bind_host) +@cli.command("patch-harbor") +def patch_harbor() -> None: + """Apply pier's patches to the installed harbor package. + + Re-run after upgrading harbor (uv tool upgrade harbor). + Patches are idempotent — safe to run multiple times. + """ + from pier.patches.apply import apply_all + + click.echo("Applying harbor patches...") + applied = apply_all(verbose=True) + if applied: + click.echo(f"Done — {applied} patch(es) applied.") + else: + click.echo("All patches already applied.") + + @cli.command() @click.argument("path", required=False) @click.option( diff --git a/pier/harbor_bridge.py b/pier/harbor_bridge.py index a8470f8..f96ffab 100644 --- a/pier/harbor_bridge.py +++ b/pier/harbor_bridge.py @@ -184,6 +184,7 @@ def _write_mounts_compose( include_bind_mount: bool = True, task_dir: Path | None = None, ports: list[int] | None = None, + environment_env: list[str] | None = None, ) -> Path: """Write a docker-compose override for workspace mounts. @@ -196,6 +197,7 @@ def _write_mounts_compose( Skills are handled by Harbor via skills_dir in task.toml. When ports is provided, exposes those container ports to the host. + When environment_env is provided, adds static environment entries to the service. """ service: dict = {"tmpfs": [f"{container_workdir}/.pier"]} volumes: list[str] = [] @@ -212,6 +214,12 @@ def _write_mounts_compose( service["volumes"] = volumes if ports: service["ports"] = [f"{p}:{p}" for p in ports] + if environment_env: + env_dict: dict[str, str] = {} + for entry in environment_env: + k, _, v = entry.partition("=") + env_dict[k] = v + service["environment"] = env_dict compose = {"services": {"main": service}} path = trial_dir / "docker-compose-pier.json" path.parent.mkdir(parents=True, exist_ok=True) @@ -226,6 +234,7 @@ def _make_environment( workspace_dir: Path | None = None, ports: list[int] | None = None, extra_mounts: list[str] | None = None, + environment_env: list[str] | None = None, ): """Reconstruct a Harbor Docker environment from pier session data. @@ -280,16 +289,18 @@ def _make_environment( include_bind_mount=False, task_dir=task_dir, ports=ports, + environment_env=environment_env, ) _patch_compose_paths(environment, mounts_path) - elif ports: - # No workspace mount, but still need ports exposed. + elif ports or environment_env: + # No workspace mount, but still need ports and/or compose env. mounts_path = _write_mounts_compose( trial_dir, workspace_dir or Path("/unused"), container_workdir, include_bind_mount=False, ports=ports, + environment_env=environment_env, ) _patch_compose_paths(environment, mounts_path) else: @@ -335,6 +346,7 @@ async def _async_start_environment( workspace_dir: Path | None = None, ports: list[int] | None = None, extra_mounts: list[str] | None = None, + environment_env: list[str] | None = None, ) -> None: environment, task, _ = _make_environment( task_dir, @@ -343,6 +355,7 @@ async def _async_start_environment( workspace_dir=workspace_dir, ports=ports, extra_mounts=extra_mounts, + environment_env=environment_env, ) await environment.start(force_build=False) @@ -389,12 +402,14 @@ def start_environment( workspace_dir: Path | None = None, ports: list[int] | None = None, extra_mounts: list[str] | None = None, + environment_env: list[str] | None = None, ) -> None: """Build (if needed), start a Harbor Docker environment. If workspace_dir is provided, it is bind-mounted into the container. If ports is provided, those container ports are exposed to the host. If extra_mounts is provided, they are added as volume mounts. + If environment_env is provided, those KEY=VALUE pairs are added to the compose service environment. """ with _placeholder_task_env_vars(task_dir): asyncio.run( @@ -405,6 +420,7 @@ def start_environment( workspace_dir=workspace_dir, ports=ports, extra_mounts=extra_mounts, + environment_env=environment_env, ) ) @@ -508,7 +524,7 @@ async def _run_interactive_setup( # Mark onboarding complete so `claude` doesn't prompt setup_parts.append( - f"echo '{{\"hasCompletedOnboarding\": true}}' > {config_dir}/.claude.json" + f'echo \'{{"hasCompletedOnboarding":true,"numStartups":1}}\' > {config_dir}/.claude.json' ) await environment.exec( # type: ignore[attr-defined] @@ -521,6 +537,8 @@ async def _async_setup_agent( harbor_session_id: str, trial_dir: Path, agent_name: str, + *, + skip_install: bool = False, ) -> None: from harbor.agents.factory import AgentFactory from harbor.models.agent.name import AgentName @@ -538,7 +556,8 @@ async def _async_setup_agent( agent = AgentFactory.create_agent_from_name( AgentName(agent_name), logs_dir=trial_paths.agent_dir, **extra_kwargs ) - await agent.setup(environment) + if not skip_install: + await agent.setup(environment) await _run_interactive_setup(agent, agent_name, environment) @@ -547,16 +566,25 @@ def setup_agent( harbor_session_id: str, trial_dir: Path, agent_name: str, + *, + skip_install: bool = False, ) -> None: """Install a Harbor agent in a running environment. Calls the agent's setup() method which uploads and runs the install script (e.g. install-claude-code.sh). Does NOT call run() — the user drives the agent interactively via ``pier exec``. + + When *skip_install* is True, only the interactive setup (onboarding + marker, skills, MCP servers) is executed — useful when the binary is + already present in the container image. """ with _placeholder_task_env_vars(task_dir): asyncio.run( - _async_setup_agent(task_dir, harbor_session_id, trial_dir, agent_name) + _async_setup_agent( + task_dir, harbor_session_id, trial_dir, agent_name, + skip_install=skip_install, + ) ) diff --git a/pier/patches/__init__.py b/pier/patches/__init__.py new file mode 100644 index 0000000..e1739e7 --- /dev/null +++ b/pier/patches/__init__.py @@ -0,0 +1 @@ +"""Harbor monkey-patches applied by ``pier patch-harbor``.""" diff --git a/pier/patches/apply.py b/pier/patches/apply.py new file mode 100644 index 0000000..3ec784c --- /dev/null +++ b/pier/patches/apply.py @@ -0,0 +1,33 @@ +"""Discover and apply all harbor patches.""" + +from __future__ import annotations + +from pier.patches import patch_skip_install, patch_trial_chown + +ALL_PATCHES = [ + ("trial-chown", patch_trial_chown), + ("skip-install", patch_skip_install), +] + + +def apply_all(*, verbose: bool = True) -> int: + """Apply all patches. Returns count of newly applied patches.""" + applied = 0 + for name, mod in ALL_PATCHES: + if mod.is_applied(): + if verbose: + print(f" {name}: already applied") + continue + result = mod.apply() + if result: + applied += 1 + if verbose: + if isinstance(result, list): + for path in result: + print(f" {name}: patched {path}") + else: + print(f" {name}: patched {result}") + else: + if verbose: + print(f" {name}: skipped (harbor not found)") + return applied diff --git a/pier/patches/patch_skip_install.py b/pier/patches/patch_skip_install.py new file mode 100644 index 0000000..71bd8a2 --- /dev/null +++ b/pier/patches/patch_skip_install.py @@ -0,0 +1,117 @@ +"""Skip agent install when the binary is already in the Docker image. + +Harbor's built-in agents (claude-code, codex, etc.) always run a full +apt-get + curl/npm install cycle in their install() method, even when +the Dockerfile already baked the agent in. This adds ~5 minutes of +redundant setup per trial. + +This patch adds an early-return guard to the install() method: if the +agent binary is already on PATH, skip the install. + +See: https://github.com/laude-institute/harbor/issues/1279 +""" + +from __future__ import annotations + +import subprocess +import sys + +MARKER = "PIER_SKIP_INSTALL_PATCH" + +_AGENTS = { + "claude_code": ("claude", 'export PATH="$HOME/.local/bin:$PATH"; which claude'), + "codex": ("codex", "which codex"), + "goose": ("goose", "which goose"), + "hermes": ("hermes", "which hermes"), +} + + +def _find_all_agent_py(module_name: str) -> list[str]: + """Find agent module in all uv tool venvs that have harbor installed.""" + from pathlib import Path + + paths: list[str] = [] + tools_dir = Path.home() / ".local" / "share" / "uv" / "tools" + interpreters = [] + if tools_dir.is_dir(): + for tool_dir in tools_dir.iterdir(): + python = tool_dir / "bin" / "python3" + if not python.exists(): + python = tool_dir / "bin" / "python" + if python.exists(): + interpreters.append(str(python)) + interpreters.append(sys.executable) + + for python in interpreters: + try: + result = subprocess.run( + [ + python, + "-c", + f"import harbor.agents.installed.{module_name}; " + f"print(harbor.agents.installed.{module_name}.__file__)", + ], + capture_output=True, + text=True, + timeout=10, + ) + p = result.stdout.strip() + if p and p not in paths: + paths.append(p) + except Exception: + continue + return paths + + +def is_applied() -> bool: + found_any = False + for module_name in _AGENTS: + for path in _find_all_agent_py(module_name): + found_any = True + if MARKER not in open(path).read(): + return False + return found_any + + +def apply() -> list[str]: + """Apply the patch. Returns list of patched file paths.""" + patched = [] + for module_name, (binary, check_cmd) in _AGENTS.items(): + for path in _find_all_agent_py(module_name): + src = open(path).read() + if MARKER in src: + continue + + guard = ( + f" # {MARKER}\n" + f" try:\n" + f" _check = await environment.exec(command={check_cmd!r})\n" + f" if _check.return_code == 0:\n" + f" return\n" + f" except Exception:\n" + f" pass\n" + ) + + target = " async def install(self, environment: BaseEnvironment) -> None:\n" + if target not in src: + continue + + lines = src.split("\n") + insert_idx = None + found_def = False + for i, line in enumerate(lines): + if "async def install(self, environment" in line: + found_def = True + continue + if found_def and line.strip() and not line.strip().startswith("#"): + insert_idx = i + break + + if insert_idx is None: + continue + + lines.insert(insert_idx, guard) + open(path, "w").write("\n".join(lines)) + patched.append(path) + + return patched diff --git a/pier/patches/patch_trial_chown.py b/pier/patches/patch_trial_chown.py new file mode 100644 index 0000000..7ea9292 --- /dev/null +++ b/pier/patches/patch_trial_chown.py @@ -0,0 +1,115 @@ +"""Fix agent log permissions before trajectory conversion. + +When Harbor runs agents in Docker with bind mounts, the agent (running as +root inside the container) creates session log files with 600 permissions. +Harbor's trajectory converter (populate_context_post_run) runs on the host +*before* Harbor's cleanup chown in stop(), so it hits "Permission denied" +reading those files and silently skips trajectory generation. + +This patch inserts a _maybe_fix_agent_log_permissions() call before each +_maybe_populate_agent_context() call in trial.py, calling _chown_to_host_user +on the agent logs directory while the container is still running. + +See: https://github.com/laude-institute/harbor/issues/178 +""" + +from __future__ import annotations + +MARKER = "PIER_CHOWN_PATCH" + + +def _find_all_trial_py() -> list[str]: + """Find trial.py in all uv tool venvs that have harbor installed.""" + import subprocess + import sys + from pathlib import Path + + paths = [] + tools_dir = Path.home() / ".local" / "share" / "uv" / "tools" + if tools_dir.is_dir(): + for tool_dir in tools_dir.iterdir(): + python = tool_dir / "bin" / "python3" + if not python.exists(): + python = tool_dir / "bin" / "python" + if not python.exists(): + continue + try: + result = subprocess.run( + [ + str(python), + "-c", + "import harbor.trial.trial; print(harbor.trial.trial.__file__)", + ], + capture_output=True, + text=True, + timeout=10, + ) + p = result.stdout.strip() + if p and p not in paths: + paths.append(p) + except Exception: + continue + + # Also check the current interpreter + try: + result = subprocess.run( + [ + sys.executable, + "-c", + "import harbor.trial.trial; print(harbor.trial.trial.__file__)", + ], + capture_output=True, + text=True, + timeout=10, + ) + p = result.stdout.strip() + if p and p not in paths: + paths.append(p) + except Exception: + pass + + return paths + + +def is_applied() -> bool: + paths = _find_all_trial_py() + if not paths: + return False + return all(MARKER in open(p).read() for p in paths) + + +def apply() -> list[str]: + """Apply the patch. Returns the patched file paths.""" + patched = [] + for path in _find_all_trial_py(): + src = open(path).read() + if MARKER in src: + continue + + helper = f"""\ + async def _maybe_fix_agent_log_permissions(self) -> None: # {MARKER} + \"\"\"Best-effort chown of agent logs so populate_context_post_run can read them.\"\"\" + if self._environment.is_mounted and hasattr(self._environment, '_chown_to_host_user'): + try: + await self._environment._chown_to_host_user( + str(EnvironmentPaths.agent_dir), recursive=True + ) + except Exception: + pass + + def _maybe_populate_agent_context(self) -> None:""" + + src = src.replace( + " def _maybe_populate_agent_context(self) -> None:", + helper, + 1, + ) + src = src.replace( + " self._maybe_populate_agent_context()", + " await self._maybe_fix_agent_log_permissions()\n self._maybe_populate_agent_context()", + ) + + open(path, "w").write(src) + patched.append(path) + + return patched diff --git a/pier/tests/test_agent_integration.py b/pier/tests/test_agent_integration.py index e27ccb2..ab6ca71 100644 --- a/pier/tests/test_agent_integration.py +++ b/pier/tests/test_agent_integration.py @@ -77,7 +77,6 @@ def test_task_free_agent_install_and_exec( TEST_IMAGE, "--agent", agent_name, - "-f", ], cwd=REPO_ROOT, ) diff --git a/pier/tests/test_pier_cli.py b/pier/tests/test_pier_cli.py index c96df13..1fdcd80 100644 --- a/pier/tests/test_pier_cli.py +++ b/pier/tests/test_pier_cli.py @@ -359,7 +359,7 @@ def test_start_with_agent_calls_setup( catch_exceptions=False, ) assert result.exit_code == 0, result.output - assert "claude-code installed" in result.output + assert "claude-code ready" in result.output mock_setup.assert_called_once() args = mock_setup.call_args[0] assert args[0] == task_dir # task_dir @@ -447,7 +447,7 @@ def test_start_agent_into_existing_session( catch_exceptions=False, ) assert result.exit_code == 0, result.output - assert "claude-code installed" in result.output + assert "claude-code ready" in result.output mock_setup.assert_called_once() session = json.loads((ws / ".pier" / "session.json").read_text()) @@ -506,7 +506,7 @@ def test_start_agent_from_cwd( catch_exceptions=False, ) assert result.exit_code == 0, result.output - assert "claude-code installed" in result.output + assert "claude-code ready" in result.output mock_setup.assert_called_once() session = json.loads((ws / ".pier" / "session.json").read_text()) assert session["agents"] == ["claude-code"] @@ -668,41 +668,27 @@ def test_start_host_existing_session_is_noop(runner, index_path, task_dir, tmp_p assert "already exists" in result.output.lower() -def test_start_nonempty_dir_blocked(runner, index_path, task_dir, tmp_path): - """Starting in a non-empty directory is blocked by default.""" - ws = tmp_path / "ws" - ws.mkdir() - (ws / "existing-file.txt").write_text("important") - result = runner.invoke(cli, ["start", str(task_dir), "--host", "-d", str(ws)]) - assert result.exit_code != 0 - assert "not empty" in result.output - - -def test_start_nonempty_dir_allowed_with_force(runner, index_path, task_dir, tmp_path): - """--force allows starting in a non-empty directory.""" +def test_start_nonempty_dir_warns(runner, index_path, task_dir, tmp_path): + """Starting in a non-empty directory succeeds with a warning.""" ws = tmp_path / "ws" ws.mkdir() (ws / "existing-file.txt").write_text("important") result = runner.invoke( cli, - ["start", str(task_dir), "--host", "-d", str(ws), "--force"], + ["start", str(task_dir), "--host", "-d", str(ws)], catch_exceptions=False, ) assert result.exit_code == 0 -def test_start_git_dir_needs_force(runner, index_path, task_dir, tmp_path): - """Directory with .git needs --force.""" +def test_start_git_dir_allowed(runner, index_path, task_dir, tmp_path): + """Directory with .git is allowed without any extra flag.""" ws = tmp_path / "ws" ws.mkdir() (ws / ".git").mkdir() - result = runner.invoke(cli, ["start", str(task_dir), "--host", "-d", str(ws)]) - assert result.exit_code != 0 - assert "not empty" in result.output - result = runner.invoke( cli, - ["start", str(task_dir), "--host", "-d", str(ws), "--force"], + ["start", str(task_dir), "--host", "-d", str(ws)], catch_exceptions=False, ) assert result.exit_code == 0 @@ -1327,6 +1313,124 @@ def test_stop_host_errors(runner, index_path, tmp_path): assert "host" in result.output.lower() +@patch("pier.harbor_bridge.stop_environment") +def test_stop_all(mock_stop, runner, index_path, tmp_path): + """pier stop --all stops all container-mode workspaces.""" + ws1 = tmp_path / "ws1" + ws1.mkdir() + _write_session(ws1, _container_session(harbor_session_id="pier-1"), index_path) + ws2 = tmp_path / "ws2" + ws2.mkdir() + _write_session(ws2, _container_session(harbor_session_id="pier-2"), index_path) + ws3 = tmp_path / "ws3" + ws3.mkdir() + _write_session(ws3, _host_session(), index_path) + + result = runner.invoke(cli, ["stop", "--all"], catch_exceptions=False) + assert result.exit_code == 0 + assert mock_stop.call_count == 2 + assert "Stopped" in result.output + + +def test_stop_all_no_workspaces(runner, index_path): + result = runner.invoke(cli, ["stop", "--all"], catch_exceptions=False) + assert result.exit_code == 0 + assert "No container-mode workspaces" in result.output + + +@patch("subprocess.run", return_value=MagicMock(returncode=0)) +@patch("pier.harbor_bridge.stop_environment", side_effect=FileNotFoundError("gone")) +@patch("pier.harbor_bridge.get_container_name", return_value="pier-ws-main-1") +def test_stop_fallback_removes_container( + mock_name, mock_stop, mock_run, runner, index_path, tmp_path +): + """When Harbor stop fails, fallback to docker rm -f and clean up session.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session(ws, _container_session(), index_path) + result = runner.invoke(cli, ["stop"], catch_exceptions=False) + assert result.exit_code == 0 + assert not (ws / ".pier" / "session.json").exists() + docker_args = mock_run.call_args[0][0] + assert "docker" in docker_args + assert "rm" in docker_args + + +# --------------------------------------------------------------------------- +# pier start --delete +# --------------------------------------------------------------------------- + + +@patch("pier.harbor_bridge.stop_environment") +@patch("pier.harbor_bridge.start_environment") +def test_start_delete_tears_down_existing( + mock_start, mock_stop, runner, index_path, task_dir, tmp_path +): + """pier start --delete removes existing workspace before starting.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session(ws, _container_session(), index_path) + (ws / "stale-file.txt").write_text("old") + + result = runner.invoke( + cli, + ["start", str(task_dir), "-d", str(ws), "--delete"], + catch_exceptions=False, + ) + assert result.exit_code == 0, result.output + assert "Removed existing workspace" in result.output + assert not (ws / "stale-file.txt").exists() + assert (ws / ".pier" / "session.json").exists() + + +# --------------------------------------------------------------------------- +# pier start --exec +# --------------------------------------------------------------------------- + + +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +@patch("pier.harbor_bridge.start_environment") +def test_start_exec_runs_command( + mock_start, mock_running, runner, index_path, task_dir, tmp_path +): + """pier start --exec runs a command in the container after start.""" + ws = tmp_path / "ws" + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + result = runner.invoke( + cli, + ["start", str(task_dir), "-d", str(ws), "--exec", "claude --help"], + catch_exceptions=False, + ) + assert result.exit_code == 0, result.output + docker_args = mock_run.call_args[0][0] + assert "claude" in docker_args + assert "--help" in docker_args + + +# --------------------------------------------------------------------------- +# pier exec --ae dedup +# --------------------------------------------------------------------------- + + +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_exec_agent_env_overrides_host_api_key( + mock_running, runner, index_path, tmp_path, monkeypatch +): + """Session agent_env overrides host API key of the same name.""" + ws = tmp_path / "ws" + ws.mkdir() + sess = _container_session() + sess["agent_env"] = ["ANTHROPIC_API_KEY=from-session"] + _write_session(ws, sess, index_path) + monkeypatch.setenv("ANTHROPIC_API_KEY", "from-host") + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + runner.invoke(cli, ["exec", "bash"]) + args = mock_run.call_args[0][0] + # Session value should win; host value should not appear + assert "ANTHROPIC_API_KEY=from-session" in args + assert "ANTHROPIC_API_KEY=from-host" not in args + + # --------------------------------------------------------------------------- # pier list # --------------------------------------------------------------------------- @@ -2297,7 +2401,7 @@ def test_start_task_free_with_agent( catch_exceptions=False, ) assert result.exit_code == 0, result.output - assert "claude-code installed" in result.output + assert "claude-code ready" in result.output mock_setup.assert_called_once() session = json.loads((ws / ".pier" / "session.json").read_text()) @@ -2437,7 +2541,6 @@ def test_start_no_mount_copies_workspace_to_container( "--image", "ubuntu:24.04", "--no-mount", - "--force", ], catch_exceptions=False, ) @@ -2468,7 +2571,6 @@ def test_start_no_mount_sets_git_safe_directory( "--image", "ubuntu:24.04", "--no-mount", - "--force", ], catch_exceptions=False, ) @@ -2500,7 +2602,6 @@ def test_start_no_mount_no_git_safe_directory_without_git( "--image", "ubuntu:24.04", "--no-mount", - "--force", ], catch_exceptions=False, ) @@ -2619,7 +2720,7 @@ def test_start_task_free_already_running_installs_agent( catch_exceptions=False, ) assert result.exit_code == 0 - assert "claude-code installed" in result.output + assert "claude-code ready" in result.output mock_setup.assert_called_once() @@ -2828,7 +2929,6 @@ def test_start_no_mount_copy_failure_errors( "--image", "ubuntu:24.04", "--no-mount", - "--force", ], )