From 90bb6d253959ef7cd10c0930eec08bfe94e4e5b8 Mon Sep 17 00:00:00 2001 From: Dominique Broeglin Date: Mon, 6 Jul 2026 11:50:33 +0200 Subject: [PATCH] Guard against unwritable (root-owned) jobs/ directories The Pier Docker backend runs task containers as root and bind-mounts the jobs/ directory, so a completed run can leave jobs/ owned by root. A later run started as the normal user then cannot create a new run directory under it and Pier fails with a bare PermissionError. Add check_jobs_dir_writable() to pier_backend.py, which resolves the run directory Pier will create (jobs/[/]), finds the nearest existing ancestor, and raises PierBackendPreflightError with a chown remediation when it is not writable. Wire it into _validate_pier_specs so both `validate` and `run` surface a clear "jobs dir" check instead of an opaque traceback. Document the failure and fix in the scaffold README template and docs/collecting-run-data.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/collecting-run-data.md | 7 +++ src/copilot_experiments/cli.py | 7 +++ src/copilot_experiments/pier_backend.py | 40 ++++++++++++++++ .../templates/experiment_repo/README.md.tmpl | 17 +++++++ tests/test_pier_backend.py | 46 +++++++++++++++++++ 5 files changed, 117 insertions(+) diff --git a/docs/collecting-run-data.md b/docs/collecting-run-data.md index df31fe1..b56b56c 100644 --- a/docs/collecting-run-data.md +++ b/docs/collecting-run-data.md @@ -102,6 +102,13 @@ jobs/ `events.jsonl`; if no native events exist, `analyze` and summaries can fall back to `agent/trajectory.json`. +> **Root-owned `jobs/` directories.** The Pier Docker backend runs task containers as `root` +> and bind-mounts `jobs/`, so a completed run can leave `jobs/` (and its contents) owned by +> `root`. A subsequent run started as your normal user then cannot create a new run directory +> under it and Pier fails with a bare `PermissionError` on `jobs/`. `validate` and +> `run` preflight this and report the `jobs dir` check as not writable with the fix; to clear it +> manually, run `sudo chown -R "$(id -u):$(id -g)" jobs` (or `sudo rm -rf jobs`) and retry. + ## Standalone capture recipe For ad hoc runs outside Pier, generate a session id and copy the session state after Copilot exits: diff --git a/src/copilot_experiments/cli.py b/src/copilot_experiments/cli.py index 5640c5f..dcc3e11 100644 --- a/src/copilot_experiments/cli.py +++ b/src/copilot_experiments/cli.py @@ -17,6 +17,7 @@ from .pier_backend import ( PierBackendPreflightError, PierJobSpec, + check_jobs_dir_writable, discover_pier_job_configs, inject_copilot_token, preflight_pier_backend, @@ -499,6 +500,12 @@ def _validate_pier_specs(specs: list[PierJobSpec]) -> list[ValidationCheck]: checks.append(ValidationCheck(f"{prefix}: backend", False, str(exc))) else: checks.append(ValidationCheck(f"{prefix}: backend", True, "preflight OK")) + try: + check_jobs_dir_writable(spec.config) + except PierBackendPreflightError as exc: + checks.append(ValidationCheck(f"{prefix}: jobs dir", False, str(exc))) + else: + checks.append(ValidationCheck(f"{prefix}: jobs dir", True, "writable")) if all(check.ok for check in checks): try: diff --git a/src/copilot_experiments/pier_backend.py b/src/copilot_experiments/pier_backend.py index 0081974..0e15d92 100644 --- a/src/copilot_experiments/pier_backend.py +++ b/src/copilot_experiments/pier_backend.py @@ -4,8 +4,10 @@ import asyncio import json +import os import shutil import subprocess +from collections.abc import Callable from dataclasses import dataclass from datetime import datetime from pathlib import Path @@ -140,6 +142,44 @@ def preflight_pier_backend(config: Any) -> None: _preflight_docker_backend() +def check_jobs_dir_writable( + config: Any, + *, + writable: Callable[[Path], bool] | None = None, +) -> None: + """Fail fast when the harness cannot create the job's run directory. + + Pier creates ``jobs_dir / job_name`` with ``Path.mkdir(parents=True)``. When an + earlier run left that tree owned by another user -- most commonly ``root``, because + Pier's Docker backend runs containers as root and bind-mounts the jobs directory -- + the current user can no longer create entries under it and Pier raises a bare + ``PermissionError``. Detect that here and surface an actionable remediation instead. + """ + + is_writable = writable or (lambda directory: os.access(directory, os.W_OK | os.X_OK)) + target = _job_dir(config) + anchor = _nearest_existing_ancestor(target) + if is_writable(anchor): + return + raise PierBackendPreflightError( + f"Cannot write to {anchor}, which is needed to create the run directory " + f"{target}. An earlier run probably created it as another user (for example " + f"root, via the Pier Docker backend), so the current user can no longer create " + f"entries under it. Reclaim ownership with " + f'`sudo chown -R "$(id -u):$(id -g)" {anchor}` ' + f"or remove the stale output directory, then retry." + ) + + +def _nearest_existing_ancestor(path: Path) -> Path: + """Return ``path`` or its closest existing ancestor directory.""" + + for candidate in (path, *path.parents): + if candidate.exists(): + return candidate + return Path(path.anchor) if path.anchor else Path(".") + + def inject_copilot_token(config: Any, token: str) -> None: """Inject a GitHub token into local Copilot agents without persisting it to config.""" diff --git a/src/copilot_experiments/templates/experiment_repo/README.md.tmpl b/src/copilot_experiments/templates/experiment_repo/README.md.tmpl index 45b18ec..628d4a6 100644 --- a/src/copilot_experiments/templates/experiment_repo/README.md.tmpl +++ b/src/copilot_experiments/templates/experiment_repo/README.md.tmpl @@ -64,6 +64,23 @@ uv run copilot-experiments inspect / --agent copilot-cli --tri uv run copilot-experiments analyze / --agent copilot-cli --trial 1 ``` +## Troubleshooting + +### `PermissionError` creating a run directory under `jobs/` + +The Pier Docker backend runs task containers as `root` and bind-mounts the `jobs/` +directory, so a completed run can leave `jobs/` (and its contents) owned by `root`. A later +run started as your normal user then cannot create a new run directory under it, and Pier +fails with a bare `PermissionError: [Errno 13] Permission denied: '.../jobs/'`. + +`validate` and `run` now preflight this and report `jobs dir` as not writable with the fix, +but if you hit it directly, reclaim ownership (or remove the stale output) and retry: + +```bash +sudo chown -R "$(id -u):$(id -g)" jobs # reclaim ownership, or +sudo rm -rf jobs # discard stale outputs +``` + ## Writing experiments See `tasks/example-fix-bug/` and `experiments/example.yaml`. diff --git a/tests/test_pier_backend.py b/tests/test_pier_backend.py index 2ddea00..144b12b 100644 --- a/tests/test_pier_backend.py +++ b/tests/test_pier_backend.py @@ -13,6 +13,7 @@ from copilot_experiments.pier_backend import ( COPILOT_CLI_AGENT_IMPORT_PATH, PierBackendPreflightError, + check_jobs_dir_writable, discover_pier_job_configs, inject_copilot_token, load_pier_job_config, @@ -218,6 +219,51 @@ def fail_if_called(): assert "Docker is unavailable" in result.output +def test_check_jobs_dir_writable_passes_for_writable_tree(tmp_path: Path): + config_path = tmp_path / "job.yaml" + config_path.write_text("job_name: smoke\njobs_dir: jobs\n", encoding="utf-8") + config = load_pier_job_config(config_path, root=tmp_path) + + # tmp_path is writable, so the not-yet-created jobs/smoke tree is reachable. + check_jobs_dir_writable(config) + + +def test_check_jobs_dir_writable_reports_nearest_existing_ancestor(tmp_path: Path): + config_path = tmp_path / "job.yaml" + config_path.write_text("job_name: smoke\njobs_dir: jobs\n", encoding="utf-8") + config = load_pier_job_config(config_path, root=tmp_path) + jobs_dir = tmp_path / "jobs" + jobs_dir.mkdir() + + blocked = {jobs_dir} + with pytest.raises(PierBackendPreflightError) as excinfo: + check_jobs_dir_writable(config, writable=lambda directory: directory not in blocked) + + message = str(excinfo.value) + # The existing but unwritable ancestor (jobs/) is reported, not the missing run dir. + assert str(jobs_dir) in message + assert "chown" in message + # The intended run directory is still named for context. + assert str(tmp_path / "jobs" / "smoke") in message + + +def test_cli_validate_flags_unwritable_jobs_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + experiments = tmp_path / "experiments" + experiments.mkdir() + (experiments / "job.yaml").write_text("job_name: smoke\n", encoding="utf-8") + + def fail_writable(_config, **_kwargs): + raise PierBackendPreflightError("not writable; run chown") + + monkeypatch.setattr("copilot_experiments.cli.check_jobs_dir_writable", fail_writable) + + result = CliRunner().invoke(app, ["validate", "--root", str(tmp_path)]) + + assert result.exit_code == 1 + assert "smoke: jobs dir" in result.output + assert "chown" in result.output + + @pytest.mark.parametrize( "example_root", [