[issue-2777][slice-2/4] Scaffold + PR-phase deletions (slice-1b)

[james-in-a-box]

Issue #2777 — clean up the sliced implementation phase of the SDLC pipeline.

The sliced implement path (_run_implement_phase_slices /
_run_one_slice_inner plus the context-PR machinery in
gateway_client.py and gateway.py) has accreted significant
complexity across #2137, #2548, #2593, and #2744. A separate
egg/<id>/context branch was introduced as a parallel stack root,
and every downstream piece of complexity exists only to service that
separate branch: temp-worktree materialisation, two-tier idempotency,
ContextBranchDiverged handling, a soft-fail wrapper called from
five sites, an observability-dedup set, and a gateway push-exemption
regex. Each prior recurrence of the "context PR not opened" bug
(#2593 → #2744 → #2769) added another call site to the scaffold
instead of removing the fragility. The PR phase is also a no-op in
slice-DAG mode (_should_skip_pr_phase_auto_pr returns True
wholesale), so there is no backstop when the context PR is silently
missed.

This stack realigns the topology and trims the accumulated mess in
four stacked PRs (linear chain 1 → 2 → 3 → 4, per the architect's
iteration-1 sub-slicing of A+D into 1a/1b/1c at the operator's
direction):

1. Slice 1 (id=1, slice-1a) — Context-PR opener + wiring.
   ADDS new primitives only: _open_context_pr_at_implement_start
   (hard-required idempotent up-front opener), a PlanPreflightError
   validator at plan-phase completion, and the surgical helpers
   _is_slice_dag_mode and _resolve_slice_base_branch (cq-10).
   Rewires the five _maybe_open_base_pr_for_plan_to_implement
   call sites at pipelines.py:16503, :22132, :23671, :24666,
   plus phases.py:500. The legacy wrapper is left in place but
   unreferenced.

2. Slice 2 (id=2, slice-1b) — Scaffold + PR-phase deletions.
   DELETES the entire egg/<id>/context scaffold:
   _open_context_pr_for_pipeline and its 21 silent return-None
   paths, _lookup_existing_context_pr, _gather_context_pr_files,
   _persist_context_pr_linkage_on_contract,
   _maybe_open_base_pr_for_plan_to_implement (now unreferenced),
   _resolve_slice_1_context_branch_from_contract, the
   _context_pr_events_emitted dedup set, the
   create_context_branch gateway-client method,
   ContextBranchDiverged, the _CONTEXT_BRANCH_RE gateway
   push-exemption (plus dangling is_context_push). Deletes the
   PR phase entirely (_should_skip_pr_phase_auto_pr + caller +
   route registration + all PipelinePhase.PR reads/writes across
   ~26 files). Removes context_branch / context_title /
   context_description from PRMetadata (schema v1.1 → v1.2
   with _migrate_schema_version_to_1_2 migrator). Rewires
   stacked_pr_reconciler.py cascade-base to derive from
   context_pr_number + _resolve_slice_base_branch. Deletes
   orchestrator/consensus.py and its 8 reference clusters
   across pipelines.py (6), phases.py (1), signals.py (1).
   Picks one _check_post_consensus_stall semantic per AC-23.

3. Slice 3 (id=3, slice-1c) — Cohesion-independent cleanup.
   Adds gh pr list idempotency pre-flight to create_slice_pr
   (cq-8). Diagnoses and stops the silent rebase of egg/<id>/work
   onto main (Pipeline work branch is being rebased onto main, breaking isolation and causing slice rebase conflicts #2570 bundle), with AC-9a HITL gate if diagnosis
   points at an OOS primitive. Audits each # noqa: BLE001
   swallow-all in the slice-loop region individually (Q2).
   Collapses the 9 dual-path except ImportError slice-loop shims
   (Q3). Structurally deletes the "umbrella" terminology
   (cq-6 subsumes Drop "umbrella" terminology from slice-PR code/docs/PR bodies #2389). Adds # noqa: ARG002 / dead-code
   markers and Add per-slice MCP controls (restart_slice, etc.) for #2137 slice scheduling #2199 docstring banners to the SliceScheduler hooks
   (cq-3). Deletes stale archaeology comments. Adds the end-to-end
   integration test for the up-front context-PR open path (Q4).

4. Slice 4 (id=4, slice-2) — Slice/phase restart hardening.
   Makes restart_phase slice-aware. Eager-persists
   parent_branch_at_creation at PENDING→IN_PROGRESS. Adds a
   merge-base fallback in _resolve_slice_base_branch. Extends
   bootstrap reconciliation for IN_PROGRESS / BLOCKED slices with
   commits-on-origin > 0. Adds per-slice consensus tracker
   reconstruction in startup_reconciliation (closes Slice-scoped consensus trackers can't reconstruct from message store after orchestrator restart #2409 —
   threading slice_id into existing
   reconstruct_tracker_from_messages).

Impact: idempotent-by-construction context PR removes the
recurring "context PR not opened" failure class (#2593, #2744,
#2769). Pipelines surviving an orchestrator-pod recycle resume
instead of re-spawning. The schema bump and PR-phase deletion are
breaking changes for the in-flight pipelines; per feedback Q5 none
exist, so the clean break is safe. Net deletions estimated at
~600 lines against ~200 added (new opener, new helpers,
BLE001 audit replacements, tests).

This slice

Scaffold + PR-phase deletions (slice-1b)

Files affected:

• orchestrator/routes/pipelines.py
• orchestrator/routes/phases.py
• orchestrator/mcp_tools.py
• orchestrator/overseer/monitor.py
• orchestrator/dag_visualizer.py
• shared/egg_contracts/models.py
• shared/egg_contracts/phase_defaults.py
• gateway/phase_filter.py
• gateway/phase_transition.py
• orchestrator/gateway_client.py
• gateway/gateway.py
• orchestrator/stacked_pr_reconciler.py
• orchestrator/consensus.py
• orchestrator/routes/signals.py
• orchestrator/peer_consensus.py
• tests/shared/egg_contracts/test_phase_defaults.py
• gateway/tests/test_pipeline_push_block.py
• gateway/tests/test_phase_api.py
• gateway/tests/test_phase_filter.py
• gateway/tests/test_phase_filter_restrictions.py
• gateway/tests/test_phase_transition.py
• tests/shared/egg_contracts/test_pr_metadata.py
• tests/docs/test_context_pr_doc_terminology.py

Tasks:

• task-2-1: Delete the entire egg/<id>/context parallel-stack-root scaffold in orchestrator/routes/pipelines.py. Specifically delete: _open_context_pr_for_pipeline (line 10002, ~640 lines including the 21 silent return None paths); _lookup_existing_context_pr (line 9735, ~150 lines); _gather_context_pr_files (line 9896); _persist_context_pr_linkage_on_contract (line 9791); _maybe_open_base_pr_for_plan_to_implement (line 10648, ~230 lines — note TASK-1-2 has replaced its single surviving call site already, so this is a pure deletion); _resolve_slice_1_context_branch_from_contract (line 10883); the _context_pr_events_emitted dict and lock at lines 10644–10645 and their touch sites at 1850, 1851, 10801, 10802; the context_pr.skipped and context_pr.failed event-bus entries at lines 291–292 and 1036–1037 (and EventType.CONTEXT_PR_SKIPPED / CONTEXT_PR_FAILED if they exist in orchestrator/events.py). Surviving context_branch / context_title / context_description read sites in pipelines.py outside the deleted function bodies (added per reviewer_plan v2 blocker 2) — these are NOT inside the function deletions above and MUST be removed in this task or routed through the new helpers from TASK-1-3: - pipelines.py:10801, 10804, 10844 — context_branch reads (plan-anchor lines). Re-anchor against HEAD; if inside a now-deleted function, drop with the function; if standalone, replace with the resolved parent branch via _resolve_slice_base_branch from TASK-1-3. - pipelines.py:11096-11097 — context_title / context_description reads in slice-PR builder. After cq-2 these no longer exist; replace with reads of contract.pr.title and contract.pr.description (the canonical fields used by TASK-1-2). - pipelines.py:11519-11542 — context_branch read in cascade-base sub-block. Reroute through _resolve_slice_base_branch. - pipelines.py:16755, 16781 — context_branch reads outside any deleted function. Reroute through _resolve_slice_base_branch or drop if dead. - pipelines.py:20193 — context_branch read at advance-phase boundary. Drop if covered by TASK-1-2's new opener path; otherwise reroute. The implement-phase coder MUST run the verification grep rg 'context_branch\|context_title\|context_description' orchestrator/routes/pipelines.py BEFORE editing to re-anchor each of the named lines against HEAD (per the global re-anchoring note in §Approach) and AGAIN after editing to confirm zero hits outside test scaffolding. Update slice-1 base resolution in pipelines.py:15394–15405 to call the new _resolve_slice_base_branch from TASK-1-3 instead of _resolve_slice_1_context_branch_from_contract. Verify no other references to deleted symbols remain via grep -rn '<symbol>' orchestrator/ shared/ gateway/ tests/ integration_tests/ — widened scope catches leaks into gateway code, gateway tests, and integration tests. Ordering note: this task depends_on: [TASK-1-2, TASK-1-1, TASK-2-4, TASK-2-5, TASK-1-3] (new opener, plan validator, schema cleanup, cascade rewire, and new slice-base resolver must all exist first; TASK-2-4 + TASK-2-5 clear the structural consumer ahead of this deletion).
  • Acceptance criteria: - The seven functions listed above are removed from pipelines.py. - The _context_pr_events_emitted dict, lock, and all four touch sites are removed. - The context_pr.skipped / context_pr.failed event-bus entries are removed (along with their EventType members if present). - Each of the seven enumerated surviving read sites (10801, 10804, 10844, 11096-11097, 11519-11542, 16755, 16781, 20193) is either removed or rerouted through _resolve_slice_base_branch / contract.pr.title / description. - grep -rn for each deleted symbol across orchestrator/ shared/ gateway/ tests/ integration_tests/ returns zero hits outside test files actively being rewritten by TASK-3-8 / TASK-2-9 / TASK-3-11. - The post-edit verification grep rg 'context_branch|context_title|context_description' orchestrator/routes/pipelines.py returns zero hits. - Slice-1 base resolution at pipelines.py:15394–15405 now calls _resolve_slice_base_branch (from TASK-1-3).
• task-2-2: Delete the PR phase entirely (cq-4). Per the risk-analyst's R1 audit, the actual surface is ~9 sites — not just the four named by cq-4. Touch ALL of these in one task so the delete is atomic: (1) _should_skip_pr_phase_auto_pr def pipelines.py:8222 and its sole caller pipelines.py:20844. (2) _finalize_pr_phase_failed at pipelines.py:8280, 21024 (owns HEAD-recovery semantics post-PR-phase-failure; becomes obsolete). (3) Two further PipelinePhase.PR references in pipelines: pipelines.py:18747 and pipelines.py:19843. (4) _get_pr_url_from_pipeline at pipelines.py:4067-4075 reads from phases['pr'].artifacts['pr_url']. After deletion, the PR URL is read directly from contract.pr.context_pr_number (set by TASK-1-2) — update this helper or remove and inline. (5) PipelinePhase.PR = 'pr' enum value at shared/egg_contracts/models.py:78 (StrEnum at line 62). Hard-remove per Q5 (no in-flight pipelines). (6) Phase-graph constants at orchestrator/routes/phases.py:70-71: PipelinePhase.IMPLEMENT: [PipelinePhase.PR] and PipelinePhase.PR: []. Change IMPLEMENT's downstream list to [] (terminal), drop the PR row entirely. Also fix the 'next_phase': 'pr' response payload at orchestrator/routes/phases.py:849. (7) mcp_tools.py:728 advance_phase MCP tool definition advertises 'pr' as a valid target value — remove. mcp_tools.py:1409 reads phases['pr'].artifacts for the PR URL on completion — re-point at contract.pr.context_pr_number. (8) Overseer monitor: _check_pr_phase_outcome and the pr_phase_no_pr alert at orchestrator/overseer/monitor.py:481, 1168-1180, 1707-1741. Delete the probe, the alert wiring, and the alert-type registration. After deletion there is no pr phase to check; an alert that would now never fire is dead code. Plus _check_post_consensus_stall semantic rewire (added per reviewer_plan v2 blocker 6): at orchestrator/overseer/monitor.py:1122-1160 the post-consensus-stall predicate short-circuits on the old "PR phase has run and recorded artifacts" signal via a getattr chain through phases['pr'].artifacts. That signal is a STRICT SUPERSET of the new "context PR exists" signal (context_pr_number is not None), because cq-4 moves the context PR to the plan→implement boundary — so context_pr_number is set throughout implement, not after a PR-phase boundary. A blanket find/replace would silently weaken the Orchestrator pipeline state fails to sync after implement-phase push + PR creation (causes false 'post-consensus-push-stall' alerts) #1911 stall signal the predicate was wired to produce. Required choice: either (a) DELETE the short-circuit entirely if the underlying stall signal becomes unreachable post-PR-phase-deletion (verify via code-walk; document the proof in the commit message), OR (b) re-derive the equivalent predicate — e.g. "all slices closed AND context_pr_number is set AND no consensus events in last N seconds" — preserving the superset semantics. Pick one explicitly in the commit message and state why; silent acceptance of the weaker predicate is a regression on Orchestrator pipeline state fails to sync after implement-phase push + PR creation (causes false 'post-consensus-push-stall' alerts) #1911. (9) dag_visualizer.py:53, 61 — PipelinePhase.PR as a node in the visualizer's graph. Remove the node and the edge from IMPLEMENT to PR. (10) Gateway-side PR-phase entries (lock-step with orchestrator): gateway/phase_filter.py:526 (PipelinePhase.PR: PhasePermissions(...)) and gateway/phase_filter.py:642 (PR: PhaseFileRestriction(...)); the PR-phase row in the state-machine transition table in gateway/phase_transition.py. Removing PipelinePhase.PR from orchestrator without lock-step gateway removal leaves the gateway state machine inconsistent — a v1.1 contract load post-deploy, or any test that invokes advance_phase target='pr', surfaces the mismatch. Delete both sites in the same task so the deploy is atomic. (11) shared/egg_contracts/phase_defaults.py:105 row removal (added per reviewer_plan v2 blocker 3): the PipelinePhase.PR: PhaseConfig(...) row in the phase-defaults table. Removing PipelinePhase.PR from the StrEnum without removing this row produces a KeyError at startup when downstream consumers iterate the defaults dict. Verified at HEAD via grep -n "PipelinePhase.PR\|'pr'" shared/egg_contracts/phase_defaults.py. DO NOT touch gateway_client.py:1441 where create_pr registers a temp gateway session with phase='pr'. That is the gateway session-namespace phase string used so the gateway accepts the gh pr create op; it is NOT the same as PipelinePhase.PR. Note (added per reviewer_plan v2 blocker 3): the prior plan also listed gateway_client.py:1409 and :2567 in the carve-out. Verified at HEAD: :1409 is in the same namesake region and remains preserved; :2567 is unrelated — it's a gh pr list CLI args list entry (the literal 'pr', argument to gh) and MUST NOT be in the carve-out enumeration. Drop :2567 from the preserve list and re-anchor :1409 against HEAD before editing (per the global re-anchoring note in §Approach). Additional preserve targets (added per reviewer_plan v2 non-blocking note): gateway/tests/test_session_manager.py:1127, 1170 and gateway/tests/test_gateway.py:4371 are namesake hits that assert the gateway-session namespace phase='pr' survives PipelinePhase.PR removal. They MUST NOT be deleted by this task's grep sweep. Verification artifact: run rg 'PipelinePhase\\.PR|phases\\["pr"\\]|phase=.pr.|phase == .pr.' orchestrator/ shared/ gateway/ BEFORE the task starts AND AFTER all 11 site-categories are addressed; the after-grep must show only the gateway-session phase='pr' hits in gateway_client.py (:1409, :1441) and the namesake test hits in gateway/tests/test_session_manager.py and gateway/tests/test_gateway.py (the documented carve-out). Commit BOTH the before-grep and the after-grep output verbatim in the commit message so reviewer_plan can spot-check the delta without rerunning the audit. Tests are owned by TASK-2-7 (schema/phase_defaults tests), TASK-2-9 (gateway PR-phase tests), TASK-3-11 (orchestrator PR-phase tests), and TASK-3-12 (docs).
  • Acceptance criteria: - All 11 site-categories above are addressed (ci: bump actions/checkout from 4 to 6 #10 covers the gateway-side phase_filter + phase_transition entries; ci: bump actions/setup-python from 5 to 6 #11 covers shared/egg_contracts/phase_defaults.py:105). - Verification grep rg 'PipelinePhase\\.PR|phases\\["pr"\\]|phase=.pr.|phase == .pr.' orchestrator/ shared/ gateway/ returns ONLY the gateway-session phase='pr' hits in gateway_client.py (:1409, :1441) and the namesake test hits in gateway/tests/test_session_manager.py:1127, 1170 and gateway/tests/test_gateway.py:4371. :2567 is NOT in the carve-out (it's a gh pr list CLI args list, not the session namespace). - PipelinePhase.PR enum member removed from BOTH shared/egg_contracts/models.py AND shared/egg_contracts/phase_defaults.py AND gateway/phase_filter.py AND gateway/phase_transition.py. - Phase graph IMPLEMENT: [] is terminal. - Overseer _check_pr_phase_outcome and its alert wiring are removed. - _check_post_consensus_stall semantic rewire is addressed per Merge Phase 1.5 docs into main #8: either the short-circuit is deleted (with proof of unreachability in commit message) or the equivalent stall predicate is re-derived from contract.pr.context_pr_number + slice-closure + consensus-quiescence. The choice and rationale are stated in the commit message. - dag_visualizer no longer renders a PR node. - The advance_phase MCP definition no longer accepts target='pr'. - Commit message contains BOTH the BEFORE and AFTER output of the verification grep, verbatim.
• task-2-3: Delete GatewayClient.create_context_branch (orchestrator/gateway_client.py:2327, ~90 lines) and ContextBranchDiverged (gateway_client.py:3453) — both are dead once TASK-2-1 removes the only callers. Delete _CONTEXT_BRANCH_RE from gateway/gateway.py:1112 and remove the regex from the push-block enforcement at gateway/gateway.py:1350 and 1362. Before deletion, confirm via grep that the gateway's pipeline-session push-allow list already accepts egg/<id>/work pushes (it does — the work branch is the canonical pipeline tip tracked by the session). If a pipeline session does NOT already cover egg/<id>/work on slice-loop entry, surface an impasse instead of silently leaving a hole. is_context_push cleanup (added per reviewer_plan v2 non-blocking R7): gateway/gateway.py:1344-1392 carries a dangling is_context_push variable that becomes unreachable once _CONTEXT_BRANCH_RE is removed (the regex was the only thing that ever flipped it to True). Locate the variable's full lifecycle via grep -n "is_context_push" gateway/gateway.py (currently ~5 hits: line 1344 narrative comment, line 1349 is_context_push = False initializer, line 1363 is_context_push = bool(_CONTEXT_BRANCH_RE.match(branch)) assignment, line 1376 read inside conditional, line 1392 elif is_context_push: branch). Two acceptable treatments: (a) hard-replace with is_context_push = False everywhere (preserves the narrative comment and the conditional structure, makes dead-codepath status obvious), OR (b) remove the variable entirely along with its narrative comment and downstream conditional branches (collapses dead branches and net-negative LOC). Pick (b) unless an audit reveals the variable is referenced by callers/audit-log emitters outside this file.
  • Acceptance criteria: - create_context_branch and ContextBranchDiverged removed from gateway_client.py. - _CONTEXT_BRANCH_RE removed from gateway/gateway.py; both push-block call sites updated to no longer reference it. - is_context_push variable + narrative comment + the four downstream conditional references at gateway/gateway.py:1344-1392 are either removed entirely (preferred) or hard-pinned to False with a comment explaining the residual is intentional scaffolding. - grep -rn 'ContextBranchDiverged\|create_context_branch\|is_context_push' returns zero hits outside test files (or only the hard-pinned-False if option (a) is chosen). - Gateway pipeline-session push-allow logic still permits egg/<id>/work pushes (manual verification step in commit message).
• task-2-4: PRMetadata schema cleanup (cq-2 — hard-remove). In shared/egg_contracts/models.py: delete the context_branch, context_title, and context_description fields from the PRMetadata class (currently at lines 499–531). KEEP context_pr_number (still used as the PR number of the egg/<id>/work → main PR). KEEP deferred_actions. Bump the schema version constant at line 763 from "1.1" to "1.2". Add a _migrate_schema_version_to_1_2 migration entry (per reviewer_plan v2 blocker 2): the operator's Q5 confirms no in-flight pipelines, but on-disk fixtures (.egg-state/contracts/issue-2777-replan.json, issue-2769.json, issue-2548.json, issue-2474.json, issue-1557-v2.json) carry the three removed fields. The contract loader for THIS very pipeline will refuse the v1.2 load without migration. The migration entry must (a) drop the three fields when present on load, (b) preserve context_pr_number and deferred_actions, (c) leave fresh-v1.2 contracts untouched (no-op). The existing migration registry pattern is the precedent — locate it via grep -rn "schemaVersion\|_migrate" shared/egg_contracts/. Search for all read sites of the three deleted fields across the codebase (grep -rn 'context_branch\|context_title\|context_description') and either delete them (if covered by TASK-2-1 or TASK-3-1) or note them for the new TASK-2-5 structural rewire (stacked_pr_reconciler.py cascade-base and the seven pipelines.py read sites enumerated in TASK-2-1's extended scope). Any read site that survives outside the deletion-task scope is a bug.
  • Acceptance criteria: - The three fields are removed from PRMetadata. - schemaVersion default is "1.2". - _migrate_schema_version_to_1_2 exists and drops the three removed fields from on-disk v1.1 contracts on load (no-op for v1.2). - No surviving read site of any deleted field outside test files AND outside the new TASK-2-5 structural rewire scope (stacked_pr_reconciler.py). - The pipeline's own contract on disk (.egg-state/contracts/issue-2777-replan.json) loads successfully under the v1.2 schema via the migration entry.
• task-2-5: NEW — added per reviewer_plan v2 blocker 2 (cascade-base rewire from context_branch to context_pr_number). orchestrator/stacked_pr_reconciler.py is a STRUCTURAL consumer of the deleted contract.pr.context_branch field — at HEAD the references are at lines 94, 112, 120, 129, 150, 157-158, 247, 275, 283 (verified via grep -n "context_branch\|context_title\|context_description" orchestrator/stacked_pr_reconciler.py). These are NOT covered by TASK-2-1's pipelines.py deletion scope nor TASK-3-1's umbrella deletion. The reconciler threads context_branch through the cascade-base fallback for orphaned slices — exactly the safety net cq-9 tries to preserve. After TASK-2-4 deletes the field, every read site here raises AttributeError at runtime. Rewire the cascade-base resolution onto the new _resolve_slice_base_branch helper from TASK-1-3 (which gains a merge-base fallback in TASK-4-3 for orphaned slices). For the specific case where the reconciler today falls back to context_branch for "PR shouldn't get here" paths (line 150 comment), the new path resolves through _resolve_slice_base_branch instead. Replace each read site with the appropriate helper call: - Default-argument context_branch: str | None = None → replace with parent_branch: str | None = None. - Docstring + comment references → update to describe the new cascade-base contract. - Argument-passing sites → switch to passing the resolved parent branch via _resolve_slice_base_branch. Add a unit test in TASK-3-8 that exercises the stacked_pr_reconciler.py cascade-base fallback with the new helper. Ordering: depends_on: [TASK-1-3, TASK-2-4] — _resolve_slice_base_branch from TASK-1-3 must exist, and TASK-2-4 must have removed the schema field so the rewire isn't redundant.
  • Acceptance criteria: - orchestrator/stacked_pr_reconciler.py no longer reads contract.pr.context_branch. - The cascade-base resolution goes through _resolve_slice_base_branch (from TASK-1-3 / TASK-4-3). - grep -n "context_branch" orchestrator/stacked_pr_reconciler.py returns zero hits. - The orphaned-slice safety net (cq-9 intent) is preserved by routing through the merge-base fallback (TASK-4-3). - Unit test in TASK-3-8 covers the new cascade-base fallback path.
• task-2-6: Delete the legacy ConsensusEvaluator module (cq-5). reviewer_plan v1→v2 + risk_analyst v2→v3 verified the production call surface. There are EIGHT reference clusters total — all must be removed in this task. Citations below give BOTH the refine-anchor 1cb235871 line range AND the HEAD line range; per the global re-anchoring note in §Approach, the coder MUST re-anchor again at implement-time HEAD before editing. In orchestrator/routes/pipelines.py (6 clusters — per architect AC-18 + risk_analyst v2 blocker 1, verified at HEAD via grep -n "get_consensus_evaluator|from consensus import|from ..consensus" orchestrator/routes/pipelines.py): (1) refine-anchor 1805-1808 / HEAD 1813-1816 — import + .clear() call (the early-cancel path). (2) refine-anchor 2844-2848 / HEAD 2859-2863 — import + evaluator = get_consensus_evaluator() handle. (3) refine-anchor 3274-3279 / HEAD 3289-3293 — import + handle + .clear() call (the restart_phase consensus-clear block named by the analysis). (4) refine-anchor ~3486-3496 / HEAD 3516-3526 — import + handle + .clear() call inside the "Failed to clear legacy consensus after hard-reset ack" block. NEW — added per risk_analyst v2 blocker 1; this cluster was missed in v2 and is a distinct call site from (3) (the hard-reset-ack path vs the restart_phase consensus-clear path). Deletion of consensus.py without removing this cluster guarantees an ImportError post-restart on the hard-reset ack path. Verified at HEAD via sed -n '3510,3530p' orchestrator/routes/pipelines.py. (5) refine-anchor 4206-4210 / HEAD 4489-4493 — import + handle (nested path). (6) refine-anchor 4215-4219 / HEAD 4498-4502 — second import + handle in the same neighbourhood. In orchestrator/routes/phases.py (1 cluster — added per reviewer_plan v2 blocker 1, verified at HEAD via grep -n "consensus" orchestrator/routes/phases.py): (7) phases.py:119-124 — try: from consensus import get_consensus_evaluator; except ImportError: from ..consensus import get_consensus_evaluator + get_consensus_evaluator().clear(pipeline_id) call inside the complete_phase route's "Clear ephemeral message store and consensus state on phase transition" block. Deletion of consensus.py without removing this cluster guarantees an ImportError at startup the first time complete_phase is invoked. In orchestrator/routes/signals.py (1 cluster — added per reviewer_plan v2 blocker 1, verified at HEAD via grep -n "get_consensus_evaluator|ConsensusEvaluator|from consensus|from .consensus" orchestrator/routes/signals.py): (8) signals.py:847-871 — try: from consensus import ReadinessState, get_consensus_evaluator; except ImportError: from ..consensus import ReadinessState, get_consensus_evaluator + evaluator = get_consensus_evaluator() + evaluator.evaluate(...) call in the READY heartbeat handler. Deletion of consensus.py without removing this cluster guarantees an ImportError the first time a READY signal fires. Each cluster has a 3-line try: from consensus import get_consensus_evaluator; except ImportError: from ..consensus import get_consensus_evaluator shim plus the actual usage. Delete all eight clusters AND the orchestrator/consensus.py module (ConsensusEvaluator class at line 38, get_consensus_evaluator() singleton at line 153, ReadinessState enum). Verify via grep -rn 'ConsensusEvaluator|get_consensus_evaluator|ReadinessState|from consensus|from .consensus|from orchestrator.consensus' orchestrator/ shared/ gateway/ tests/ integration_tests/ that no other module imports from it. The BRC PeerConsensusTracker (orchestrator/peer_consensus.py:69) is the only consensus path; nothing else needs to change. Notes / follow-on cleanup (added per risk_analyst v2 non-blocking): the peer_consensus.py:1604 alias method is a compatibility shim on the surviving tracker (not the deleted module). It is NOT a runtime-breaker for this task's deletion, but verify whether it remains dead post-deletion via grep -rn '<alias-method-name>'. If dead, remove it lockstep with this task; if reachable (test scaffolding or future-Add per-slice MCP controls (restart_slice, etc.) for #2137 slice scheduling #2199 hook), leave it and file a follow-up issue noting the residual coupling. Commit the after-grep output in the commit message for reviewer_plan to spot-check.
  • Acceptance criteria: - orchestrator/consensus.py is deleted. - All EIGHT reference clusters are removed: 6 in pipelines.py (refine-anchored lines 1805-1808, 2844-2848, 3274-3279, ~3486-3496, 4206-4210, 4215-4219; HEAD-anchored 1813-1816, 2859-2863, 3289-3293, 3516-3526, 4489-4493, 4498-4502), 1 in phases.py (119-124), 1 in signals.py (847-871). Re-anchored against HEAD before editing. - The hard-reset-ack consensus-clear block at HEAD 3516-3526 (cluster 4) is explicitly removed — verified by the post-edit grep returning zero hits in that line range. - No surviving import of the deleted module (or its ReadinessState enum) across the repo (verified by the widened grep). - peer_consensus.py:1604 alias method's reachability is verified post-deletion; if dead, removed lockstep; if reachable, noted in commit message with follow-up issue link. - Commit message contains the after-grep output.
• task-2-7: Rewrite tests/shared/egg_contracts/test_phase_defaults.py to reflect the removal of PipelinePhase.PR from shared/egg_contracts/phase_defaults.py:105 (and the PipelinePhase StrEnum at shared/egg_contracts/models.py:62-78 per TASK-2-2 (5)). Specifically: drop any test that asserts PR is in the phase-defaults table; assert IMPLEMENT is the terminal phase with no downstream; add a default-deny coverage test that asserts a planner trying to default to phase 'pr' is rejected. The test runs under make test.
  • Acceptance criteria: - tests/shared/egg_contracts/test_phase_defaults.py passes with the PR-phase removed. - The test file no longer references PipelinePhase.PR. - A new default-deny test asserts that 'pr' is not an accepted phase string.
• task-2-8: Rewrite gateway/tests/test_pipeline_push_block.py to reflect _CONTEXT_BRANCH_RE deletion (TASK-2-3). The existing context-branch allow-test class at lines 994-1052 becomes obsolete because the exemption regex no longer exists. Delete the class. Add a replacement regression test that verifies egg/<id>/context pushes are now BLOCKED (the exemption is gone — the branch itself is gone — but a misbehaving caller might still try to push to it; assert the gateway rejects the push with a clear policy-violation error). Run under make test to confirm.
  • Acceptance criteria: - Lines 994-1052 (the context-branch allow-test class) are deleted from gateway/tests/test_pipeline_push_block.py. - A replacement regression test asserts that a push to egg/<id>/context is rejected by the gateway with a policy-violation error message. - make test passes.
• task-2-9: Rewrite the four gateway PR-phase test files to drop PR-phase assertions and add default-deny coverage for target='pr' (architect v2 AC-4c, lock-step with TASK-2-2's PipelinePhase.PR removal): (1) gateway/tests/test_phase_api.py — drop any test that asserts PR-phase advancement succeeds; add a test that asserts advance_phase target='pr' is rejected (default-deny). (2) gateway/tests/test_phase_filter.py — drop any assertion that PipelinePhase.PR exists in the phase-permissions table; add a test that asserts PR is not a valid permission key. (3) gateway/tests/test_phase_filter_restrictions.py — drop any assertion that PR-phase has a PhaseFileRestriction entry; add a test that asserts looking up restrictions for 'pr' returns the default-deny. (4) gateway/tests/test_phase_transition.py — drop any assertion that IMPLEMENT → PR or PR → COMPLETE is an accepted transition; add a test that asserts IMPLEMENT is the terminal phase. Run make test to confirm green.
  • Acceptance criteria: - All four files updated per the above. - Each file has at least one new default-deny test verifying 'pr' is no longer accepted. - make test passes.
• task-2-10: Update schema and doc-terminology tests for the PRMetadata field removal in TASK-2-4. Three changes: (1) tests/shared/egg_contracts/test_pr_metadata.py:91-142 currently has ~18 asserts on the three deleted fields (context_branch, context_title, context_description). Delete those asserts; add one positive test that asserts PRMetadata no longer accepts those field names (Pydantic rejects with extra='forbid' validation error); add one positive test that asserts context_pr_number and deferred_actions still work as before. (2) tests/docs/test_context_pr_doc_terminology.py:70-243 has doc-terminology asserts on the deleted field names (the test asserts docs mention the fields). Delete those asserts; add a replacement test that asserts docs DO mention context_pr_number (still live) but do NOT mention the three removed fields (regression test that docs were updated). (3) Any test in tests/ or orchestrator/tests/ that imports context_branch / context_title / context_description from PRMetadata — grep tests/ orchestrator/tests/ integration_tests/ before completing to catch stragglers. Run make test-all to confirm a green suite.
  • Acceptance criteria: - test_pr_metadata.py:91-142 asserts on deleted fields are removed; positive tests for the field removal exist. - test_context_pr_doc_terminology.py:70-243 doc asserts on deleted fields are removed; replacement doc-update regression test exists. - No surviving test imports the three deleted fields (verified by grep -rn 'context_branch\|context_title\|context_description' tests/ orchestrator/tests/ integration_tests/).

Test Plan

• Automated (slice-1, id=1, slice-1a opener + wiring):
  • Unit tests for _open_context_pr_at_implement_start
    (happy / idempotent / hard-required paths),
    PlanPreflightError validator (5 rejection cases per AC-1a),
    _is_slice_dag_mode, _resolve_slice_base_branch (without
    merge-base fallback — lands in slice-4).
• Automated (slice-2, id=2, slice-1b deletions):
  • Unit tests that the deleted helpers / class / fields no
    longer import. test_dag_visualizer.py rewritten for
    IMPLEMENT-terminal. test_pr_metadata.py rewritten for the
    removed fields + migration tests for
    _migrate_schema_version_to_1_2. Gateway-side test files
    rewritten for default-deny on target='pr'. stacked_pr_reconciler
    cascade-base test exercises the new derivation.
    _check_post_consensus_stall test asserts the new semantic
    (per AC-23 choice).
• Automated (slice-3, id=3, slice-1c cleanup tail):
  • Unit tests: create_slice_pr idempotency pre-flight; no
    umbrella string anywhere; 3-5 BLE001 sample sites; Pipeline work branch is being rebased onto main, breaking isolation and causing slice rebase conflicts #2570
    invariant test asserts merge-base unchanged after N≥3 phase
    transitions with M≥2 main PRs merged in parallel.
  • Integration test under integration_tests/regression/
    (kubectl-gated recovery/regression tier; the legacy
    integration_tests/local_pipeline/ directory was deleted
    on 2026-05-11 in commit f7803637d1) that spawns a sliced-
    DAG pipeline, asserts a single context PR exists with
    head=egg/<id>/work base=main, clears context_pr_number
    and re-triggers implement-start hook, asserts no duplicate
    PR is opened (AC-8). SDLC integration tests rewritten for
    implement→complete transition.
• Automated (slice-4, id=4, slice-2 restart hardening):
  • Unit tests for slice-aware restart_phase (per-slice tracker
    clear), eager-persist of parent_branch_at_creation (field
    present at PENDING→IN_PROGRESS), the merge-base fallback in
    _resolve_slice_base_branch, and the extended bootstrap
    reconciliation that resumes non-COMPLETE slices without
    re-spawning.
  • Integration test under integration_tests/regression/
    that kills the orchestrator pod mid-implement on a sliced
    pipeline, restarts, and asserts per-slice consensus trackers
    reconstruct (Slice-scoped consensus trackers can't reconstruct from message store after orchestrator restart #2409 closure proof / AC-16).
• Manual:
  • After slice-1 (id=1) merges: confirm the new opener helper
    is callable; no behavior change observable yet.
  • After slice-2 (id=2) merges: run a small sliced pipeline;
    confirm context PR opens automatically; confirm PR phase
    removed.
  • After slice-3 (id=3) merges: confirm create_slice_pr
    idempotency; confirm no umbrella string; make test-all
    green.
  • After slice-4 (id=4) merges: kill orchestrator mid-implement
    on a sliced pipeline; restart; confirm slice resumes without
    respawning, per-slice consensus trackers report prior state.

Manual Steps

Pre-merge (slice-1, id=1): None. Slice-1a only ADDS code.

Pre-merge (slice-2, id=2):

• Confirm there are NO in-flight slice-DAG pipelines in RUNNING
  state at deploy time (feedback Q5 confirmed none; re-confirm
  at merge). The PRMetadata schema bump (v1.1 → v1.2) auto-
  migrates via _migrate_schema_version_to_1_2.
• Verify the gateway's pipeline-session push-allow list already
  accepts pushes to egg/<id>/work; removing _CONTEXT_BRANCH_RE
  must not leave a hole.

Pre-merge (slice-3, id=3):

• The Pipeline work branch is being rebased onto main, breaking isolation and causing slice rebase conflicts #2570 fix bundle may surface an AC-9a HITL via
  mcp__sdlc__register_open_question if Phase-1 diagnosis
  identifies the root cause inside an OOS primitive. The HITL
  is mid-implement, not pre-merge.

Pre-merge (slice-4, id=4): None.

Post-merge (slice-1, id=1): None.
Post-merge (slice-2, id=2): None (PR-phase concept closed
structurally).
Post-merge (slice-3, id=3): Close #2389 with a reference to
slice-3's PR (cq-6 subsumes). Close #2570 with a reference to
slice-3's PR (AC-9 invariant test); if AC-9a HITL was option (c)
xfail-and-defer, close instead with the follow-up issue
reference.
Post-merge (slice-4, id=4): Close #2409 with a reference to
slice-4's PR (subsumed).

Stack

• Position: slice 2 of 4 in pipeline issue-2777-replan
• Stacked on top of egg/issue-2777-replan/slice-1

Slice slice-2 of pipeline issue-2777-replan. Stacked on top of egg/issue-2777-replan/slice-1.

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflicts with egg/issue-2777-replan/slice-1:

File	Category	Resolution
orchestrator/routes/pipelines.py	Semantic (additive + scaffold-deletion)	5 conflict blocks — see breakdown below

Per-conflict breakdown:

1. Lines 9795–11018 (scaffold deletion) — additive vs. scaffold-deletion. slice-1 added tombstone-marked _build_pr_body / _auto_create_pr / _ExistingPRLookup / _lookup_existing_context_pr / _persist_context_pr_linkage_on_contract / _gather_context_pr_files / _open_context_pr_for_pipeline per egg-reviewer non-blocking Phase 4: CLI implementation #6. Accepted HEAD — slice-2 deletes these functions per TASK-2-1 + TASK-2-2.

2. Lines 11109–11119 (docstring) — additive. Both sides describe _persist_context_pr_number differently. Accepted slice-1's improved persistence-surface description (egg-reviewer non-blocking Phase 1.5: Documentation extraction #3 doc improvement; slice-1's wording is strictly clearer).

3. Lines 11131–11179 (docstring) — additive. HEAD has the TASK-2-2 pipeline-mirror description; slice-1 has the egg-reviewer non-blocking Phase 1.5: Documentation extraction #3 persistence-surface description. Combined both — they describe complementary behavior present in HEAD's code (pipeline-level field mirror + on-disk-pending-runner-commit semantics).

4. Lines 11581–11593 (docstring) — semantic. HEAD references the new extant_branches parameter on _resolve_slice_base_branch; slice-1 had a "Consumed by slice-2 TASK-2-1" tombstone. Accepted HEAD — we ARE slice-2, so the tombstone is now obsolete; HEAD's wording is the current docstring.

5. Lines 11718–11985 (scaffold deletion) — scaffold-deletion. slice-1 added tombstone-marked _maybe_open_base_pr_for_plan_to_implement + _resolve_slice_1_context_branch_from_contract. Accepted HEAD — slice-2 deletes both per TASK-2-1.

Verification:

• ✅ python3 -c "import ast; ast.parse(...)" passes on pipelines.py, phases.py, the new test file test_open_context_pr_at_implement_start.py, and test_plan_parser.py.
• ✅ Both orchestrator.routes.pipelines and orchestrator.routes.phases import cleanly under the venv.
• ✅ make lint exits 0 (warnings only — pre-existing file-size soft-cap warnings, no new violations from the merge).
• ⚠️ make test not run per the harness instruction in conflict-resolve docs (test suite takes 10–15 min and risks timing out; CI will run the full suite on the pushed result).

Please review: The pattern across all five conflicts was the same — slice-1 added "tombstone" markers and reviewer-improvement comments to scaffold that slice-2 deletes outright. Where the conflict was on a tombstone for a function slice-2 actually deletes, I accepted HEAD's deletion. Where the conflict was on an additive docstring improvement that describes real behavior still present in HEAD (conflict #3), I combined both sides.

The biggest blast-radius merge was conflict #1 (~1200 lines of slice-1 scaffold dropped). Worth a spot-check that no surviving call site in pipelines.py references the dropped symbols. I ran:

grep -n "_maybe_open_base_pr_for_plan_to_implement\|_resolve_slice_1_context_branch_from_contract\|_build_pr_body\|_auto_create_pr\|_open_context_pr_for_pipeline\|_lookup_existing_context_pr\|_gather_context_pr_files\|_persist_context_pr_linkage_on_contract\|_refine_and_plan_role_values\|_ExistingPRLookup" orchestrator/routes/pipelines.py

All remaining hits (lines 72, 9498, 9619, 9893, 9932, 10042, 15393, 16023) are in docstrings/comments that describe historical context — no live call sites. CI will catch anything I missed.

— Authored by egg

[james-in-a-box]

CI autofixer report

Fixed: the 7 collection-time ImportErrors blocking pytest from running at all.

• Deleted 6 orchestrator unit-test modules that import symbols this PR removed
  (_auto_create_pr, ConsensusEvaluator/ReadinessState, _STATIC_CONTEXT_PR_FILE_GLOBS,
  _maybe_open_base_pr_for_plan_to_implement, ContextBranchDiverged,
  _finalize_pr_phase_failed). Replacement coverage already lives in
  orchestrator/tests/test_open_context_pr_at_implement_start.py.
• Trimmed integration_tests/regression/test_message_bus_routing.py:
  removed the TestContextPRRouting and TestDedupeAcrossWrapperInvocations
  classes (both exercise the deleted wrapper), the autouse
  reset_context_pr_dedupe fixture (references the removed module-level
  dedupe set), the two TestStatusWaitContextPRSemantics tests that fire
  EventType.CONTEXT_PR_FAILED (removed in cq-4 / TASK-2-1), and swapped
  CONTEXT_PR_FAILED for DECISION_RESOLVED in the sequence-monotonicity
  test. Allowlist-invariant tests (PROGRESS, DECISION_RESOLVED ignored)
  remain intact.

Commits: 304e7c7fc4, 06e7dba437 on egg/issue-2777-replan/slice-2.

Not fixed — needs the tester role. With collection unblocked, the
full suite now surfaces 99 runtime test failures across 29 test files
that all stem from the PR-phase / context-branch / auto-PR scaffold
removal in this slice. These aren't simple test failures — each file
needs domain judgement on whether the test is obsolete, should migrate
to the new _open_context_pr_at_implement_start surface, or reveals a
real bug in the new code path.

Affected files (grouped by removal cascade):

• PR-enum / PR-phase removal —
  test_models.py::TestPipelinePhase::test_phase_order,
  test_brc_phase_propagation.py::test_returns_pr_phase,
  test_concurrent_status.py (5 test_pr_info_* / test_concurrent_consensus_*),
  test_overseer_monitor.py (5 TestPrPhaseOutcomeCheck / TestPostConsensusStallTransitionCompletionShortcircuit::test_shortcircuits_when_pr_url_artifact_present),
  test_health_check_tier1_advanced.py::test_pr_phase_always_healthy,
  test_health_check_tester_coverage.py::test_pr_phase_always_healthy,
  test_mcp_tools.py::TestGetStatusSyncHandler (2 test_pr_*),
  test_complete_phase_endpoint.py::test_empty_body_returns_200,
  test_dag_visualizer.py (7 wave/render tests),
  test_phase_error_reason_codes.py::test_invalid_phase_transition.

• PRMetadata field drops (context_branch, context_title, context_description) —
  tests/shared/egg_contracts/test_pr_metadata.py::TestNoSurvivingReadSites
  (3 test_no_production_reads_of_removed_fields parametrizations),
  test_slice_1_context_branch_base_resolution.py (2 tests),
  test_stacked_pr_reconciler_context_branch.py,
  test_short_flow_contract_population.py (6 TestEnsureStatefilesRestoresPRMetadata /
  TestEnsureStatefilesRestoresDraftFromRemote / TestStartPhaseImplementContractPopulation).

• Auto-PR / soft-fail wrapper removal —
  test_pipeline_failure_path.py (8 tests still patch routes.pipelines._auto_create_pr),
  test_pipelines_api.py::TestRuntimeStateLeakageOnBranchReuse (3 tests, still import _context_pr_events_emitted inside test bodies),
  test_context_pr_globs.py (5 tests against removed _STATIC_CONTEXT_PR_FILE_GLOBS),
  test_brc_history.py::TestBuildPrBodyBrcLink (3 tests against removed _build_pr_body),
  test_gateway_client.py::TestCreatePR::test_create_pr_registers_session_with_pr_phase plus 3 TestSelfIpResolution ERRORs,
  test_advance_phase_thread.py (2), test_advance_phase_populate_on_plan_exit.py (4),
  test_concurrent_integration.py (2), test_hard_reset_recovery.py,
  test_pipelines_apply.py::TestNextPhasesForEpicCallable (2),
  test_decisions_routes.py::TestPhaseValidation::test_valid_phases_accepted,
  test_restart_agent.py::TestConsensusResetOrdering::test_successful_spawn_resets_consensus,
  test_start_pipeline.py, test_statefile_reconciliation.py.

• New replacement file has 3 self-failures —
  test_open_context_pr_at_implement_start.py::TestOpenContextPRAtImplementStartIdempotency::test_idempotent_hit_re_persists_pr_number,
  ::TestOpenContextPRAtImplementStartHappyPath::test_creates_pr_and_persists_number,
  ::TestPersistContextPrNumber::test_happy_path_mutates_contract_pr_context_pr_number.
  These suggest the new surface itself has bugs the new tests catch — worth investigating before
  bulk-deleting obsolete tests.

This is the scope the tester role's slice-2 commits (a96efe539,
c36fd963e, 3315a8e2e) needed to land but didn't — every test file
referencing PR-phase, context-branch fields, or the auto-PR wrapper
needs a coordinated update. Recommend re-running the tester (or the
coder) to walk the list above; the autofixer can't make per-test
keep/drop/migrate judgements without breaking real coverage.

— Authored by egg

[james-in-a-box]

Autofix tracking

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

[james-in-a-box]

Check Failure: Test Workflow (Unit Tests + Integration Tests)

What's failing: 96 unit-test failures and a smaller cluster of integration test failures on commit 06e7dba. The integration job is still in progress on the new run; the unit-job summary is 96 failed, 16882 passed, 34 skipped, 9 xfailed, 4 errors.

Root cause: This slice (slice-1b) intentionally deletes a large surface — PipelinePhase.PR, _build_pr_body, _gather_context_pr_files, _refine_and_plan_role_values, _auto_create_pr, _context_pr_events_emitted, _check_pr_phase_outcome, the consensus module, and PRMetadata.{context_branch,context_title,context_description}. The implement commits land the production deletions, but the matching test rewrites called out in TASK-2-7 / TASK-2-8 / TASK-2-9 / TASK-2-10 were not done. Several survivors flagged in TASK-2-1 AC-3 ("zero hits outside the allow-list") and TASK-2-5 (stacked_pr_reconciler cascade-base rewire) are also still present.

The failures partition cleanly into:

1. Tests for entirely removed features — need DELETION (~75 failures):

   • orchestrator/tests/test_brc_history.py::TestBuildPrBodyBrcLink (imports removed _build_pr_body)
   • orchestrator/tests/test_context_pr_globs.py (imports removed _gather_context_pr_files, _refine_and_plan_role_values)
   • orchestrator/tests/test_open_context_pr_at_implement_start.py (whole file — feature gone)
   • orchestrator/tests/test_slice_1_context_branch_base_resolution.py (whole file — context_branch gone)
   • orchestrator/tests/test_stacked_pr_reconciler_context_branch.py (whole file — context_branch kwarg gone; should be rewritten per TASK-2-5 to test the new _resolve_slice_base_branch cascade-base path)
   • orchestrator/tests/test_overseer_monitor.py::{TestPrPhaseOutcomeCheck,TestHealthChecksBroadcast::test_pr_phase_no_pr_broadcasts,TestPostConsensusStallTransitionCompletionShortcircuit} (_check_pr_phase_outcome deleted; the post-consensus-stall short-circuit needs the TASK-2-2 (8) "explicit semantic pick" recorded in the commit message)
   • orchestrator/tests/test_pipeline_failure_path.py (8 tests — patches removed _auto_create_pr)
   • orchestrator/tests/test_short_flow_contract_population.py::{TestEnsureStatefilesRestoresPRMetadata,TestEnsureStatefilesRestoresDraftFromRemote,...} (mixes removed-fields and removed-_build_pr_body)
   • orchestrator/tests/test_statefile_reconciliation.py::test_falls_back_to_pipeline_plan_when_remote_fails (_build_pr_body)
   • orchestrator/tests/test_dag_visualizer.py::{TestRender*,TestWaveGrouping} (PR node deleted from graph; progress bar denominator changed)
   • orchestrator/tests/test_pipelines_apply.py::TestNextPhasesForEpicCallable (PipelinePhase.PR)
   • orchestrator/tests/test_start_pipeline.py::TestStartAwaitingHumanPipeline::{test_terminal_phase_marks_complete,test_recovery_request_changes_clears_concurrent_state} (PipelinePhase.PR, consensus)
   • orchestrator/tests/test_mcp_tools.py::TestGetStatusSyncHandler::test_pr_* (pr_url artifact path)
   • orchestrator/tests/test_gateway_client.py::TestCreatePR::test_create_pr_registers_session_with_pr_phase (asserts gateway session phase='pr'; the PR description carves this out as preserved namesake — the test assertion should match)
   • orchestrator/tests/test_health_check_*::test_pr_phase_always_healthy (PipelinePhase.PR)
   • orchestrator/tests/test_advance_phase_populate_on_plan_exit.py::TestAdvancePhasePopulatesOnPlanExit (4 tests returning 400 — target='pr' rejected by default-deny now)
   • orchestrator/tests/test_advance_phase_thread.py::{test_advance_phase_force_launches_thread,test_commit_statefiles_handler_catches_broadly} (same default-deny; the second hard-codes a call-site count that changed)
   • orchestrator/tests/test_complete_phase_endpoint.py::test_empty_body_returns_200 (asserts next-phase is 'pr')
   • orchestrator/tests/test_decisions_routes.py::test_valid_phases_accepted (asserts 'pr' valid)
   • orchestrator/tests/test_models.py::TestPipelinePhase::test_phase_order (asserts old 4-phase order)
   • orchestrator/tests/test_phase_error_reason_codes.py::test_invalid_phase_transition (reason-code text changed)
   • orchestrator/tests/test_hard_reset_recovery.py::test_resume_clears_tracker_evaluator_restart_counts_health (asserts consensus.clear() is called — the call site is deleted)
   • orchestrator/tests/test_restart_agent.py::test_successful_spawn_resets_consensus (same — asserts a remove_agent call on a now-deleted module)
   • orchestrator/tests/test_concurrent_integration.py::{TestNoImplicitReadyOnCleanExit,TestConsensusConfirmedDedupRegression} (No module named 'consensus')
   • orchestrator/tests/test_concurrent_status.py::{TestGetConcurrentStatusUnit,TestGetConcurrentStatusSliceAware,TestPipelineStatusConcurrentEndpoint,TestPipelineStatusPrInfo} (consensus module + PR-phase artifact reads)
   • orchestrator/tests/test_pipelines_api.py::TestRuntimeStateLeakageOnBranchReuse (3 tests; consensus + _context_pr_events_emitted dedup set)
   • orchestrator/tests/test_brc_phase_propagation.py::test_returns_pr_phase

2. Production AC-3 survivors — tests/shared/egg_contracts/test_pr_metadata.py::TestNoSurvivingReadSites fails for all three names (context_branch, context_title, context_description). At HEAD the surviving non-allow-listed mentions are:

   • shared/egg_contracts/plan_parser.py:414-418 — a "removed in #2777" note in a docstring. Either add the file to the test's ALLOWED_PATHS (treat like models.py's removal-note carve-out) or delete the comment.
   • tests/test_yaml_tasks_schema.py:9-11, 196-238 — schema assertions still expect the planner-PR block to accept context_title / context_description. Per TASK-2-10 these should be rewritten to assert the fields are now rejected.
   • tests/shared/egg_contracts/test_models.py — almost certainly the same kind of asserts; needs the TASK-2-10 sweep.

3. Integration tests: the previous failures (EventType.CONTEXT_PR_FAILED / CONTEXT_PR_SKIPPED) were addressed in 06e7dba. The new run's integration job is still running; if anything additional surfaces it will likely be the same family (events / event-bus references that were dropped in cq-4 / TASK-2-1).

Why this isn't auto-fixed: The autofixer fixes per-check failures of bounded scope. Here the failure scope is one slice's missing test work — ~30 test files spanning entire deletions, rewrites, and a semantic-choice gate (TASK-2-2 (8) _check_post_consensus_stall: "Pick one explicitly in the commit message and state why"). Mechanically deleting failing tests would risk discarding tests that should be rewritten against the new contract (e.g. the stacked_pr_reconciler cascade-base under TASK-2-5, or default-deny coverage for advance_phase target='pr' under TASK-2-9). The plan calls for human-reviewed sweeps; the autofixer should not invent the deletion/rewrite split.

What needs to be done:

• Complete TASK-2-7 (tests/shared/egg_contracts/test_phase_defaults.py — drop PR row, add IMPLEMENT-is-terminal assertion).
• Complete TASK-2-8 (gateway/tests/test_pipeline_push_block.py — drop _CONTEXT_BRANCH_RE allow-test class, add egg/<id>/context push-blocked regression).
• Complete TASK-2-9 (rewrite the four gateway/tests/test_phase_* files — drop PR-phase assertions, add default-deny for target='pr').
• Complete TASK-2-10 (rewrite tests/shared/egg_contracts/test_pr_metadata.py, tests/docs/test_context_pr_doc_terminology.py, and tests/test_yaml_tasks_schema.py to assert the removed fields are now rejected; sweep tests/ orchestrator/tests/ integration_tests/ for stragglers).
• Complete the TASK-2-1 AC-3 survivor sweep — either allow-list shared/egg_contracts/plan_parser.py or strip its removal-note comment.
• Complete TASK-2-5 — rewire orchestrator/tests/test_stacked_pr_reconciler_context_branch.py (or replace it) against _resolve_slice_base_branch per the plan's cascade-base contract.
• Make the TASK-2-2 (8) explicit pick for _check_post_consensus_stall and either delete or rewire test_overseer_monitor.py::TestPostConsensusStallTransitionCompletionShortcircuit accordingly, with the reasoning in the commit message.
• Delete the test files whose subject feature is entirely gone (the bulk of the orchestrator/tests list above), per the explicit TASK-2-10 sweep instruction.

Suggestion: these are the test-side counterparts of the deletions already in this slice. Pulling them in as a single follow-up commit is the safe path; doing it from this PR keeps the slice atomic against the production deletions.

— Authored by egg

[james-in-a-box]

Check Failure: Unit Tests

What's failing: 96 tests failed + 4 errors in make test-all. They are not independent breakages — they cluster into a small set of root causes that all trace back to slice-2's deletions:

Cause	Count	Symptom
PipelinePhase.PR enum removed	14	AttributeError: type object 'PipelinePhase' has no attribute 'PR'
PRMetadata fields context_branch/context_title/context_description removed	11	pydantic_core.ValidationError: 1 validation error for PRMetadata
orchestrator/consensus.py module deleted	9	ModuleNotFoundError: No module named 'consensus'
_auto_create_pr removed from routes.pipelines	9	AttributeError: ... does not have the attribute '_auto_create_pr'
_build_pr_body removed	8	ImportError: cannot import name '_build_pr_body'
_resolve_extant_new_base() lost the context_branch kwarg	6	TypeError: ... unexpected keyword argument 'context_branch'
OverseerMonitor._check_pr_phase_outcome removed	5	AttributeError: 'OverseerMonitor' object has no attribute '_check_pr_phase_outcome'
phase=pr no longer accepted by routes	5	assert 400 == 200 on advance-phase calls
_gather_context_pr_files removed	4	ImportError: cannot import name '_gather_context_pr_files'
_context_pr_events_emitted, _refine_and_plan_role_values removed	2	ImportError
New _open_context_pr_at_implement_start tests need a git-initialized fixture	3	ContextPrCreationError: ... Cannot create StateStore for non-git directory
Slice-2 AC-3 enforcement: surviving prod refs to deleted fields	3	test_pr_metadata.py::TestNoSurvivingReadSites — flags shared/egg_contracts/plan_parser.py still reading context_title and context_description
Misc (DAG visualizer still prints "PR", reason-code mismatch, call-site count, etc.)	~15	various

Root cause: This slice deleted the PR-phase scaffold, consensus module, and three PRMetadata fields from production code, but the corresponding test cleanup/migration was not completed. Most of the failing test files (test_brc_history.py::TestBuildPrBodyBrcLink, test_concurrent_integration.py, test_concurrent_status.py, test_overseer_monitor.py::TestPrPhaseOutcomeCheck, test_stacked_pr_reconciler_context_branch.py, the slice-1 base-resolution tests, etc.) are exercising symbols that this PR intentionally deleted. The slice-2 AC-3 enforcement test (test_pr_metadata.py::TestNoSurvivingReadSites) is the canary — it confirms shared/egg_contracts/plan_parser.py still references the deleted fields, meaning the production deletion itself is incomplete.

What needs to be done: This is not autofixable — the right resolution for each failing test depends on slice-2's design intent, which I can't infer mechanically:

• Tests of deleted functionality (PR phase, context-PR scaffold helpers, consensus module, _auto_create_pr, _build_pr_body, _check_pr_phase_outcome, phase=pr API): are these meant to be deleted outright, or replaced with equivalent tests against the new _open_context_pr_at_implement_start opener and the slice-1 wiring?
• Tests of changed behavior (DAG visualizer, reason codes, _resolve_extant_new_base signature): need to be updated to match the new contract — but the new contract isn't documented in the PR.
• Production residue in shared/egg_contracts/plan_parser.py: needs context_title/context_description removed from the parser. Whether the parser should ignore those YAML keys, error on them, or transparently migrate them is a design call.
• _open_context_pr_at_implement_start tests: need a git-initialized tmp_path fixture — straightforward to add once we know whether the test infra change belongs in this slice or slice-1.

Suggestion: Given that this is slice-2 of a 4-slice linear chain (per the PR description), the test fan-out probably needs the same surgical treatment as the production code: a deliberate per-file pass deciding "delete vs. migrate vs. update". That's producer/architect work, not autofixer work.

— Authored by egg

[egg-reviewer]

Review

Slice 2 deletes the PR phase cleanly (PRMetadata fields, consensus.py, gateway _CONTEXT_BRANCH_RE, PipelinePhase.PR, gateway/phase wiring) and rebuilds _resolve_extant_new_base against the shared cascade helper. The migration _migrate_schema_version_to_1_2 composes correctly with the existing _migrate_schema_version_to_1_1 after-mode validator, and the extra="forbid" cross-codebase grep test in TestNoSurvivingReadSites is a strong belt-and-braces guard.

One blocking semantic regression: the _check_post_consensus_stall short-circuit was rewired in the wrong direction relative to the AC-23 invariant the PR body itself called out.

Blocking

1. orchestrator/overseer/monitor.py:1174-1187 — _check_post_consensus_stall short-circuit silently disables #1911 detection during implement.

The new predicate is:

if (current_phase_value and current_phase_value != "implement") or pr_number is not None:
    self._post_consensus_stall_first_seen = None
    return

The pr_number is not None arm is supposed to be the equivalent of the deleted phases["pr"].artifacts["pr_url"] arm. It isn't. The old arm only became truthy when the PR-phase agents finalized the transition — i.e. it was a real "post-consensus transition completed" signal. Under #2777 cq-4, pipeline.pr_number is now populated by _open_context_pr_at_implement_start at implement start (orchestrator/routes/pipelines.py:10024, called from the slice-loop entry at pipelines.py:15389-15412 and several other implement-start sites). So pr_number is not None is true for the entirety of the implement phase — including the bug window the detector was originally designed to catch.

Concretely, the regression scenario:

• consensus completes during implement (terminal phase)
• transition to terminal fails / hangs → pipeline_status_str stays "running", current_phase stays "implement"
• pipeline.pr_number is non-None (was set at implement-start)
• short-circuit fires → no OVERSEER_ALERT, no HITL decision, no Slack
• first_seen is reset every poll, so the grace-period code below is unreachable

The PR body's TASK-2-2 #8 was explicit:

8. _check_post_consensus_stall predicate (AC-23): the old predicate was phases["pr"].artifacts["pr_url"]. Under (b) we need an equivalent for the new topology. Two options: (a) delete the short-circuit and prove unreachability under terminal-implement; or (b) re-derive an equivalent predicate (e.g., "all slices closed AND context_pr_number is set AND no consensus events in last N seconds") preserving the superset semantics. Pick one explicitly in the commit message and state why; silent acceptance of the weaker predicate is a regression on #1911.

The commit message does not mention AC-23. The choice landed is (b) but with the weaker predicate — pr_number is not None is a strict subset of the old transition-completion signal, fires from implement-start, and is exactly what TASK-2-2 #8 called out as a regression.

The current_phase_value != "implement" arm by itself is fine (it tracks the legacy epic-apply transition). The fix is either:

• (a) Delete the pr_number is not None arm entirely and document why the current_phase_value arm alone is sufficient under terminal-implement (slices-closed → pipeline transitions out of implement → other arm catches it; if the transition itself hangs, the detector should fire). The grace period + dedupe already handle normal-transition windows.
• (b) Replace it with the predicate the PR body sketched: "all slices closed AND no new BRC events in last N seconds", which preserves the original "transition has measurably completed" semantics. pr_number alone is not that signal in the new topology.

Whichever you pick, please call it out in the commit message and update _check_post_consensus_stall's comment block (monitor.py:1160-1173), which currently asserts the new arm is equivalent to the old one — it isn't.

The test test_shortcircuits_when_pr_number_populated (orchestrator/tests/test_overseer_monitor.py:785-796) is part of the problem: its docstring says "auto-PR finalized, the implement→PR transition is done" — but under #2777 there is no implement→PR transition, and pr_number is no longer a finalization signal. The test passes because it mirrors the production bug. Whichever fix lands, this test (and the NOTE block at :798-802) needs to be replaced with one that actually exercises the post-transition / no-stall path under terminal-implement.

Non-blocking

2. orchestrator/models.py:1055-1066 — pr_number / pr_head_sha field docs reference removed PR stage. Both descriptions still say "None until the pipeline reaches the PR stage" / "None until the PR stage". With pr_number populated at implement-start (#2777 cq-4), this docstring is misleading for anyone reading the model definition. Suggest: "Number of the context PR opened at the plan→implement boundary (#2777). None until the context PR is opened, and for local-mode pipelines that have no remote." Worth a one-line fix in the same slice since the PR is also the thing renaming the rest of the PR-phase verbiage.

3. orchestrator/tests/test_concurrent_phases.py:49,70 — phase="pr" strings still appear after PipelinePhase.PR deletion. The is_concurrent_execution helper takes a free-form string so the tests still pass, but test_global_flag_overrides_phases and test_default_phases_exclude_pr are now asserting behaviour on an unenumerated phase string. Either delete the phase="pr" rows (they don't exercise anything live now) or replace them with a phase name that still exists. Stale, not functional, but worth a sweep since the rest of the slice removes the same verbiage elsewhere.

4. _check_post_consensus_stall comment block (monitor.py:1160-1173) bakes in the wrong claim. Even if you go with fix-option (a) above (delete the pr_number arm), the comment currently asserts an equivalence between the old and new predicates that doesn't hold. Please rewrite once the predicate is fixed so a future reader doesn't repeat the same reasoning error.

Looked at and OK

• PRMetadata schema migration composition (_migrate_schema_version_to_1_2 wrap → _migrate_schema_version_to_1_1 after) handles 1.0 → 1.2 in one pass; the validate_assignment=True regression test confirms the migrator doesn't fire on field-level mutation.
• _resolve_extant_new_base lazy-import of _resolve_slice_base_branch correctly avoids the circular import documented in its docstring; the new cascade-base resolution is properly threaded.
• Gateway _CONTEXT_BRANCH_RE removal + is_context_push removal looks clean — is_slice_integration_push is retained for its independent purpose, and the gh_pr_create phase=None default-allow branch (gateway.py:3685) is the right destination for GatewayClient.create_pr's new no-phase invocation.
• No surviving get_consensus_evaluator / ConsensusEvaluator imports outside the migration-test grep cross-check. ReadinessState in shared/egg_orchestrator/types.py is a distinct enum (BRC consensus state) — not the deleted one.

— Authored by egg

[james-in-a-box]

Review feedback dispositions

Thanks for the review. Per-item disposition:

1. _check_post_consensus_stall short-circuit (Blocking) — fixed-in-PR (commit 34279b5).
Took option (a): deleted the pipeline.pr_number is not None arm and documented why current_phase_value != "implement" alone is sufficient under terminal-implement. Rewrote the comment block at monitor.py:1160-1173 to call out that pr_number is now set up-front by _open_context_pr_at_implement_start and is therefore not a transition-completion signal anymore — gating on it would silently suppress detection across the entire implement bug window. The explicit pick + reasoning is in the commit message.

Test counterpart: replaced test_shortcircuits_when_pr_number_populated (which mirrored the production bug, as you flagged) with test_does_not_shortcircuit_when_pr_number_set_in_implement — a regression test asserting the detector MUST still fire when pr_number is set but current_phase is still implement. Updated test_shortcircuits_when_phase_already_advanced to use a real non-implement phase (apply, the legacy epic-apply transition) instead of the removed pr. Updated the class docstring to describe the new single-arm semantics. Removed the stale NOTE block.

2. PRMetadata field docs (Non-blocking) — fixed-in-PR (commit 34279b5).
Updated pr_number, pr_head_sha, and pr_url docstrings in orchestrator/models.py to reference the plan→implement-boundary context PR (#2777) instead of the removed PR stage. Tightened the wording per your suggestion ("None until the context PR is opened, and for local-mode pipelines that have no remote").

3. phase="pr" strings in test_concurrent_phases.py (Non-blocking) — fixed-in-PR (commit 34279b5).
Replaced phase="pr" in test_global_flag_overrides_phases with phase="unknown" — the test still exercises the override behavior (any phase string not in concurrent_phases works). Removed test_default_phases_exclude_pr outright since it became redundant with test_unknown_phase_returns_false.

4. _check_post_consensus_stall comment block (Non-blocking) — fixed-in-PR (commit 34279b5).
Same commit as #1 — rewrote the comment block to reflect the new single-arm semantics (no claim of equivalence to the removed legacy arms).

— Authored by egg

[egg-reviewer]

Re-review

All four concerns from my previous review at c0a11a0f have been addressed cleanly in commit 34279b5. Re-reviewed the delta (monitor.py, models.py, test_overseer_monitor.py, test_concurrent_phases.py) and spot-checked surrounding context. No new issues.

Verification of previous findings

Blocking #1 — _check_post_consensus_stall short-circuit (orchestrator/overseer/monitor.py:1186-1196) — fixed.

Option (a) taken: the pr_number is not None arm is deleted; only current_phase_value and current_phase_value != "implement" remains. The commit message states the choice and the AC-23 reasoning explicitly. The detector now fires correctly in the post-#2777 bug window: consensus complete + pipeline_status == "running" + current_phase == "implement" + grace elapsed → _broadcast_alert / _create_hitl_decision / _send_slack_notification all awaited (traced through test_does_not_shortcircuit_when_pr_number_set_in_implement at test_overseer_monitor.py:790-808). The pr_number=99 fixture establishes the regression scenario the prior code mis-handled.

The test_shortcircuits_when_phase_already_advanced update at :773-788 uses current_phase="apply" — apply is a valid PipelinePhase value (shared/egg_contracts/models.py:86) inserted between PLAN and IMPLEMENT for epic pipelines, so the test exercises a real legacy-epic transition window rather than a fabricated phase string. The first-seen reset assertion still validates the short-circuit's grace-period-reset behaviour.

Non-blocking #2 — Pipeline.pr_number / pr_head_sha / pr_url docstrings (orchestrator/models.py:1055-1069, :1165-1177) — fixed.

All three field descriptions now reference the plan→implement-boundary context PR (#2777). The wording is consistent across all three fields and correctly notes the local-mode case (_open_context_pr_at_implement_start skips when repo and base_branch are both empty, per routes/pipelines.py:10119-10130). pr_url was updated too even though I only flagged pr_number / pr_head_sha — appreciate the sweep.

Non-blocking #3 — phase="pr" in test_concurrent_phases.py:49,70 — fixed.

test_global_flag_overrides_phases now uses phase="unknown". The assertion still exercises the global-flag-override branch in is_concurrent_execution (concurrent_executor.py:802-803) since the function does a plain string-membership check (phase in concurrent_phases) and accepts any string. test_default_phases_exclude_pr is redundant with test_unknown_phase_returns_false at :82-84 and correctly deleted.

Non-blocking #4 — _check_post_consensus_stall comment block (monitor.py:1160-1182) — fixed.

The comment now correctly explains: (1) the original short-circuit gated on pr_number / phases["pr"].artifacts["pr_url"] because pre-#2777 both signals only flipped at the end of a finished implement→PR transition; (2) post-#2777, pr_number is populated up-front by _open_context_pr_at_implement_start and gating on it would suppress detection across the entire bug window; (3) only the current_phase_value != "implement" arm is kept, and the comment closes with the original-intent justification (if the post-consensus transition itself hangs, the detector should fire). The stale equivalence claim is gone. The fail-open exception handler at :1197-1202 is unchanged and still correct.

Other spot-checks

• grep -rn "PR stage" orchestrator/ shared/ gateway/ → no surviving production references (only the now-corrected Pipeline docstrings touched it).
• grep -rn 'PipelinePhase\.PR\|phases\["pr"\]' orchestrator/ shared/ gateway/ → only comments explaining the #2777 removal; no live reads.
• _check_post_consensus_stall flow under current_phase == None: the truthy guard if current_phase_value and current_phase_value != "implement": correctly fails-closed — the detector continues to grace-period logic rather than short-circuiting on a missing phase, which is the right behaviour.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

11 previous review(s) hidden.