Fix #1993: default agent cwd to $EGG_REPO_PATH

[jwbron]

Summary

• Sandbox agents were starting at cwd=/home/egg (HOME/WORKDIR) instead of the repo at /home/egg/repos/<repo>. Early relative-path tool calls against .egg-state/… failed on the first few attempts and the agent spent tokens rediscovering the layout on every run.
• EGG_REPO_PATH was already populated by the orchestrator/entrypoint; wire it in as the default cwd when no explicit cwd is passed to run_agent_async.
• Covers both SDK paths (claude-sdk default + the opt-in EGG_HARNESS=egg harness) and the harness factory's project CLAUDE.md lookup. Explicit cwd= arguments still take precedence; os.getcwd() remains the final fallback for local CLI invocation outside the orchestrator.

Fixes #1993.

Notes

• The "stale cwd across bash calls" footgun in the issue is not a separate bug — shared/egg_harness/tools/bash.py spawns each call as a fresh subprocess with start_new_session=True, so cwd does not actually persist. The agent was chaining cd repos/egg because the initial cwd was wrong; fixing the initial cwd removes the need to chain.
• Dockerfile WORKDIR /home/egg is untouched — HOME semantics still apply for shell/config discovery; the agent session just starts in the repo.

Test plan

• pytest tests/shared/egg_agent/test_client.py — 48 passing (2 new cases cover EGG_REPO_PATH fallback and explicit-cwd precedence, existing fallback test updated to scrub env)
• pytest shared/tests/test_egg_harness/ — 507 passing
• ruff check + ruff format pass
• Observed on next pipeline run: first-call Read('.egg-state/drafts/…') succeeds; session-init log shows cwd=/home/egg/repos/<repo>

🤖 Generated with Claude Code

[egg-reviewer]

Clean, well-scoped fix. Precedence chain (explicit cwd > EGG_REPO_PATH > os.getcwd()) is correctly implemented in both SDK paths. Tests cover the key cases. No blocking issues.

Non-blocking suggestions:

1. Empty-string EGG_REPO_PATH defense (client.py:102, harness_factory.py:116): os.environ.get("EGG_REPO_PATH") returns "" if the var is set but empty. That empty string gets passed to ClaudeAgentOptions(cwd="") / create_bash_tool(cwd=""), but the log line at client.py:288 masks it ("" or os.getcwd() → shows os.getcwd()). The entrypoint always sets a real path so this won't fire today, but os.environ.get("EGG_REPO_PATH") or None would be strictly safer and keep the log/actual-cwd in sync:

   resolved_cwd: str | None = str(cwd) if cwd else (os.environ.get("EGG_REPO_PATH") or None)

   Same pattern in harness_factory.py:116.

2. Redundant fallback in harness_factory.py:168: After the line-116 change, cwd_str already includes the EGG_REPO_PATH fallback, so repo_path = cwd_str or os.environ.get("EGG_REPO_PATH") is now a no-op or branch when called from client.py. It still serves direct callers of create_egg_harness (e.g. validate_harness_parity.py), so leaving it is fine — might just be worth a comment noting the intentional redundancy.

3. Test assertions could be stronger: The new tests (test_init_log_cwd_falls_back_to_egg_repo_path, test_explicit_cwd_wins_over_egg_repo_path) verify the logged cwd, not the cwd actually passed to ClaudeAgentOptions. Since the same resolved_cwd variable feeds both paths this is fine in practice, but an assertion on mock_query.call_args to check the options object would catch any future refactor that splits the two apart.

— Authored by egg

[james-in-a-box]

Agreed. All three suggestions addressed in 4977e2c:

1. Empty-string defense — both client.py:102 and harness_factory.py:116 now use os.environ.get("EGG_REPO_PATH") or None so an empty env var is treated the same as unset.
2. Redundancy comment — added a note at harness_factory.py:168 explaining the intentional or branch for direct callers like validate_harness_parity.py.
3. Stronger test assertions — all four cwd tests now assert on mock_query.call_args.kwargs["options"].cwd in addition to the logged value.

— Authored by egg

[egg-reviewer]

All three suggestions from the previous review are properly addressed:

1. Empty-string defense (client.py:102, harness_factory.py:116): os.environ.get("EGG_REPO_PATH") or None correctly collapses "" to None, letting the fallback chain continue to os.getcwd().

2. Redundancy comment (harness_factory.py:168-171): Accurately explains why the or os.environ.get("EGG_REPO_PATH") branch at line 172 is kept — direct callers of create_egg_harness bypass client.py's resolution layer.

3. Stronger test assertions: All four cwd tests now assert on mock_query.call_args.kwargs["options"].cwd, verifying the value actually passed to the SDK rather than just the logged cwd. Assertions are correct (None for the no-arg/no-env fallback, explicit paths for the other three cases).

Non-blocking suggestion:

There is no test exercising the specific scenario the or None defense guards against — EGG_REPO_PATH="" (set but empty). A test like patch.dict(os.environ, {"EGG_REPO_PATH": ""}) asserting that options.cwd is None and the logged cwd falls back to os.getcwd() would lock in the behavior. Not blocking since the code is correct and the scenario is unlikely given the orchestrator always sets a real path.

— Authored by egg

[james-in-a-box]

Agreed. Added test_empty_egg_repo_path_treated_as_unset in 2de0500 — patches EGG_REPO_PATH="" and asserts that options.cwd is None (SDK falls back to os.getcwd()) and the logged cwd matches os.getcwd().

— Authored by egg

[egg-reviewer]

Re-review of changes since 4977e2c (one new commit: 2de0500).

The new test_empty_egg_repo_path_treated_as_unset test correctly exercises the EGG_REPO_PATH="" edge case from the previous suggestion. It patches the env var to an empty string, then verifies both the logged cwd (os.getcwd()) and the options object (cwd is None), confirming the or None defense at client.py:102 works as intended.

No issues found. All previous feedback has been properly addressed across both rounds.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

4 previous review(s) hidden.