docs: steer agents to the structured contract tool, not Bash-composed egg-contract by jwbron · Pull Request #2741 · jwbron/egg

jwbron · 2026-05-20T19:41:44Z

Summary

Agents corrupt HITL decision/feedback text by composing egg-contract command strings for the Bash tool. Shell metacharacters in LLM-authored prose (backticks, $(...), $VAR, <, >) get interpreted by the shell — silently mangling the text, or executing a backtick/$(...) span as a command.
A shell-free path already exists on both harnesses (mcp__sdlc__* on claude_agent_sdk, the EggContract tool on EGG_HARNESS=egg). This updates the agent-facing steering so it actually lands.
Fixes the two reasons the prior steering failed: it argued efficiency ("avoids a subprocess + JSON parsing step") rather than correctness, and it named only the mcp__sdlc__* tools, which do not exist on EGG_HARNESS=egg.

Changes

sandbox/agent-config/rules/contract.md — callout leads with the real failure mode (shell corruption) and covers both harnesses, naming the EggContract tool for EGG_HARNESS=egg. The MCP-equivalents section is scoped to claude_agent_sdk and no longer uses the efficiency rationale.
shared/egg_harness_integration/egg_tools.py — the EggContract tool description now instructs the agent to pass each flag and value as a separate args element and never route egg-contract through the Bash tool.

Notes

This is a mitigation, not a guarantee — prompt steering lowers recurrence but cannot eliminate it. Command substitution happens in the agent's own Bash tool (bash -c), not the gateway (which runs everything via argv subprocess.run, no shell). The clean structural escalation, if corruption recurs, is to relocate egg-contract out of the shared sandbox/bin PATH directory so the Bash tool cannot resolve it.

Test plan

make lint — clean (only pre-existing soft-cap warnings on untouched files)
make test — 18,139 passed, 44 skipped, 0 failed
Docs + tool-description change only; no runtime code paths affected

… egg-contract Agents register HITL decisions and feedback by composing an `egg-contract` command string and running it through the `Bash` tool. The free-text fields (--question, --options, --notes) carry LLM-authored prose; in a shell command string the shell interprets backticks, $(...), $VAR, <, >, etc. — silently corrupting the text, or executing a backtick/$(...) span as a command. A shell-free path already exists on both harnesses (mcp__sdlc__* on claude_agent_sdk, the EggContract tool on EGG_HARNESS=egg), but the existing steering did not land: contract.md justified "prefer MCP tools" with an efficiency rationale and named only the claude_agent_sdk MCP tools, which do not exist on EGG_HARNESS=egg. - contract.md: lead with the real reason (shell corruption) and cover both harnesses, naming the EggContract tool for EGG_HARNESS=egg. - EggContract tool description: instruct the agent to pass each flag and value as a separate `args` element and never route egg-contract through the Bash tool. Mitigation, not a guarantee: it removes the efficiency-only framing and the harness gap, the two reasons the prior steering failed to land.

egg-reviewer

No agent-mode design concerns. Steering agents toward the existing structured contract tools (mcp__sdlc__* / EggContract) instead of Bash-composed egg-contract is good agent-tool-selection guidance — it informs rather than constrains, and the PR description appropriately frames this as a soft mitigation with the structural escalation path (relocating egg-contract out of the Bash PATH) called out explicitly.

— Authored by egg

egg-reviewer

Summary

Documentation/tool-description change only. The reasoning is sound: agents composing egg-contract … --question "…" --options "…" --notes "…" for the Bash tool can have free-text prose silently mangled (<, >, ;, |, &) or executed (backticks, $(...), $VAR) by bash -c. Both harnesses already expose a shell-free path — mcp__sdlc__* / mcp__task__* / mcp__phase__* on claude_agent_sdk, and the EggContract tool (asyncio.create_subprocess_exec, no shell) on EGG_HARNESS=egg. Switching the steering from "efficiency" to "correctness / shell corruption" and naming both harness paths is the right framing.

I verified the structural claims:

shared/egg_harness_integration/egg_tools.py:_make_cli_handler builds argv = [executable, command] + args and runs it via create_subprocess_exec — no shell, confirming the description's claim that fields are passed as data.
tests/tools/test_rule_doc_drift.py scans for Prefer this over \egg-…`` lines per rule doc; every list item in the renamed "MCP tool equivalents" section preserves that phrasing, so the drift gate still passes.
No documents link to the renamed #prefer-mcp-tools-over-the-cli anchor (grepped both .md and .py).
sandbox/entrypoint.py:938 setup_agent_rules concatenates contract.md into ~/.claude/CLAUDE.md, so the steering reaches the agent at session start.

No blocking issues.

Non-blocking suggestions

The same failure mode applies to egg-orch, and the PR leaves that rule file with the rationale it just deprecated. sandbox/agent-config/rules/orchestrator.md:33 still leads with "avoid a subprocess + JSON parsing step" — the efficiency framing this PR explicitly identifies as the reason the prior steering failed. And several egg-orch subcommands carry LLM-authored free text that is just as exposed in a Bash command string:
- egg-orch overseer alert --summary "…" --detail "…" --recommend "…"
- egg-orch progress emit --step "…" --detail "…" --blocker "…"
- egg-orch signal error --error "…"
- egg-orch anchor init --task "…"
An agent dropping a markdown code span or a <thing> placeholder into any of these via Bash hits the same corruption. A follow-up that lifts the same callout into orchestrator.md (and probably checkpoint.md, though its free-text surface is smaller) would actually close the failure mode — leaving it scoped to contract.md for now is fine, but worth filing.
EggContract description undersells its scope. The text says "task completion, phase management, decisions, and feedback" but the tool also handles show, verify-criterion, add-commit, and update-notes. Same omission existed before — but since the description is being revised anyway, "every egg-contract subcommand" or an explicit "show, verify-criterion, add-commit, update-notes, complete-task, complete-phase, add-decision, add-feedback" reads as a more accurate steering signal to a model deciding whether EggContract covers its case.
Section-heading inconsistency across rule docs. This PR renames ## Prefer MCP tools over the CLI → ## MCP tool equivalents (\claude_agent_sdk` harness)incontract.mdwhileorchestrator.md:33andcheckpoint.md:23keep the old heading. If the rename signals "this is harness-specific," the same is true of the other two files (themcp__*tools are only registered onclaude_agent_sdk`). Worth normalizing in the follow-up suggested in (1).

— Authored by egg

… rule docs Closes the failure mode the prior commit only scoped to contract.md. The same shell-metacharacter corruption applies to egg-orch (overseer alert --summary/--detail/--recommend, progress emit --step/--detail/ --blocker, signal error --error, anchor init --task) and to egg-checkpoint search --text — both have shell-free paths on each harness. - sandbox/agent-config/rules/orchestrator.md, checkpoint.md — add the same callout (use structured tool, not Bash) and replace the efficiency-framed rationale ("avoid a subprocess + JSON parsing step") with the shell-corruption framing. Section heading normalized to "MCP tool equivalents (claude_agent_sdk harness)" matching contract.md. - shared/egg_harness_integration/egg_tools.py — EggContract description now enumerates every egg-contract subcommand (show, add-commit, complete-task, complete-phase, update-notes, verify-criterion, add-decision, add-feedback) rather than the partial "task completion, phase management, decisions" list. EggOrch and EggCheckpoint descriptions gain the matching "do not invoke through Bash" guidance so the EGG_HARNESS=egg path has the same steering as the rule docs. Addresses review feedback on #2741.

james-in-a-box · 2026-05-20T20:24:09Z

Thanks for the thorough review. All three non-blocking suggestions addressed in 0ddf4ba.

Per-item disposition:

Propagate the callout to orchestrator.md and checkpoint.md — fixed-in-PR (commit 0ddf4ba). The same shell-corruption mode applies to egg-orch overseer alert --summary/--detail/--recommend, progress emit --step/--detail/--blocker, signal error --error, and anchor init --task, and to egg-checkpoint search --text. Both rule docs now lead with the corruption framing, point at the shell-free path on each harness (mcp__brc__* / mcp__progress__* / mcp__checkpoint__* on claude_agent_sdk; EggOrch / EggCheckpoint on EGG_HARNESS=egg), and drop the efficiency rationale.
EggContract description undersells its scope — fixed-in-PR (commit 0ddf4ba). The description now enumerates every subcommand the tool handles: show, add-commit, complete-task, complete-phase, update-notes, verify-criterion, add-decision, add-feedback. Took the explicit-list option rather than "every egg-contract subcommand" because the enumeration also serves as inline doc when an agent reads the tool description while deciding which tool to reach for. Applied the same expansion to EggOrch and EggCheckpoint so the EGG_HARNESS=egg path has matching guidance.
Section-heading inconsistency across rule docs — fixed-in-PR (commit 0ddf4ba). orchestrator.md and checkpoint.md now both use ## MCP tool equivalents (\claude_agent_sdk` harness), matching the rename in contract.md. The drift-gate (tests/tools/test_rule_doc_drift.py) still passes since the Prefer this over `egg-…`` idiom on each list item is preserved (28 tests pass).

Verification:

tests/tools/test_rule_doc_drift.py — 28 passed (the drift gate over all rule docs)
shared/tests/test_egg_harness/ — 507 passed, 12 skipped (covers egg_tools.py's registration path)
make lint — clean (only pre-existing soft-cap warnings on untouched files)

— Authored by egg

egg-reviewer

No agent-mode design concerns. The delta extends the shell-corruption framing from contract.md into orchestrator.md and checkpoint.md, and enumerates the full subcommand surface in the EggOrch / EggContract / EggCheckpoint tool descriptions. This is good agent-tool-selection steering — it orients the agent toward the existing shell-free structured surface (so free-text fields stop being mangled by bash -c) without pre-fetching context or constraining how the agent works. The enumerated subcommands serve as useful inline documentation at tool-selection time.

— Authored by egg

egg-reviewer

Summary

Re-reviewed the delta from f3122ff → 0ddf4ba (commit docs: propagate shell-corruption callout to orchestrator + checkpoint rule docs). All three non-blocking suggestions from the prior review are addressed; no new issues introduced.

Verification

Suggestion 1 — callout propagation. sandbox/agent-config/rules/orchestrator.md:5-23 and sandbox/agent-config/rules/checkpoint.md:5-20 now lead with the shell-corruption framing, list the exact metacharacter set (backticks, $(...), $VAR, <, >, ;, |, &), enumerate the LLM-authored free-text flags per CLI, and point at the structured tool on each harness. The efficiency rationale is gone from both files (grep -rn "subprocess + JSON parsing step" returns no source-file hits — only one .egg-state/brc-history/ artifact).

I spot-checked that every flag named in the callouts actually exists:

egg-orch overseer alert --summary/--detail/--recommend — confirmed via --help.
egg-orch progress emit --step/--detail/--blocker — confirmed.
egg-orch signal error --error — confirmed.
egg-orch anchor init --task — confirmed at orchestrator/cli.py:1078.
egg-checkpoint search --text — confirmed; it's the only LLM-authored free-text field across the six egg-checkpoint subcommands.

Suggestion 2 — EggContract scope. shared/egg_harness_integration/egg_tools.py:137-147 now enumerates every subcommand (show, add-commit, complete-task, complete-phase, update-notes, verify-criterion, add-decision, add-feedback). The expansion was also applied to EggOrch (lines 124-135) and EggCheckpoint (lines 149-158), both of which now carry the same "Never invoke through Bash" steering with the specific free-text flags named — a tighter signal to the model than the previous one-liners.

Suggestion 3 — section-heading normalization. All three rule docs (contract.md:41, orchestrator.md:53, checkpoint.md:40) now use ## MCP tool equivalents (\claude_agent_sdk` harness). No broken anchor references — grep -rn "prefer-mcp-tools-over-the-cli"returns nothing in.md/.py`.

Drift gate. I ran tests/tools/test_rule_doc_drift.py locally — 28 tests pass. The regex (Prefer this over \egg-…`) still matches every list item in the renamed sections, and the (path, tool_name, cli_str)parametrize set still resolves to liveTOOL_REGISTRY` entries.

Harness tests. shared/tests/test_egg_harness/ — 507 passed, 12 skipped. The _make_cli_handler factory still builds argv = [executable, command] + args and runs it through asyncio.create_subprocess_exec (no shell), which is what the new descriptions promise.

Non-blocking

Heading naming inconsistency outside the three rule docs. sandbox/egg_lib/data/hitl_editing_rules.md:22 still uses ## MCP-tool equivalents (HITL-edit harness) — different hyphenation (MCP-tool vs MCP tool) and a different parenthetical scope (HITL-edit harness vs \claude_agent_sdk` harness). The HITL editing harness has a different audience and the free-text-corruption argument doesn't apply to its read-only tool surface (show_contract, check_hitl_answers`), so this may be intentional — but if you're closing the harness-naming-drift gap, this is the one file left out. Not blocking and out of scope for this PR.

— Authored by egg

Drops the hyphen in 'MCP-tool' so the heading matches the other three rule docs (contract.md, orchestrator.md, checkpoint.md), all of which use 'MCP tool equivalents'. The parenthetical scope is kept as '(HITL-edit harness)' rather than '(claude_agent_sdk harness)' because this rule doc genuinely targets a different harness with a read-only tool surface (show_contract, check_hitl_answers) where the shell-corruption argument doesn't apply.

james-in-a-box · 2026-05-20T20:55:24Z

Thanks for the re-review. The one non-blocking suggestion is addressed in e83449c.

Per-item disposition:

Heading naming inconsistency outside the three rule docs (sandbox/egg_lib/data/hitl_editing_rules.md:22) — fixed-in-PR (commit e83449c) for the hyphenation half; disagree (scope is genuinely different) for the parenthetical half.
- Hyphenation (MCP-tool → MCP tool): normalized. The heading now reads ## MCP tool equivalents (HITL-edit harness), matching the formatting of contract.md:41, orchestrator.md:53, and checkpoint.md:40. The drift gate (tests/tools/test_rule_doc_drift.py) already covers hitl_editing_rules.md via _RULE_DOC_GLOBS and still passes (28 tests) since the rename touches the section heading, not the per-line Prefer this over regex.
- Parenthetical scope ((HITL-edit harness) kept as-is, not changed to (claude_agent_sdk harness)): you flagged this as possibly intentional, and it is. The HITL editing harness is a separate audience — read-only contract reads (show_contract, check_hitl_answers), no shell-composition surface, and the shell-corruption framing the other three rule docs lead with does not apply. Renaming the parenthetical would mis-describe the file's scope. Leaving it as (HITL-edit harness) keeps the section labeled accurately.

Verification:

tests/tools/test_rule_doc_drift.py — 28 passed (the drift gate over all rule docs, including hitl_editing_rules.md).
make lint — clean (only pre-existing soft-cap warnings on untouched files).

— Authored by egg

egg-reviewer

No agent-mode design concerns. The delta since 0ddf4ba is a one-character heading normalization in sandbox/egg_lib/data/hitl_editing_rules.md (MCP-tool → MCP tool) for consistency with the three sibling rule docs. The parenthetical scope ((HITL-edit harness)) is correctly kept distinct since the HITL-edit harness exposes a read-only contract surface where the shell-corruption framing doesn't apply. No new design surface introduced.

— Authored by egg

egg-reviewer

Summary

Re-reviewed the delta from 0ddf4ba → e83449c (single commit: docs: normalize 'MCP tool equivalents' heading in hitl_editing_rules.md). One-character whitespace fix in a markdown heading. Addresses the single non-blocking suggestion from the prior re-review.

Verification

The heading rename: sandbox/egg_lib/data/hitl_editing_rules.md:22 now reads ## MCP tool equivalents (HITL-edit harness) — matching the formatting of contract.md:41, orchestrator.md:53, and checkpoint.md:40 on the MCP tool (unhyphenated) half.
Parenthetical-scope disagreement is well-reasoned and I agree. The HITL-edit harness's tool surface is read-only (show_contract, check_hitl_answers) — the shell-corruption framing the other three rule docs lead with does not apply, and renaming the parenthetical to (claude_agent_sdk harness) would mis-describe the file's audience. Keeping (HITL-edit harness) is correct.
No broken anchors. grep -rn '#mcp-tool-equivalents\|#prefer-mcp-tools-over-the-cli' returns no source-file hits (.egg-state/ pipeline artifacts excluded). The remaining MCP-tool hyphenated occurrences in source (shared/prompts/security-review-criteria.md:192, docs/reference/agent-tools.md:463, orchestrator/substrate/claude_code/hook_entry.py:46) are compound-modifier prose, not heading anchors.
Drift gate impact. The change touches a section heading; the Prefer this over \egg-…`regex intests/tools/test_rule_doc_drift.py` operates on list items, so the gate is unaffected — confirmed by the author's local run (28 passing).

No new issues. The PR closes the harness-naming-drift gap cleanly.

— Authored by egg

james-in-a-box · 2026-05-20T21:08:00Z

egg review completed. View run logs

9 previous review(s) hidden.

Blocking fixes: - Replace every `claude -p` reference with `python3 -m egg_agent` via `build_agent_command()` per refine analysis lines 41-58 + the in-production primitive at consensus_wrapper.py:748-759. `claude --print` is EGG100-linted per agent-mode-design.md:90-104. - Spell out cq-3 durability fix: NEW `commit_and_push_sync` variant of save_pipeline + NEW `_startup_reconciliation_replay_safety_budget` step. Existing best-effort-async-push (state_store.py:11/:804-805/ :890-928) does NOT meet cq-3 — contract_store.py:139-142 confirms 'pre-push crashes' failure mode. - Subdivide v1 slice-2 into 2 (schema), 3 (endpoint), 4 (CLI verbs) along orchestrator-route / sandbox-CLI / contract-schema seams. - Commit slice-5 (was slice-3) memory shape to distilled / rewrite- and-distill default with rationale on BOTH axes (cache cost AND orient-don't-constrain per agent-mode-design.md). Non-blocking addressed: - patterns.py allowlist citation tightened to span all BRC roles (362-516+); matchers.py:33 evidence for prefix-glob subdir coverage. - slice-5 explicit dict-arg handler path (not argv — preserves #2741). - slice-6 explicit use of existing wait-loop CLI (preserves #2323). - slice-6 cutover playbook (drain in-flight before deploy). - slice-7 prior-fix preservation audit (#2323/#2064/#2482/#1995/ #2036/#2451/#2142/#2725). - R5 mitigation contract clarity. - Slice DAG grows from 6 to 8 slices; forest constraint preserved.

…nalysis + scaffold Pre-emptively addresses the three blocking-shape concerns surfaced by risk_analyst's plan-phase risk register, before reviewer_plan rolls them into a NACK: - BC-1: slice-1 spike measurement constraint updated. Cache numbers must come from `python3 -m egg_agent` invocations with the production BRC preamble + 31 MCP tool schemas, not raw `claude`. cost_callback source corrected (kubectl logs against litellm pod, not host ~/.local/state/clm/). Folded as decision d-9 + risk. - BC-2: slice-5 wrapper rewrite must pass per-event prompts via shlex.quote argv OR stdin/tempfile (mirrors the existing consensus_wrapper.py:759-760 pattern). Regression test for shell metachars mandated. Folded as decision d-10 + risk; #2741 explicit. - BC-3: slice-4 durable safety budget gets explicit success + partial-failure semantics. Typed exception on push failure; consumer falls back to in-memory snapshot + OVERSEER_ALERT; bounded retry documented. Folded as decision d-11 + risk. No structural slice change — still 8 slices in a linear forest. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…fix slice-2 verification Addresses both blockers in reviewer_plan's v1 NACK and folds in both reviewers' non-blocker suggestions. Blocker 1 (reviewer_plan + risk_analyst R4): split the former MCP→CLI collapse slice at the additive/deletion seam. slice-5 ships the additive stdin/file prose plumbing for `consensus propose/ack/nack` and the two net-new BRC CLI subcommands (`brc resolve-obligation`, `brc read-peer-artifact`) with the #2741 regression-guard test. slice-6 then mechanically deletes the 28 MCP tools, the SYSTEM_PROMPT_NUDGE block, and the EGG_MCP_TOOLS env-flag check (including the client.py:311 gate — no orphan flag), and migrates the test surface so the e2e exercise survives as a stdin/file `consensus ack/nack` exercise. Blocker 2 (reviewer_plan): rewrote slice-2 verification. ScriptedProvider is unit-test-only and cannot drive a deployed event pump end-to-end (per integration_tests/regression/test_brc_concurrency.py:1-25 and #2474). Slice-2 now verifies via wrapper-rendering snapshot tests + slice_id-propagation assertion (risk_analyst R9) + in-process PeerConsensusTracker regressions, with TRUE end-to-end deferred to the slice-4 spike on issue-2270/qwen3.7-max. Non-blockers folded in (both reviewers): - §design.memory_schema added with `last_reviewed_commit_sha` per producer (risk_analyst R6, reviewer_plan adversarial-re-review concern), atomic-write contract (tempfile + os.replace via shared/egg_overseer/state.py:266 helper), fail-closed path construction (raise if EGG_AGENT_ROLE empty) - slice-3 acceptance: per-event prompt MUST include the full git-log delta `git log {last_reviewed_commit_sha}..HEAD --not origin/{base_branch} -p` per REVIEWER-SYNC.md, not just `changed_artifacts` - Pseudocode corrected: `consensus confirmed` not `progress complete`; `--append-context` flagged as illustrative-only (does NOT exist in build_agent_command today); added od-6 to pin the memory-delivery mechanism - slice-3 verification adds sandbox-image rebuild + agent restart for mission.md BEFORE slice-4 flag-flip - Three-point WS7 cache measurement schedule (baseline, post-slice-3, post-slice-6) - Explicit rollback plan for slice-4 spike falsification (git revert slices 1-3) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Updates the plan to track the architect v2 scaffold (6342b2d) that split slice-5 along the additive/deletion seam: slice-5 ships the stdin/file prose plumbing (#2741 regression guard) + two new BRC subcommands, slice-6 ships the MCP deletion + test migration. Total tasks now 42 across 6 slices (20 coder + 7 doc + 15 tester). v2-incorporated changes from architect/risk_analyst review: - Memory schema carries six required fields per architect design.memory_schema.required_fields incl. last_reviewed_commit_sha per producer (slice-1 TASK-1-6/1-7/1-9 acceptance pins all six). - Atomic memory writes via promoted shared/egg_overseer/state.py:266 _persist_atomic_template helper (slice-1 TASK-1-6). - Fail-closed memory-path constructor — raises if EGG_AGENT_ROLE unset/empty (slice-1 TASK-1-6 + TASK-1-9). - Slice-2 verification revised: wrapper-rendering unit test + heartbeat unit test with slice_id propagation assertion + in-process PeerConsensusTracker regression. ScriptedProvider cannot drive a deployed pod end-to-end; true E2E deferred to slice-4 (TASK-2-7 documents the rationale). - Slice-3 hands the agent the full git log {last_reviewed_commit_sha}..HEAD --not origin/{base_branch} -p delta per producer (NOT just changed_artifacts) per REVIEWER-SYNC.md (TASK-3-1/3-2/3-6 pin the command shape). - Memory delivery via inline tail position (architect od-6 Option B) rather than the illustrative --append-context flag that does not exist on build_agent_command (TASK-3-2). - mission.md sandbox-image rebuild pinned to slice-3 (TASK-3-4) BEFORE slice-4 flag flip; TASK-4-1 has a pre-flight that fails fast if the rebuild is not deployed. - Three-point WS7 cache measurement schedule: TASK-3-7 (baseline post-slice-3), TASK-4-1 (post-slice-4 spike), TASK-6-7 (post-slice-6 deletion). Regression > 20% at any boundary triggers HITL pause per risk_analyst R8. - Wrapper calls egg-orch consensus confirmed (existing CLI at orch_cli.py:2753) on role_complete, NOT a new progress complete command — old TASK-1-6 (progress complete) dropped. - Slice-6 deletes EGG_MCP_TOOLS env flag (no orphan); migrated E2E test preserves SDK-spawn exercise via stdin/file consensus ack/nack (NOT direct-handler collapse).

…t_prompt Slice-2's ``invoke_agent_for_event`` shipped a minimal stub prompt ("BRC event-pump handler / Role / Slice / Action / Event payload"); this commit wires it to the slice-3 ``compose_event_prompt`` via a CLI entry-point on ``orchestrator/routes/event_prompt.py``. The CLI is the wrapper-bash injection seam — calling ``compose_event_prompt`` directly from a Python heredoc would force the ``orchestrator.routes`` package ``__init__.py`` (which imports Flask) to load on the agent pod, and Flask is not in the sandbox runtime. The CLI bypasses that by being the script's ``if __name__ == "__main__":`` entry-point: the wrapper bash invokes ``python3 /opt/egg-runtime/orchestrator/routes/event_prompt.py <action>`` with the event_payload JSON on stdin, and the script imports its own neighbouring helpers without traversing the package init. CLI responsibilities (matches plan TASK-3-2): * Read ``EGG_AGENT_ROLE`` / ``EGG_BASE_BRANCH`` / ``EGG_REPO_PATH`` / ``EGG_BRC_MEMORY`` from env (set on every agent pod per ``orchestrator/kubernetes_spawner.py:818-823``). * Read the per-role memory file at ``.egg-state/agent-outputs/<role>/brc-memory.md`` iff ``EGG_BRC_MEMORY=full`` (slice-1 writer's read gate; slice-4 flips the default to ``full``). * Parse per-producer ``last_reviewed_commit_sha`` from the memory file's structured ``### <role>`` blocks (slice-1 writer schema) even in ``write-only`` mode — the architect plan splits the memory-excerpt gate (full only) from the SHA-lookup gate (always) so the wrapper still renders the per-producer delta against the fallback baseline. * For each ``last_reviewed_commit_sha``, run ``git log {sha}..HEAD --not origin/{base_branch} -p`` via subprocess inside the worktree. The gateway allows ``--not`` and ``-p`` on ``git log`` per #2905. * Extract the open-NACK list (``event_payload['nacks']`` / ``aggregated_nacks``) and forward it through. * Call ``compose_event_prompt`` and write the rendered prompt to stdout. The wrapper bash captures stdout into ``$prompt`` and passes it as the positional argument to ``python3 -m egg_agent``. On any failure (script missing, schema drift, git log subprocess crash) the wrapper falls back to the slice-2 stub so the event-pump keeps running rather than failing the agent invocation — the idle-budget safety net catches a wedged event-pump even under a degraded composer. The composer call obeys the env-flag split: * ``EGG_BRC_MEMORY=full`` — memory excerpt included; per-producer delta rendered. * ``EGG_BRC_MEMORY=write-only`` (slice-1 default) — memory excerpt omitted from prompt; per-producer delta still rendered against the memory file's stored SHAs as a fallback baseline. Matches the plan TASK-3-2 wording verbatim. * ``EGG_BRC_MEMORY=off`` — both omitted. Defensive safety rails added: * ``_GIT_LOG_TIMEOUT_SECS = 60`` so a hung gateway doesn't deadlock the event-pump. * ``_GIT_LOG_DELTA_MAX_BYTES = 256 KiB`` per producer with explicit truncation sentinel so a pathologically large refactor doesn't blow past Claude's context budget silently. * Stdin-based event_payload (#2741 prose-arg discipline) instead of argv. The TASK-3-3 preamble collapse and slice-3 tests land in follow-up commits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Document the slice-5 additive CLI surface across the four docs that already carry consensus-protocol prose: - docs/reference/orchestrator-cli.md * New "Prose-bearing args: stdin and --*-file channels (#2741)" subsection under ## BRC Consensus Protocol covering --summary-file, --reason-file, --files-reviewed-file, and the stdin sentinel `-`. * New "## BRC verb-level operations (egg-orch brc)" section documenting the next-action / get-state / list-blocking / resolve-obligation / read-peer-artifact subcommands. * Deprecation-warning note on the argv --summary / --reason path. - docs/reference/agent-tools.md * MCP↔CLI table: mcp__brc__get_state, mcp__brc__list_blocking (slice-1), and mcp__brc__read_peer_artifact, mcp__brc__resolve_obligation (slice-5) flipped from "no CLI" to their new egg-orch brc subcommands. * cli_command=None rationale list: drop the four promoted verbs and add a callout summarizing the slice-5 promotion. * Schema-derivation paragraph: shrink the "tools with no CLI" list accordingly. - docs/reference/agent-wait-patterns.md * Update re-propose / stale-version examples to use --summary-file / --reason-file (the canonical idiom for any wrapper-composed CLI). * New "Prose-bearing args use stdin / --*-file, not argv (#2741)" subsection under §1 with channel table, examples, and rationale. * Related Documentation: cross-link to the new orchestrator-cli.md BRC verb-level operations section and to #2741. - docs/guides/concurrent-execution.md * Refresh the worked Consensus Protocol example to use --summary-file for propose, --reason-file for ack, and the stdin sentinel for nack / withdraw. Add a brc resolve-obligation example. * New "egg-orch brc — verb-level read/derive surface" subsection cross-linking the canonical reference in orchestrator-cli.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…rface Slice-5 of the BRC event-pump rollout extends the CLI surface for the in-bash wrapper that ships in slice-2. Three coder tasks land here: task-5-1 — Prose-arg channels for consensus propose/ack/nack/withdraw. The wrapper composes CLI invocations via ``bash -c``, so argv-only prose (``--summary "$RAW"``, ``--reason "$RAW"``) gets corrupted by shell metacharacters (``$VAR`` / backticks / ``$()`` / ``;`` / ``&&`` / embedded newlines) — the #2741 failure mode. This slice generalises the mitigation: every prose-bearing arg now offers a paired ``--FOO-file PATH`` flag and accepts ``-`` as the argv sentinel for stdin. Argv prose still works for humans and during transition but emits ``DeprecationWarning`` so a regression to argv-only inside the wrapper surfaces. ``--files-reviewed-file PATH`` carries an array with one path per line (blank lines and ``#`` comments stripped), per architect v2 §verification_strategy.slice_5. Two helpers in orch_cli.py — ``_resolve_prose_arg`` and ``_resolve_files_reviewed_arg`` — handle channel selection (file → stdin → argv), enforce mutual exclusion, and emit the deprecation. task-5-2 — ``egg-orch brc resolve-obligation`` CLI. Verb-level wrapper around ``mcp__brc__resolve_obligation`` (#2338). Slice-6 deletes the agent-side MCP server, so the wrapper bash needs this verb reachable without an MCP round-trip. Args mirror the handler: ``--reviewer-role`` and ``--producer-role`` are required; ``--commit-sha`` and ``--note`` are optional. The ``--note`` flag uses the same prose-arg plumbing as the other reason / summary args. task-5-3 — ``egg-orch brc read-peer-artifact`` CLI. Verb-level wrapper around ``mcp__brc__read_peer_artifact``. Stdout JSON; pagination via ``--limit`` + opaque ``--cursor`` round-trip; ``--message-type`` is ``action="append"`` for repeated use; ``--no-include-unattributed`` opts out of the slice-scoped + cross-cutting merge (default on, per the handler's per-slice-partition contract from #2548). Tests authored by the coder (tester reviews-and-hardens): * ``tests/sandbox/egg_lib/test_orch_cli_prose_args.py`` — #2741 regression-guard. Parametrises seven representative prose payloads (``$VAR`` / backticks / ``$()`` / shell-control / newline+tab / UTF-8 / quotes+escapes) across each delivery channel (file, stdin sentinel, argv) for ``consensus propose``, ``ack``, ``nack``, and ``withdraw``. Asserts byte-equality between the on-disk / stdin input and the request body received by the orchestrator fake. Argv-path tests assert the ``DeprecationWarning`` fires. Mutual- exclusion paths return exit 2 with helpful stderr. ``--files- reviewed-file`` one-path-per-line semantics covered (blank lines + ``#`` comments stripped). The ``consensus propose --file`` JSON payload path (from issue #1738) is explicitly tested to NOT emit the deprecation warning — only the per-arg argv channels are deprecated. * ``tests/sandbox/egg_lib/test_orch_cli_brc.py`` — extends slice-1's test file with ``TestBrcResolveObligation`` (happy path / commit SHA / note via file / note via stdin / help / parser registration) and ``TestBrcReadPeerArtifact`` (happy path / peer-role filter / message-type list / limit+cursor pagination round-trip / no-include-unattributed default flip / phase choices restricted / help / parser registration). All 306 tests pass on the changed paths; existing consensus-push, slice-1 BRC, and CLI parity tests continue to pass unchanged. Files: sandbox/egg_lib/orch_cli.py; tests/sandbox/egg_lib/test_orch_cli_brc.py; tests/sandbox/egg_lib/test_orch_cli_prose_args.py. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…2949) * docs(#2908 slice-3 task-3-4..3-5): event-handler mission.md + slice-3 doc surface Documenter tasks for slice-3 of #2908: task-3-4: Rewrite the Concurrent Execution Mode section of sandbox/agent-config/rules/mission.md to the event-handler contract: the agent is invoked one-shot per actionable event by the BRC event-pump wrapper; act on the event, update BRC memory, exit naturally. Removes "never exit before the orchestrator stops you", the producer/reviewer STAY-ALIVE wording, and the explicit wait-loop / cursor plumbing — under the post-#2908 model the wrapper bash owns the lifecycle. Preserves Anti-Sycophancy, Structured Progress Reporting, HITL vs OVERSEER_ALERT, and Handling Agent Failures sections verbatim. A legacy-path note preserves the pre-flip contract until slice-4 flips the default. sandbox/claude-rules/mission.md is a symlink to sandbox/agent-config/rules/, so both paths reflect the change and the diff-empty acceptance assertion holds trivially. task-3-5: Extend docs/architecture/orchestrator.md and docs/reference/agent-wait-patterns.md with the slice-3 surface: the compose_event_prompt composer (10 KB envelope, 2 KB memory truncation, NACK payload from peer_consensus.py:949-1024, verbatim per-producer git-log delta scaled by change size), the full-delta adversarial re-review rationale (REVIEWER-SYNC.md contract + risk_analyst R6), tail-position memory delivery (architect od-6 Option B, sidesteps the non-existent --append-context flag), composer interplay with EGG_BRC_MEMORY across off / write-only / full modes, the _build_brc_preamble collapse (kept/removed table + the three caller sites unchanged), the sandbox-image rebuild trigger gating slice-4's flag flip, and the slice-3 unit/snapshot verification stance. Documents the architect's open-decision resolutions od-1 / od-2 / od-3 / od-4 / od-6 with their slice-1 / slice-2 / slice-3 implementation cites and explicitly marks od-5 as deferred to slice-6's MCP→CLI latency baseline. Cross-links between brc-memory.md (slice-1 writer), orchestrator.md (slice-3 architecture), and agent-wait-patterns.md §10.9 (slice-3 wait-side companion); brc-memory.md "How slice-3 reads it" now points to the landed reader-side sections. Refs #2908 tasks 3-4, 3-5 * docs(#2908 slice-3): address reviewer_code v1 non-blocking observations Three substantive corrections from reviewer_code's v1 ACK observations: 1. **Preamble collapse is unconditional, not gated by EGG_BRC_EVENT_PUMP.** The legacy-path note in mission.md previously implied the legacy wrapper still carried "the full event-blocking / cursor / persistent-session plumbing" in the preamble — but task-3-3 collapses _build_brc_preamble unconditionally, so both wrapper paths see the collapsed preamble. The legacy wrapper re-supplies wait / restart instructions through its own recovery system prompt baked into _CONSENSUS_WRAPPER_TEMPLATE, not through the preamble. Reword to clarify what the EGG_BRC_EVENT_PUMP flag actually selects (the wrapper, not the preamble), and propagate the same clarification through orchestrator.md and §10.9 of agent-wait-patterns.md. 2. **Heading "(behind EGG_BRC_MEMORY=full)" was misleading.** The composer itself runs whenever the event-pump branch runs (EGG_BRC_EVENT_PUMP), and only the memory-excerpt *content* is gated by EGG_BRC_MEMORY. The matrix table got this right but the section heading suggested the whole composer was behind the memory flag. Drop the "behind ..." qualifier from the H2/H3 headings and add a "Flag mapping (read this first)" callout at the top of both the orchestrator.md and agent-wait-patterns.md sections. Update all four cross-link anchors (orchestrator.md, agent-wait-patterns.md, brc-memory.md) to the new slug. 3. **Line-number references in a 24.5k-line file are doc-drift prone.** Add an explicit caveat noting the numbers come from the slice-3 contract spec and reflect post-collapse positions, and lean on function- and banner-name references (_build_brc_preamble, "the dual-mandate banner") for reading the live file. Apply the same softening in agent-wait-patterns.md §10.9.5. The reviewer also flagged that the sandbox image rebuild is not exercised by this slice — that's a slice-4 coordination obligation sitting outside the BRC review surface (documenter cannot exercise make build / make k3s-import / make deploy from the pod); the docs record the rebuild-trigger as required by task-3-4's acceptance criterion so slice-4 can verify the new image deployed before flipping EGG_BRC_EVENT_PUMP. No content change needed for that one. Refs #2908 tasks 3-4, 3-5 (re-propose v2 after reviewer_code ACK) * docs(#2908 slice-3): address reviewer_code v2 non-blocking observations Three substantive corrections from reviewer_code's v2 ACK observations. None are blocking; each addresses a factual / drift risk the reviewer surfaced inline. 1. **mission.md doc cross-references now use $EGG_REPO_PATH/docs/... convention.** The three new links at mission.md:153 (BRC memory artifact) and :165 (BRC Event-Pump Wrapper + agent-wait-patterns §10) previously used 2-dot relative paths (../../docs/...). The sibling rule files in the same directory (checkpoint.md, contract.md, orchestrator.md) use the $EGG_REPO_PATH/docs/... convention, and the same mission.md file's other doc references already follow that pattern. The 2-dot paths resolve to sandbox/docs/... from the source tree and /docs/... from the runtime mount at /opt/claude-rules/, neither of which exists. Switch to the dominant convention so the links are followable from any context. 2. **"exits 0 once role_complete flips" replaced with the actual flag name.** The role_complete flag does not exist anywhere in the code. The brc next-action route returns {"action": "complete"} (orchestrator/routes/consensus.py _VALID_ACTIONS + role-complete short-circuit) and the wrapper checks is_complete from the consensus status payload. Reword the third event-handler step to "exits 0 once `brc next-action` returns the `complete` action (i.e. the role is marked complete in `consensus status`)" so the doc no longer references a non-existent flag. 3. **peer_consensus.py line-number caveat propagated.** The slice-3 prompt-composer tables in both orchestrator.md (§"Per-event prompt composer") and agent-wait-patterns.md §10.9.1 cited peer_consensus.py:949-1024 for _open_nacks_barrier_response. The actual function spans 949–1046 in this branch — the same doc-drift risk the v2 commit already softened for the 24.5k-line pipelines.py. Extend the same caveat to the peer_consensus.py citation so future drift on either file is contained by the function-name reference rather than caught only by the line number. The reviewer also surfaced a coordination note: task-3-4's acceptance criterion is "rebuild produces a new image tag", which the documenter cannot exercise from the BRC pod (no sudo / k3s access). That half is structurally deferred to slice-4's pre-flag-flip kubectl-exec assertion (task-4-1 per the contract). The docs already record the rebuild trigger (make build / make k3s-import / make deploy) as required by the acceptance criterion. No content change needed — the slice-3 PR body should surface the deferral explicitly so slice-4 doesn't have to rediscover it. Refs #2908 tasks 3-4, 3-5 (re-propose v3 after reviewer_code v2 ACK with non-blocking observations) * feat(#2908 slice-3 task-3-1): compose_event_prompt per-event prompt composer Adds ``orchestrator/routes/event_prompt.py::compose_event_prompt`` — the slice-3 replacement for the slice-2 wrapper's minimal stub prompt at ``orchestrator/consensus_wrapper.py::invoke_agent_for_event``. Per the slice-3 plan TASK-3-1: - Positional signature ``(role, event_payload, memory_excerpt, nacks, git_log_delta, base_branch) -> str`` matches the contract verbatim so the wrapper bash's ``python3 -c`` call site is stable across refactors. - Memory excerpt rendered at the user-prompt TAIL position (architect od-6 Option B); the illustrative ``--append-context`` flag from the analysis pseudocode does not exist on ``build_agent_command`` (verified at ``shared/egg_agent/command.py:11-46``). - Per-producer ``git log {sha}..HEAD --not origin/{base_branch} -p`` delta rendered verbatim alongside the executed command (NOT a ``changed_artifacts``-only shortcut — per ``docs/architecture/REVIEWER-SYNC.md`` the re-review must audit the full delta as a fresh review). - Open-NACK payload rendered per-reviewer with reason and artifact_refs (#2142 aggregated-NACK barrier shape from ``peer_consensus.py:_open_nacks_barrier_response``). - Envelope bound at ``PROMPT_ENVELOPE_MAX_BYTES = 10 KB`` (excluding the delta, which scales with the change); memory excerpt capped at ``MEMORY_EXCERPT_MAX_CHARS = 2 KB``. Placed in a sibling module rather than at the bottom of ``orchestrator/routes/pipelines.py`` (the plan explicitly allows the sibling-module form when the host file is over the decomposition cap, which ``pipelines.py`` at ~24 800 lines already is). ``pipelines.py`` adds a one-line re-export so callers importing via ``orchestrator.routes.pipelines.compose_event_prompt`` continue to work — the contract assigns task-3-1 to ``pipelines.py`` so the re-export keeps the public surface aligned with that. Wiring (TASK-3-2), the BRC preamble collapse (TASK-3-3), and tests land in follow-up commits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(#2908 slice-3 task-3-2): wire event-pump wrapper to compose_event_prompt Slice-2's ``invoke_agent_for_event`` shipped a minimal stub prompt ("BRC event-pump handler / Role / Slice / Action / Event payload"); this commit wires it to the slice-3 ``compose_event_prompt`` via a CLI entry-point on ``orchestrator/routes/event_prompt.py``. The CLI is the wrapper-bash injection seam — calling ``compose_event_prompt`` directly from a Python heredoc would force the ``orchestrator.routes`` package ``__init__.py`` (which imports Flask) to load on the agent pod, and Flask is not in the sandbox runtime. The CLI bypasses that by being the script's ``if __name__ == "__main__":`` entry-point: the wrapper bash invokes ``python3 /opt/egg-runtime/orchestrator/routes/event_prompt.py <action>`` with the event_payload JSON on stdin, and the script imports its own neighbouring helpers without traversing the package init. CLI responsibilities (matches plan TASK-3-2): * Read ``EGG_AGENT_ROLE`` / ``EGG_BASE_BRANCH`` / ``EGG_REPO_PATH`` / ``EGG_BRC_MEMORY`` from env (set on every agent pod per ``orchestrator/kubernetes_spawner.py:818-823``). * Read the per-role memory file at ``.egg-state/agent-outputs/<role>/brc-memory.md`` iff ``EGG_BRC_MEMORY=full`` (slice-1 writer's read gate; slice-4 flips the default to ``full``). * Parse per-producer ``last_reviewed_commit_sha`` from the memory file's structured ``### <role>`` blocks (slice-1 writer schema) even in ``write-only`` mode — the architect plan splits the memory-excerpt gate (full only) from the SHA-lookup gate (always) so the wrapper still renders the per-producer delta against the fallback baseline. * For each ``last_reviewed_commit_sha``, run ``git log {sha}..HEAD --not origin/{base_branch} -p`` via subprocess inside the worktree. The gateway allows ``--not`` and ``-p`` on ``git log`` per #2905. * Extract the open-NACK list (``event_payload['nacks']`` / ``aggregated_nacks``) and forward it through. * Call ``compose_event_prompt`` and write the rendered prompt to stdout. The wrapper bash captures stdout into ``$prompt`` and passes it as the positional argument to ``python3 -m egg_agent``. On any failure (script missing, schema drift, git log subprocess crash) the wrapper falls back to the slice-2 stub so the event-pump keeps running rather than failing the agent invocation — the idle-budget safety net catches a wedged event-pump even under a degraded composer. The composer call obeys the env-flag split: * ``EGG_BRC_MEMORY=full`` — memory excerpt included; per-producer delta rendered. * ``EGG_BRC_MEMORY=write-only`` (slice-1 default) — memory excerpt omitted from prompt; per-producer delta still rendered against the memory file's stored SHAs as a fallback baseline. Matches the plan TASK-3-2 wording verbatim. * ``EGG_BRC_MEMORY=off`` — both omitted. Defensive safety rails added: * ``_GIT_LOG_TIMEOUT_SECS = 60`` so a hung gateway doesn't deadlock the event-pump. * ``_GIT_LOG_DELTA_MAX_BYTES = 256 KiB`` per producer with explicit truncation sentinel so a pathologically large refactor doesn't blow past Claude's context budget silently. * Stdin-based event_payload (#2741 prose-arg discipline) instead of argv. The TASK-3-3 preamble collapse and slice-3 tests land in follow-up commits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(#2908 slice-3 task-3-3): collapse _build_brc_preamble for event-pump model Strips wait-loop / STAY-ALIVE / cursor-threading guidance from the BRC consensus preamble emitted to every concurrent-mode agent prompt. The slice-2 event-pump wrapper owns lifecycle now; the agent's contract is one-shot per actionable event, not "stay alive in a wait-loop until SIGTERM". What changed in _build_brc_preamble: * Producer Lifecycle step 4 (RESPOND TO REVIEWS) — removed the pre-confirm wait-loop invocation (``_brc_preconfirm_wait_line``) and the "Do not include CONSENSUS_CONFIRMED in this pre-confirm wait" foot-gun guidance + the directed STATUS-nudge prose (#2531). Kept the #2142 aggregation rule (still relevant) and the NACK pushback paragraph in condensed form. * Producer step 6 (STAY ALIVE) — deleted entirely. * Producer step 7 (HANDLE RE-REVIEW) — re-numbered to step 6 and rewritten in event-pump terms ("when you are re-invoked with a CONSENSUS_RE_REVIEW event"). Step 8 (RESOLVE OBLIGATIONS) becomes step 7. * Reviewer step 2 (POLL) — replaced with INVOKED PER EVENT framing. The wrapper invokes the reviewer when the producer's CONSENSUS_PROPOSE lands; the reviewer no longer self-drives a wait-loop. * Reviewer step 7 (STAY ALIVE) — deleted. * Reviewer step 8 — re-numbered to step 7, simplified the recovery-trigger language. The adversarial-re-review dual-mandate banner (the "TWO equal-weight mandates" + "Both must pass to ACK" paragraph) is preserved per plan acceptance. * Dual-Role Execution Order banner — kept structurally (per plan acceptance) but updated to describe the event-pump invocation pattern: the coder's PROPOSE re-invokes the tester rather than triggering an in-process wait-loop return. * Pre-seeded empty-producer shortcut (#2581) — references to step 6 STAY ALIVE replaced with "exit; the wrapper re-invokes you with the next event". The detailed STATUS-nudge / wait-loop filter set guidance collapses to "the wrapper will re-invoke you on the next event". * Trailing "If you exit before the orchestrator stops you, you have FAILED your role" warning — replaced with the event-handler contract: "the wrapper drives lifecycle, exit naturally after acting". * Reviewer ACK/NACK section — condensed the #1998 conditional-ACK example (kept the rule + flag, dropped the verbatim command example), folded the #2336 alternative-obligation block into one sentence at the tail of the #2338 drop-obligation block, and trimmed the stale-version paragraph. * Helpers ``_brc_preconfirm_wait_line`` and ``_brc_stay_alive_wait_line`` — deleted (zero callers post-collapse). Byte-size delta (per the plan acceptance ≥ 25% target softened from the original ≥ 40%): * coder 9664 -> 7238 bytes (25.1% drop) * reviewer_code 12606 -> 9346 bytes (25.9% drop) * tester 24139 -> 17635 bytes (26.9% drop) The "Both must pass to ACK" phrase from the dual-mandate banner is preserved (verified across all three role variants); the byte-size drop is across all three. Coverage caveat per the plan: slice-3 ships the collapsed preamble against the LEGACY wrapper path by default (the EGG_BRC_EVENT_PUMP flag stays off until slice-4). The intermediate state (slice-3 merged but slice-4 not yet) is intentionally inert because the legacy wrapper's capped-restart safety net catches the resulting "agent exits after one pass" behavior — slice-4's flag flip plus deletion of the legacy template close that window. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(#2908 slice-3): add compose_event_prompt + preamble-collapse tests, update legacy preamble tests The coder-owns-tests policy (#2936) means the slice-3 coder authors the initial test scaffold for TASK-3-6 (compose_event_prompt) and TASK-3-7 (collapsed preamble snapshot); the tester reviews-and-hardens in their own pass. New tests: * ``orchestrator/tests/test_compose_event_prompt.py`` (TASK-3-6) — prompt-shape tests per role variant (producer / reviewer / dual-role), memory-excerpt truncation at the 2 KB cap, open-NACK rendering across 0 / 1 / 2+ reviewers (the #2142 aggregated barrier case), verbatim ``git log {sha}..HEAD --not origin/{base_branch} -p`` command emission (with a regression guard against future ``changed_artifacts``-only shortcut per REVIEWER-SYNC.md + risk_analyst R6), envelope-budget assertion (≤ 10 KB excluding the rendered delta, which scales with the change), and defensive shape (None inputs, empty role, empty base branch). * ``orchestrator/tests/test_brc_preamble_collapsed.py`` (TASK-3-7) — three role-shape snapshots (coder / reviewer_code / tester); absent-strings (STAY-ALIVE, positive wait-loop instructions, cursor-threading, ready_to_confirm STATUS-nudge); kept-strings ("Both must pass to ACK", dual-mandate banner, agent roster, producer/reviewer lifecycle skeleton); event-handler contract framing replaces the legacy "you have FAILED your role" warning; byte-size drop ≥ 25% per role variant. Legacy preamble tests updated to match the slice-3 collapsed shape in ``orchestrator/tests/test_pipeline_prompts.py`` and ``orchestrator/tests/test_concurrent_integration.py``: * Updated (retargeted assertions, preserved purpose): - ``TestBrcPreambleSyncStep::test_reviewer_sync_step_after_poll`` (POLL → INVOKED PER EVENT) - ``TestBrcPreambleSyncStep::test_reviewer_lifecycle_renumbered`` (steps 1-7, STAY ALIVE deleted) - ``TestAdversarialReReviewPriming::test_reviewer_lifecycle_step8_carries_adversarial_framing`` (banner moved from step 8 to step 7; substantive content preserved) - ``TestAdversarialReReviewPriming::test_producer_respond_to_reviews_legitimizes_new_findings`` (NACK pushback paragraph condensed but preserved) - ``TestDirectedCoordinationGuidance::test_directed_coordination_before_exit_warning`` (precedes the Event-handler contract instead of the deleted "you have FAILED" warning) - Three ``TestDualRoleExecutionOrdering`` tests (banner-presence + REVIEW token; wait-loop allowlist references dropped) - ``TestConcurrentPromptLifecycle::test_concurrent_prompt_includes_lifecycle_preamble`` (asserts the slice-3 Event-handler-contract framing instead of the legacy STAY-ALIVE / "FAILED your role" framing) * Deleted (entirely about deleted functionality): - ``TestReviewerPollUsesWaitLoop`` (POLL is gone) - ``TestReviewerWaitLoopMentionsAutoCursor`` (cursor-threading is gone) - ``TestProducerRespondToReviewsWaitLoop::test_step4_lists_pre_confirm_allowlist`` and ``test_step4_explains_status_ready_to_confirm_nudge`` (the wait-loop allowlist is gone; the orchestrator's ``brc next-action`` route drives the producer pre-confirm waits now). The ``test_step4_excludes_consensus_confirmed`` regression guard is preserved. - Seven ``TestDualRoleExecutionOrdering`` tests that pinned the wait-loop allowlist content of the banner. - ``TestConcurrentPromptLifecycle::test_reviewer_stay_alive_uses_canonical_for_list`` (STAY ALIVE is gone). The deletions preserve issue cross-references in adjacent comments so the audit trail (#1943, #2064, #2323, #2482, #2531, #2749) is not lost. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * style(#2908 slice-3): ruff format on slice-3 source files Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-3): rebase reconciliation for older slice-3 base The slice-3 task-3-3 patch was authored against a base that included the #2936 coder-owns-tests rewrite; rebasing onto origin/slice-3 (which precedes #2936) required two small reconciliations: * Dual-role banner preface — restored ``propose right after`` and ``self-block`` phrasings (required by ``test_dual_role_banner_states_propose_first``) while keeping the event-pump framing from the slice-3 collapse. * ``test_no_positive_wait_loop_instructions`` — broadened the negative-qualifier allowlist to accept ``Do NOT call`` / ``do NOT call`` so the producer-orientation copy (``Do NOT call `wait-loop`...``) and the new banner preface (``Do NOT block on a reviewer wait...``) both satisfy the regression guard. The intent is unchanged: any line mentioning ``wait-loop`` must qualify it with a negative directive. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-3): address reviewer_contract v1 NACKs Three blocking findings addressed; one additional non-blocker drop: 1. **Env-var prefix bug in wrapper bash** (NACK #1). The form ``EGG_AGENT_ROLE=... printf '%s' "$event_payload" | python3 ...`` attached env-vars only to ``printf``, not to ``python3``. The agent pod's parent shell currently exports the needed vars so this works in production, but the comment claimed the re-export protected against a parent shell that hadn't propagated them — which the construct didn't actually do. Moved the env prefix to the RHS of the pipe so ``python3`` actually gets the named vars. Also captured stderr to a temp file and surfaced the first line in the fallback ``cw_log`` so the operator can tell script-not-found / schema-drift / subprocess crash apart (non-blocking observation from reviewer_concurrency). 2. **CLI tests for the memory-mode handling** (NACK #2 — plan TASK-3-2 acceptance "snapshot test verifies both branches"). Added three subprocess-level tests in ``test_compose_event_prompt.py`` driving the ``_cli`` entry point against a real tmp-repo + populated memory file: * ``test_cli_full_mode_emits_memory_and_delta`` * ``test_cli_write_only_mode_omits_memory_keeps_delta`` * ``test_cli_off_mode_omits_memory_and_uses_changed_artifacts_fallback`` Each one verifies the ``EGG_BRC_MEMORY``-mode-gated behaviour of the slice-1 default (``write-only``) and the slice-4 target (``full``). 3. **``changed_artifacts`` fallback implemented in ``_build_delta_entries``** (NACK #3 — plan TASK-3-2 acceptance + documenter's docs at ``docs/architecture/orchestrator.md`` / ``docs/reference/agent-wait-patterns.md`` describe the same degraded-baseline fallback). When no per-producer SHA is recorded yet (off mode, first-ever ACK before any memory write, parse failure, file missing) and the event payload carries a ``changed_artifacts`` list, render a single fallback entry naming the producer and the artifact list — explicitly labelled as a degraded baseline so the agent does NOT mistake it for an adversarial-re-review-grade diff. Adds ``_extract_changed_artifacts`` and ``_extract_producer_role`` helpers and threads ``event_payload`` through the ``_cli`` call. Four new unit tests cover (a) the fallback fires when no SHA + non-empty ``changed_artifacts``, (b) the producer role is pulled from ``event_payload.producer`` / ``event_payload.producer_role``, (c) no fallback when neither SHA nor ``changed_artifacts``, (d) a real SHA always wins over the fallback. 4. **Drop dead ``type`` fallback in ``_render_event_section``** (non-blocking observation). The ``next-action`` route emits ``action`` only (``consensus.py::_VALID_ACTIONS``); the ``type`` fallback was hedging against a schema that doesn't exist in this codebase. All 629 tests in the directly-affected suites pass; ruff lint + format clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-3): address tester + reviewer_contract v1 NACKs Two follow-on findings from the aggregated v1 NACK barrier: * **tester NACK** — ``ruff format --check`` failed on ``orchestrator/routes/pipelines.py`` (quote-style on the ``#2338`` drop-obligation paragraph). Ran ``ruff format`` on the file; ``make lint`` now passes literally. * **reviewer_contract NACK #2** — ``test_consensus_wrapper.py`` had no test that pinned the wrapper template's ``invoke_agent_for_event`` invocation shape. The ``TestEventPumpInvokesComposer`` class adds six snapshot tests that fail if a future refactor: * drops the ``invoke_agent_for_event`` function definition; * changes the script path reference or removes the ``EGG_EVENT_PROMPT_SCRIPT`` env-var indirection; * breaks the ``EGG_AGENT_ROLE`` / ``EGG_BASE_BRANCH`` / ``EGG_BRC_MEMORY`` env-var re-export contract to the CLI; * reverts the v1-NACK fix (env-var prefix landing on ``printf`` instead of ``python3``); the ordering test asserts the textual order ``printf '%s' ... | EGG_AGENT_ROLE=... python3`` so a re-introduction of the bug trips the test; * changes the ``python3 "$script_path" "$action"`` call shape; * accidentally references ``event_prompt.py`` from the legacy flag-off template (the composer is event-pump-only). All 635 tests in the directly-affected suites pass; ruff lint + format clean across all slice-3 files. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2908 slice-3): documenter consensus participation marker Empty commit recording documenter participation in slice-3 BRC consensus restart (#2908). The slice-3 documenter work for task-3-4 (mission.md event-handler rewrite) and task-3-5 (orchestrator.md + agent-wait-patterns.md slice-3 doc surface) is already on the slice-3 integration branch from prior cycles (commits 6137694, e3ab9e9, 90b2cb5, plus reconciliation commits dfad765, 63c6bfe, fc2e82e, 66b7e73); this commit anchors a fresh BRC propose by the slice-3 documenter at restart. Verified on the slice-3 branch HEAD prior to this commit: - diff sandbox/agent-config/rules/mission.md sandbox/claude-rules/mission.md returns empty (byte-identical via symlink). - rg 'STAY-ALIVE\b|wait-loop|never exit' inside the rewritten Concurrent Execution Mode section of mission.md returns zero matches. - The "BRC Per-Event Prompt Composer + Preamble Collapse (slice-3, #2908)" section in docs/architecture/orchestrator.md is present and reflects the reviewer_code v1/v2 corrections (heading does not say "behind EGG_BRC_MEMORY"; preamble collapse documented as unconditional; byte-size drop softened to ≥ 25%; line-number references carry the "drift-prone — prefer function-name" caveat). - The "Per-Event Prompt Shape and Memory Consumption (slice-3, #2908)" section in docs/reference/agent-wait-patterns.md is present at §10.9 as a subsection of slice-2's §10 (BRC Event-Pump Wrapper), wiring the $EGG_REPO_PATH/docs/... convention. - EGG_BRC_MEMORY env var entry is in the orchestrator.md Environment Variables table. - Cross-refs to brc-memory.md (slice-1) and the BRC Event-Pump Wrapper section (slice-2) all resolve on this branch (slice-1/slice-2 docs are in the slice-3 branch's history). Refs #2908 tasks 3-4, 3-5 * feat(#2908 slice-3): coder consensus participation marker Empty commit recording coder participation in slice-3 BRC consensus restart (#2908). The slice-3 coder work for tasks 3-1 (compose_event_prompt), 3-2 (wire event-pump wrapper to composer), and 3-3 (collapse _build_brc_preamble) is already on the slice-3 integration branch from prior cycles (commits ed8a4c5, 27ed9d0, 2261d5c, plus the test-author commit 7cff8d1, lint-fix 63c6bfe, reconciliation dfad765, and the NACK-address commits fc2e82e, 66b7e73); this commit anchors a fresh BRC propose by the slice-3 coder at restart. Verified on slice-3 branch HEAD prior to this commit: - orchestrator/tests/test_compose_event_prompt.py: 23/23 pass — covers prompt shape per role (producer / reviewer / dual-role), memory-excerpt truncation at the 2 KB cap, NACK rendering for 0/1/2+ reviewers, full 'git log SHA..HEAD --not origin/BASE -p' delta command emitted verbatim with per-producer last_reviewed_commit_sha substituted (no changed_artifacts-only shortcut), envelope bounded ≤ 10 KB. - orchestrator/tests/test_brc_preamble_collapsed.py: 26/26 pass — covers STAY-ALIVE / wait-loop / cursor strings absent (per task-3-3), dual-mandate 'Both must pass to ACK' phrase preserved, agent roster preserved, producer/reviewer lifecycle skeletons preserved, preamble byte size drops ≥ 25% vs the pre-collapse snapshot baseline. - orchestrator/tests/test_pipeline_prompts.py: 431/431 pass — full pipeline-prompts regression suite (event-handler contract present, dual-role banner integration, etc.). - orchestrator/tests/test_consensus_wrapper.py: passes — including the TestEventPumpInvokesComposer suite added in 66b7e73 that pins the wrapper template's invoke_agent_for_event invocation shape. - ruff check + ruff format on the four touched source files passes literally (orchestrator/routes/event_prompt.py, orchestrator/consensus_wrapper.py, orchestrator/routes/pipelines.py, the two new test files). * fix(#2908 slice-3): address reviewer_code v2 + reviewer_code_holistic v2 NACKs Addresses four blocking findings from the slice-3 v2 NACK barrier (reviewer_code v2 finding #1 + reviewer_code_holistic v2 findings #1/#2/#3). All four are cross-module asymmetries between the slice-3 composer (orchestrator/routes/event_prompt.py) and the next-action route (orchestrator/routes/consensus.py). ### 1. reviewer_code v2 finding #1 — REVIEWER-SYNC.md path The composer's module docstring (event_prompt.py:16) and the "Per-producer re-review delta" prompt section (event_prompt.py:153) both cited 'docs/architecture/REVIEWER-SYNC.md' — a file that does not exist at that path. The real location is 'shared/prompts/REVIEWER-SYNC.md' (verified with git show origin/egg/issue-2908-impl2/slice-3:shared/prompts/REVIEWER-SYNC.md; every other reference in the codebase uses the correct shared/ path). Reviewers following the cited link would 404; this is the exact 'documented snippet that doesn't work as a copy-paster reads it' regression the review criteria flag. Fix: replace both occurrences with 'shared/prompts/REVIEWER-SYNC.md'. Regression guards: test_per_producer_delta_section_cites_correct_reviewer_sync_path and test_module_docstring_cites_correct_reviewer_sync_path now assert the rendered prompt contains 'shared/prompts/REVIEWER-SYNC.md' AND does not contain 'docs/architecture/REVIEWER-SYNC.md' (so a future move/rename can't silently re-break this). ### 2. reviewer_code_holistic v2 finding #2 — unresolved_nacks _extract_nacks (event_prompt.py:602-630) accepted only 'nacks' and 'aggregated_nacks' keys, but next-action's _derive_next_action (consensus.py lines 329/346) emits 'unresolved_nacks' for the single-reviewer NACK propose path — the COMMON case. The open-NACK barrier shape ('nacks' key) requires 2+ distinct reviewers; a single-reviewer NACK uses 'unresolved_nacks' and was silently dropped from the per-event prompt. Fix: add 'unresolved_nacks' as a third accepted key (priority: nacks → unresolved_nacks → aggregated_nacks). Producer re-invoked to address a single-reviewer NACK now sees the structured Open-NACKs section with reviewer 'reason' + 'artifact_refs' inline (the round-trip-per-NACK signal #2142 was built to enforce). Regression guards: - test_extract_nacks_accepts_unresolved_nacks_key_from_next_action pins the new key against the real _derive_next_action payload shape. - test_compose_event_prompt_renders_unresolved_nacks_section asserts end-to-end that the structured Open NACKs section renders with the reviewer's identity / reason / artifact_refs. - test_extract_nacks_priority_order_nacks_over_unresolved_nacks pins barrier-shape priority over the convenience key. ### 3. reviewer_code_holistic v2 finding #3 — scoping to current producer _build_delta_entries iterated EVERY producer in the reviewer's memory file (for producer in sorted(per_producer.keys())) and emitted a delta for each, irrespective of which producer the CURRENT event named. The user-visible failure: reviewer-A ACKed coder at v1 → memory stores coder's SHA. Tester then proposes for the first time. Reviewer-A is re-invoked with event_payload = {pending_reviews: [{producer: tester, ...}]}. The renderer emitted ONLY coder's stale delta — tester's first review had no delta at all, and the section title 'Per-producer re-review delta' implied the rendered delta WAS the producer being reviewed (but it was the wrong one). Fix: scope delta enumeration to producers named in event_payload.pending_reviews (or top-level producer/producer_role on the producer side). Treat memory's per-producer SHA as a per-producer LOOKUP keyed by the current producer, not as an ENUMERATION source. Legacy / synthetic-test paths with no pending_reviews key fall back to enumerating all stored SHAs (backward compat). New helpers: - _extract_current_producers(event_payload) walks pending_reviews / top-level producer keys, de-dupes in first-seen order. - _extract_artifacts_for_producer(event_payload, producer) pulls artifact_refs from the matching pending_reviews entry first (production path), falls back to top-level changed_artifacts only when the top-level producer matches (prevents cross-producer artifact leak). Regression guards: - test_build_delta_entries_scopes_to_pending_reviews_producer pins the scoping invariant (memory has X SHA, event names Y → render Y, not X). - test_build_delta_entries_pending_reviews_with_sha_renders_real_delta asserts _run_git_log invoked only for the current producer's SHA (not the stale one). - test_build_delta_entries_multiple_pending_reviews_renders_each covers the multi-pending-review case. - test_build_delta_entries_no_pending_reviews_falls_back_to_memory_enum pins the backward-compat path. - test_extract_current_producers_* / _extract_artifacts_for_producer_* pin the new helper behaviour. ### 4. reviewer_code_holistic v2 finding #1 — changed_artifacts fallback wired through The documented changed_artifacts fallback (docs/architecture/orchestrator.md + docs/reference/agent-wait-patterns.md) was dead code in production: next-action's _derive_next_action never emits a top-level changed_artifacts key, but the fallback path in _build_delta_entries looked for one. First-time reviewers of any producer (no stored SHA, no top-level changed_artifacts in the real payload) silently saw an empty 'Per-producer re-review delta' section. Fix: enrich next-action's reviewer-side pending_reviews entries with artifact_refs sourced from PeerConsensusTracker.get_current_proposal_snapshot(producer). The lock is reentrant (threading.RLock at peer_consensus.py:101) so the call inside _derive_next_action's locked block is safe. The composer's _extract_artifacts_for_producer prefers pending_reviews[i].artifact_refs over the legacy top-level changed_artifacts key, restoring the documented fallback. Regression guard: - test_next_action_reviewer_pending_reviews_includes_artifact_refs asserts a pending_reviews entry from the production next-action route carries artifact_refs mirroring the producer's current proposal artifacts (the seeded a.py from _propose). ### Tests 660 tests pass under the slice-3 regression scope (compose + preamble + pipeline_prompts + consensus_wrapper + consensus_next_action + concurrent_integration). Ruff lint + format clean on the four touched files. * test(#2908 slice-3): tester hardening on compose_event_prompt + preamble collapse Adversarial coverage added on top of the coder-authored test scaffolds for TASK-3-6 (compose_event_prompt) and TASK-3-7 (collapsed preamble). Per #2936 the coder authors its tests; the tester reviews-and-hardens in their own pass. Probes the boundaries the coder-authored happy-path tests do not: * ``_truncate`` exact-boundary + just-over-boundary semantics (the off-by-one at the cap was not pinned). * Multi-byte UTF-8 memory excerpt — truncation respects the code-point cap not the byte cap; envelope still respected. * Whitespace-only memory excerpt → section omitted (strip() guard). * Producer-delta iteration order preserves caller order (no implicit sort — sorting belongs to ``_build_delta_entries``'s memory-enum fallback, not the renderer). * ``(no commits in range — re-review is a no-op)`` sentinel for an empty delta string (the post-confirm re-confirm shape). * Producer-delta entries with missing keys render defensive defaults (``(unknown)`` producer label, ``<no prior review>`` SHA sentinel). * Non-string delta is coerced via ``str()`` rather than crashing. * NACK with missing ``reviewer`` / ``reason`` / non-list ``artifact_refs`` renders sentinels rather than crashing. * ``_parse_per_producer_sha`` edge cases — ``-`` sentinel skip, first-match-wins per heading, orphan bullet ignored, backtick-wrapped heading canonicalised. * ``_extract_nacks`` third-priority ``aggregated_nacks`` fallback (the coder pins ``nacks`` > ``unresolved_nacks``; this pins the third tier). * ``_extract_nacks`` drops non-dict entries silently. * ``_extract_changed_artifacts`` filters ``None`` / whitespace; defensive against non-dict payloads. * ``_extract_current_producers`` robust against mixed-shape ``pending_reviews`` entries; defensive against non-dict payloads. * ``_extract_artifacts_for_producer`` priority: ``pending_reviews`` > top-level ``changed_artifacts``; empty / whitespace producer returns empty list. * ``_run_git_log`` subprocess error paths — timeout sentinel, non-zero rc sentinel with stderr, 256 KiB truncation marker. * CLI: ``--event-payload-file`` reads from disk; missing file returns rc=2 with explicit error; malformed-JSON stdin falls back to surfacing the raw payload under ``raw``; empty stdin falls back to ``{"action": <argv>}`` action-only payload. Hardens the preamble-collapse contract beyond the line-by-line ``wait-loop`` qualifier check: * Positive ``egg-orch message wait-loop`` invocations forbidden (each occurrence must sit inside a ±200-char window with a ``do NOT`` / ``Do NOT`` / ``not block on`` / ``not call`` / ``not issue`` negation anchor — catches multi-line code blocks). * ``--for CONSENSUS_*`` filter-flag patterns absent (legacy wait-loop plumbing). * ``never exit`` warning gone (TASK-3-3 explicit collapse target; pairs with the coder's ``"you have FAILED your role"`` check). * ``/tmp/egg-wait-cursor-`` plumbing path absent (full-text scan, complements the substring check). * Agent roster names the producer + reviewer roles, not just the heading. * No ``Ready to confirm — all confirm preconditions satisfied`` STATUS-nudge anchors (#2531 plumbing collapsed). * Tester preamble fits inside a 20 KB ceiling (absolute bound complements the ≥ 25% relative-drop assertion against the hardcoded baseline — a runaway re-expansion fails on both anchors). * Unknown role doesn't crash; the agent-roster section still renders. * Event-handler contract surfaces the wrapper-owned-wait framing (wrapper / drives your lifecycle / one-shot) — pins the central intent of the collapse beyond merely the contract heading. All 115 tests pass under PYTHONPATH=. pytest; ruff lint + format clean on both files. Configured ``make test`` / ``make lint`` / ``make security`` cannot run end-to-end in this pod because the venv-sync step fails to fetch dev wheels (no PyPI egress) — see the propose attestation for the ``tests_execution_blocked`` rationale and the slice-3-scoped check counts that DID execute directly. * Persist BRC history for slice-3 (#2548) * Fix checks: apply automated formatting fixes * Address reviewer_holistic v2 feedback - Enforce PROMPT_ENVELOPE_MAX_BYTES in compose_event_prompt by byte-truncating the NACKs section (the variable-size driver) with an explicit sentinel when the envelope would otherwise overflow (reviewer_holistic minor #1). Memory tail-position contract and delta-exclusion are preserved. - Re-export EGG_REPO_PATH on the python3 invocation in the event-pump wrapper so the env-prefix matches the in-source comment listing all four wrapper-supplied vars (reviewer_holistic minor #2). Test updated to assert all four env-var re-exports. - Update stale pipelines.py line anchors in docs/architecture/ orchestrator.md and docs/reference/agent-wait-patterns.md to the actual post-collapse positions (12180 for _build_brc_preamble, 13366/13399/13427 for callers, 12561-12573 for the dual-mandate banner, 12567 for the 'Both must pass to ACK' anchor); add a reminder that the function/banner-name anchor is the drift-resistant reference (reviewer_holistic minor #4). - Add an operator-telemetry note to agent-wait-patterns.md §10.9.5 calling out the slice-3 default-off restart-budget consumption (one restart per agent per phase) so operator SLOs aren't surprised by the slice-3-vs-slice-4 metric step (reviewer_holistic minor #3). * Strip NACK payload from event_section JSON to honour envelope cap Reviewer re-review of commit aaaa17f found that Minor #1 (envelope-cap enforcement) was only partially fixed: the NACK list rendered into nacks_section was truncated under the cap, but the same payload was ALSO baked into event_section via json.dumps(event_payload, ...). Production payloads from _producer_has_open_barrier (consensus.py:248) and _producer_has_unresolved_nacks_on_current_version (:340-358) put the full nacks/unresolved_nacks list directly inside event_payload, so the reviewer's worked example (6 reviewers x ~2.8 KB reason) still sailed past PROMPT_ENVELOPE_MAX_BYTES because the JSON copy was untouched. Fix: strip the NACK keys (nacks / unresolved_nacks / aggregated_nacks) from event_payload before json.dumps in _render_event_section, replacing each with a cross-reference marker pointing the agent at the dedicated nacks_section. The single source of truth for the rendered NACK bytes is now nacks_section, which the envelope-cap truncation pass already governs. Existing tests masked the bug because they passed event_payload={"action": "propose"} (no nacks in payload) with the nacks list separately, which never happens in production. Two new regression tests pin the production-shaped payload: - test_production_shaped_open_barrier_payload_honours_envelope_cap (multi-reviewer 'nacks' barrier shape from _producer_has_open_barrier) - test_production_shaped_unresolved_nacks_payload_honours_envelope_cap (single-reviewer 'unresolved_nacks' shape from _derive_next_action) Both assert the envelope stays under the cap AND that the NACK reason text does NOT appear in the JSON block. * Fix dual-role banner contradiction with coder-owns-tests orientation The merge resolution in 39a08bb kept HEAD's banner verbatim while main's #2936 (coder-owns-tests) auto-merged into surrounding sections. The banner directed the tester to "execute Producer steps 1-3 FIRST" and "draft scaffolding" while the surrounding producer orientation tells the tester "Orient only until the coder proposes" and "do NOT write test files before the coder's CONSENSUS_PROPOSE". Two opposite directives in the same prompt. Fix (banner-only edit; surrounding sections already correct): - Remove "draft scaffolding" from the opportunistic-prepare list (it's the scaffold-first directive in disguise). - Reword "Producer steps 1-3 come FIRST" to "Producer ORIENT (step 1) comes FIRST" + an explicit pivot on the role-specific orientation for whether WORK (step 2) runs immediately or is gated on the upstream CONSENSUS_PROPOSE. - Name the tester case explicitly (the dual-role agent whose WORK is gated per #2936) so the agent reading the banner can reconcile it with its role-specific orientation. - Keep "the wrapper re-invokes you when that proposal arrives" and "propose right after" — slice-3's event-pump framing remains correct. Add ``test_dual_role_banner_does_not_contradict_coder_owns_tests`` that scopes assertions to the banner SUBSTRING (between ``### Dual-Role Execution Order`` and ``### Producer Lifecycle``) so a future banner-only edit can't sneak the scaffold-first prose back in. Pairs with the existing ``test_tester_orientation_directs_review_and_harden_after_propose`` which covers ``_build_producer_orientation`` — together they pin both halves of the prompt the merge resolution desynced. --------- Co-authored-by: egg <egg@example.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: egg-orchestrator <egg@localhost> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>

#2952) * feat(#2908 slice-4 task-4-1): flip EGG_BRC_EVENT_PUMP and EGG_BRC_MEMORY defaults Slice-4 task-4-1 makes the event-pump wrapper the production default by flipping two env-flag defaults: * ``EGG_BRC_EVENT_PUMP`` flips from unset→OFF (legacy) to unset→ON (event-pump). Setting ``EGG_BRC_EVENT_PUMP=false`` (or ``0`` / ``no`` / ``off``, case-insensitive) keeps the legacy capped-restart template available for a one-release rollback window. Unrecognised tokens fall through to event-pump so a typo cannot silently downgrade the production path. Slice-4 task-4-2 will delete the legacy template entirely and the env flag with it. * ``EGG_BRC_MEMORY`` flips from unset→``off`` (slice-1 inert) to unset→``full`` (event-pump composer reads memory by default). Setting ``EGG_BRC_MEMORY=off`` is the one-release rollback escape hatch. Unknown values still fail-safe to ``off`` (the fallback target stays restrictive — an undocumented value is a misconfiguration signal, NOT a write-bearing default to mask). Files touched: * ``orchestrator/consensus_wrapper.py``: - ``_event_pump_enabled()`` default flipped; falsy-token allowlist captures rollback path; docstring + module-level reframe updated. - Wrapper template's inline ``EGG_BRC_MEMORY:-off`` → ``...:-full`` so the wrapper's invocation of ``event_prompt.py`` inherits the new default even on shells that don't export the var explicitly. * ``sandbox/egg_agent_tools/handlers/brc_memory.py``: - ``get_memory_mode()`` defaults to ``MODE_FULL``; new ``MODE_DEFAULT`` constant pins the contract. * ``orchestrator/routes/event_prompt.py``: - CLI ``memory_mode`` default flipped from ``"off"`` to ``"full"``. * Tests updated to match the new defaults: - ``orchestrator/tests/test_consensus_wrapper.py``: ``TestEventPumpTemplateSelection`` rewritten — unset-env now pins event-pump, ``EGG_BRC_EVENT_PUMP=false`` pins legacy. ``TestBuildConsensusWrappedCommand``, ``TestConsensusWrapperBehavior``, ``TestBufferOverflowDetection``, ``TestEventDrivenWait``, ``TestSSESigtermGrace`` gain an autouse ``_force_legacy_template`` fixture that engages the rollback escape hatch so they continue to drive the legacy template. Slice-4 task-4-2 deletes the entire fixture + these classes alongside the legacy template. - ``tests/sandbox/egg_agent_tools/test_handlers_brc.py``: renamed ``test_unset_defaults_to_off`` → ``test_unset_defaults_to_full``; the unset-env pin now asserts the memory file is written. - ``orchestrator/tests/test_compose_event_prompt.py``: docstring note that ``write-only`` is the rollback target, not the default. Verified manually with Python smoke tests that the flag-flip works for unset, truthy, and the full falsy-token allowlist (``false`` / ``0`` / ``no`` / ``off`` / case variants), and that ``EGG_BRC_MEMORY=writeonly`` (typo) still fails safe to ``off`` with a warning while ``EGG_BRC_MEMORY=full`` and unset both enable writes + reads. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2908 slice-4 task-4-4): post-deletion consensus wrapper docs Rewrite docs/architecture/orchestrator.md "BRC Consensus Wrapper" section (renamed from "BRC Event-Pump Wrapper (slice-2, behind EGG_BRC_EVENT_PUMP)") to describe the post-deletion steady state. Event-pump is now the only consensus-wrapper path; the legacy capped-restart template and the agent-side heartbeat / keep-alive path were removed in slice-4 task-4-2. Changes: - docs/architecture/orchestrator.md - Renamed section to "BRC Consensus Wrapper"; updated anchor link from #brc-event-pump-wrapper-slice-2-behind-egg_brc_event_pump. - Replaced the slice-2 caveat blockquote with a four-slice rollout summary that names the deleted symbols (_CONSENSUS_WRAPPER_TEMPLATE, _RECOVERY_SYSTEM_PROMPT, SSE consensus.reached, MAX_CONSENSUS_RESTARTS). - Reframed "Why a new wrapper template" → "Why the wrapper drives the loop"; rewrote in past tense so the doc reads as if the event-pump has always been the only model. - Rewrote "Wrapper-side heartbeat (#2036 migration)" and "Wrapper-side gateway-session keep-alive (#2451 migration)" with "completed in slice-4" qualifier; described agent-side deletion. - Rewrote "Idle / no-progress safety budget" to drop the comparison table with the legacy 3-restart cap; replaced with a single behaviour table for EGG_BRC_IDLE_BUDGET_MIN. - Added new "Rollback plan" subsection documenting git revert of slice-4 → slice-3 → slice-2 → slice-1 in reverse-merge order, the integration check operators must run, and the partial-revert interaction (reverting only slice-4 restores the dual-emission state). - Renamed "Slice-2 verification stance — unit-test-only" to "Verification stance — unit-test-only"; explained that the snapshot tests pinning the byte-for-byte legacy template emission were retired in slice-4 task-4-3. - Renamed "BRC Per-Event Prompt Composer + Preamble Collapse (slice-3)" to drop the slice marker; reframed "Flag mapping" to "What's gated by what" since EGG_BRC_EVENT_PUMP no longer gates anything. - Updated EGG_BRC_MEMORY table: full is now the slice-4 default; write-only is the opt-in regression path. - Updated env vars table: EGG_BRC_EVENT_PUMP is a deprecated no-op pointing at the rollback plan; EGG_BRC_IDLE_BUDGET_MIN is no longer gated on EGG_BRC_EVENT_PUMP=true. - docs/guides/concurrent-execution.md - Replaced the slice-2 "two emission paths" caveat with a single post-deletion summary linking to the new orchestrator.md section. - Rewrote the "Consensus Wrapper" body to describe the deterministic event-pump loop (steps 1–6) as the only path; removed MAX_CONSENSUS_RESTARTS-based restart cap, the recovery system prompt, and the final-consensus-check restart cycle. - Updated the configuration table: dropped `max_restarts` row; added EGG_BRC_IDLE_BUDGET_MIN; updated transient-crash recovery paragraph to reference the idle/no-progress budget instead of the deleted MAX_CONSENSUS_RESTARTS hard cap. - docs/architecture/README.md - Updated the cross-link card to point at the renamed section and summarise the slice-4 deletion + rollback plan. Cross-links to docs/architecture/brc-memory.md (slice-1) retained throughout. The wait-side companion at agent-wait-patterns §10 is referenced from each cross-link card. Satisfies contract task-4-4. Acceptance: doc reads as if the event pump has always been the only model; legacy-path caveats removed; cross-links present; rollback plan documented; markdown renders clean (no conflict markers; section anchors resolve). * feat(#2908 slice-4 task-4-2): delete legacy capped-restart template and agent-side heartbeat Slice-4 task-4-2 collapses ``consensus_wrapper.py`` onto the event-pump template that slice-2 introduced and slice-3 wired the per-event composer into. The event-pump is now the only production path; rollback under a regression is a ``git revert`` of slices 1-3 per the PR body, not an env-flag flip. Deleted from ``orchestrator/consensus_wrapper.py``: * ``_CONSENSUS_WRAPPER_TEMPLATE`` (the ~600-line legacy capped-restart bash template). * ``_RECOVERY_SYSTEM_PROMPT`` and ``_RECOVERY_USER_PROMPT`` — the restart-time recovery prompts. * The SSE ``consensus.reached`` curl path (issue #1897) that lived inside the legacy template — the event-pump uses ``egg-orch message wait-loop`` instead. * ``MAX_CONSENSUS_RESTARTS`` (issue #2806) and its companion constants ``MAX_READY_POLL_CYCLES``, ``TRANSIENT_RESTART_BACKOFF_INITIAL``, ``STARTUP_FAILURE_WINDOW_SECONDS``. The idle/no-progress safety budget (env ``EGG_BRC_IDLE_BUDGET_MIN``, default 30 min) is the replacement liveness ceiling. * ``_event_pump_enabled()`` — the ``EGG_BRC_EVENT_PUMP`` env-flag read. The flag is now silently inert; operators with it lingering in k8s manifests can leave it set to either truthy or falsy and still get the event-pump template. * The legacy-template branch in ``build_consensus_wrapped_command``, which is now a thin alias for ``build_event_pump_wrapped_command``. Preserved by relocating into ``_EVENT_PUMP_WRAPPER_TEMPLATE`` (per task-4-2 acceptance, "Keep ``is_buffer_overflow`` / ``is_transient_crash`` / ``is_startup_failure`` classifiers"): * ``is_buffer_overflow()`` — Claude Agent SDK 1 MiB JSON reader overflow detector (#2804). * ``is_transient_crash()`` — signal-based exits (134, 136, 137, 139, 255). * ``is_startup_failure()`` — exit 1 within a 30 s startup window. * ``STARTUP_FAILURE_WINDOW_SECONDS`` — kept as a bash-scope shell variable inside the template (was a Python module constant). The classifiers are not yet wired into the event-pump's ``propose|ack|nack`` agent-invocation failure path (which uses ``AGENT_FAIL_STREAK`` + idle-budget escalation today); they live as named helpers for future revisions. Deleted from ``sandbox/egg_agent_tools/handlers/message.py``: * ``_WAIT_LOOP_HEARTBEAT_INTERVAL_SECS`` — the 60-s cadence constant. * ``_default_emit_wait_loop_heartbeat`` — the agent-side ``WAITING_FOR_EVENT`` / ``WORKING`` heartbeat emitter (#2036). * ``_start_wait_loop_heartbeat`` — the threaded periodic-tick helper. * The per-iteration ``emit_hb`` / ``stop_hb`` calls inside ``message_wait_loop``, including the ``try/finally`` block that drove the final ``WORKING`` beat on wait exit. The event-pump wrapper now owns both heartbeat liveness (#2036) and slice-scoped gateway-session keep-alive (#2451) via the wrapper- owned ``start_background_heartbeat`` subshell. ``message_heartbeat`` (the explicit handler invoked by ``egg-orch message heartbeat``) is unchanged — the wrapper calls it. Test updates: * ``orchestrator/tests/test_consensus_wrapper.py``: deleted the ``TestBuildConsensusWrappedCommand`` / ``TestConsensusWrapperBehavior`` / ``TestBufferOverflowDetection`` / ``TestEventDrivenWait`` / ``TestSSESigtermGrace`` classes (and the ``_force_legacy_template`` fixture that fed them). The buffer-overflow / SSE / capped-restart surfaces they covered no longer exist. The event-pump classes (``TestEventPumpTemplateSelection`` and siblings) cover the new production path; ``TestEventPumpTemplateSelection`` is reworked to pin the post-task-4-2 invariant that ``EGG_BRC_EVENT_PUMP`` is silently inert (any value, including ``false`` / ``0`` / ``no`` / ``off``, emits the event-pump template). * ``orchestrator/tests/test_consensus_wrapper_anchor.py``: deleted in full — every test pinned ``_RECOVERY_SYSTEM_PROMPT`` / ``_CONSENSUS_WRAPPER_TEMPLATE`` symbols that no longer exist. * ``orchestrator/tests/test_brc_nack_iteration.py``: removed ``TestConsensusWrapperNackFeedback`` (4 tests) that pinned the legacy recovery prompt's NACK-feedback placeholder + helper. The equivalent event-pump assertion lives in ``orchestrator/tests/test_compose_event_prompt.py``. * ``tests/sandbox/egg_agent_tools/test_handlers_message.py``: removed ``TestMessageWaitLoopHeartbeat`` (16 tests) that pinned the agent-side heartbeat path. ``TestMessageHeartbeat`` (the explicit handler tests) is unchanged. * ``integration_tests/regression/test_brc_concurrency.py``: updated the slice-2 verification-stance docstring to reflect the slice-4 post-deletion steady state (E2E deferred to #2585 via ``egg_stack``; in-process tracker coverage unchanged). Defensive grep assertions all return zero matches on ``orchestrator/consensus_wrapper.py``: rg 'consensus\.reached|sse_url|_RECOVERY_SYSTEM_PROMPT|MAX_CONSENSUS_RESTARTS' \ orchestrator/consensus_wrapper.py # → 0 hits Smoke-verified that the event-pump template is emitted regardless of ``EGG_BRC_EVENT_PUMP`` value, that the three classifiers survive in the event-pump template, and that the deleted agent-side heartbeat helpers are no longer importable from ``handlers.message``. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2908 slice-4 task-4-4 v2): address reviewer_code v1 NACK Five blocking findings + three non-blockers from reviewer_code's v1 NACK on PR-proposal v1. Address all five blockers; defer the non-blockers (env-var coordination is gated on coder task-4-1 / task-4-2 landing first). Blocker 1+2+3 — restored post-#2936 coder-owns-tests content in docs/guides/concurrent-execution.md (the v1 proposal overwrote the post-#2936 wording when I copied the slice-4 base, which predates the #2936 merge): - §"HANDOFF" table row example: "Coder can't push test files → HANDOFF to tester" → "Tester can't push a .github/ CI fix → HANDOFF to coder with the required end-state" (matches docs/reference/agent-roles.md). - §"Worked Example: Role-Boundary Handoff" rewrite: drop the reinstated pre-#2936 coder→tester test-handoff example, restore the post-#2936 tester→coder .github/-staging example, and keep the explicit lead sentence "the coder→tester test handoff that used to live here is gone: the coder now authors and pushes its own tests". - §"Rebase rarely conflicts" paragraph: "Rebase cannot conflict because agents have mutually exclusive file write permissions" / "role restrictions guarantee non-overlapping file sets" was a doc lie after #2936 — restore the pre-rewrite "rarely conflicts" wording and the follow-up paragraph that names the shared test scope and the serialize-by-time-not-concurrent invariant. Blocker 4+5 — dead anchors and stale §10 / §10.9 framing in docs/reference/agent-wait-patterns.md and docs/architecture/brc-memory.md: - agent-wait-patterns.md §10 retitled from "BRC Event-Pump Wrapper (slice-2, behind EGG_BRC_EVENT_PUMP)" to "BRC Consensus Wrapper (event-pump model)"; intro blockquote rewritten to drop the slice-2 "OFF by default" framing and instead describe the post-deletion steady state + rollback path; §10.8 retitled from "Flag-off as the temporary default — when slice-4 flips it" to "Rollout completed in slice-4" with body rewritten accordingly; §10.9 retitled to drop the (slice-3) suffix and the "Flag mapping" blockquote rewritten to "What's gated by what" since EGG_BRC_EVENT_PUMP no longer gates anything. - All five dead inbound anchor references repointed to the new anchors: agent-wait-patterns.md lines 1178, 1411, 1653, 1654 and brc-memory.md lines 235, 237. - Reverse direction — repointed orchestrator.md, README.md, and concurrent-execution.md cross-links from #10-brc-event-pump-wrapper-slice-2-behind-egg_brc_event_pump to #10-brc-consensus-wrapper-event-pump-model (3 occurrences in orchestrator.md, 1 each in README.md and concurrent-execution.md); same for #109-brc-per-event-prompt-composer--preamble-collapse-slice-3 → #109-brc-per-event-prompt-composer--preamble-collapse. - Verified via grep across docs/ that no remaining link points at the old anchors and no remaining body text carries the "(slice-2, behind EGG_BRC_EVENT_PUMP)" framing. Non-blockers deferred: - Rollback-plan example precision (compose_event_prompt slice-3 vs slice-2 wrapper) — useful tightening but doesn't change the correctness of the doc. - "schema is unchanged" past-tense alignment — minor. - EGG_BRC_EVENT_PUMP "no-op" vs "removed" wording — gated on coder's task-4-1 / task-4-2 final state. Will re-pass once the coder's proposal lands so the doc and code agree. * docs(#2908 slice-4 task-4-4 v3): address reviewer_code v2 NACK v2 cleared mandate 1 (all v1 blockers fixed) but mandate 2 found four new blocking findings in agent-wait-patterns.md §10.3 / §10.4 / §10.5 / §10.7 — subsection bodies still described the flag-off vs flag-on dual-emission world in present tense as if both paths still shipped, contradicting the §10 intro blockquote rewritten in v2. Address all four: - §10.3 (Heartbeat ownership) — dropped the two-row flag-off vs flag-on table; rewrote in past-tense post-migration framing mirroring orchestrator.md §"Wrapper-side heartbeat (#2036 migration completed in slice-4)" — the wrapper owns heartbeating now and the pre-#2908 agent-side path in `message_wait_loop` was deleted in slice-4 task-4-2. - §10.4 (Gateway-session keep-alive) — struck the closing "With the flag off the agent-side keep-alive still runs" sentence and replaced it with the slice-4-deletion qualifier matching §10.3 / orchestrator.md. - §10.5 (Idle / no-progress safety budget) — dropped the parenthetical "(replaces the 3-restart FAIL cap)" from the heading; dropped the two-row flag-off vs flag-on table; replaced with a single-row `EGG_BRC_IDLE_BUDGET_MIN` table mirroring orchestrator.md's steady-state version; rewrote the present-tense "MAX_CONSENSUS_RESTARTS = 3 cap" framing in past tense. - §10.7 (Verification stance) — dropped "Slice-2" from the heading; rewrote the body in past tense matching orchestrator.md's §"Verification stance — unit-test-only"; removed the "snapshot equality for the flag-off path" and "deferred to slice-4" framing (slice-4 is this work; flag-off snapshot tests were retired in slice-4 task-4-3); flipped the integration-tests bullet from "runs with EGG_BRC_EVENT_PUMP=false" to "runs against the event-pump wrapper". Adjacent cleanups for body/header coherence: - §10.1 ASCII diagram: relabelled "LEGACY (flag off, today's default)" → "PRE-#2908 (deleted in slice-4 task-4-2 — kept here for git-blame readers)" and "EVENT-PUMP (flag on)" → "STEADY STATE (event-pump, the only path after slice-4)". - §10.9.4 EGG_BRC_MEMORY mode table: marked `full` as the slice-4 default (mirrors orchestrator.md); dropped the slice-3-rollout "operators opt into full just as they opt into EGG_BRC_EVENT_PUMP=true" paragraph since EGG_BRC_EVENT_PUMP is no longer consulted. - §10.9.5 `_build_brc_preamble` collapse: rewrote the closing paragraph in past tense — the collapse runs unconditionally now because the event-pump wrapper is the only path; flipped "Slice-4 flips the wrapper default" → "Slice-4 flipped the wrapper default" so the doc reads as steady state. - §10.9.6 `mission.md` sandbox-rebuild paragraph: flipped "Slice-4's flag-flip is gated" → past-tense "The slice-4 default flip was gated". - §10.9.7 Composer / preamble verification stance: dropped "Slice-3" from the heading; rewrote in past tense matching the §10.7 rewrite; removed "deferred to slice-4" since slice-4 is this work. - §10.9.8 Architect open-decision resolutions: "resolved across slices 1–3" → "resolved across slices 1–4". - §11 Related Documentation cross-link: updated the Concurrent Execution Wrapper card from "how the wrapper uses SSE + wait-loop" (SSE machinery was deleted in slice-4 task-4-2) to "the deterministic event-pump bash loop driver". The two §10.7 non-blockers (slice-2 contract back-reference at §10.7 tail, architect-corrected-pseudocode parenthetical) survive as audit history — the reviewer marked them non-blocking and the context is still useful for future maintainers tracing the slice-2 design review. * docs(#2908 slice-4 task-4-4 v3 follow-up): EGG_BRC_EVENT_PUMP removed not no-op Reviewer_code v2 non-blocker #3 was deferred awaiting coder task-4-1 / task-4-2 final state. The coder's task-4-2 commit (15664e8) has now landed and the docstring at orchestrator/consensus_wrapper.py:35 confirms the env var itself was deleted ("the EGG_BRC_EVENT_PUMP env flag itself"), not just left as a dead branch. Update the docs to match: - docs/architecture/orchestrator.md env-vars table EGG_BRC_EVENT_PUMP row: "Deprecated no-op after slice-4" → "Removed in slice-4 task-4-2"; default "unset (no-op)" → "n/a (removed)"; added the helm-values / pod-spec drop-row note for operators that referenced it explicitly. - docs/architecture/orchestrator.md §"Operator-facing env vars (cross-link)": "the EGG_BRC_EVENT_PUMP selector is no longer consulted — setting it has no effect because the legacy template it selected to is gone" → "was removed in slice-4 task-4-2 — the env var is no longer read by the orchestrator, so setting it has no effect on a post-slice-4 codebase." - docs/architecture/orchestrator.md §"Rollback plan" partial-revert paragraph: tightened the post-slice-4-revert narrative to say the env var itself comes back when slice-4 is reverted (because task-4-2 is what deleted it), and operators wanting event-pump back set EGG_BRC_EVENT_PUMP=true (not =false — the defaults flip back to off). Also tightened the example of why reverse-merge order matters (slice-2 wrapper template references a composer slice-3 added, not "a composer that no longer exists"). - docs/reference/agent-wait-patterns.md §10.8: same shift — env var was deleted alongside the legacy template, so setting it has no effect; rollback path is reverse-merge order. * fix(#2908 slice-4 v2): address reviewer_code_holistic NACK on v1 Fix the six broken tests and four stale docstrings the holistic reviewer surfaced on v1 (the gateway-blocked test execution missed them; the structural issues are all visible from grep alone). Tests (orchestrator/tests/test_consensus_wrapper.py + orchestrator/tests/test_brc_nack_iteration.py): * Restored ``import os`` / ``import shlex`` / ``import subprocess`` — the surviving event-pump test classes still need them (``TestEventPumpConfirmFailureRaisesIdleAlert`` uses ``shlex.quote`` for stubbed PATH binaries; ``TestEventPumpHeartbeatSubshellLifecycle`` and the brc_snapshot tests use ``os.environ``). * Deleted ``TestEventPumpHeartbeatCadence::test_flag_off_heartbeat_path_unchanged`` — its invariant ("legacy template does not emit ``egg-orch message heartbeat``") no longer applies; the legacy template is gone. Replaced with an inline comment cross-linking to the post-deletion positive invariant. * Deleted ``TestEventPumpKeepAliveCadence::test_flag_off_keep_alive_remains_agent_side`` — same reason. * Deleted ``TestEventPumpIdleBudgetAlert::test_flag_off_idle_budget_not_used`` — same reason. * Deleted ``TestEventPumpRoleCompleteConfirm::test_flag_off_legacy_path_does_not_auto_call_consensus_confirmed`` — the legacy template is gone; the event-pump's confirm invocation is strictly orchestrator-driven via the ``case "$ACTION"`` arms, not auto-invoked on agent exit, so the symmetry guard is structurally satisfied. * Renamed ``TestEventPumpFlagIsolation::test_flag_on_does_not_inherit_legacy_max_restarts`` to ``test_event_pump_relies_on_idle_budget_not_legacy_restart_cap`` and dropped the ``max_restarts=7`` kwarg (the legacy kwarg was deleted from ``build_consensus_wrapped_command`` by task-4-2). The remaining assertion — ``EGG_BRC_IDLE_BUDGET_MIN`` is in the script — is the salient invariant. * Deleted ``TestEventPumpInvokesComposer::test_flag_off_legacy_template_does_not_reference_event_prompt`` — same legacy-path-only invariant. * Removed the orphaned ``assert "unresolved_nacks" in _CONSENSUS_WRAPPER_TEMPLATE`` line at the bottom of ``test_brc_nack_iteration.py`` (was left outside any function by the original ``TestConsensusWrapperNackFeedback`` deletion; this is a pure cleanup of slice-4 v1 commit 15664e8). Docstrings: * ``sandbox/egg_agent_tools/handlers/brc_memory.py:546`` — ``record_review_event`` docstring updated to reflect the slice-4 task-4-1 default flip (``EGG_BRC_MEMORY`` defaults to ``full`` now, not ``off``). * ``orchestrator/routes/event_prompt.py:787`` — CLI docstring updated to ``default full``; documents that ``off`` is the one-release rollback escape hatch and ``write-only`` keeps the writer warm without consuming the excerpt. * ``orchestrator/consensus_wrapper.py:81`` — module-level template comment rewritten: the env-flag predicate is gone, the event-pump template is the only template path post-task-4-2. * ``orchestrator/consensus_wrapper.py:723`` — ``build_event_pump_wrapped_command`` docstring rewritten to describe the post-task-4-2 reality (no env-flag gate; legacy template deleted; ``compose_event_prompt`` already wired). Defensive (addresses the non-blocking observation #1): * ``tests/sandbox/egg_agent_tools/test_handlers_message.py:TestMessageHeartbeat`` gains an autouse ``_isolate_slice_id_env`` fixture that clears ``EGG_SLICE_ID``. ``message_heartbeat`` auto-attaches ``slice_id`` from that env via ``_maybe_attach_slice_id``, so a developer-machine ``EGG_SLICE_ID`` (e.g. inside the egg sandbox) would otherwise add an unexpected key to the request body and fail the strict-equality assertions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-4 v3): address reviewer_code v1 NACK on coder v2 Blocking finding: * test_consensus_wrapper.py top-level imports missed ``import sys``; ``test_persistent_confirm_failure_fires_overseer_alert`` (the §1 + §6.2 lock-in test, the most operator-critical assertion in the file) uses ``sys.executable`` at line ~1092 and would raise NameError on execution, silently disabling the regression guard. The reviewer caught it via grep — same shape as the reviewer_code_holistic v1 NACK that surfaced the missing os/shlex/subprocess imports. Fix: add ``import sys`` alongside os/shlex/subprocess. Non-blocking findings (all addressed in this v3 since they're cheap): * TestEventPumpIdleBudgetAlert class docstring rewritten — ``The old template keeps MAX_CONSENSUS_RESTARTS verbatim`` was present-tense framing for the legacy template that task-4-2 deleted. Now reads ``The legacy template that owned the historical restart cap was deleted in slice-4 task-4-2; the idle budget is now the only liveness ceiling in the wrapper.`` * TestEventPumpFlagIsolation class renamed to TestEventPumpIdleBudgetCeiling with docstring rewritten — after task-4-2 there is no flag-on / flag-off partition to police, so the original name and ``cross-cutting guards`` framing no longer apply. The class retains its single surviving test (``test_event_pump_relies_on_idle_budget_not_legacy_restart_cap``) which is correct against the post-deletion state. * test_persistent_confirm_failure_fires_overseer_alert inline comment rewritten — ``_event_pump_enabled`` was deleted by task-4-2; the ``monkeypatch.setenv("EGG_BRC_EVENT_PUMP", "true")`` is harmlessly retained as a defensive guard against a future regression that re-introduces a flag-gated branch. Comment now reads as such. The reviewer flagged the docstring drift as non-blocking but I'm folding it into the same commit because the cost is one edit each and the docstring↔code mismatch the holistic v1 NACK called out is the same class of issue. Keeping the surface honest. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-4 v3 follow-up): address reviewer_code_holistic v2 blocker #2 (test_pipeline_prompts fixture) Follow-up to v3 (e093f67 pushed) that addressed the reviewer_code v2 blocker (missing import sys). This commit addresses the reviewer_code_holistic v2 blocker (2): two pre-existing test failures in orchestrator/tests/test_pipeline_prompts.py. Root cause: the slice-4 base-merge in 06c5a6c resolved the conflict on test_pipeline_prompts.py by keeping slice-3's _PLAN_WITH_MISASSIGNED_TASK fixture (``role: coder`` + ``files: integration_tests/conftest.py``). But main's #2936 ("coder authors its own tests; tester reviews-and- hardens") explicitly excluded coder→test-files from the role↔files alignment validator. The fixture no longer trips the reject path, breaking TestPlannerRoleAlignmentValidation::test_rejects_misassigned_plan_at_propose_time and ::test_rejected_proposal_does_not_mutate_tracker. Fix: cherry-pick main's fixture update — switch the misassignment fixture from a test-file path to a docs path (docs/fixtures.md), which IS still a misassignment, since docs remain the documenter's scope. Added an explanatory comment above the fixture citing #2936 and the slice-3 merge-resolution context so future readers do not re-revert under a conflict resolution that "looks like" the slice-3 text. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-4 v4): restore _auto_populate_contract + ruff I001 fix (tester v3 NACK) Tester v3 NACK had two blockers: 1. ``_auto_populate_contract_at_implement_start`` was deleted from ``orchestrator/routes/pipelines.py`` during the slice-4 base merge (commit 06c5a6c). The orphan import in ``orchestrator/tests/test_auto_populate_contract.py`` broke ``pytest --collect-only`` and blocked ``make test`` from running any tests at all (collection aborts on the first ImportError). Verified by the tester via ``git diff origin/main..origin/egg/issue-2908-impl2/slice-4`` that the function was dropped, not renamed. Fix: restored the function body verbatim from ``origin/main`` (the #2915 production implementation) and re-added the call site inside the slice-loop-mode gate where it lived on main. The function: * lives between ``_check_origin_has_plan_draft`` and ``_populate_contract_from_plan_safe`` (matches main's ordering). * is called from the ``_use_slice_loop`` check in ``_run_pipeline`` when ``_slice_count == 0``, exactly as on main. * uses ``_populate_contract_from_plan``, ``PopulateOutcome``, ``ForestValidationError``, ``_commit_statefiles_to_worktree``, and ``_pipeline_identifier`` — all present in the current file (no further imports needed). The function has a slice-4 v4 banner in its docstring explaining the restore so future merge resolutions don't re-drop it. 2. ``orchestrator/consensus_wrapper.py:50`` had a ruff I001 unsorted imports failure — an extra blank line between ``import shlex`` and the next module-level constant. Fix: removed the extra blank line (one-line deletion). Verified locally: * ``pytest --collect-only`` no longer aborts on ``ImportError: cannot import name '_auto_populate_contract_at_implement_start'``. * ``orchestrator/tests/test_auto_populate_contract.py`` imports clean. * ``orchestrator.routes.pipelines`` module imports clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-4 v7): address reviewer_code NACK — 4 ruff failures Reviewer_code re-reviewed coder v6 and NACKed with 4 blocking ruff failures + 1 ruff-format failure that would block ``make lint`` in CI: 1. ``orchestrator/tests/test_consensus_wrapper.py:13-23`` — I001 unsorted-import-block (resolved as a side-effect of fixes 2 and 3 reducing the import block to a single from-import). 2. ``orchestrator/tests/test_consensus_wrapper.py:18`` — F401 ``pytest`` imported but unused. The two surviving call sites inside function bodies use ``import pytest as _pytest`` so the top-level name was dead after the v2 test deletions. Fix: remove the top-level ``import pytest``. 3. ``orchestrator/tests/test_consensus_wrapper.py:22`` — F401 ``consensus_wrapper.build_event_pump_wrapped_command`` imported but unused (zero references in the file after the test-deletion sweep). Fix: drop the second name from the from-import. 4. ``tests/sandbox/egg_agent_tools/test_handlers_message.py:10`` — F401 ``threading`` imported but unused. Slice-4 task-4-2 (15664e8) deleted the threaded ``message_wait_loop`` heartbeat machinery; the test cases that exercised it were also removed but the top-level ``import threading`` was left behind. Fix: remove the now-dead import. 5. ``orchestrator/tests/test_pipeline_prompts.py:5129-5131`` — ruff format-check failure on a multi-line assertion message. Pre- existing from the slice-3 tester commit 7cff8d1 but surfaced only now that the file is in lint scope. Fix: ``ruff format`` collapses the two-string concatenation into a single line. Verified locally: * ``ruff check .`` → ``All checks passed!`` * ``ruff format --check .`` → ``872 files already formatted`` * ``pytest orchestrator/tests/test_consensus_wrapper.py`` → 33 passed. * ``pytest tests/sandbox/egg_agent_tools/test_handlers_message.py`` → 24 passed. * ``pytest orchestrator/tests/test_pipeline_prompts.py`` → 431 passed. Non-blocking observations from reviewer_code v6 (the _auto_populate_contract restore in routes/pipelines.py and the v4 consensus_wrapper.py I001 deletion) were already verified-clean in the prior review and remain unchanged in v7. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist BRC history for slice-4 (#2548) * docs(#2908 slice-5 task-5-4): prose-arg channels + brc verb-level CLI Document the slice-5 additive CLI surface across the four docs that already carry consensus-protocol prose: - docs/reference/orchestrator-cli.md * New "Prose-bearing args: stdin and --*-file channels (#2741)" subsection under ## BRC Consensus Protocol covering --summary-file, --reason-file, --files-reviewed-file, and the stdin sentinel `-`. * New "## BRC verb-level operations (egg-orch brc)" section documenting the next-action / get-state / list-blocking / resolve-obligation / read-peer-artifact subcommands. * Deprecation-warning note on the argv --summary / --reason path. - docs/reference/agent-tools.md * MCP↔CLI table: mcp__brc__get_state, mcp__brc__list_blocking (slice-1), and mcp__brc__read_peer_artifact, mcp__brc__resolve_obligation (slice-5) flipped from "no CLI" to their new egg-orch brc subcommands. * cli_command=None rationale list: drop the four promoted verbs and add a callout summarizing the slice-5 promotion. * Schema-derivation paragraph: shrink the "tools with no CLI" list accordingly. - docs/reference/agent-wait-patterns.md * Update re-propose / stale-version examples to use --summary-file / --reason-file (the canonical idiom for any wrapper-composed CLI). * New "Prose-bearing args use stdin / --*-file, not argv (#2741)" subsection under §1 with channel table, examples, and rationale. * Related Documentation: cross-link to the new orchestrator-cli.md BRC verb-level operations section and to #2741. - docs/guides/concurrent-execution.md * Refresh the worked Consensus Protocol example to use --summary-file for propose, --reason-file for ack, and the stdin sentinel for nack / withdraw. Add a brc resolve-obligation example. * New "egg-orch brc — verb-level read/derive surface" subsection cross-linking the canonical reference in orchestrator-cli.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(#2908 slice-5 task-5-1/5-2/5-3): prose-arg channels + brc CLI surface Slice-5 of the BRC event-pump rollout extends the CLI surface for the in-bash wrapper that ships in slice-2. Three coder tasks land here: task-5-1 — Prose-arg channels for consensus propose/ack/nack/withdraw. The wrapper composes CLI invocations via ``bash -c``, so argv-only prose (``--summary "$RAW"``, ``--reason "$RAW"``) gets corrupted by shell metacharacters (``$VAR`` / backticks / ``$()`` / ``;`` / ``&&`` / embedded newlines) — the #2741 failure mode. This slice generalises the mitigation: every prose-bearing arg now offers a paired ``--FOO-file PATH`` flag and accepts ``-`` as the argv sentinel for stdin. Argv prose still works for humans and during transition but emits ``DeprecationWarning`` so a regression to argv-only inside the wrapper surfaces. ``--files-reviewed-file PATH`` carries an array with one path per line (blank lines and ``#`` comments stripped), per architect v2 §verification_strategy.slice_5. Two helpers in orch_cli.py — ``_resolve_prose_arg`` and ``_resolve_files_reviewed_arg`` — handle channel selection (file → stdin → argv), enforce mutual exclusion, and emit the deprecation. task-5-2 — ``egg-orch brc resolve-obligation`` CLI. Verb-level wrapper around ``mcp__brc__resolve_obligation`` (#2338). Slice-6 deletes the agent-side MCP server, so the wrapper bash needs this verb reachable without an MCP round-trip. Args mirror the handler: ``--reviewer-role`` and ``--producer-role`` are required; ``--commit-sha`` and ``--note`` are optional. The ``--note`` flag uses the same prose-arg plumbing as the other reason / summary args. task-5-3 — ``egg-orch brc read-peer-artifact`` CLI. Verb-level wrapper around ``mcp__brc__read_peer_artifact``. Stdout JSON; pagination via ``--limit`` + opaque ``--cursor`` round-trip; ``--message-type`` is ``action="append"`` for repeated use; ``--no-include-unattributed`` opts out of the slice-scoped + cross-cutting merge (default on, per the handler's per-slice-partition contract from #2548). Tests authored by the coder (tester reviews-and-hardens): * ``tests/sandbox/egg_lib/test_orch_cli_prose_args.py`` — #2741 regression-guard. Parametrises seven representative prose payloads (``$VAR`` / backticks / ``$()`` / shell-control / newline+tab / UTF-8 / quotes+escapes) across each delivery channel (file, stdin sentinel, argv) for ``consensus propose``, ``ack``, ``nack``, and ``withdraw``. Asserts byte-equality between the on-disk / stdin input and the request body received by the orchestrator fake. Argv-path tests assert the ``DeprecationWarning`` fires. Mutual- exclusion paths return exit 2 with helpful stderr. ``--files- reviewed-file`` one-path-per-line semantics covered (blank lines + ``#`` comments stripped). The ``consensus propose --file`` JSON payload path (from issue #1738) is explicitly tested to NOT emit the deprecation warning — only the per-arg argv channels are deprecated. * ``tests/sandbox/egg_lib/test_orch_cli_brc.py`` — extends slice-1's test file with ``TestBrcResolveObligation`` (happy path / commit SHA / note via file / note via stdin / help / parser registration) and ``TestBrcReadPeerArtifact`` (happy path / peer-role filter / message-type list / limit+cursor pagination round-trip / no-include-unattributed default flip / phase choices restricted / help / parser registration). All 306 tests pass on the changed paths; existing consensus-push, slice-1 BRC, and CLI parity tests continue to pass unchanged. Files: sandbox/egg_lib/orch_cli.py; tests/sandbox/egg_lib/test_orch_cli_brc.py; tests/sandbox/egg_lib/test_orch_cli_prose_args.py. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2908 slice-5 task-5-4 v2): address reviewer_code v1 NACK Address three blocking findings plus the in-scope non-blocking ones raised by reviewer_code on the v1 documenter proposal. Blocking 1 — `--files-reviewed-file` is one-path-per-line (NOT JSON): The `_resolve_files_reviewed_arg` handler in orch_cli.py:884-922 reads the file as newline-delimited paths, strips blank lines and `#`-prefixed comments, and never calls `json.loads`. Update the example comment in orchestrator-cli.md to say "one path per line; blank lines and `#` comments stripped" and rewrite the heredoc example to demonstrate the comment-stripping behavior. Mirror the clarification in agent-wait-patterns.md. Blocking 2 — schema-derivation claim was wrong: The four BRC tool registrations in sandbox/egg_agent_tools/tools/brc.py (get_state / list_blocking / read_peer_artifact / resolve_obligation) ALL still declare `cli_command=None`. Slice-1 / slice-5 added thin CLI wrappers (`egg-orch brc <verb>`) over the same handlers but deliberately did NOT flip the registrations. The MCP-side schemas continue to be hand-authored in `schemas.py`; the `derive_schema_from_argparse` path is skipped. Restore the four BRC verbs to the `cli_command=None` bullet list with the additional context that a thin CLI wrapper exists; revise the "promoted to CLI" callout to "CLI surface added (registration unchanged)"; revise the schema-derivation paragraph; tag the CLI- counterpart cells with "thin wrapper, registration still cli_command=None — see callout below". Blocking 3 — `brc read-peer-artifact` does NOT use the gateway: The handler reads `.egg-state/brc-history/<identifier>-<phase>.json` files from local disk (verified: no `orchestrator_request(...)` call in `brc_read_peer_artifact`). EGG_ORCHESTRATOR_URL / EGG_LIFECYCLE_SECRET do not apply. Rewrite the "all five subcommands" sentence in orchestrator-cli.md to scope the auth claim to the other four and explain the local-disk semantics so operators don't misdiagnose missing-secret failures. Non-blocking (in-scope to the row I touched): - agent-tools.md: fix the pre-existing handler typo `handlers.brc.read_peer_artifact` → `handlers.brc.brc_read_peer_artifact` (every sibling row uses the brc_ prefix). - agent-tools.md: tighten the read_peer_artifact description to mention `<identifier>-<phase>.json` (not `<pipeline_id>`), the per-slice `<identifier>-implement-<slice_id>.json` partition, the unattributed sibling merge + `include_unattributed=False` toggle, and the `message_type` filter (single value or list). - concurrent-execution.md: author a distinct `reviewer-code-cond-ack.md` for the conditional ACK example so the prose narrative matches the obligation case (instead of re-using the unconditional ACK file). - concurrent-execution.md: show the `cat > /tmp/obligation-resolved.md` heredoc step on the `brc resolve-obligation` example (every other prose-arg example in the same section creates the file inline). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#2908 slice-5 v2): address tester v1 NACK — catch UnicodeDecodeError, return rc=2 Tester v1 NACK on coder v1 (commit 0a8a7f6): `_resolve_prose_arg` caught `OSError` but not `UnicodeDecodeError`, so a binary or non-UTF-8 file passed to `--reason-file` / `--summary-file` / `--note-file` / `--files-reviewed-file` raised a raw traceback to the wrapper bash instead of the actionable `Error: failed to read ...` message. The tester's one-line fix recommendation was to add `UnicodeDecodeError` to the `except` clauses. That's done. While there, also aligned the helpers to the established orch_cli pattern of `return 2` from `cmd_*` (the same pattern `cmd_consensus_ack` already uses for its `--pre-merge-condition-resolved-in-diff` guard): * Added `_ProseArgError` sentinel exception. `_resolve_prose_arg` and `_resolve_files_reviewed_arg` now `raise _ProseArgError` on any CLI-level validation failure (mutual exclusion, missing required arg, file-read failure incl. `UnicodeDecodeError`); the cmd_* functions catch it and return rc=2. The stderr error message is emitted by the helper before the raise — cmd_* only translates the exception to the exit code. * No more `sys.exit(2)` inside the helpers — `sys.exit` from within a cmd_* call raises `SystemExit`, which fails pytest tests that expect a clean returned rc (the tester's bug-finding test, `test_non_utf8_file_surfaces_clean_error_not_traceback`, makes this explicit by checking `assert rc == 2` after the call). Tester's adversarial test file pulled in (tester committed it as `tests/sandbox/egg_lib/test_orch_cli_prose_args_adversarial.py` at 847985f3d8). Two of the tester's tests that used `pytest.raises(SystemExit)` updated to the new `return rc=2` contract: * `TestProseFileReadErrors::test_missing_reason_file_path_surfaces_clean_error` * `TestProseArgEmptyEdges::test_empty_string_argv_treated_as_missing` Both now assert `rc == 2` returned. The `test_invalid_phase_rejected_at_parse_time` test continues to use `pytest.raises(SystemExit)` because argparse's `choices=` rejection is genuinely a parse-time SystemExit, not a cmd_* validation path. Mirror change in `tests/sandbox/egg_lib/test_orch_cli_prose_args.py`: the coder-authored `test_reason_and_reason_file_mutually_exclusive`, `test_missing_reason_fails_cleanly`, and `test_files_reviewed_and_file_mutually_exclusive` likewise switch from `pytest.raises(SystemExit)` to `assert rc == 2` + `capsys` stderr inspection. Verification: * All 142 tests in `tests/sandbox/egg_lib/` pass. * Broader regression: 193 tests across `egg_agent_tools/test_handlers_brc.py`, `test_cli_parity.py`, `test_orch_cli_consensus_push.py`, and `test_orch_cli_slice_id.py` pass unchanged. * ruff check + ruff format clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(#2908 slice-5 task-5-7): MCP-surface latency baseline capture Adds the kubectl-gated integration test ``integration_tests/test_mcp_baseline_capture.py`` that drives a real-LLM 5-role consensus through the still-live MCP surface (slice-5 is additive only) and writes per-event wall-clock samples to ``.egg-state/agent-outputs/latency-mcp-baseline.json``. The schema (``samples: [{role, event_type, start_ts, end_ts, duration_seconds, exit_code}]`` plus aggregate p50/p95/max/sum) is the input slice-6 TASK-6-6 will compare against the CLI-only baseline it captures after the MCP→CLI collapse lands. The test skips cleanly when ``_kubectl_available()`` returns False via the session-scoped ``egg_stack`` fixture, and also skips when the gateway is unhealthy (dummy GH creds in CI) — mirroring the guard used by ``test_orchestrator_mcp_contract.py``. Also commits a synthetic placeholder baseline JSON marked ``_meta.synthetic: true`` so slice-6 has a file to read while the capture test waits on a real-LLM run; ``_meta.synthetic_reason`` explains exactly how to regenerate. No ``ScriptedProvider`` import / reference (per the slice-5 plan re-scope: real LLM route, not in-process provider swap). * Persist BRC history for slice-5 (#2548) * Fix mypy errors: assert file_path non-None before open() in orch_cli * fix(#2908 slice-4): migrate test assertions off deleted capped-restart wrapper The CI Unit Tests failure on PR #2951 surfaced 18 broken tests; this commit fixes the 6 caused by Group A — call sites in two test files that the slice-4 task-4-3 sweep ("delete tests of the retired capped- restart cap") missed because they referenced ``RESTART_COUNT`` / "Restarting" / "BRC Consensus Recovery" / ``max_restarts`` / ``startup_failure_window_seconds`` rather than the symbol names listed in the original task. orchestrator/tests/test_concurrent_integration.py * ``test_spawn_agent_uses_wrapped_command``: assert event-pump markers (``event-pump``, ``egg-orch brc get-state``, ``egg-orch brc next-action``) instead of the deleted ``RESTART_COUNT`` / "BRC Consensus Recovery" strings. * Rename ``test_wrapper_contains_restart_logic`` → ``test_wrapper_drives_event_pump_loop`` and re-assert against the event-pump template. The original invariant ("orchestrator must not fake consensus on behalf of agents") is preserved — the event-pump never auto-signals READY either. orchestrator/tests/test_consensus_race_on_exit.py * Delete ``TestWrapperStaleTrackerFallback`` (4 tests) plus its unused ``os`` / ``shlex`` / ``subprocess`` / ``sys`` / ``tempfile`` imports. The class exercised ``build_consensus_wrapped_command(max_restarts=..., startup_failure_window_seconds=...)`` which slice-4 deleted in favour of the event-pump template; the event-pump reads BRC state directly via ``egg-orch brc get-state`` every loop iteration, so the wrapper no longer has a "stale tracker" of its own to fall back from. Module-docstring updated to point future readers at that history. Remaining 12 Group B failures (test_short_flow_contract_population, test_slice_4_restart_hardening) reference orchestrator production code (``_slice_agents_alive``, ``_resolve_slice_base_branch``'s ``parent_branch_exists`` kwarg, contract-runtime preservation in ``_populate_contract_from_plan``) that exists on ``origin/main`` but is missing from this branch — see PR-thread comment for the merge- regression analysis and proposed recovery paths (decision required). * fix(#2908 slice-4): restore _slice_agents_alive, parent-branch probe, runtime preservation The slice-3 → slice-4 merge resolution accidentally reverted three fixes that landed on main after slice-3 forked. This re-applies them verbatim from origin/main so the unit tests pass: * _slice_agents_alive (#2914): k8s alive guard called from the Layer-C bootstrap resume branch. Without it, a restart_phase that tore down agents but left the contract IN_PROGRESS wedges with no agents. * _resolve_slice_base_branch parent_branch_exists callback (#2928): fresh non-root slices now probe whether the derived parent branch exists on origin via ls_remote_branch_strict, replacing the pre-#2928 merge-base probe that mis-routed every fresh non-root slice onto work whenever work had advanced ahead of the parent. * _merge_preserved_slice_runtime (#2908): _populate_contract_from_plan re-parses the plan into fresh PENDING slices on every restart; the safety-net populator outside the contract_synced guard would otherwise reset COMPLETE slices and strand the pipeline on slice-1. Authored-by: egg * Address PR #2952 review feedback (egg-reviewer) Finding 1: convert three Python-2-looking ``except E1, E2:`` clauses in integration_tests/test_mcp_baseline_capture.py (lines 172, 210, 369) to parenthesized tuple form. Ruff format actively strips parens off bare ``except (E1, E2):`` (no binding) — pin with ``# fmt: skip`` so the clearer form survives the formatter. Finding 2: extend the slice-5 prose-arg plumbing to the two remaining prose-bearing flags the reviewer flagged. ``consensus propose --risk`` gains ``--risk-file PATH`` and ``--risk -`` stdin sentinel; ``consensus ack --pre-merge-condition`` likewise gains ``--pre-merge-condition-file PATH`` and stdin sentinel. Argv path still works but emits the same DeprecationWarning as ``--summary`` / ``--reason``. Docs and prose-arg test surface updated; ``--pre-merge-condition-resolved-in-diff`` deliberately not exposed (it carries a commit SHA, not prose). Finding 3: add a TODO(slice-6 TASK-6-6) block to the test_mcp_baseline_capture.py module docstring naming the synthetic- baseline trip-wire — slice-6's TASK-6-6 must hard-gate on ``_meta.synthetic`` so the 5% latency budget cannot pass by coincidence against placeholder p50/p95 numbers. --------- Co-authored-by: egg <egg@example.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: egg-orchestrator <egg@localhost> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: James Wiesebron <jameswiesebron@khanacademy.org> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

… masking failures (#3305) * fix(#3302): guard against orphan test roots + stop lint short-circuit Two toolchain-hygiene gaps from #3298 (class 2): a new test root could be silently uncollected by CI and left untyped, and `make lint-python` short-circuited so one lint failure hid another. Guard: add scripts/check-test-roots.py (runs in `make lint-custom`, i.e. the CI lint job). It discovers every `*/tests` dir containing `test_*.py` and asserts each is wired into all four test-root lists — pyproject testpaths, the `make test-all` roots, the `make test` full-suite fallback, and `TEST_ROOT_DIRS` — plus, for roots under a mypy package root, the `mypy --exclude` list. A new test root now fails CI until every list is updated atomically. Wiring: the guard surfaced four pre-existing orphan roots that CI never ran — sandbox/tests, scripts/tests, shared/egg_anchor/tests, shared/egg_contracts/tests (added by #3200/#3077/#1991, never wired). Wired all four into the lists above. This re-collected ~27 test files whose code had rotted while invisible; fixed them: - egg_contracts: bump fake commit hashes to valid 7-char hex (the AgentExecutionModel.commit pattern now requires ^[a-f0-9]{7,40}$). - sandbox/test_overseer_alert_cli: patch the handler's orchestrator_request (cmd_overseer_alert delegates to progress_overseer_alert now); the posted payload is unchanged so assertions stand. - sandbox/test_brc_cli_args: --reason moved from argparse-required to handler-layer enforcement (#2741/#2908); assert at the command layer. Lint: rewrite `make lint-python` to run ruff check, ruff format --check, and mypy independently and aggregate failures instead of aborting on the first, so one failure no longer masks another. * Fix checks: align newly-wired test roots with CI environment The #3302 orphan-test-root guard wired previously-orphaned roots (sandbox/tests, scripts/tests, shared/egg_anchor/tests, shared/egg_contracts/tests) into pytest testpaths, so make test-all now collects pre-existing tests there. Three surfaced failures under CI: - test_ci_config: expand the expected testpaths set to the full list now in pyproject.toml so the pinned set matches the wired roots. - test_brc_slice_routing: propose tests omitted commit_sha, so brc_propose fell back to 'git rev-parse HEAD' in EGG_REPO_PATH, absent on the runner. Supply an explicit commit_sha; these tests cover slice_id routing, not HEAD resolution. - test_build_host_repo_map: the CLI test ran the script via its 'env python3' shebang, which can resolve to an interpreter without PyYAML. Invoke it through sys.executable instead. * Fix checks: align retry test with post-#2270 overseer spawn path The #2270 §1.5 refactor replaced the bespoke spawn_overseer_container method with the generic _spawn_overseer_agent -> spawner.spawn_agent_job path, but TestWorktreeCreationRetry.test_retry_succeeds_on_second_attempt still asserted the removed spawn_overseer_container was called, so the Unit Tests job failed. Assert on spawn_agent_job instead. * Harden fallback-root regex anchor + document mypy exclude assumption Address review suggestions on #3302 guard: - Anchor parse_fallback_roots on the >"$selected_file" redirect so it cannot match the unrelated printf '%s\n' "$cur_id" marker write if Makefile recipes are reordered. - Document that mypy --exclude values are regexes but compared literally (every current entry is a plain literal path). --------- Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

egg-reviewer Bot approved these changes May 20, 2026

View reviewed changes