[issue-2908][slice-5/6] Additive CLI surface: stdin/file prose plum...

[james-in-a-box]

Context

BRC consensus today depends on the agent re-entering a blocking
egg-orch message wait-loop between every event. That re-entry is a
seam the model can fall out of by emitting a final assistant
message instead of re-entering the wait. Claude usually re-enters;
qwen3.7-max does not (#2906) — it exits success=True after one
match, the wrapper sees no CONSENSUS_CONFIRMED, the 3-restart cap
trips (#2806), and the pipeline FAILs after ~$1 and ~20 min of
churn. Prompt-only mitigations narrow the seam for one model; the
seam itself exists for every model (lineage: #2323, #2064, #2482,
#2036, #1995, #2451).

Changes

Reframe consensus-agent execution from a long-lived participant
that holds blocking waits into a deterministic wrapper-driven
event pump that invokes the agent one-shot per actionable event,
with continuity carried by a durable per-role memory file. Lands
in six linear slices behind EGG_BRC_EVENT_PUMP (default false
until slice-4):

1. slice-1 — Foundations. New egg-orch brc next-action,
   brc get-state, brc list-blocking, phase get-context CLI
   subcommands. Durable BRC memory artifact at
   .egg-state/agent-outputs/<role>/brc-memory.md with
   action-scaffolded writes into brc_ack / brc_nack handlers,
   atomic writes via promoted _persist_atomic_template,
   last_reviewed_commit_sha per producer in the schema,
   fail-closed path construction. Gated by
   EGG_BRC_MEMORY=write-only (writes accumulate; reads land
   in slice-3). Purely additive — zero behavior change.
2. slice-2 — Event-pump wrapper. Rewrite
   orchestrator/consensus_wrapper.py as a deterministic event
   pump gated by EGG_BRC_EVENT_PUMP. Drop the 3-restart cap;
   replace with idle/no-progress safety budget. Migrate the
   heartbeat (Overseer flags BRC reviewers/testers as stalled when they're correctly blocked in mcp__brc__wait_loop (implement-phase false positive) #2036) and gateway-session keep-alive (Orchestrator heartbeat-session lookup fails: container_id missing slice-N segment for non-coder roles #2451) from
   the agent-side handler into the wrapper's blocking wait.
   Heartbeat payload carries slice_id from EGG_SLICE_ID
   (regression guard). Verification is unit-test-only — no
   in-process test double can drive a deployed pod end-to-end
   (the pod-injection avenue was ruled out per Expand integration test coverage #2474); true
   E2E deferred to slice-4 via the egg_stack real-pod
   fixture. Old path retained verbatim alongside.
3. slice-3 — Delta + prompt collapse. Wire per-event
   invocation to hand the agent the memory delta plus the full
   git log {last_reviewed_commit_sha}..HEAD --not origin/{base_branch} -p
   delta per producer (NOT just orchestrator-side
   changed_artifacts — per REVIEWER-SYNC.md the re-review must
   audit the full delta as a fresh review). Strip the
   STAY-ALIVE / wait-loop / cursor-threading guidance from
   _build_brc_preamble and mission.md; replace with a lean
   event-handler contract. Sandbox image rebuilt + agent pod
   restarted BEFORE slice-4 flag flip.
4. slice-4 — Flag flip + delete old path. Flip the
   EGG_BRC_EVENT_PUMP / EGG_BRC_MEMORY defaults to on, gated on
   the slice-2 / slice-3 unit + BRC in-process regression suites
   passing on the new default; delete the capped-restart bash, the
   _RECOVERY_SYSTEM_PROMPT, the SSE machinery, and the
   agent-side wait_loop heartbeat code. Rollback plan: revert
   slices 1–3 via git revert if production traffic regresses. The
   qwen3.7-max qwen3.7-max BRC agents end their agentic loop early (success=True) without reaching CONSENSUS_CONFIRMED → consensus-wrapper restart churn #2906 k3s spike that previously validated this
   end-to-end was removed (qwen route unavailable in k3s;
   Claude-route E2E deferred to Rewrite TestCredentialIsolation as k3s-native (currently skipped under k3s) #2585).
5. slice-5 — Additive CLI prose plumbing + 2 new BRC
   subcommands. Add stdin / --reason-file / --summary-file
   / --files-reviewed-file plumbing to
   consensus propose --summary, consensus ack --reason,
   consensus nack --reason, consensus withdraw --reason
   (docs: steer agents to the structured contract tool, not Bash-composed egg-contract #2741 regression guard). Add egg-orch brc resolve-obligation
   and egg-orch brc read-peer-artifact CLI subcommands the
   slice-6 deletion depends on. Argv kept as fallback during
   transition (deprecation warning).
6. slice-6 — MCP → CLI deletion. Delete the 28 agent-facing
   MCP tools (~1,515 LOC across 7 namespace files + the 4
   infra files + server.py), the SYSTEM_PROMPT_NUDGE, and the
   MCP registration block in shared/egg_agent/client.py:299-353
   INCLUDING the EGG_MCP_TOOLS env flag at :311 (no orphan
   flag). Retire tests/tools/test_mcp_cli_drift.py. Migrate the
   MCP E2E test so the agent's first action is consensus ack/nack via stdin/file (preserves SDK-spawn exercise).
   Verify per-event wall-clock latency within 5%.

Impact

Operator-facing: the BRC consensus subsystem becomes
model-portable — any agent that exits naturally after handling
one event reaches CONFIRMED, instead of needing prompt nudges to
keep re-entering an in-process wait. Cost-per-phase drops because
restart churn disappears, replaced by short bounded per-event
invocations that hit the prefix cache (≥ 60-min TTL on both
routes per WS7 closure). Net code deletion: the capped-restart
template, the SSE machinery, the recovery system prompt, the 28
MCP tool schemas, the EGG_MCP_TOOLS flag, the cursor-threading
guidance, and the agent-side heartbeat all go away. The agent
primitive (pod / worktree / SDK / permissions / restrictions) is
untouched.

This slice

Additive CLI surface: stdin/file prose plumbing + new BRC subcommands

Files affected:

• sandbox/egg_lib/orch_cli.py
• docs/reference/agent-tools.md
• docs/reference/agent-wait-patterns.md
• tests/sandbox/egg_lib/test_orch_cli_prose_args.py
• tests/sandbox/egg_lib/test_orch_cli_brc.py
• integration_tests/test_mcp_baseline_capture.py

Tasks:

• task-5-1: Add stdin / file alternative for prose-bearing args on cmd_consensus_propose --summary (parser at sandbox/egg_lib/orch_cli.py:3265), cmd_consensus_ack --reason (parser at orch_cli.py:3485,3523), cmd_consensus_nack --reason (parser at orch_cli.py:3573), and cmd_consensus_withdraw --reason (parser at orch_cli.py:3600). Today these args are argv-only and re-introduce the shell-metachar corruption mitigated in docs: steer agents to the structured contract tool, not Bash-composed egg-contract #2741 when the wrapper bash composes the command. Reuse the --file PATH pattern from existing cmd_consensus_propose (orch_cli.py:2552). New flags: --summary-file PATH (propose), --reason-file PATH (ack / nack / withdraw), --files-reviewed-file PATH (ack / nack — JSON array on disk, one path per line per architect v2 §verification_strategy.slice_5), stdin sentinel --summary - / --reason -. Keep argv --summary / --reason working for now (deprecation lives in a later cycle) but emit a deprecation warning when used.
  • Acceptance criteria: --summary-file PATH / --reason-file PATH / --files-reviewed-file PATH round-trip multi-line UTF-8 prose containing shell metacharacters intact ($VAR, backticks, ;, &&, newlines); stdin sentinel works for echo … | egg-orch consensus ack --reason -; argv path emits deprecation warning to stderr; existing CLI behavior preserved on argv path (regression test).
• task-5-2: Add egg-orch brc resolve-obligation CLI subcommand to sandbox/egg_lib/orch_cli.py. Wraps the existing mcp__brc__resolve_obligation handler in sandbox/egg_agent_tools/handlers/brc.py. Args: --reviewer-role, --producer-role, --commit-sha (optional), --note (optional; via stdin or --note-file PATH per the docs: steer agents to the structured contract tool, not Bash-composed egg-contract #2741 prose-arg rule from TASK-5-1).
  • Acceptance criteria: CLI subcommand registered; round-trip against the orchestrator succeeds; help text mirrors the MCP-tool description; prose --note exercised via stdin and via --note-file PATH.
• task-5-3: Add egg-orch brc read-peer-artifact CLI subcommand to sandbox/egg_lib/orch_cli.py. Wraps the existing mcp__brc__read_peer_artifact handler. Args: --phase (required), --peer-role (optional), --message-type (optional, repeatable), --limit (default 50, max 500), --cursor (opaque token), --include-unattributed (default true). Stdout JSON.
  • Acceptance criteria: CLI subcommand registered; matches handler behaviour for slice-scoped and unattributed reads; pagination tested with --limit + --cursor round-trip.
• task-5-4: Documenter: update docs/reference/agent-tools.md (or equivalent — locate via Grep docs/ for "consensus propose" / "consensus ack") and docs/reference/agent-wait-patterns.md to document the new --summary-file / --reason-file / --files-reviewed-file flags and stdin sentinel, the two new brc resolve-obligation / brc read-peer-artifact subcommands, and the deprecation warning on the argv --summary / --reason path. Cross-link to docs: steer agents to the structured contract tool, not Bash-composed egg-contract #2741 for the shell-metachar rationale.
  • Acceptance criteria: Docs reflect the new CLI surface; deprecation note on the argv path included; docs: steer agents to the structured contract tool, not Bash-composed egg-contract #2741 cross-link present.
• task-5-5: docs: steer agents to the structured contract tool, not Bash-composed egg-contract #2741 regression-guard test at tests/sandbox/egg_lib/test_orch_cli_prose_args.py (new file). For each of consensus propose --summary, consensus ack --reason, consensus nack --reason, consensus withdraw --reason: round-trip prose containing each of $VAR, single backticks, $(), ;, &&, embedded newlines, UTF-8 non-ASCII characters — via stdin sentinel - AND via --*-file PATH — and assert byte-equality between the on-disk input and the request body received by the orchestrator stub. Also test the --files-reviewed-file one-path-per-line semantics. Argv-path tests verify the deprecation warning lands on stderr.
  • Acceptance criteria: Tests pass under make test; one parametrized test per (CLI command × prose payload × delivery channel) case; deprecation-warning assertion present on the argv-path tests.
• task-5-6: Unit tests for TASK-5-2 / TASK-5-3 CLI subcommands at tests/sandbox/egg_lib/test_orch_cli_brc.py (extending the file added in slice-1 TASK-1-8). Cover brc resolve-obligation happy path + --note via stdin and via --note-file; brc read-peer-artifact paginated round-trip; lifecycle-secret auth on both.
  • Acceptance criteria: Tests pass under make test; one test per subcommand's happy path plus pagination / prose-channel edge case.
• task-5-7: Capture MCP-surface latency baseline for slice-6's comparison test (TASK-6-6 revised acceptance). Add a fixture at integration_tests/test_mcp_baseline_capture.py (directly under integration_tests/; local_pipeline/ does not exist). Drive a real-LLM 5-role consensus on the still-live MCP surface (slice-5 is additive only — MCP tools are still registered) using the session-scoped egg_stack fixture at integration_tests/conftest.py:340 (kubectl-gated; k3s-backed; agents run real Claude / Qwen via the litellm route configured for the test stack — no ScriptedProvider reference; that class does not exist per reviewer_plan v2). Record per-event wall-clock samples (event_type, start_ts, end_ts, agent-process exit code as captured from the orchestrator's pipeline-status events, NOT from a non-existent in-process provider) and write to .egg-state/agent-outputs/latency-mcp-baseline.json. The committed JSON file is slice-6's baseline; capturing it in slice-5 sidesteps the vendored-tarball maintenance burden the original TASK-6-6 carried.
  • Acceptance criteria: Test runs against the egg_stack fixture (kubectl-gated; skips if _kubectl_available() returns False); produces latency-mcp-baseline.json with schema documented in the test file (samples: [{event_type, start_ts, end_ts, exit_code}] plus aggregate p50/p95); JSON file committed at the end of slice-5; slice-6 TASK-6-6 reads this file to derive its baseline. No ScriptedProvider import or reference.

Test Plan

Per-slice automated coverage:

• slice-1: unit tests for next-action derivation (producer /
  reviewer / dual-role / open-NACK barrier Producer can re-propose after only one reviewer NACK in multi-reviewer BRC, wasting review cycles #2142 /
  conditional ACK / stale-version Producer agent calls 'egg-orch message wait-loop --for CONSENSUS_CONFIRMED'; validator rejects #2482), memory-write
  side-effects on brc_ack/brc_nack covering all six
  required fields (incl. last_reviewed_commit_sha per
  producer); atomic-write contract test; fail-closed path
  constructor test (raise on unset EGG_AGENT_ROLE); CLI
  round-trip with lifecycle-secret auth.
• slice-2: wrapper template snapshot for event-pump branch;
  wrapper-side heartbeat unit test asserting slice_id
  propagation from EGG_SLICE_ID; idle-budget overseer-alert
  threshold test; in-process PeerConsensusTracker regression
  (integration_tests/regression/test_brc_*.py) with flag off
  establishes zero orchestrator-side regression; E2E deferred
  to slice-4 via egg_stack (no in-process test double can
  drive a deployed pod end-to-end per Expand integration test coverage #2474).
• slice-3: compose_event_prompt unit tests; collapsed
  preamble snapshot; full git-log delta in per-event prompt
  asserted; per-event prompt envelope (excluding delta) ≤ 10 KB;
  sandbox-image rebuild verification (new mission.md reachable
  in pod) BEFORE slice-4 flag flip.
• slice-4: no new end-to-end test — the qwen3.7-max qwen3.7-max BRC agents end their agentic loop early (success=True) without reaching CONSENSUS_CONFIRMED → consensus-wrapper restart churn #2906 k3s
  spike was removed (qwen route unavailable in k3s; Claude-route
  E2E blocked on ScriptedProvider pod injection, deferred to
  Rewrite TestCredentialIsolation as k3s-native (currently skipped under k3s) #2585). The flag flip is gated on the slice-2 / slice-3 unit +
  BRC in-process regression suites passing on the new default;
  the deletions in TASK-4-2 are covered by the updated unit tests
  in TASK-4-3.
• slice-5: stdin / --reason-file / --summary-file /
  --files-reviewed-file round-trip tests for prose containing
  $VAR / backticks / ; / && / embedded newlines (docs: steer agents to the structured contract tool, not Bash-composed egg-contract #2741
  regression guard); CLI subcommand tests for brc resolve-obligation and brc read-peer-artifact; argv
  --reason deprecation-warning test.
• slice-6: BRC actions reachable via CLI with no agent MCP
  server registered; migrated E2E runs first action as
  consensus ack/nack via stdin/file (preserves SDK-spawn
  exercise); per-event wall-clock latency within 5% of
  pre-slice-6 baseline.

Manual verification:

• slice-3 pre-merge: sandbox image rebuilt + agent pod restarted
  so the new mission.md is reachable in the pod BEFORE slice-4
  flag flip.
• slice-4 pre-flag-flip: human inspects spike output
  (brc-memory content for reasoning fidelity;
  last_reviewed_commit_sha updated per producer; cost-per-phase
  delta vs restart-churn baseline) and
  consents to default-on flip via PR review.
• slice-6 pre-merge: human verifies no in-flight pipelines are
  mid-run against pre-slice agents; gate deploy on drain or
  cancel.

Manual Steps

Pre-merge:

• slice-3: sandbox image rebuilt + agent pod restarted BEFORE
  slice-4's flag flip (the new mission.md is the slice-4
  assumption; if pods are still running the old image when the
  flag flips, the event-pump path will reference STAY-ALIVE
  semantics that have been deleted from the preamble).
• slice-4: human review of spike output (memory content,
  last_reviewed_commit_sha correctness, cost delta)
  before the EGG_BRC_EVENT_PUMP default flips to true.
• slice-6: confirmation that no
  in-flight pipelines exist before deploy (MCP tools are deleted
  so an old wrapper that still injects them starts with no MCP
  server registered).

Post-merge:

• slice-4: monitor 24 h of production BRC traffic for any
  "Agent exited without BRC consensus" entries; fall back via
  EGG_BRC_EVENT_PUMP=false deployment env if seen. Rollback
  path for full spike falsification: git revert slices 1–3 (no
  production traffic touched the new path because the flag
  stayed off until slice-4).
• slice-6: monitor latency dashboards 24 h for per-event
  wall-clock regression beyond the 5% budget.

Stack

• Position: slice 5 of 6 in pipeline issue-2908-impl2
• Stacked on top of egg/issue-2908-impl2/slice-4

Slice slice-5 of pipeline issue-2908-impl2. Stacked on top of egg/issue-2908-impl2/slice-4.

[james-in-a-box]

Autofix tracking

{"Lint/Python": 1, "Test/Integration Tests / Integration Tests": 2, "Test/Unit Tests": 3}

[james-in-a-box]

Check Failures: Unit Tests & Integration Tests

Investigated CI run 26864555653. Both failures are out of scope for this PR — slice-5 only touches sandbox/egg_lib/orch_cli.py and its tests, but the failures live elsewhere.

Unit Tests — 13 failures, inherited from slice-4

orchestrator/routes/pipelines.py (blob 7e003565b on this branch tip) is missing three pieces of code that the tests on this branch depend on:

Test failure	Missing symbol/behavior	Source commit
7× ImportError: cannot import name '_slice_agents_alive' from 'routes.pipelines'	_slice_agents_alive() helper	#2916 (3398005b1)
4× TypeError: _resolve_slice_base_branch() got an unexpected keyword argument 'parent_branch_exists'	parent_branch_exists kwarg replacing merge_base_lookup	#2928 (b8573924d)
1× AssertionError: assert <SliceStatus.PENDING> == <SliceStatus.COMPLETE> in test_populate_contract_from_plan_preserves_slice_and_task_runtime	Runtime-preserving merge inside _populate_contract_from_plan (currently blindly assigns contract.slices = contract_slices at L19766)	#2923 (664592205)
1× pytest-timeout 60s	Downstream of _slice_agents_alive missing	—

Root cause: those three commits exist in slice-4 (PR #2951) and slice-6 history, but are absent from this branch's pipelines.py. The merge commit 39a08bb6f (slice-3 merging origin/main) appears to have resolved the pipelines.py conflict by keeping HEAD's version while still accepting the new test files — so the tests came in but the source fixes they depend on did not.

Why this PR can't fix it: slice-5's diff is purely additive CLI surface (orch_cli.py + docs + new test files). Touching pipelines.py here would be out of scope and would conflict with whatever lands on slice-4. The fix needs to land on slice-4 (#2951), then rebase up through slice-5.

Integration Tests — Docker build infrastructure flake

The integration job failed at add-apt-repository -y ppa:deadsnakes/ppa during image build:

ln: failed to create symbolic link '/etc/resolv.conf': Device or resource busy
Failed to connect to socket /run/dbus/system_bus_socket: No such file or directory
exit code 1

This is runner-level (dbus socket / resolv.conf), not code. A re-run should clear it.

Recommended action

• Slice-4 (PR [issue-2908][slice-4/6] Flag flip + delete old capped-restart wrapp... #2951): restore the three missing edits in pipelines.py (_slice_agents_alive, parent_branch_exists signature, runtime-preserving slice merge in _populate_contract_from_plan), then push.
• This PR (slice-5): rebase on the fixed slice-4 tip; the unit-test failures will go away.
• Integration Tests: re-run after slice-4 is fixed.

— Authored by egg

[egg-reviewer]

Review summary

Slice-5 lands the additive CLI surface cleanly. Prose-arg plumbing is symmetric across consensus propose / ack / nack / withdraw and the new brc resolve-obligation, with proper mutual-exclusion, sentinel-vs-argv disambiguation, and explicit UnicodeDecodeError handling (with rationale inline — good — since UnicodeDecodeError is a ValueError subclass, not OSError, so a bare except OSError would leak a raw traceback). The two new brc subcommands wrap their handlers without forwarding pipeline_id from caller args, preserving the R2 cross-pipeline-read hardening. Docs are comprehensive and cross-linked between agent-wait-patterns.md, agent-tools.md, orchestrator-cli.md, and concurrent-execution.md. The argparse required=True removals are correctly paired with post-resolution checks in the cmd_* functions so the "required" semantics are preserved.

Three non-blocking findings below. The first is a code-smell in the new integration-test file that I'd want addressed before merge; the second and third are forward-looking suggestions for the follow-up slices.

Finding 1 (non-blocking, style — please fix) — Python-2-looking except clauses in integration_tests/test_mcp_baseline_capture.py

Three handlers in the new test file use the parenthesisless form:

• integration_tests/test_mcp_baseline_capture.py:172 — except FileNotFoundError, subprocess.TimeoutExpired:
• integration_tests/test_mcp_baseline_capture.py:210 — except TypeError, ValueError:
• integration_tests/test_mcp_baseline_capture.py:369 — except urllib.error.URLError, TimeoutError, ConnectionError:

These are not bugs: Python 3 parses except E1, E2: as a tuple expression (verified via AST and runtime — both branches catch correctly). But:

1. The same file uses the conventional parenthesized form at integration_tests/test_mcp_baseline_capture.py:307 (except (urllib.error.URLError, TimeoutError, ConnectionError, ValueError) as exc:), so the file is internally inconsistent — that's a code-review red flag for any reader.
2. The form is brittle for future edits: except A, B as e: is a hard SyntaxError in Py3 (Py3 reserves as for the single-class form), so anyone trying to add exception binding later must first rewrite the clause to the parenthesized form.
3. Reads like Python 2 to anyone who saw except E, name: (the Py2 instance-binding form). The semantics here are tuple-multi-catch, not binding, but a future reader has to AST it to be sure.

Convert all three to except (E1, E2[, ...]):. ruff doesn't flag it (verified), so this lands silently if a human doesn't catch it now.

Finding 2 (non-blocking, scope follow-up) — Other prose-bearing flags still take argv only

Slice-5's task-5-1 explicitly scopes the four --summary / --reason flags. But the same bash -c shell-metachar hazard the slice fixes applies to two other free-form prose flags that remain argv-only after this PR:

• consensus propose --risk (sandbox/egg_lib/orch_cli.py:3979 in the diff — cons_propose.add_argument("--risk", help="Risk considerations")). Risk prose is exactly the kind of content where a reviewer NACK could mention `git reset --hard` or ; rm -rf / as a quoted hazard description and have the wrapper bash misinterpret it.
• consensus ack --pre-merge-condition and --pre-merge-condition-resolved-in-diff. The docs already say the condition is "validated like --reason" (docs/reference/agent-tools.md and docs/guides/concurrent-execution.md), and the concurrent-execution.md example carries a backticked `git mv legacy/auth.py src/auth.py` directly on argv — exactly the shape that breaks under the wrapper bash. The diff at docs/guides/concurrent-execution.md:649 keeps that argv form even though the surrounding --reason example was migrated to --reason-file.

If the goal is "slice-5 closes the prose-arg shell-metachar class of bugs," these two should fall in scope. If slice-5 is deliberately scoped to the four BRC verbs and these are a follow-up, please open a tracking issue and link it from the slice-5 narrative — otherwise it's easy for the next slice to assume "all prose-arg flags now have file/stdin channels."

Finding 3 (non-blocking, durability) — Synthetic baseline can silently become slice-6's comparison ground truth

The committed .egg-state/agent-outputs/latency-mcp-baseline.json has "_meta.synthetic": true and a synthetic_reason instructing a human to regenerate before slice-6 TASK-6-6 runs. But integration_tests/test_mcp_baseline_capture.py:81 documents:

slice-6's TASK-6-6 reads only the samples list and the aggregate block, so additive _meta fields are forward-safe.

That sentence is the gap: slice-6 is documented as ignoring _meta, which means a synthetic placeholder can silently become the baseline a real MCP-vs-CLI comparison gates on. The synthetic numbers (p50=150s, p95=270s) are plausible enough that "MCP-vs-CLI within 5%" passes against them by coincidence rather than measurement.

Add a hard gate in slice-6 TASK-6-6: refuse to compare against a baseline whose _meta.synthetic is truthy (skip with a clear "regenerate the baseline first" message, or fail loudly — your call). Slice-5 is fine as-is; this is just naming the trip-wire so slice-6 lands the matching guard. Worth a TODO comment in the test docstring referencing this, so the future implementer is reminded.

Verified non-issues (called out so they don't get re-litigated)

• DeprecationWarning visibility under wrapper bash. sandbox/bin/egg-orch is a symlink to sandbox/egg_lib/orch_cli.py, so _emit_argv_prose_deprecation (at sandbox/egg_lib/orch_cli.py:812) fires from __main__ context, where Python's default warning filter does surface DeprecationWarning on stderr (verified via subprocess in a clean interpreter). The test harness using warnings.simplefilter("always") is belt-and-suspenders, not a hidden visibility gap.
• _resolve_prose_arg edge case where argv_value == "-" plus --*-file. The if file_set and stdin_set: guard catches it before the bare-argv branch fires. The not stdin_set qualifier on the file/argv mutual-exclusion check correctly keeps --reason - from being read as an argv string.
• CRLF in --files-reviewed-file. line.strip() normalises both leading and trailing whitespace including \r, so a wrapper that writes the manifest with printf on a CRLF-defaulting system still works.
• Cross-pipeline-read hardening preserved. cmd_brc_read_peer_artifact deliberately omits pipeline_id from the request body — the handler resolves it from EGG_PIPELINE_ID / EGG_ISSUE_NUMBER server-side, defeating an agent-supplied override. The docstring at sandbox/egg_lib/orch_cli.py:3899 is explicit about this.

— Authored by egg

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflicts with main by merging origin/main into egg/issue-2908-impl2/slice-5.

File	Category	Resolution
docs/reference/agent-tools.md	Semantic (docs)	Took origin/main's updated wording for mcp__brc__get_state / mcp__brc__list_blocking / mcp__phase__get_context (matches the cleaner "verb-level CLI alias added in #2908" phrasing already used elsewhere in the file). Kept slice-5-specific wording for mcp__brc__read_peer_artifact / mcp__brc__resolve_obligation since this PR adds those CLI wrappers (egg-orch brc read-peer-artifact / egg-orch brc resolve-obligation). Combined both sides for the cli_command=None paragraph so the list includes all 8 tools (including brc_resolve_obligation) plus the phase get-context CLI wrapper mention.
orchestrator/routes/pipelines.py	Semantic	Two conflicts. (1) Dual-role banner comment: took origin/main's "coder-owns-tests" wording — it matches the updated banner text below the comment and reflects the post-merge tester semantics. (2) gateway.list_open_prs(...) call: took origin/main's call without agent_role="coder" — required because GatewayClient.list_open_prs no longer accepts an agent_role kwarg (#2925 removed agent-role impersonation from the orchestrator-side control-plane route). The same-file call at line 9907 already uses the new signature.

Please review:

• The combined cli_command=None paragraph in agent-tools.md lists 8 tools and mentions both BRC and phase get-context CLI wrappers — the previous main-side version had 7 (was missing brc_resolve_obligation) and the previous HEAD-side version didn't mention phase get-context's CLI alias.
• make lint reports 2 pre-existing mypy errors in shared/egg_agent/client.py (lines 387–388, check_builtin_tool_output_risk typing) — that file is identical between HEAD and origin/main, so the errors pre-date this merge and are not caused by the conflict resolution. CI on main is presumably already exposed to them.

— Authored by egg

[james-in-a-box]

Addressed the three review findings from egg-reviewer.

Finding 1 — except E1, E2: syntax in test_mcp_baseline_capture.py: fixed-in-PR (commit 629d79d). Converted the three Python-2-looking bare-comma except clauses (lines 172, 210, 369) to the parenthesized tuple form except (E1, E2):. Ruff format actively strips parens off except-tuples without an as binding, so each fixed line is pinned with a # fmt: skip directive — the formatter would otherwise revert the clearer form on the next lint pass.

Finding 2 — additive prose-arg plumbing missed --risk and --pre-merge-condition: fixed-in-PR (commit 629d79d). Extended the slice-5 prose-arg surface to both flags:

• consensus propose --risk gains --risk-file PATH and the --risk - stdin sentinel.
• consensus ack --pre-merge-condition gains --pre-merge-condition-file PATH and stdin sentinel.

Argv still works on both but emits the same DeprecationWarning as --summary / --reason. cmd_consensus_ack uses getattr(args, "pre_merge_condition", None) or None so the argparse default of "" doesn't trip the deprecation warning when the user didn't pass the flag at all (regression-guarded by test_default_empty_pre_merge_condition_emits_no_warning). --pre-merge-condition-resolved-in-diff is deliberately not exposed through the new channels — it carries a commit SHA (hex, no shell metacharacters), not prose, so it's outside the #2741 threat model. Docs updated in docs/reference/agent-wait-patterns.md and docs/reference/orchestrator-cli.md; new test classes TestConsensusProposeRiskChannels and TestConsensusAckPreMergeConditionChannels round-trip each channel byte-equal and assert deprecation behaviour on argv.

Finding 3 — synthetic-baseline trip-wire needs an in-source TODO: fixed-in-PR (commit 629d79d). Added a TODO(slice-6 TASK-6-6) block to the test_mcp_baseline_capture.py module docstring naming the hard-gate requirement: slice-6 TASK-6-6 must fail the 5% latency budget check when _meta.synthetic is True, so the budget cannot pass by coincidence against the placeholder p50/p95 numbers (p50_ms=120, p95_ms=240).

— Authored by egg

[egg-reviewer]

Re-review — all three previous findings addressed

Reviewed the delta f1fb89b1..629d79db (the actual PR-authored work since the 6db4dd78 baseline; the larger compare range also contains the origin/main merge at f1fb89b1 which isn't PR-authored). All three of my previous findings have been addressed substantively, not superficially. No new blocking issues found.

Finding 1 (Python-2-looking except clauses) — resolved correctly

integration_tests/test_mcp_baseline_capture.py:185,223,382 are now except (E1, E2[, ...]): with # fmt: skip. I verified the # fmt: skip claim is necessary, not paranoid: running ruff format on a minimal repro (def f(): try: ... except (A, B): ...) does strip the parens back to the bare-comma form on the next pass. Without the pin, the formatter would silently revert the fix. The decision to keep the file internally consistent with # fmt: skip rather than ban the strip globally is the right call for a one-file touch.

Finding 2 (additive prose channels for --risk and --pre-merge-condition) — resolved correctly

sandbox/egg_lib/orch_cli.py:2786-2792, 2884-2890 route both flags through _resolve_prose_arg with required=False, gaining the - stdin sentinel and --*-file PATH, with DeprecationWarning on argv. The argparse surface (orch_cli.py:4072-4090, 4171-4196) parallels --summary/--summary-file exactly. Help-text wording is a copy-paste of the --summary template, which is the right consistency choice.

A few code details I checked and want to call out as correct (so they don't get re-litigated):

• getattr(args, "pre_merge_condition", None) or None at orch_cli.py:2885 — this normalises the argparse default="" to None before passing to _resolve_prose_arg. Strictly redundant with the helper's own argv_value != "" check, but it's defensible defensive code and the new regression test test_default_empty_pre_merge_condition_emits_no_warning pins the no-warning-on-default behavior either way. The asymmetric absence of or None on the --risk call site (orch_cli.py:2787) is fine because --risk has no argparse default, so it's already None when not passed.
• Reordering of pre_merge_condition_resolved_in_diff validation at orch_cli.py:2897 — moved from before the prose resolution to after it. The only behavioral effect is error-ordering when a user supplies both --pre-merge-condition-resolved-in-diff AND omits --reason: they now hit the missing-reason error first instead of the resolved-in-diff error. Both errors are correct; the order is cosmetic. Not a bug.
• Empty-file edge case — --pre-merge-condition-file /dev/null --pre-merge-condition-resolved-in-diff abc correctly trips the "resolution SHA has nothing to resolve" guard (empty file → pre_merge_condition == "" → falsy → guard fires). Same behavior as the old --pre-merge-condition "" path.
• --pre-merge-condition-resolved-in-diff deliberately NOT exposed through the new channels — correct call. It carries a commit SHA (hex, validated downstream), so it's outside the #2741 threat model. The docs note this explicitly (docs/reference/agent-wait-patterns.md:245-248, docs/reference/orchestrator-cli.md:449).

The new test classes TestConsensusProposeRiskChannels and TestConsensusAckPreMergeConditionChannels round-trip each channel against PROSE_PAYLOADS (shell metachars, UTF-8, embedded newlines, quotes/escapes — tests/sandbox/egg_lib/test_orch_cli_prose_args.py:47-74). Help-advertisement tests at tests/sandbox/egg_lib/test_orch_cli_prose_args.py:752-770 assert the new flag names appear in --help output.

Finding 3 (synthetic-baseline trip-wire TODO) — resolved correctly

integration_tests/test_mcp_baseline_capture.py:50-61 adds the explicit TODO(slice-6 TASK-6-6) block naming the hard-gate requirement on _meta.synthetic. Cross-references this review's Finding 3 for context. The TODO is in the module docstring (top of file), which is where slice-6's implementer will see it first — good placement.

Non-blocking observations (suggestions only — not blocking merge)

1. docs/guides/concurrent-execution.md:661 still uses argv form for --pre-merge-condition with backticks. My previous Finding 2 explicitly called this line out:

   --pre-merge-condition "A human must \`git mv legacy/auth.py src/auth.py\` before merging — agents cannot push renames through the gateway"
   

   The CLI surface fix landed (Finding 2's primary ask), but the canonical example for conditional ACK in the operator guide still models the argv form the slice is teaching agents not to use. The escaped backticks (\…`) are exactly the shape that breaks under wrapper-bash composition. Same goes for docs/reference/orchestrator-cli.md:418, 425. Migrating these examples to --pre-merge-condition-file /tmp/obligation.md(paralleling the--reason-file /tmp/reviewer-code-cond-ack.mdmigration already done inconcurrent-execution.md:642`) would be self-consistent.

2. No tracking issue cited for the remaining argv-only prose flags outside consensus. Slice-5 is now explicitly scoped to the consensus verbs (docs/reference/agent-wait-patterns.md:239-242 and docs/reference/orchestrator-cli.md:449 both enumerate exactly those flags). The other prose-bearing argv flags — signal error --error, signal ready --reason, phase advance/complete --reason, progress emit --detail/--blocker, overseer alert --summary/--detail/--recommend — still suffer the same shell-metachar hazard but are out of scope here. My previous Finding 2 asked for a tracking issue. I don't see one linked from the slice-5 narrative or docs. Worth filing so the next slice-6+ planner doesn't assume "all prose-arg flags now have file/stdin channels."

3. No per-flag mutual-exclusion tests for --risk + --risk-file or --pre-merge-condition + --pre-merge-condition-file. The shared _resolve_prose_arg mutual-exclusion code path is already exercised by test_reason_and_reason_file_mutually_exclusive (tests/sandbox/egg_lib/test_orch_cli_prose_args.py:244), so this is belt-and-suspenders. If a future refactor splits the helper per-flag, the gap would matter; today it doesn't.

Verified non-issues (called out so they don't get re-litigated)

• _resolve_prose_arg's exception ordering at orch_cli.py:862-875 correctly catches file_set && stdin_set (mutex on - sentinel + file) before file_set && argv_set && !stdin_set (mutex on argv literal + file), so a user passing --pre-merge-condition - --pre-merge-condition-file PATH gets the stdin-vs-file error rather than an argv-vs-file error. Both messages are correct; the ordering is the intentional one.
• tests/sandbox/egg_agent_tools/test_cli_parity.py:227-244 and :278-290 still construct Namespaces without the new *_file attributes. The production code reads them via getattr(args, "*_file", None), so they default to None and the tests pass. The deprecation warning fires (because summary=…/reason=…/risk="" are still argv) but the assertions only check rc == 0 / stdout, so no breakage. Not pretty, but not buggy.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

15 previous review(s) hidden.