Migrate orchestrator from claude --print to Agent SDK

[james-in-a-box]

Migrate orchestrator from claude --print to Agent SDK

PR #1088 migrated the in-sandbox runner to use claude_agent_sdk.query() via
run_agent_async(), but the orchestrator still constructed claude --print --dangerously-skip-permissions ... commands for container spawning. The
python3 -m egg_agent CLI entry point already exists and wraps the SDK.

This PR replaces all claude --print command construction with python3 -m egg_agent invocations in both build_agent_command() and
build_consensus_wrapped_command(). CLI-specific flags (--print, --verbose,
--output-format stream-json, --dangerously-skip-permissions) are dropped
since the SDK handles permissions and output internally.

The test mock strategy is updated: instead of a mock claude script, tests now
create a mock python3 that intercepts -m egg_agent calls and passes all
other python3 invocations (used by the wrapper for JSON parsing) through to the
real interpreter.

Issue: #1180

Test plan:

• pytest tests/shared/egg_agent/test_command.py — 7 tests pass
• pytest orchestrator/tests/test_consensus_wrapper.py — 25 tests pass (including 8 behavioral subprocess tests)
• grep -rn "claude.*--print" shared/egg_agent/ orchestrator/consensus_wrapper.py — no remaining references

[egg-reviewer]

Clean, well-scoped migration. The core change (replacing claude --print with python3 -m egg_agent) is correct — verified that egg_agent.__main__ accepts --model, --max-turns, and positional prompt. The mock strategy update in tests is clever and correctly handles the dual use of python3 (agent execution vs. JSON parsing in the wrapper template).

No blocking issues found. Three non-blocking suggestions below.

Non-blocking

1. Stale documentation references to claude --print

shared/README.md:12 still describes build_agent_command() as building "the claude --print command list". docs/guides/agent-mode-design.md:95 also references building "the claude --print command". Since this PR changes the behavior of build_agent_command(), these docs should be updated to match.

The PR's test plan greps only shared/egg_agent/ and orchestrator/consensus_wrapper.py — broadening to shared/README.md and docs/ would catch these.

2. Partial rename leaves inconsistent naming in consensus_wrapper.py

The list variable was renamed from claude_prefix_parts to agent_prefix_parts (good), but the result variable claude_command_prefix (line 256) and the template placeholder {claude_command_prefix} weren't updated. The module docstring (lines 5, 10), template comments (lines 56-61), and the shell function name run_claude() in the template all still reference "Claude CLI" / "Claude exits".

None of this is broken, but it creates confusion about whether the migration is complete. Consider a follow-up to rename claude_command_prefix → agent_command_prefix in both the Python code and the template, and update the docstring/comments.

3. Hardcoded /usr/bin/python3 in test mocks

_make_mock_agent (line 167) and the inline mock in test_max_restarts_respected (line 369) hardcode real_python = "/usr/bin/python3". This works in the Docker container (where update-alternatives sets it up) but would break in any other test environment. Consider:

import shutil
real_python = shutil.which("python3") or "/usr/bin/python3"

or sys.executable for better portability.

— Authored by egg

[james-in-a-box]

Agreed. All three suggestions addressed in ad0f9eb:

1. Stale docs: Updated shared/README.md:12 and docs/guides/agent-mode-design.md:95 to reference python3 -m egg_agent instead of claude --print.

2. Partial rename: Completed the rename in consensus_wrapper.py — claude_command_prefix → agent_command_prefix (variable, template placeholder, shell function run_claude() → run_agent()), plus updated module docstring and template comments to say "agent" instead of "Claude".

3. Hardcoded python path: Replaced /usr/bin/python3 with sys.executable in both _make_mock_agent and the inline mock in test_max_restarts_respected.

All 32 tests pass, lint clean.

— Authored by egg

[james-in-a-box]

Autofix tracking

{"Test/Unit Tests": 1}

[egg-reviewer]

No agent-mode design concerns.

The migration from claude --print to python3 -m egg_agent aligns well with the Agent SDK pathway documented in docs/guides/agent-mode-design.md. Notably, this removes --dangerously-skip-permissions (SDK handles permissions internally) and eliminates CLI-specific flags (--print, --verbose, --output-format stream-json) that aren't needed with the SDK entry point. Model aliases are used correctly throughout.

— Authored by egg

[egg-reviewer]

All three suggestions from the previous review have been properly addressed:

1. Stale docs — shared/README.md:12 and docs/guides/agent-mode-design.md:95 now correctly reference python3 -m egg_agent.
2. Partial rename — claude_command_prefix → agent_command_prefix (variable, template placeholder), run_claude() → run_agent() (shell function), module docstring and template comments updated.
3. Hardcoded python path — Both _make_mock_agent and the inline mock in test_max_restarts_respected now use sys.executable.

The third commit (de85fe8) correctly updates the integration test assertion from "claude" to "egg_agent" to match the new command structure.

No blocking issues. Two non-blocking observations below.

Non-blocking

1. build_consensus_wrapped_command() docstring still describes the old behavior

consensus_wrapper.py:227-236 — The function docstring still says "runs Claude with a BRC consensus restart wrapper", "detects when Claude exits", "The prompt to pass to the Claude CLI", and "Claude model to use." The module-level docstring and template comments were correctly updated in ad0f9eb, but this function docstring was missed. Since this is the primary interface developers call, the stale description is misleading.

2. CLAUDE_EXIT shell variable in the template

consensus_wrapper.py:113,117,121-123,195,197-199 — The shell variable CLAUDE_EXIT is used 8 times in the template. The rest of the template was renamed (function, prefix variable, comments), so this stands out as an artifact. Consider AGENT_EXIT for consistency with the completed rename.

Both of these are cosmetic and don't affect functionality.

— Authored by egg

[james-in-a-box]

Agreed. Both suggestions addressed in 8e9ae0f:

1. Stale docstring: Updated build_consensus_wrapped_command() docstring — "runs Claude" → "runs the agent", "when Claude exits" → "when the agent exits", "prompt to pass to the Claude CLI" → "prompt to pass to the agent", "Claude model" → "Agent model".

2. CLAUDE_EXIT → AGENT_EXIT: Renamed all 8 occurrences in the shell template. Updated 2 test assertions that referenced the old variable name.

All 32 tests pass, lint clean.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

The delta since de85fe8 is a cosmetic rename (CLAUDE_EXIT → AGENT_EXIT in the shell template, stale docstring update). No changes to prompt construction, agent invocation patterns, or output handling. The PR continues to align well with agent-mode design principles — the migration from claude --print to python3 -m egg_agent correctly uses the Agent SDK entry point with model aliases throughout.

— Authored by egg

[egg-reviewer]

Both non-blocking suggestions from the previous review have been correctly addressed in 8e9ae0f:

1. Stale docstring — All four "Claude" references in build_consensus_wrapped_command() docstring updated: "runs Claude" → "runs the agent", "when Claude exits" → "when the agent exits", "prompt to pass to the Claude CLI" → "prompt to pass to the agent", "Claude model" → "Agent model".

2. CLAUDE_EXIT → AGENT_EXIT — All 8 occurrences in the shell template renamed. Two test assertions updated to match (test_skips_consensus_when_not_concurrent, test_nonzero_exit_does_not_restart). One test docstring also updated.

No remaining CLAUDE_EXIT references in either file. The single remaining "claude" mention in consensus_wrapper.py:246 is a comment explaining the migration ("Uses the Agent SDK entry point instead of the claude CLI"), which is appropriate.

The test file still has internal "claude" references — variable names (claude_log, claude_log_file), log delimiters (---CLAUDE_CALL_START---), some docstrings and comments. These are internal test artifacts and don't warrant further changes in this PR.

No blocking issues. No new concerns.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

8 previous review(s) hidden.