Fix orchestrator crash on host paths in egg-state dirs

[james-in-a-box]

Fix orchestrator crash creating .egg-state dirs on host paths

The orchestrator receives host-translated worktree paths from the gateway
(e.g. /home/jwies/.egg-worktrees/...), but the orchestrator container
only sees these at /home/egg/... via Docker volume mounts. Two bugs:

1. ensure_egg_state_dirs() crashed with PermissionError: [Errno 13]
   trying to mkdir on inaccessible host paths
2. phase_readonly_mounts() silently produced no mounts because
   is_dir() returned False on host paths, leaving .egg-state/
   subdirs writable during implement phase

Adds _host_to_local_volumes() helper to translate host paths using the
existing HOST_HOME env var, and a local_volumes parameter to
phase_readonly_mounts() so it checks local paths for existence while
keeping host paths as Docker mount sources.

Issue: Pipeline issue-732 PermissionError

Test plan:

• pytest tests/shared/egg_container/test_phase_mounts.py — 32 tests pass (3 new)
• pytest orchestrator/tests/test_container_spawner.py — 38 tests pass (6 new)
• New tests cover: host→local translation, prefix-only replacement, passthrough when HOST_HOME matches or is unset, local_volumes for is_dir checks with host mount sources

Authored-by: egg

[egg-reviewer]

Review: Fix orchestrator crash on host paths in egg-state dirs

Reviewed all 4 changed files. The fix is correct and well-scoped — it addresses a real crash (PermissionError) and a silent bug (empty readonly mounts) caused by the orchestrator trying to operate on host-absolute paths it can't access.

Approach

The solution is clean: translate host paths to orchestrator-local paths using the existing HOST_HOME env var, then pass the local paths for filesystem operations while keeping host paths as Docker mount sources. This mirrors the inverse of the gateway's translate_to_host_path() in gateway/gateway.py:249.

Minor issues

1. HOST_HOME with trailing slash
orchestrator/container_spawner.py:109 — path.replace(host_home, container_home, 1) will produce a double-slash if HOST_HOME has a trailing slash (e.g., /home/jwies/ → /home/egg//...). The gateway's translate_to_host_path() has the same latent issue, so this is consistent with existing behavior, but both could be tightened with host_home.rstrip("/"). Not blocking.

2. Test for "HOST_HOME not set" is unnecessarily convoluted
orchestrator/tests/test_container_spawner.py:626-631 — The double-nested patch.dict with __import__("os") is hard to read. A simpler approach:

with patch.dict("os.environ", {}, clear=True):
    result = _host_to_local_volumes(repo_volumes)

If the test needs to preserve some env vars but remove HOST_HOME, use patch.dict("os.environ", {"HOST_HOME": ""}) since the function already treats empty string as unset. Not blocking — the test is correct, just noisy.

No issues found

• Security: No path traversal concern — HOST_HOME is set by the deployment operator (via .env / docker-compose), not by user input. The startswith + replace(..., 1) pattern correctly handles the prefix-only replacement.
• Correctness: ensure_egg_state_dirs now receives local paths it can actually mkdir/chown on. phase_readonly_mounts correctly uses local paths for is_dir() checks while keeping host paths as Docker mount sources. The check_volumes.get(repo_name, host_path) fallback is safe — an inaccessible path just returns is_dir() == False, skipping the mount.
• Backward compatibility: The local_volumes parameter defaults to None, preserving existing behavior for all callers outside the orchestrator.
• Test coverage: Good — 9 new tests covering translation, passthrough, prefix-only replacement, multiple repos, and the local_volumes/repo_volumes separation in phase_readonly_mounts.

Looks good to merge.

— Authored by egg

[james-in-a-box]

Addressed both suggestions from the review:

1. HOST_HOME trailing slash — Added .rstrip("/") to _host_to_local_volumes so a trailing slash on HOST_HOME (e.g. /home/jwies/) no longer produces double-slashes in translated paths. Added a test for this case.

2. Simplified test_passthrough_when_host_home_empty — Replaced the double-nested patch.dict with the simpler patch.dict("os.environ", {"HOST_HOME": ""}) approach, since the function already treats empty string as unset.

All tests passing (39 container_spawner + 32 phase_mounts).

— Authored by egg

[egg-reviewer]

Re-review: Fix orchestrator crash on host paths in egg-state dirs

Reviewed delta 4878b65..df748f0 (1 commit, 2 files changed).

Previous feedback verification

Both suggestions from the initial review have been properly addressed:

1. HOST_HOME trailing slash — container_spawner.py:104 now applies .rstrip("/") before any path operations. This correctly prevents double-slashes when HOST_HOME ends with /. Edge cases are safe: HOST_HOME="/" or "///" strip to empty string, which hits the if not host_home early return.

2. Simplified test — test_passthrough_when_host_home_empty replaced the double-nested patch.dict with patch.dict("os.environ", {"HOST_HOME": ""}). Clean and correct — the function treats empty string as unset.

3. New test coverage — test_trailing_slash_on_host_home (line 663) directly covers the trailing-slash scenario.

No new issues found

The changes are minimal and focused. No correctness, security, or design concerns.

Phase mounts tests pass (32/32). Container spawner tests cannot run in this sandbox (missing docker SDK) but the PR author reports 39/39 passing.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Re-review confirmed no new issues. All previous feedback was already addressed in df748f0. No further changes needed.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

4 previous review(s) hidden.