From 36f8a0ef41e9838d1b3b10d363577169b21e4c5f Mon Sep 17 00:00:00 2001 From: Jonathan Bragg Date: Fri, 10 Apr 2026 09:21:50 -0700 Subject: [PATCH] Prevent agent log loss during pier exec --- DEVELOPER.md | 110 +++++++++++++ README.md | 36 +---- pier/cli.py | 152 ++++++++++++++++-- pier/harbor_bridge.py | 193 ++++++++++++++++++++-- pier/tests/test_agent_integration.py | 19 +++ pier/tests/test_docker_integration.py | 43 +++++ pier/tests/test_harbor_bridge.py | 198 ++++++++++++++++++++++- pier/tests/test_pier_cli.py | 220 +++++++++++++++++++++++++- 8 files changed, 902 insertions(+), 69 deletions(-) create mode 100644 DEVELOPER.md diff --git a/DEVELOPER.md b/DEVELOPER.md new file mode 100644 index 0000000..3a6fb97 --- /dev/null +++ b/DEVELOPER.md @@ -0,0 +1,110 @@ +# Developer Guide + +## Setup + +```bash +git clone https://github.com/allenai/pier && cd pier +make check # run tests, lint, and typecheck +uv run pre-commit install # optional: auto-lint and format on commit +uv tool install -e . # editable install — use `pier` from any directory +``` + +## Testing + +```bash +uv run --extra dev pytest -rs # fast — skips Docker agent tests +``` + +Docker-backed agent integration suite: + +```bash +PIER_RUN_DOCKER_INTEGRATION=1 uv run --extra dev pytest -rs pier/tests/test_agent_integration.py +PIER_RUN_DOCKER_INTEGRATION=1 PIER_TEST_AGENTS=codex uv run --extra dev pytest -rs pier/tests/test_agent_integration.py +PIER_RUN_DOCKER_INTEGRATION=1 PIER_TEST_IMAGE=ubuntu:24.04 uv run --extra dev pytest -rs pier/tests/test_agent_integration.py +``` + +## Agent Log Capture + +When agents run inside containers via `pier exec`, their output and session +logs need to survive container teardown. Harbor mounts +`workspace/.pier/_harbor/agent/` → `/logs/agent` in the container, so +anything written there persists on the host. + +### Two capture mechanisms + +**Session recording** — `pier exec` auto-detects agent commands by matching +the command name against Harbor's agent registry (`get_binary_agent_map()`). +When an agent is detected, the command is wrapped with `script -q -c` to +record terminal output to `/logs/agent/exec//.txt` while preserving +full TTY behavior (colors, cursor, interactive prompts). + +**Config-dir env vars** — `CLAUDE_CONFIG_DIR` and `CODEX_HOME` are set on +every container exec, pointing into the per-session directory. These agents +write structured session logs (JSONL, session dirs) that survive container +teardown. User `-e` overrides win via `setdefault`. + +### Per-session isolation + +Every `pier exec` creates a timestamped directory directly under +the agent log dir: + +``` +workspace/.pier/_harbor/agent/ + exec/ + 2026-04-09_18-30-00-a1b2c3/ # pier exec claude ... + claude-code.txt # session recording + sessions/projects/hash/ # Claude JSONL (via CLAUDE_CONFIG_DIR) + 2026-04-09_18-35-00-d4e5f6/ # pier exec goose ... + goose.txt # session recording + 2026-04-09_18-40-00-g7h8i9/ # pier exec bash + sessions/projects/hash/ # Claude JSONL if run inside bash + setup/ # Harbor agent install logs (not pier-managed) +``` + +Each session directory mirrors Harbor's flat `/logs/agent/` layout. When +`pier verify` or `pier capture` extracts trajectory, +`_latest_session_dir()` (called by `extract_agent_context()`) scans by +reverse timestamp to find the most recent directory containing the +requested agent's files. + +### What pier hardcodes vs derives from Harbor + +| Knowledge | Source | Location | +|-----------|--------|----------| +| CLI binary names (claude, codex, goose...) | Harbor's `get_version_command()` | `get_binary_agent_map()` | +| Log-dir env vars (CLAUDE_CONFIG_DIR, CODEX_HOME) | Harbor's `EnvironmentPaths` | `get_log_capture_env()` | +| Post-run artifact commands (gemini, hermes) | Hardcoded from Harbor's `run()` | `get_post_run_commands()` | +| Behavior env vars (IS_SANDBOX, PATH prefixes) | Hardcoded per-agent | `get_agent_exec_env()` | +| Session recording filename | Convention: `.txt` | `_exec_container()` | + +Binary names and log-dir values are derived from Harbor at runtime. +The rest is hardcoded in `harbor_bridge.py` — all grouped under the +"Agent log capture" section. When Harbor exposes APIs for +`get_log_env_vars()`, `get_post_run_commands()`, and `get_cli_binary()`, +these can be replaced. + +### Post-run artifact collection + +Some agents produce structured artifacts beyond raw output. Harbor's +`run()` collects these after the agent exits: + +- **gemini-cli**: copies `~/.gemini/tmp/session-*.json` → `gemini-cli.trajectory.json` +- **hermes**: runs `hermes sessions export` → `hermes-session.jsonl` + +Pier replicates these via `get_post_run_commands()`, run after the agent +exits but while the container is still up. If a new Harbor agent adds +post-run steps, add them there. + +### Multiple sessions + +`pier verify` and `pier capture` auto-find the most recent session +for the requested agent. Older sessions are preserved on disk and can be +selected with `--session ` on verify/capture. + +### Limitations + +- **`pier exec bash` + manual agent** — only config-dir agents + (Claude, Codex) get structured logs captured. Other agents need the + `script` wrapper, which only applies when pier detects the agent command. +- **Agents with complex entry points** (openhands, swe-agent) aren't + auto-detected — their binary is `python`/`pip`, not a unique name. diff --git a/README.md b/README.md index 9fad81a..d6a8d30 100644 --- a/README.md +++ b/README.md @@ -160,11 +160,11 @@ Run a command in the workspace context. Sets workspace env vars so task CLIs fin ```bash pier exec bash -pier exec claude +pier exec claude # agent auto-detected → full output captured 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. Agent commands are auto-detected and their output is recorded via `script`. Each exec gets its own timestamped directory so sessions don't overwrite each other. `CLAUDE_CONFIG_DIR` and `CODEX_HOME` are always set so structured session logs land in the mounted volume. - **Host mode**: runs the command directly with `TASK_WORKSPACE` set - `-d` / `--detach`: runs in the background (useful for servers like quarto preview) @@ -223,34 +223,4 @@ Install task skills for your coding agent (host mode only). Reads `skills_dir` f ## Development -### Architecture - -Pier stays **agent-agnostic** — it should not need changes when Harbor adds a new agent. Agent-specific details (session layouts, env vars, detection heuristics) belong in Harbor, not pier. `pier/harbor_bridge.py` is the thin boundary between the two; when it must contain agent-specific code, it should be treated as temporary scaffolding until Harbor exposes the right API. `pier/cli.py` should never reference individual agent names. - -```bash -git clone https://github.com/allenai/pier && cd pier -make check # run tests, lint, and typecheck -uv run pre-commit install # optional: auto-lint and format on commit -uv tool install -e . # editable install — use `pier` from any directory -``` - -### Testing - -Default test runs stay fast and skip Docker-backed agent install smoke tests: - -```bash -uv run --extra dev pytest -rs -``` - -Run the Docker-backed agent integration suite explicitly: - -```bash -PIER_RUN_DOCKER_INTEGRATION=1 uv run --extra dev pytest -rs pier/tests/test_agent_integration.py -``` - -Optional selectors for the integration suite: - -```bash -PIER_RUN_DOCKER_INTEGRATION=1 PIER_TEST_AGENTS=codex uv run --extra dev pytest -rs pier/tests/test_agent_integration.py -PIER_RUN_DOCKER_INTEGRATION=1 PIER_TEST_IMAGE=ubuntu:24.04 uv run --extra dev pytest -rs pier/tests/test_agent_integration.py -``` +See [DEVELOPER.md](DEVELOPER.md) for setup, testing, and architecture. diff --git a/pier/cli.py b/pier/cli.py index 534dae4..2d8e581 100644 --- a/pier/cli.py +++ b/pier/cli.py @@ -18,13 +18,14 @@ import hashlib import json -import tempfile import logging import os import re import shutil import subprocess +import tempfile import tomllib +import uuid from datetime import datetime, timezone from pathlib import Path @@ -1058,6 +1059,9 @@ def exec_cmd(detach: bool, command: tuple[str, ...]) -> None: Container mode uses docker exec (streaming output). Host mode runs the command directly with workspace env vars set. + Agent commands (claude, codex, goose, etc.) are auto-detected and + their output is captured to /logs/agent/ for trajectory extraction. + \b Examples: pier exec bash @@ -1075,8 +1079,41 @@ def exec_cmd(detach: bool, command: tuple[str, ...]) -> None: _exec_container(sess, ws, list(command), detach=detach) +def _validate_session_flags( + sess: dict | None, session: str | None, session_dir: str | None +) -> None: + """Validate --session and --session-dir flags.""" + if session and session_dir: + raise click.ClickException( + "--session and --session-dir are mutually exclusive." + ) + if session and (not sess or sess.get("mode") != "container"): + raise click.ClickException("--session is for container mode only.") + if session and ("/" in session or "\\" in session or ".." in session): + raise click.ClickException(f"Invalid session timestamp: {session!r}") + + +def _detect_agent_from_command(command: list[str]) -> str | None: + """Match a command against all known Harbor agent binaries. + + Uses Harbor's agent registry to build a binary→agent map (cached + after first call). Returns the agent name or None. + """ + if not command: + return None + try: + cmd_basename = Path(command[0]).name + return harbor_bridge.get_binary_agent_map().get(cmd_basename) + except Exception: + return None + + def _exec_container( - sess: dict, workspace: Path, command: list[str], *, detach: bool = False + sess: dict, + workspace: Path, + command: list[str], + *, + detach: bool = False, ) -> None: hsid = _get_hsid(sess, workspace) if not harbor_bridge.is_environment_running(hsid): @@ -1095,14 +1132,35 @@ def _exec_container( if key: env[key] = val - # Merge agent env vars and PATH prefixes. + # Apply agent env vars and PATH prefixes for registered agents + # and the auto-detected agent (if not already registered). + active_agent = _detect_agent_from_command(command) + agent_names = list(sess.get("agents", [])) + if active_agent and active_agent not in agent_names: + agent_names.append(active_agent) + path_prefixes: list[str] = [] - for agent_name in sess.get("agents", []): + for agent_name in agent_names: agent_env, pfx = harbor_bridge.get_agent_exec_env(agent_name) env.update(agent_env) - if pfx: + if pfx and pfx not in path_prefixes: path_prefixes.append(pfx) + # Per-session log isolation: each pier exec gets a timestamped directory + # under the agent log dir so sessions don't overwrite each other. + session_name = ( + datetime.now(timezone.utc).strftime("%Y-%m-%d_%H-%M-%S") + + f"-{uuid.uuid4().hex[:6]}" + ) + host_session_dir = _harbor_trial_dir(workspace) / "agent" / "exec" / session_name + host_session_dir.mkdir(parents=True, exist_ok=True) + + container_session_dir = f"/logs/agent/exec/{session_name}" + log_path = f"{container_session_dir}/{active_agent}.txt" if active_agent else None + + for k, v in harbor_bridge.get_log_capture_env(container_session_dir).items(): + env.setdefault(k, v) + rc = harbor_bridge.exec_in_container( hsid, task_dir, @@ -1110,7 +1168,17 @@ def _exec_container( env=env, path_prefix=":".join(path_prefixes), detach=detach, + log_path=log_path, ) + + # Run post-run artifact collection (e.g. gemini trajectory copy, + # hermes session export) while the container is still running. + if active_agent and not detach and rc == 0: + for post_cmd in harbor_bridge.get_post_run_commands( + active_agent, container_session_dir + ): + harbor_bridge.exec_in_container(hsid, task_dir, ["sh", "-c", post_cmd]) + raise SystemExit(rc) @@ -1141,10 +1209,16 @@ def _exec_host(sess: dict, workspace: Path, command: list[str]) -> None: default=None, help="Agent session/log directory (container path in container mode, host path otherwise).", ) +@click.option( + "--session", + default=None, + help="Session timestamp to use (from pier exec). Defaults to latest.", +) def verify( trial_dir: str | None, agent: str | None, session_dir: str | None, + session: str | None, ) -> None: """Run the verifier for the current workspace. @@ -1164,6 +1238,8 @@ def verify( ) raise click.ClickException(msg) + _validate_session_flags(sess, session, session_dir) + # Default agent from session if not explicitly provided if not agent: agents = sess.get("agents", []) @@ -1201,6 +1277,7 @@ def verify( trial_dir=trial_dir, agent=agent, session_dir=session_dir, + session=session, ) @@ -1225,6 +1302,7 @@ def _verify_container( trial_dir: str | None = None, agent: str | None = None, session_dir: str | None = None, + session: str | None = None, ) -> None: hsid = _get_hsid(sess, workspace) @@ -1265,10 +1343,30 @@ def _verify_container( # reimplementing its detection logic in pier. container_agent_context = None if agent and not session_dir: - click.echo(f"Extracting {agent} trajectory from container logs.") - container_agent_context = harbor_bridge.extract_agent_context( - agent, harbor_td / "agent" - ) + agent_dir = harbor_td / "agent" + if session: + # Explicit --session: use that specific timestamp dir. + session_path = agent_dir / "exec" / session + if not session_path.is_dir(): + raise click.ClickException( + f"Session {session!r} not found in {agent_dir}." + ) + click.echo(f"Extracting {agent} trajectory ({session}).") + container_agent_context = harbor_bridge.extract_agent_context( + agent, session_path + ) + else: + sessions = harbor_bridge.get_agent_session_dirs(agent_dir, agent) + ts = f" ({sessions[0].name})" if sessions else "" + click.echo(f"Extracting {agent} trajectory{ts}.") + if len(sessions) > 1: + click.echo( + f" (using latest of {len(sessions)} sessions" + " — pass --session to select)" + ) + container_agent_context = harbor_bridge.extract_agent_context( + agent, agent_dir + ) if not container_agent_context: click.echo( "Warning: trajectory extraction returned no data. " @@ -1547,7 +1645,12 @@ def _install_skills_from_dir(skills_dir: Path, workspace: Path) -> None: default=None, help="Agent session/log directory (container path in container mode, host path otherwise).", ) -def capture(agent: str | None, session_dir: str | None) -> None: +@click.option( + "--session", + default=None, + help="Session timestamp to use (from pier exec). Defaults to latest.", +) +def capture(agent: str | None, session_dir: str | None, session: str | None) -> None: """Capture the agent's trajectory for the current workspace. In container mode, the session directory is detected automatically. @@ -1567,6 +1670,8 @@ def capture(agent: str | None, session_dir: str | None) -> None: sess = None ws = Path.cwd().resolve() + _validate_session_flags(sess, session, session_dir) + # Discover session directory resolved_session_dir: Path | None = None container_agent_context = None @@ -1604,10 +1709,29 @@ def capture(agent: str | None, session_dir: str | None) -> None: ) if agent: harbor_td = _harbor_trial_dir(ws) - click.echo(f"Extracting {agent} trajectory from container logs.") - container_agent_context = harbor_bridge.extract_agent_context( - agent, harbor_td / "agent" - ) + agent_dir = harbor_td / "agent" + if session: + session_path = agent_dir / "exec" / session + if not session_path.is_dir(): + raise click.ClickException( + f"Session {session!r} not found in {agent_dir}." + ) + click.echo(f"Extracting {agent} trajectory ({session}).") + container_agent_context = harbor_bridge.extract_agent_context( + agent, session_path + ) + else: + sessions = harbor_bridge.get_agent_session_dirs(agent_dir, agent) + ts = f" ({sessions[0].name})" if sessions else "" + click.echo(f"Extracting {agent} trajectory{ts}.") + if len(sessions) > 1: + click.echo( + f" (using latest of {len(sessions)} sessions" + " — pass --session to select)" + ) + container_agent_context = harbor_bridge.extract_agent_context( + agent, agent_dir + ) if not container_agent_context: raise click.ClickException( "Could not extract agent session from container.\n" diff --git a/pier/harbor_bridge.py b/pier/harbor_bridge.py index 309ac9c..19c6ac0 100644 --- a/pier/harbor_bridge.py +++ b/pier/harbor_bridge.py @@ -750,25 +750,114 @@ def _bridge_claude_code(session_dir: Path, logs_dir: Path) -> None: } +_CONFIG_DIR_AGENTS = {"claude-code", "codex"} + + +def get_agent_session_dirs(harbor_agent_dir: Path, agent_name: str) -> list[Path]: + """Return session directories containing files for *agent_name*, newest first. + + Scans ``harbor_agent_dir/exec/`` for timestamped directories that + contain the agent's output (``.txt``). For config-dir + agents (claude-code, codex), a ``sessions/`` subdirectory also + counts as a match. + """ + exec_dir = harbor_agent_dir / "exec" + if not exec_dir.is_dir(): + return [] + + def _matches(d: Path) -> bool: + if (d / f"{agent_name}.txt").exists(): + return True + if agent_name in _CONFIG_DIR_AGENTS and (d / "sessions").is_dir(): + return True + return False + + return [ + d + for d in sorted(exec_dir.iterdir(), key=lambda d: d.name, reverse=True) + if d.is_dir() and _matches(d) + ] + + +def _latest_session_dir(harbor_agent_dir: Path, agent_name: str) -> Path | None: + """Return the most recent session directory for *agent_name*, or None.""" + dirs = get_agent_session_dirs(harbor_agent_dir, agent_name) + return dirs[0] if dirs else None + + +# --------------------------------------------------------------------------- +# Agent log capture — pier-side knowledge of Harbor agent internals +# +# Harbor's run() handles env vars, output tee, and post-run artifact +# collection. Pier doesn't call run() — agents are driven interactively +# — so these functions replicate the parts needed for log persistence. +# +# TODO: Replace when Harbor exposes APIs for get_log_env_vars(), +# get_post_run_commands(), and get_cli_binary(). +# --------------------------------------------------------------------------- + + +def get_log_capture_env(base_dir: str | None = None) -> dict[str, str]: + """Env vars that direct agent session logs to the mounted volume. + + Only claude-code and codex use config-dir env vars for log routing; + all other agents are covered by tee wrapping. When *base_dir* is + provided (e.g. ``/logs/agent/``), the default agent_dir + prefix is replaced so logs land in the per-run directory. + """ + from harbor.models.trial.paths import EnvironmentPaths + + agent_dir = str(EnvironmentPaths.agent_dir) + env = { + "CLAUDE_CONFIG_DIR": _claude_config_dir(), + "CODEX_HOME": _codex_home_dir(), + } + if base_dir: + env = {k: v.replace(agent_dir, base_dir) for k, v in env.items()} + return env + + +def get_post_run_commands(agent_name: str, log_dir: str) -> list[str]: + """Shell commands to collect agent artifacts after an interactive run. + + Replicates the post-run steps from Harbor's ``run()`` methods that + copy or export structured session data to the log directory. + """ + import shlex + + safe_dir = shlex.quote(log_dir) + if agent_name == "gemini-cli": + return [ + "find ~/.gemini/tmp -type f -name 'session-*.json' 2>/dev/null | " + f"head -n 1 | xargs -r -I{{}} cp {{}} {safe_dir}/gemini-cli.trajectory.json" + ] + if agent_name == "hermes": + return [ + 'export PATH="$HOME/.local/bin:$PATH" && ' + f"hermes sessions export {safe_dir}/hermes-session.jsonl " + "--source cli 2>/dev/null || true" + ] + return [] + + def get_agent_exec_env(agent_name: str) -> tuple[dict[str, str], str]: """Return env vars and PATH prefix needed to run an agent interactively. Returns ``(env_dict, path_prefix)`` where *path_prefix* is a string like ``"$HOME/.local/bin"`` or empty. - TODO: Harbor should expose an API for this so pier doesn't hardcode - agent-specific env vars. + Log-dir env vars (CLAUDE_CONFIG_DIR, CODEX_HOME) are set unconditionally + by :func:`get_log_capture_env` — this function only adds agent-specific + behavior flags and PATH prefixes. """ env: dict[str, str] = {} path_prefix = "" if agent_name == "claude-code": - env["CLAUDE_CONFIG_DIR"] = _claude_config_dir() env["IS_SANDBOX"] = "1" env["CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC"] = "1" path_prefix = _local_bin_path_prefix() elif agent_name == "codex": - env["CODEX_HOME"] = _codex_home_dir() path_prefix = _codex_path_prefix() elif agent_name in {"cursor-cli", "kimi-cli", "goose", "hermes"}: path_prefix = _local_bin_path_prefix() @@ -777,6 +866,64 @@ def get_agent_exec_env(agent_name: str) -> tuple[dict[str, str], str]: return env, path_prefix +def get_agent_binary(agent_name: str) -> str | None: + """Extract the CLI binary name for an agent from Harbor's version command. + + Instantiates the agent via Harbor's factory and parses the binary + from ``get_version_command()``. Returns None if the binary can't + be determined (e.g. agents that use ``python -m ...``). + """ + from harbor.agents.factory import AgentFactory + from harbor.models.agent.name import AgentName + + _NON_AGENT_BINARIES = {"python", "pip", "uv"} + + try: + agent = AgentFactory.create_agent_from_name( + AgentName(agent_name), logs_dir=Path("/tmp/pier-probe") + ) + cmd = agent.get_version_command() + if not cmd: + return None + # Take the last semicolon-separated part and extract the first token. + last_part = cmd.split(";")[-1].strip() + binary = last_part.split()[0] if last_part else None + if binary and Path(binary).name in _NON_AGENT_BINARIES: + return None + return Path(binary).name if binary else None + except Exception: + return None + + +# Cached binary→agent map, built once per process from Harbor's agent registry. +_binary_agent_map: dict[str, str] | None = None + + +def get_binary_agent_map() -> dict[str, str]: + """Return a mapping of CLI binary names to Harbor agent names. + + Built from Harbor's agent registry by inspecting each agent's + ``get_version_command()``. Cached after first call. + """ + global _binary_agent_map + if _binary_agent_map is not None: + return _binary_agent_map + + from harbor.agents.factory import AgentFactory + + result: dict[str, str] = {} + for agent_cls in AgentFactory._AGENTS: + try: + name = agent_cls.name() + binary = get_agent_binary(name) + if binary: + result[binary] = name + except Exception: + continue + _binary_agent_map = result + return result + + def resolve_task_env(task_dir: Path) -> dict[str, str]: """Resolve [environment.env] from task.toml using Harbor's resolver. @@ -808,11 +955,16 @@ def exec_in_container( env: dict[str, str] | None = None, path_prefix: str = "", detach: bool = False, + log_path: str | None = None, ) -> int: """Run a command in a running container via docker exec. Returns the process exit code. Handles TTY allocation, env vars, - PATH prefix, and detached mode. + PATH prefix, detached mode, and optional session recording. + + When *log_path* is set (an absolute container path), the command is + wrapped with ``script -q`` to record terminal output while preserving + full TTY behavior (colors, cursor, interactive prompts). """ import shlex @@ -823,10 +975,22 @@ def exec_in_container( for var, val in (env or {}).items(): env_flags.extend(["-e", f"{var}={val}"]) - if path_prefix: - shell_cmd = f"export PATH={path_prefix}:$PATH && exec " + " ".join( - shlex.quote(c) for c in command - ) + # Build the shell command. When path_prefix or log_path is set we + # must wrap in sh -c; otherwise run the command directly. + needs_shell = bool(path_prefix) or bool(log_path) + + if needs_shell: + parts = [] + if path_prefix: + parts.append(f"export PATH={path_prefix}:$PATH") + cmd_str = " ".join(shlex.quote(c) for c in command) + if log_path: + # Use script(1) to record output while preserving TTY. + cmd_str = f"script -q -c {shlex.quote(cmd_str)} {shlex.quote(log_path)}" + else: + cmd_str = f"exec {cmd_str}" + parts.append(cmd_str) + shell_cmd = " && ".join(parts) run_command = [container, "sh", "-c", shell_cmd] else: run_command = [container, *command] @@ -894,11 +1058,14 @@ def extract_agent_logs( def extract_agent_context(agent_name: str, logs_dir: Path) -> dict | None: """Extract trajectory and usage from an agent's logs directory. - Expects ``logs_dir`` to already be in the layout Harbor's agent reader - expects (e.g. container mode where CLAUDE_CONFIG_DIR points into the - trial's agent dir). :func:`extract_agent_logs` calls this after first - bridging an external session directory into the right layout. + When *logs_dir* contains per-session timestamped directories (created + by ``pier exec``), the latest session for *agent_name* is used + automatically. Otherwise expects the standard Harbor layout directly + under *logs_dir*. """ + session = _latest_session_dir(logs_dir, agent_name) + if session: + logs_dir = session from harbor.agents.factory import AgentFactory from harbor.models.agent.context import AgentContext from harbor.models.agent.name import AgentName diff --git a/pier/tests/test_agent_integration.py b/pier/tests/test_agent_integration.py index e27ccb2..175ebc5 100644 --- a/pier/tests/test_agent_integration.py +++ b/pier/tests/test_agent_integration.py @@ -95,6 +95,25 @@ def test_task_free_agent_install_and_exec( ) assert version.returncode == 0, version.stdout + version.stderr assert version.stdout.strip() + + # Auto-detected agent command should have tee'd output to a + # per-session directory under .pier/_harbor/agent/. + exec_dir = workspace / ".pier" / "_harbor" / "agent" / "exec" + session_dirs = sorted(d for d in exec_dir.iterdir() if d.is_dir()) + assert session_dirs, "at least one session dir should exist after pier exec" + tee_output = session_dirs[-1] / f"{agent_name}.txt" + assert tee_output.exists(), ( + f"tee output for {agent_name} not found at {tee_output}" + ) + assert tee_output.stat().st_size > 0 + + # Log-capture env vars should be set inside the container. + env_check = _run( + ["uv", "run", "pier", "exec", "sh", "-c", "echo $CLAUDE_CONFIG_DIR"], + cwd=workspace, + ) + assert "/logs/agent/" in env_check.stdout + assert "/sessions" in env_check.stdout finally: if (workspace / ".pier" / "session.json").exists(): _run(["uv", "run", "pier", "stop"], cwd=workspace) diff --git a/pier/tests/test_docker_integration.py b/pier/tests/test_docker_integration.py index b816581..1b05aea 100644 --- a/pier/tests/test_docker_integration.py +++ b/pier/tests/test_docker_integration.py @@ -416,6 +416,49 @@ def test_exec_nonexistent_command_fails( finally: _cleanup_container(workspace) + def test_exec_sets_log_capture_env(self, runner, index_path, task_dir, workspace): + """pier exec always sets CLAUDE_CONFIG_DIR and CODEX_HOME.""" + try: + _start_workspace(runner, task_dir, workspace) + + r = self._run_pier_exec(workspace, "sh", "-c", "echo $CLAUDE_CONFIG_DIR") + assert r.returncode == 0, r.stderr + assert "/logs/agent/" in r.stdout + assert "/sessions" in r.stdout + + r = self._run_pier_exec(workspace, "sh", "-c", "echo $CODEX_HOME") + assert r.returncode == 0, r.stderr + assert "/logs/agent/" in r.stdout + + finally: + _cleanup_container(workspace) + + def test_exec_log_capture_env_writable( + self, runner, index_path, task_dir, workspace + ): + """CLAUDE_CONFIG_DIR inside the container is writable (bind-mounted).""" + try: + _start_workspace(runner, task_dir, workspace) + + r = self._run_pier_exec( + workspace, + "sh", + "-c", + 'mkdir -p "$CLAUDE_CONFIG_DIR/projects/test" && ' + 'echo "session" > "$CLAUDE_CONFIG_DIR/projects/test/log.jsonl" && ' + 'cat "$CLAUDE_CONFIG_DIR/projects/test/log.jsonl"', + ) + assert r.returncode == 0, r.stderr + assert "session" in r.stdout + + # Verify the file persists on the host via the bind mount + agent_dir = workspace / ".pier" / "_harbor" / "agent" + session_files = list(agent_dir.rglob("*.jsonl")) + assert session_files, "session file should be on host via bind mount" + + finally: + _cleanup_container(workspace) + # --------------------------------------------------------------------------- # pier verify diff --git a/pier/tests/test_harbor_bridge.py b/pier/tests/test_harbor_bridge.py index e184547..f7d9397 100644 --- a/pier/tests/test_harbor_bridge.py +++ b/pier/tests/test_harbor_bridge.py @@ -3,18 +3,23 @@ import json from datetime import datetime, timezone from pathlib import Path +from unittest.mock import MagicMock, patch import pytest from pier.harbor_bridge import ( _bridge_claude_code, + _latest_session_dir, create_synthetic_task_dir, _get_dockerfile_workdir, _write_mounts_compose, build_trial_result_json, + get_agent_binary, get_compose_project, get_agent_exec_env, + get_post_run_commands, get_container_name, + get_log_capture_env, is_valid_agent, ) @@ -278,7 +283,8 @@ def test_idempotent(self, tmp_path: Path): class TestGetAgentExecEnv: def test_claude_code_env(self): env, path_prefix = get_agent_exec_env("claude-code") - assert env["CLAUDE_CONFIG_DIR"] == "/logs/agent/sessions" + # CLAUDE_CONFIG_DIR is now in get_log_capture_env(), not here. + assert "CLAUDE_CONFIG_DIR" not in env assert env["IS_SANDBOX"] == "1" assert path_prefix == "$HOME/.local/bin" @@ -295,10 +301,8 @@ def test_local_bin_agent_path_prefix(self, agent_name: str): ) def test_nvm_agent_path_prefix(self, agent_name: str): env, path_prefix = get_agent_exec_env(agent_name) - if agent_name == "codex": - assert env["CODEX_HOME"] == "/logs/agent" - else: - assert env == {} + # CODEX_HOME is now in get_log_capture_env(), not here. + assert env == {} assert ( path_prefix == '$(find "$HOME/.nvm/versions/node" -mindepth 1 -maxdepth 1 -type d ' @@ -425,3 +429,187 @@ def test_path_prefix(self, tmp_path: Path): cmd_str = " ".join(args) assert "$HOME/.local/bin" in cmd_str assert "claude" in cmd_str + + +class TestGetLogCaptureEnv: + def test_returns_config_dir_env_vars(self): + env = get_log_capture_env() + assert env["CLAUDE_CONFIG_DIR"] == "/logs/agent/sessions" + assert env["CODEX_HOME"] == "/logs/agent" + + def test_values_derived_from_harbor(self): + """Env var values match what Harbor's EnvironmentPaths says.""" + from harbor.models.trial.paths import EnvironmentPaths + + env = get_log_capture_env() + assert ( + env["CLAUDE_CONFIG_DIR"] + == (EnvironmentPaths.agent_dir / "sessions").as_posix() + ) + assert env["CODEX_HOME"] == EnvironmentPaths.agent_dir.as_posix() + + +class TestGetAgentBinary: + def test_claude_code(self): + assert get_agent_binary("claude-code") == "claude" + + def test_codex(self): + assert get_agent_binary("codex") == "codex" + + def test_cursor_cli(self): + assert get_agent_binary("cursor-cli") == "cursor-agent" + + def test_goose(self): + assert get_agent_binary("goose") == "goose" + + def test_unknown_agent_returns_none(self): + assert get_agent_binary("nonexistent-agent") is None + + +class TestExecInContainerTee: + def test_script_wraps_command(self, tmp_path: Path): + from pier.harbor_bridge import exec_in_container + + with ( + patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run, + patch("sys.stdin") as mock_stdin, + ): + mock_stdin.isatty.return_value = True + exec_in_container( + "pier-ws", + tmp_path, + ["claude", "--print", "hello"], + log_path="/logs/agent/exec/2026-01-01_00-00-00/claude-code.txt", + ) + args = mock_run.call_args[0][0] + assert "sh" in args + assert "-c" in args + cmd_str = args[args.index("-c") + 1] + assert "script -q -c" in cmd_str + assert "/logs/agent/exec/2026-01-01_00-00-00/claude-code.txt" in cmd_str + # script preserves TTY — -it should still be present + assert "-it" in args + + def test_no_tee_without_flag(self, tmp_path: Path): + from pier.harbor_bridge import exec_in_container + + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + exec_in_container("pier-ws", tmp_path, ["bash"]) + args = mock_run.call_args[0][0] + cmd_str = " ".join(args) + assert "tee" not in cmd_str + + +class TestLogCapturePipeline: + """End-to-end: raw agent output → trajectory → trial result.""" + + def test_script_output_produces_valid_trial(self, tmp_path: Path): + """script(1) output with ANSI codes → extract → assemble → valid + TrialResult that pier view/summarize can read. + """ + from pier.harbor_bridge import extract_agent_logs + from pier.trajectory import assemble_trial + + # Simulate script(1) output with ANSI codes. + session_dir = tmp_path / "session" + session_dir.mkdir() + (session_dir / "goose.txt").write_text( + "Script started on 2026-04-10 00:00:00+00:00\n" + "\x1b[1m> goose\x1b[0m starting session...\n" + "--- \x1b[32mtool_call\x1b[0m: bash ---\n" + "echo hello\n" + "--- \x1b[32mresult\x1b[0m ---\n" + "hello\n" + "Script done on 2026-04-10 00:01:00+00:00\n" + ) + + # Step 1: extract_agent_logs (same as verify/capture). + trial_dir = tmp_path / "trial" + trial_dir.mkdir() + agent_context = extract_agent_logs("goose", session_dir, trial_dir / "agent") + assert (trial_dir / "agent" / "trajectory.json").exists() + + # Step 2: assemble_trial (same as verify/capture). + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.toml").write_text( + '[metadata]\nauthor_name = "test"\n[environment]\n[verifier]\n[agent]\n' + ) + (task_dir / "instruction.md").write_text("") + assemble_trial( + trial_dir, + task_dir, + "test-task", + "test-session", + {"reward": 1.0}, + agent_name="goose", + agent_context=agent_context, + ) + + # Step 3: result.json must be valid for Harbor's viewer/summarizer. + result_json = trial_dir / "result.json" + assert result_json.exists() + from harbor.models.trial.result import TrialResult + + result = TrialResult.model_validate_json(result_json.read_text()) + assert result.agent_info.name == "goose" + + +class TestLatestRunDir: + def test_returns_none_when_no_runs(self, tmp_path: Path): + agent_dir = tmp_path / "agent" + agent_dir.mkdir() + assert _latest_session_dir(agent_dir, "claude-code") is None + + def test_returns_none_when_empty(self, tmp_path: Path): + agent_dir = tmp_path / "agent" + (agent_dir / "exec").mkdir(parents=True) + assert _latest_session_dir(agent_dir, "claude-code") is None + + def test_finds_run_with_tee_file(self, tmp_path: Path): + agent_dir = tmp_path / "agent" + run = agent_dir / "exec" / "2026-01-01_00-00-00" + run.mkdir(parents=True) + (run / "claude-code.txt").write_text("output\n") + assert _latest_session_dir(agent_dir, "claude-code") == run + + def test_finds_run_with_sessions(self, tmp_path: Path): + agent_dir = tmp_path / "agent" + run = agent_dir / "exec" / "2026-01-01_00-00-00" + (run / "sessions").mkdir(parents=True) + assert _latest_session_dir(agent_dir, "claude-code") == run + + def test_picks_most_recent(self, tmp_path: Path): + agent_dir = tmp_path / "agent" + old = agent_dir / "exec" / "2026-01-01_00-00-00" + old.mkdir(parents=True) + (old / "claude-code.txt").write_text("old\n") + new = agent_dir / "exec" / "2026-01-01_00-05-00" + new.mkdir(parents=True) + (new / "claude-code.txt").write_text("new\n") + assert _latest_session_dir(agent_dir, "claude-code") == new + + def test_skips_runs_for_other_agents(self, tmp_path: Path): + agent_dir = tmp_path / "agent" + goose_run = agent_dir / "exec" / "2026-01-01_00-05-00" + goose_run.mkdir(parents=True) + (goose_run / "goose.txt").write_text("output\n") + assert _latest_session_dir(agent_dir, "claude-code") is None + + +class TestGetPostRunCommands: + def test_gemini_copies_trajectory(self): + cmds = get_post_run_commands("gemini-cli", "/logs/agent/ts") + assert len(cmds) == 1 + assert "gemini-cli.trajectory.json" in cmds[0] + assert "/logs/agent/ts" in cmds[0] + + def test_hermes_exports_session(self): + cmds = get_post_run_commands("hermes", "/logs/agent/ts") + assert len(cmds) == 1 + assert "hermes-session.jsonl" in cmds[0] + assert "/logs/agent/ts" in cmds[0] + + def test_unknown_agent_returns_empty(self): + assert get_post_run_commands("claude-code", "/logs/agent/ts") == [] + assert get_post_run_commands("goose", "/logs/agent/ts") == [] diff --git a/pier/tests/test_pier_cli.py b/pier/tests/test_pier_cli.py index b97fe3c..410a103 100644 --- a/pier/tests/test_pier_cli.py +++ b/pier/tests/test_pier_cli.py @@ -787,25 +787,29 @@ def test_exec_container_with_agent_prepends_path( assert "IS_SANDBOX=1" in args +@patch( + "pier.harbor_bridge.get_binary_agent_map", + return_value={}, +) @patch( "pier.harbor_bridge.get_agent_exec_env", return_value=({}, ""), ) @patch("pier.harbor_bridge.is_environment_running", return_value=True) def test_exec_container_without_path_prefix_runs_directly( - mock_running, mock_env, runner, index_path, tmp_path + mock_running, mock_env, mock_map, runner, index_path, tmp_path ): - """When agent has no PATH prefix, command runs directly (no shell wrapper).""" + """When no agent is detected and no PATH prefix, command runs directly.""" ws = tmp_path / "ws" ws.mkdir() sess = _container_session(agents=["codex"]) _write_session(ws, sess, index_path) with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: - runner.invoke(cli, ["exec", "codex", "run"]) + runner.invoke(cli, ["exec", "my-tool", "run"]) args = mock_run.call_args[0][0] # Should NOT wrap in sh -c assert "sh" not in args - assert "codex" in args + assert "my-tool" in args assert "run" in args @@ -961,6 +965,214 @@ def test_exec_container_not_running(mock_running, runner, index_path, tmp_path): assert "not running" in result.output +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_exec_always_sets_log_capture_env(mock_running, runner, index_path, tmp_path): + """CLAUDE_CONFIG_DIR and CODEX_HOME are set even with no agents registered.""" + ws = tmp_path / "ws" + ws.mkdir() + sess = _container_session(agents=[]) + _write_session(ws, sess, index_path) + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + runner.invoke(cli, ["exec", "bash"]) + args = mock_run.call_args[0][0] + env_str = " ".join(args) + assert "CLAUDE_CONFIG_DIR=" in env_str + assert "CODEX_HOME=" in env_str + + +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_exec_user_env_overrides_log_capture( + mock_running, runner, index_path, tmp_path +): + """User -e env vars override default log capture env vars.""" + ws = tmp_path / "ws" + ws.mkdir() + sess = _container_session(agents=[]) + sess["extra_env"] = ["CLAUDE_CONFIG_DIR=/custom/path"] + _write_session(ws, sess, index_path) + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + runner.invoke(cli, ["exec", "bash"]) + args = mock_run.call_args[0][0] + env_str = " ".join(args) + assert "CLAUDE_CONFIG_DIR=/custom/path" in env_str + assert "CLAUDE_CONFIG_DIR=/logs/agent/sessions" not in env_str + + +@patch( + "pier.harbor_bridge.get_binary_agent_map", + return_value={"claude": "claude-code"}, +) +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_exec_auto_detects_agent(mock_running, mock_map, runner, index_path, tmp_path): + """pier exec claude auto-detects the agent, creates a run dir, and tees output.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session(ws, _container_session(agents=[]), index_path) + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + runner.invoke(cli, ["exec", "claude", "--print", "hello"]) + args = mock_run.call_args[0][0] + cmd_str = args[args.index("-c") + 1] + assert "script -q -c" in cmd_str + assert "/logs/agent/" in cmd_str + assert "claude-code.txt" in cmd_str + # Per-run dir should exist on host. + exec_dir = ws / ".pier" / "_harbor" / "agent" / "exec" + assert exec_dir.is_dir() + session_dirs = [d for d in exec_dir.iterdir() if d.is_dir()] + assert len(session_dirs) == 1 + + +@patch( + "pier.harbor_bridge.get_binary_agent_map", + return_value={"claude": "claude-code"}, +) +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_exec_multiple_runs_create_separate_dirs( + mock_running, mock_map, runner, index_path, tmp_path +): + """Two pier exec claude calls create two separate run dirs.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session(ws, _container_session(agents=[]), index_path) + + with patch("subprocess.run", return_value=MagicMock(returncode=0)): + runner.invoke(cli, ["exec", "claude", "--print", "first"]) + with patch("subprocess.run", return_value=MagicMock(returncode=0)): + runner.invoke(cli, ["exec", "claude", "--print", "second"]) + + exec_dir = ws / ".pier" / "_harbor" / "agent" / "exec" + session_dirs = sorted(d for d in exec_dir.iterdir() if d.is_dir()) + assert len(session_dirs) == 2 + + +@patch( + "pier.harbor_bridge.get_binary_agent_map", + return_value={"claude": "claude-code"}, +) +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_exec_no_tee_for_non_agent_cmd( + mock_running, mock_map, runner, index_path, tmp_path +): + """pier exec bash does not tee even when agents exist in Harbor.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session(ws, _container_session(agents=[]), index_path) + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + runner.invoke(cli, ["exec", "bash"]) + args = mock_run.call_args[0][0] + cmd_str = " ".join(args) + assert "tee" not in cmd_str + # Session dir still created (for CLAUDE_CONFIG_DIR), but no tee. + exec_dir = ws / ".pier" / "_harbor" / "agent" / "exec" + assert exec_dir.is_dir() + + +@patch( + "pier.harbor_bridge.get_binary_agent_map", + return_value={"goose": "goose"}, +) +@patch( + "pier.harbor_bridge.get_agent_exec_env", + return_value=({"IS_SANDBOX": "1"}, "$HOME/.local/bin"), +) +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_exec_auto_detect_applies_env( + mock_running, mock_env, mock_map, runner, index_path, tmp_path +): + """Auto-detected agent gets env vars, PATH, and per-run tee.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session(ws, _container_session(agents=[]), index_path) + with patch("subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: + runner.invoke(cli, ["exec", "goose", "session", "start"]) + args = mock_run.call_args[0][0] + cmd_str = args[args.index("-c") + 1] + assert "script -q -c" in cmd_str + assert "goose.txt" in cmd_str + + +# --------------------------------------------------------------------------- +# pier verify/capture --session +# --------------------------------------------------------------------------- + + +@patch("pier.harbor_bridge.extract_agent_context", return_value={"cost_usd": 0.05}) +@patch("pier.harbor_bridge.verify_environment", return_value={"reward": 1.0}) +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_verify_session_flag( + mock_running, mock_verify, mock_extract, runner, index_path, task_dir, tmp_path +): + """--session selects a specific session directory.""" + ws = tmp_path / "ws" + ws.mkdir() + sess = _container_session(task_dir=str(task_dir), agents=["claude-code"]) + _write_session(ws, sess, index_path) + + # Create two session dirs + exec_dir = ws / ".pier" / "_harbor" / "agent" / "exec" + old = exec_dir / "2026-01-01_00-00-00-000000" + old.mkdir(parents=True) + (old / "claude-code.txt").write_text("old session\n") + new = exec_dir / "2026-01-01_00-05-00-000000" + new.mkdir(parents=True) + (new / "claude-code.txt").write_text("new session\n") + + result = runner.invoke( + cli, + ["verify", "--session", "2026-01-01_00-00-00-000000"], + catch_exceptions=False, + ) + assert result.exit_code == 0 + # Should use the specified session, not latest + assert "2026-01-01_00-00-00-000000" in result.output + + +@patch("pier.harbor_bridge.verify_environment", return_value={"reward": 1.0}) +@patch("pier.harbor_bridge.is_environment_running", return_value=True) +def test_verify_session_invalid_errors( + mock_running, mock_verify, runner, index_path, task_dir, tmp_path +): + """--session with nonexistent timestamp errors.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session( + ws, + _container_session(task_dir=str(task_dir), agents=["claude-code"]), + index_path, + ) + result = runner.invoke(cli, ["verify", "--session", "nonexistent"]) + assert result.exit_code != 0 + assert "not found" in result.output.lower() + + +def test_verify_session_host_mode_errors(runner, index_path, tmp_path): + """--session in host mode errors.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session(ws, _host_session(), index_path) + result = runner.invoke(cli, ["verify", "--session", "2026-01-01_00-00-00-000000"]) + assert result.exit_code != 0 + assert "container mode" in result.output.lower() + + +def test_verify_session_and_session_dir_exclusive( + runner, index_path, task_dir, tmp_path +): + """--session and --session-dir are mutually exclusive.""" + ws = tmp_path / "ws" + ws.mkdir() + _write_session( + ws, + _container_session(task_dir=str(task_dir), agents=["claude-code"]), + index_path, + ) + result = runner.invoke( + cli, ["verify", "--session", "ts", "--session-dir", "/tmp", "-a", "claude-code"] + ) + assert result.exit_code != 0 + assert "mutually exclusive" in result.output.lower() + + # --------------------------------------------------------------------------- # pier exec (host mode) # ---------------------------------------------------------------------------