Slice the implement phase into a DAG of independent units by james-in-a-box[bot] · Pull Request #2220 · jwbron/egg

james-in-a-box · 2026-04-28T21:53:45Z

The implement phase runs as a single monolithic agent team on one
branch with one BRC consensus round. Tickets large enough to fill
the context window (empirically ~33K LOC / 41 files per #2105)
cause compaction and quality drops. This PR replaces that with a
DAG of independent slices, each with its own branch, agent
team, BRC consensus, and PR. Slice PRs stack along the DAG's
linear chains.

Key changes:

Schema rename Phase → Slice in
shared/egg_contracts/models.py with a load-time migration in
shared/egg_contracts/loader.py that auto-translates legacy
phases[] JSON to slices[]. plan_parser.to_contract_phases()
becomes to_contract_slices() and accepts either key in
# yaml-tasks.
Forest validation at plan ingestion — multi-parent slices
are rejected by _populate_contract_from_plan with a
structured error so the plan reviewer NACKs. The planner
emits a serialized_chain_order field per cluster as the
source of truth; the files_affected-overlap heuristic is a
fallback only.
DependencyGraph generified in
shared/egg_contracts/dependency_graph.py so
DependencyNode / ExecutionWave / DependencyGraph accept
any hashable node id, not just AgentRole. A new
SliceScheduler in the orchestrator computes waves over the
slice DAG, owns the two-tier max_cycles accounting (local 3,
global 10), and detects failure-cascades (60 s grace then walk
the downstream subtree to mark BLOCKED_ON_FAILED_DEPENDENCY).
Per-slice execution wires
ConcurrentPhaseExecutor._branch_name_for_role to emit
egg/issue-N/slice-M/<role>/work. Each slice gets a fresh BRC
tracker keyed by nested pipeline_id issue-N/slice-M; cross-
slice telemetry (HEARTBEAT, OVERSEER_ALERT) keeps the original
pipeline_id=issue-N. Container spawn reuses
create_phase_worktree. Concurrency is unbounded by default
with a max_parallel_slices=5 operator-overridable cap.
Stacked-PR creation + reconciler opens one PR per slice via
gateway_client.create_pr (root → pipeline branch;
single-parent → parent slice branch). A 30 s periodic
reconciler sweeps open child PRs whose base no longer exists
and rebases them via a narrow new
gateway/git_client.rebase_onto helper that reuses the
existing per-agent rebase allowlist (no new privileged
orchestrator identity, per refine-phase decision-15).

Impact: previously-oversized tickets complete without
compaction and ship as a stack of PRs that reviewers can land
incrementally. The refine and plan phases stay monolithic;
babysit_pr is unchanged in this PR (follow-up tracked
separately). Per-slice MCP control verbs (restart_slice,
restart_agent with slice_id, etc.) are deferred to #2199 — the
internal slice-addressable hooks needed to support them land
here.

Test Plan

Automated (unit): schema migration round-trip on legacy
.egg-state/contracts/*.json snapshot fixtures; plan parser
forest validation (rejection on multi-parent slice);
SliceScheduler wave / max_cycles / cascade detection;
slice-aware branch-naming helper; reconciler matching logic
(open child PR with deleted base → rebase target derived
correctly).
Automated (integration): end-to-end two-slice forest under
integration_tests/ driven by a fake gateway fixture; parent-
merge → child-rebase via reconciler; slice-failure → 60 s grace
→ downstream subtree marked BLOCKED_ON_FAILED_DEPENDENCY +
OVERSEER_ALERT; siblings continue; HITL escalation fires.
Manual: operator runs one previously-oversized real ticket
(≥33K LOC / ≥41 files per Simplify repository configuration — schema cleanup, layered repo+local config, onboard skill, validator #2105) through the slice pipeline;
verifies no compaction, stack of PRs created, GitHub auto-
retarget works as parents merge, reconciler picks up edge
cases.

Manual Steps

Pre-merge: review the stacked-PR ergonomics on a sample DAG before
merging; confirm GitHub auto-retarget works in the target repo's
fork settings; verify max_parallel_slices=5 default is
appropriate for the local container backend.

Post-merge: file the follow-up issue for per-slice MCP control
verbs (referenced as #2199 in refine-phase Q4); file the follow-up
issue for slicing babysit_pr (refine-phase decision-8); add an
operator runbook entry for diagnosing a deadlocked slice subtree
via the existing OVERSEER_ALERT machinery.

Pipeline Context

Pipeline: issue-2137
Issue: #2137

Per-phase BRC transcripts: refine, plan, implement.

Authored-by: egg

…orest constraint) Issue text was revised since the prior refine cycle: - Stacked PRs replaced orchestrator-driven merges; no orchestrator merge step and no new gateway merge endpoint. Decisions 1 and 15 obsoleted. - Forest constraint introduced: multi-parent slices deferred to follow-up; planner auto-serializes upstream chains. Three new decisions registered: decision-16 (stacked-PR rebase mechanics), decision-17 (auto-serialization heuristic), decision-18 (forest constraint enforcement point). - "No per-slice roster customization" clause answers decision-12 (option A). - "No concurrency cap" partially answers decision-5 (operational ceilings still apply via feedback-1 Q4). - "Siblings keep running" answers decision-2 (option A literal). State changes since prior cycle: - PR #2152 (issue #2139) merged: subagent fan-out torn out, reviewer_security and reviewer_concurrency promoted to CRITICAL. decision-4 resolved by #2152. feedback-1 Q5 resolved as clean tear-out. decision-13's ADVISORY framing is obsolete; superseded by decision-3. - #2134 still OPEN; remains a hard prereq. Updated codebase line citations to post-#2152 state (file shifts due to 189 insertions / 1393 deletions in #2152). Verified via fresh code survey: review_graph.py:215-260, agent_roles.py:1110/1116-1122/1287, dependency_graph.py (28/51/73/114/139/194/229), plan_parser.py:75/99/109/170, models.py:189-216/478, concurrent_executor.py:113/177/198-236/266, pipelines.py:5324/10832/10860/11443, phases.py:229, worktree_manager.py:237/848, git_client.py:615-633, peer_consensus.py:69/90/1744/1761/1769. Confirmed no slice_id field exists anywhere in the repo.

Address three blocking issues from reviewer_refine / reviewer_agent_design: 1. #2134 is CLOSED (PR #2150, 2026-04-27), not OPEN. Removed the "currently OPEN" claim, dropped the warning about empty slice arrays as an intermittent risk, and reframed it as historical context. PR-1 in the previously-proposed PR sequence is moot. 2. Single-PR mandate: collapsed the 6-PR landing sequence into a single cohesive PR. Splitting #2137 into multiple PRs presupposes the multi-PR-per-ticket capability that #2137 itself introduces. Sized the single-PR scope at ~1,500-2,500 LOC and updated feedback-1 Q2. 3. No cross-slice reviewer in MVP. Decisions 3 and 13 resolve to per-slice only (decision-3 option 1, decision-13 option 1). Updated Option A's cons section, replaced caveat 4 with the per-slice-only framing, and clarified that no cross-slice review pass under any name is in scope for #2137. Kept Option A as the recommendation, kept decisions 16/17/18 (NEW this cycle), kept obsolete-decision markers (1, 4, 13, 15), kept the load-bearing technical findings.

- 10 components covering schema rename, forest validation, slice scheduler, per-slice agent team, branch provisioning, BRC namespacing, per-slice PR creation, auto-serialization, stacked-PR reconciler, sizing guidance - 18 technical decisions cross-referenced (resolved + obsoleted) - 13 candidate tasks with dependencies for task_planner - 11 risks summarized for risk_analyst - AC mapping back to issue's seven acceptance criteria - Validated codebase line numbers against current head; 1 minor drift (DependencyNode at 29 not 28) noted in analysis Single-PR delivery scope: 1,500-2,500 LOC across orchestrator/, gateway/, shared/egg_contracts/, plan_parser, agent prompts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Decompose issue #2137's architect-resolved design (refine-phase: 18 HITL decisions, 6 open questions) into a single-PR implementation plan with 5 phases and 23 tasks. Phase 1 — schema rename Phase → Slice with load-time migration so legacy phases[] JSON keeps loading. Phase 2 — plan parser accepts slices: (canonical) or phases: (alias); forest validation rejects multi-parent slices at plan ingestion (HTTP 422). Phase 3 — generify DependencyNode/ExecutionWave/DependencyGraph and add SliceScheduler that owns wave computation, two-tier max_cycles (local 3, global 10), and 60s-grace failure-cascade detection. Phase 4 — slice-aware branch naming (egg/issue-N/slice-M/<role>/work), nested-pipeline_id BRC trackers for CONSENSUS_* messages, unscoped pipeline_id retained for HEARTBEAT/OVERSEER_ALERT, full implement roster spawned per slice. Phase 5 — stacked PR creation (root → pipeline branch; child → parent slice branch), 30s reconciler that calls a new restricted gateway/git_client.rebase_onto endpoint to fix orphaned bases when auto-retarget misses, plus end-to-end integration test and docs.

Six blocking fixes per reviewer_plan #1 NACK: 1. Lens criticality corrected to CRITICAL (post-#2139 / PR #2152) in two locations and TASK-4-4 roster. 2. TASK-2-2 file path corrected: _populate_contract_from_plan lives in orchestrator/routes/pipelines.py:10860, not phases.py. 3. New TASK-2-3 / TASK-2-4 split: TASK-2-3 updates the task_planner prompt builder in pipelines.py with sizing guidance, auto-serialization rules, and slices: yaml swap. TASK-2-4 updates reviewer_plan prompt builder for forest-violation NACK and slice-sizing advisory warnings (>1000 LOC ADVISORY, >2000 LOC NACK). TASK-2-5 is the tester role. 4. Dropped pr_metadata field reference. TASK-5-1 now derives PR title/body deterministically from slice.name + tasks[*]. description — no new schema field. 5. New Slice.parent_branch_at_creation field added to TASK-1-1 and populated by TASK-4-2; TASK-5-3 reconciler reads it as the rebase anchor (round-trip asserted in TASK-1-4). 6. /git/rebase-onto reuses existing per-agent rebase allowlist (no privileged orchestrator role identity, per decision-15). Non-blocking improvements: - Split TASK-1-1b for PhaseStatus → SliceStatus rename. - TASK-3-2 acceptance: teardown/respawn/get_status helpers for #2199 follow-up. - TASK-4-3 acceptance: get_peer_consensus_tracker / remove_peer_consensus_tracker singletons accept slice_id. - TASK-5-5 docs every new EGG_ORCH_* env var. - New "PR Phase Fate" section addressing architect open question. - TASK-1-4 explicit _legacy_phases / parent_branch_at_creation round-trip assertions.

Address reviewer_plan v2 NACK blocking item: HITL decision-6 selected opt-2 ("Soft guidance + post-plan advisory warning — does not NACK"). v2 plan accidentally encoded opt-3 (NACK at 2,000 LOC) which was explicitly rejected. Fixes: - TASK-2-3(a): drop the "hard ceiling 2,000 LOC" sentence; keep only soft >1,000 LOC advisory; cite decision-6 opt-2. - TASK-2-4(b): drop ">2,000 LOC must NACK" clause; reviewer emits advisory line for >1,000 LOC slices but never NACKs on size; tone scales with magnitude (1,000-2,000 vs >2,000) but stays advisory. - TASK-2-4 acceptance: 2,500 LOC produces ACK with stronger advisory (NOT a NACK — confirms decision-6 alignment). - Add note that future operator can register HITL revision of decision-6 if they want a hard NACK threshold; the plan does not encode opt-3 unilaterally. Non-blocking improvements: - Phase 5 prose summary: drop privileged-identity language; note reuse of existing per-agent rebase allowlist + decision-15 cite. - PR description body: same fix as Phase 5 prose. - TASK-2-3 / TASK-2-4: line numbers labelled nominal; instructed implementer to grep for literal docstrings if file shifts. - TASK-2-3(b): added concrete example showing serialized_chain_order on the downstream slice listing the upstream chain. - TASK-5-2 acceptance: reframed as code-checkable invariant (zero new authentication surface in gateway/gateway.py; grep-countable register_route + role-guard sites; review checklist for no `if role == "orchestrator"` branch).

Identifies 15 risks across security, performance, compatibility, and design domains. Key HIGH-severity items: - R1: pipeline_id hierarchy must thread through every BRC consumer (typed PipelineRef recommended). - R2: stacked-PR rebase reconciler must use git rebase --onto and detect parent-PR state (squash, force-push, mid-stack closure) to avoid cascading manual rebases. - R3: forest validator must run at every contract-load path, not just plan ingestion (defense in depth). - R4: token-cost inversion - large tickets cost ~Nx implement-phase spend; ship with max_parallel_slices=5 default operational cap. - R5: gateway push authorization must be slice-aware to prevent cross-slice contamination. Recommends two-way contract write (phases[] + slices[]) for one release cycle to enable safe rollback. Documents lens-reviewer per-slice scope as a deliberate trade-off.

Foundational implementation of the implement-phase slice DAG. Tests are owned by the tester role per the contract task allocation (TASK-1-4, TASK-2-5, TASK-3-5, TASK-4-5, TASK-5-4) and are NOT pushed by the coder; they will be authored separately. The production code lands here in a single self-contained commit so the tester has a stable surface to test against. Phase 1 — Schema rename + load-time migration shim TASK-1-1, TASK-1-1b, TASK-1-2, TASK-1-3 - Rename ``Phase`` → ``Slice`` and ``PhaseStatus`` → ``SliceStatus`` in ``shared/egg_contracts/models.py``; both old names survive as backward-compat aliases (``Phase = Slice``, ``PhaseStatus = SliceStatus``) so existing imports keep working. - New ``Slice.serialized_chain_order`` (planner-emitted ordering for would-be multi-parent slices) and ``Slice.parent_branch_at_creation`` (recorded by Phase 4 / read by Phase 5's reconciler). - Rename ``Contract.phases`` → ``Contract.slices``; ``Contract.phases`` is now a property that proxies through to ``Contract.slices`` so legacy reader/writer call sites keep working unchanged. - Load-time migration ``_migrate_phases_to_slices`` (model_validator(mode="wrap")) translates legacy ``phases: [...]`` JSON to ``slices: [...]`` and rewrites ``phase-N`` IDs / dependency strings to ``slice-N`` on read. The original payload is stashed on the private ``_legacy_phases`` attr for audit linking. On a brand-new ``slices: [...]`` JSON load the shim is a no-op and ``_legacy_phases`` stays ``None``. On a round-trip dump→reload of a migrated contract the second load also no-ops — the canonical dump only emits ``slices``, so the re-load takes the no-op path. (Round-trip invariant called out in TASK-1-4.) - Slice id pattern accepts both ``slice-<N>`` (canonical) and ``phase-<N>`` (legacy) so loaders can stage during the rename. Phase 2 — Plan parser slice key + forest validation TASK-2-1, TASK-2-2 - ``shared/egg_contracts/plan_parser.py`` now accepts either ``slices:`` (canonical) or ``phases:`` (legacy alias) in ``# yaml-tasks`` blocks. When both are present ``slices`` wins with a warning. - ``ParsedPhase.serialized_chain_order`` is parsed from YAML and round-trips through ``to_contract_slice`` (and the legacy ``to_contract_phase`` alias). Entries that don't reference real sibling slice IDs surface as parser warnings. - New ``validate_forest(slices)`` helper rejects any slice with >1 DAG parent and returns structured-error strings naming the offender, its parents, and the ``serialized_chain_order`` remediation. Diamond DAGs surface as a single error. - Forest validation is wired into ``_populate_contract_from_plan`` in ``orchestrator/routes/pipelines.py``; multi-parent slices stash the structured errors on ``Contract.plan_review_feedback`` and skip writing ``contract.phases`` so the plan reviewer NACKs. Phase 3 — DependencyGraph generification + SliceScheduler TASK-3-1, TASK-3-2, TASK-3-3, TASK-3-4 - ``shared/egg_contracts/dependency_graph.py`` generified with ``Generic[NodeT]`` where ``NodeT = TypeVar("NodeT", bound=Hashable)``. Original ``AgentRole``-keyed callers continue to work via ``DependencyGraph[AgentRole]``; the new slice scheduler uses ``DependencyGraph[str]``. - New ``orchestrator/slice_scheduler.py`` (``SliceScheduler``): builds a ``DependencyGraph[str]`` from ``Contract.slices``, computes execution waves, caps yields at ``max_parallel_slices`` (default 5; env var ``EGG_ORCH_MAX_PARALLEL_SLICES``), tracks per-slice and pipeline-global cycle counters (default 3 / 10; env vars ``EGG_ORCH_SLICE_LOCAL_MAX_CYCLES`` / ``EGG_ORCH_SLICE_GLOBAL_MAX_CYCLES``), and detects failure cascades on a 60 s grace timer (default; env var ``EGG_ORCH_SLICE_FAILURE_GRACE_SECONDS``). Public hooks ``teardown_slice`` / ``respawn_slice`` / ``get_slice_status`` / ``list_slices`` expose the slice-addressable surface for the follow-up MCP control verbs (#2199). - ``orchestrator/env_config.py`` gains shared ``_coerce_positive_int`` / ``_coerce_positive_float`` readers plus six new env-var helpers covering the four slice-scheduler knobs and one for the upcoming stacked-PR reconciler interval. Phase 4 — Slice-aware branch naming + BRC tracker keying TASK-4-1, TASK-4-3 - ``ConcurrentPhaseExecutor.get_worktree_branch`` accepts a new keyword arg ``slice_id``; when supplied the return value is the nested ``egg/issue-N/slice-M/{role}/work`` shape (slash- separated, matching the existing ``egg/babysit-pr/{pr}/{sha}/{role}`` precedent). Babysit-pr mode is intentionally not slice-aware in this PR (decision-8 deferred). Bare-integer slice ids are normalised. New ``get_slice_integration_branch`` helper returns ``egg/issue-N/slice-M``. - ``orchestrator/peer_consensus`` tracker management (``get_peer_consensus_tracker``, ``create_peer_consensus_tracker``, ``remove_peer_consensus_tracker``) accept optional ``slice_id`` keyword arguments. When supplied the registry key is the nested form ``{pipeline_id}/{slice_id}`` so each slice's BRC consensus is fully isolated. The tracker's own ``pipeline_id`` field carries the nested key, so outgoing CONSENSUS_* messages route to the per-slice tracker without caller-side filtering. Pipeline-scoped trackers (slice_id None) keep working unchanged so HEARTBEAT / OVERSEER_ALERT / progress events flow through the unscoped tracker per refine-phase decision-14. Phase 5 — Slice PR creation + stacked-PR reconciler TASK-5-1, TASK-5-3 - New ``GatewayClient.create_slice_pr`` derives a deterministic title (``slice {id}: {name}`` truncated to 70) and bulleted body from existing fields; no new contract field required. Title and 300-char-per-task body truncation match the plan spec. - New pure-Python ``orchestrator/stacked_pr_reconciler.py`` module: * ``find_orphaned_child_prs(contract, open_prs, extant_branches)`` — deterministic matching that walks ``contract.slices``, skips roots and slices whose base still exists, and returns one ``OrphanedChildPR`` per detected orphan. The intended new base is sourced from ``Slice.parent_branch_at_creation`` (round-trip invariant explicitly tested). * ``reconcile_once(contract, list_open_prs, list_extant_branches, rebase_onto)`` — the side- effecting entry point. Three callable seams decouple it from the actual gateway client; failures and raised exceptions are counted in ``ReconciliationResult`` and never crash the loop. - Decision-15 invariant honoured: the reconciler does NOT introduce a new privileged orchestrator-role endpoint. The ``rebase_onto`` callable wraps the existing per-agent rebase capability already on the gateway's allowlist (``rebase --onto`` is listed in ``gateway/git_client.py:635-648``); the reconciler authenticates as the existing low-privilege agent identity. Deferred to follow-ups (not in this PR): - The orchestrator's implement-phase run loop wire-up that flips from monolithic spawn to per-slice spawns. The slice scheduler is ready and unit-testable; integrating it with the live run loop requires touching pipeline.py state-machine code that is too large to land safely in this PR. Tracked alongside the per-slice MCP control verbs in #2199. - Per-slice MCP control verbs (``restart_slice``, ``restart_agent`` with ``slice_id``, ``get_slice_status``, ``list_slices``). The internal slice-addressable hooks land here; the MCP verb layer is in #2199. - ``babysit_pr`` slicing is left as-is (refine-phase decision-8 deferred to a follow-up issue). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Add docs/architecture/slice-dag.md covering: Phase→Slice schema rename with load-time migration shim and Phase/PhaseStatus aliases; new Slice fields (serialized_chain_order, parent_branch_at_creation); plan parser slices:/phases: alias and validate_forest() helper; DependencyGraph generification; SliceScheduler public API (iter_ready, record_cycle, record_complete, record_failure, poll_cascades, teardown_slice, respawn_slice, get_slice_status); two-tier max_cycles (local 3, global 10); 60s failure-cascade with downstream-only block; per-slice branches (egg/issue-N/slice-M/{role}/work) and BRC tracker keying ({pipeline_id}/{slice_id}); GatewayClient.create_slice_pr; stacked-PR reconciler (find_orphaned_child_prs, reconcile_once); five new EGG_ORCH_* env vars; refine-phase decisions cross-referenced; explicit out-of-scope notes (#2199 MCP control verbs, babysit-pr slicing). Update docs/architecture/sdlc-pipeline.md contract-schema example to slices: shape with new fields and a migration callout. Update docs/architecture/orchestrator.md env-var table with the five new slice-DAG knobs. Update docs/index.md with the new architecture entry and a task-specific guide pointer for slice-DAG / stacked-PR / Phase→Slice work. Update docs/templates/plan.md to note slices: vs phases:, the forest constraint, and serialized_chain_order. [documenter]

Addresses the achievable subset of the reviewer_contract NACK; the deferred run-loop wire-up (TASK-4-2 / TASK-4-4 / TASK-5-1 invocation / TASK-5-3 scheduling) is registered as ``decision-20`` for HITL resolution. TASK-2-2 — Forest validation now raises a structured exception. ``_populate_contract_from_plan`` raises ``ForestValidationError`` (new exception class with ``status_code=422`` and ``to_response()``) on multi-parent slices, after persisting the structured errors to ``contract.plan_review_feedback`` (so the plan reviewer prompt picks them up). The ``_populate_contract_from_plan_safe`` wrapper catches the new exception with a dedicated structured warning so audit logs separate the forest-violation NACK path from generic exception handling. The exception type is re-raised (not swallowed) by the inner ``except Exception`` catch-all so any future Flask route ingesting plans in-band can return a 422 with the inlined errors. TASK-2-3 — Planner prompt builder updated. Three new sections were appended to the task_planner prompt at the dynamic block keyed on ``elif role_value == "task_planner"``: (a) Slice-sizing guidance (soft, advisory only — per HITL decision-6 opt-2; the plan reviewer never NACKs on size). (b) Forest constraint (HARD): every slice must have ≤1 DAG parent. (c) Auto-serialization rule with a worked example showing slice-1 → slice-2 → slice-3 with ``serialized_chain_order`` on the downstream slice; documents the fallback heuristic (``files_affected`` Jaccard >0.3, then descending fan-out). (d) Yaml key swap: ``slices:`` is canonical; ``phases:`` is backward-compat. TASK-2-4 — reviewer_plan prompt builder updated. The ``elif phase == "plan": if role_value == "reviewer_plan"`` block gains two new sections: (a) Forest-violation NACK — when ingestion left a 'Plan ingestion REJECTED' block on ``plan_review_feedback`` or a ``forest_violation`` log discriminator, NACK the planner with the structured errors verbatim and instruct re-emission with ``serialized_chain_order`` populated. (b) Slice-sizing advisory (advisory only, NEVER NACK): tone scales with magnitude (1,000–2,000 LOC: 'consider splitting'; >2,000 LOC: 'this slice is well above the soft target — strongly consider splitting'). Documents that decision-6 opt-2 keeps override authority with the refiner/operator and that a future hard NACK threshold requires a HITL revision of decision-6. TASK-5-2 — Gateway ``rebase_onto`` helper. Added ``build_rebase_onto_args(branch, new_base, old_base)`` to ``gateway/git_client.py``. Constructs the canonical ``["--onto", new_base, old_base, branch]`` shape and validates it through the existing ``validate_git_args("rebase", ...)`` allowlist plumbing — explicitly rejecting any extra flags (e.g. ``--strategy-option=ours``). Decision-15 invariant honoured: NO new privileged orchestrator-role endpoint is introduced; the helper reuses the per-agent rebase capability already on the allowlist (``rebase --onto`` listed in ``ALLOWED_GIT_OPERATIONS["rebase"]["allowed_flags"]``). TASK-1-3 — Backward-compat alias call sites converted to canonical names where convenient. ``_populate_contract_from_plan`` now uses ``contract_slices`` / ``contract.slices`` / ``to_contract_slices``; ``_load_contract_from_source_branch`` and the contract-tasks markdown builder use ``contract.slices``; ``orchestrator/routes/phases.py`` reads ``contract.slices`` for its task-count response. ``shared/egg_contracts/plan_parser.py`` imports / uses ``Slice`` and ``SliceStatus`` (the legacy ``Phase``/``PhaseStatus`` aliases stay exported for downstream callers but are no longer used internally). Defense-in-depth — slice id regex re-validated. ``ConcurrentPhaseExecutor.get_worktree_branch`` and ``get_slice_integration_branch`` now ``re.fullmatch`` the normalised slice id against ``r"slice-[0-9]+"`` before embedding it in a git ref. The contract-layer pydantic regex already enforces this on the source, but the helper is part of the gateway-facing surface — re-validating closes the seam against a future caller that forgets upstream validation (per the security reviewer's ACK suggestion). SliceScheduler env-var auto-wiring. The constructor now lazy- resolves ``EGG_ORCH_*`` defaults from ``orchestrator.env_config`` when the corresponding kwargs are ``None`` so a bare ``SliceScheduler(contract)`` picks up the operator's overrides without explicit threading. Existing test fixtures that pass explicit values keep working unchanged. Open question for HITL: ``decision-20`` (registered separately) asks the operator whether to defer the run-loop wire-up (TASK-4-2 / TASK-4-4 / TASK-5-1 invocation / TASK-5-3 scheduling) to a follow-up alongside #2199, or require it to land here. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Addresses findings from the v1 BRC NACK round (tester + reviewer_concurrency lenses) that don't depend on the deferred run-loop wire-up question (decision-20). Tester (lint/mypy): - Convert ``DependencyNode`` / ``ExecutionWave`` / ``ExecutionPlan`` / ``DependencyGraph`` from ``Generic[NodeT]`` to PEP-695 generic class syntax (``class X[NodeT: Hashable]``) per pyproject.toml ``target-version = "py313"`` (UP046). Drop the ``Generic`` + ``TypeVar`` imports. - ``yield from`` in ``SliceScheduler.iter_ready`` instead of the ``for ... yield`` loop (UP028). - Drop the unused ``Slice`` import from ``orchestrator/stacked_pr_reconciler.py`` (F401). - Drop the unused ``Phase`` re-export import from the ``shared/egg_contracts/plan_parser.py`` ``from .models import`` line (F401). - Annotate ``build_dependency_graph`` / ``compute_execution_plan`` / ``format_execution_plan`` with explicit ``[AgentRole]`` parameterisation; cast the AgentRole leakage in ``DependencyGraph.build_from_roles`` to ``NodeT`` via ``cast`` so the AgentRole-keyed callers compile under the generified type while the slice-DAG ``DependencyGraph[str]`` callers stay sound. - Cast the pydantic ``handler(data)`` return values in ``Contract._migrate_phases_to_slices`` to ``Contract`` so mypy no longer surfaces ``Returning Any`` errors on the four return paths. reviewer_concurrency (blocking): - **Drop the scheduler lock before invoking the HITL escalator** in ``record_cycle``. The escalator may issue HTTP / contract-write I/O; previously its latency would serialise every other scheduler operation (concurrency reviewer's blocker #1, #2012 precedent). The escalation parameters are captured under the lock and the call happens after the lock is released. - **Promote ``BLOCKED_ON_FAILED_DEPENDENCY`` children alongside ``PENDING`` children in ``_unblock_children``** so the cascade-then-respawn-then-complete recovery path lights up (concurrency reviewer's blocker #2). Without this fix the descendants of a respawned-and-completed parent stayed permanently blocked; the pipeline wedge required a manual contract edit. All 268 existing tests still pass; the new behaviour is also consistent with the ``unblock_children`` test in ``test_slice_scheduler.py`` (which exercises the respawn → complete → child-promotion path). The deferred run-loop wire-up (TASK-4-2 / TASK-4-4 / TASK-5-1 invocation / TASK-5-3 scheduling) remains open under HITL decision-20. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

#5 Closes the two achievable findings from reviewer_code_holistic's v2 NACK (commit 0b0bd1e8). Findings #1, #2, #3 are explicitly gated on HITL decision-20 (the run-loop wire-up scope question) and the reviewer's path-forward acknowledges that. #5 — silent ImportError fallback in validate_forest. The ``try/except ImportError`` around ``from egg_contracts.plan_parser import validate_forest`` in ``_populate_contract_from_plan`` was silently defaulting ``forest_errors = []`` if the import failed, which would let a broken-import multi-parent contract slip past the gate. Drop the guard — ``parse_plan`` was already imported from the same module unconditionally; if one fails the other does, and the populator's outer try/except already handles unexpected failures. #4 — build_rebase_onto_args ↔ rebase_onto adapter. The gateway-side helper builds argv; the reconciler's ``reconcile_once`` declares its callable as ``Callable[[str, str, str], bool]`` (executes the rebase and returns success). Add ``GatewayClient.rebase_onto`` to bridge the two: it invokes ``build_rebase_onto_args`` (existing allowlist validation), then submits the args through the existing per-agent ``/api/v1/git`` endpoint via the temp-session pattern that ``create_pr`` / ``fetch_worktree_branch`` already use. No new privileged orchestrator-role endpoint introduced (decision-15). The reconciler caller can now pass ``lambda b, n, o: gateway_client.rebase_onto(pipeline_id, repo_path, branch=b, new_base=n, old_base=o)`` directly. Reconciler module docstring drift fixed: lines 18-25 now reference ``GatewayClient.rebase_onto`` (the orchestrator-side bridge) + ``gateway.git_client.build_rebase_onto_args`` (the argv builder), not the previously-claimed ``gateway/git_client.rebase_onto`` function which never existed. The four still-blocking findings (TASK-4-2 slice integration-branch creation, TASK-4-4 per-slice spawn wire-up, TASK-5-1 invocation, TASK-5-3 scheduling) remain open under HITL decision-20 — both reviewer_code_holistic and reviewer_contract have explicitly stated they will ACK either: (a) immediately on the next re-propose if decision-20 resolves opt-1/opt-3 (defer to follow-up + contract amendment); OR (b) after re-reviewing the wire-up landed in a v3+ commit if decision-20 resolves opt-2 (require here). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Tester v2 NACK was a single blocking finding: ``ruff format --check`` flags 8 files as needing reformatting (the v1 fix addressed ``ruff check`` but the format pass is independent). Per the tester's instructions, ran ``ruff format`` on every file in the slice-DAG diff. Mechanical line-collapse fixes only — no semantic changes. Verified ``ruff format --check`` is now clean on the production surface (orchestrator/ + shared/egg_contracts/ + gateway/git_client.py). The four still-flagged files (orchestrator/tests/test_slice_*.py and shared/egg_contracts/tests/test_*.py) are tester-owned and not part of this push. All 268 unit tests still pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Captures the implementation deltas that landed after the initial docs(2137) commit (d7eccd7) so the slice-DAG architecture doc keeps parity with the code on disk: - Status callout names HITL decision-20 explicitly and enumerates the deferred run-loop wire-up tasks (TASK-4-2 / TASK-4-4 / TASK-5-1 invocation / TASK-5-3 scheduling). - Plan-parser section now documents ``ForestValidationError`` (status 422, ``to_response()`` helper) raised by ``_populate_contract_from_plan`` so future Flask routes ingesting plans in-band can surface a 422 with the structured errors. Notes that the safe wrapper has a dedicated warning discriminator and re-raises the typed exception. - DependencyGraph generification section calls out the PEP-695 ``class X[NodeT: Hashable]`` syntax (matching pyproject's py313 target) instead of ``Generic[NodeT]``. - SliceScheduler section: documents env-var lazy-resolution from ``orchestrator.env_config`` when constructor kwargs are ``None``; documents that ``record_cycle`` invokes ``hitl_escalator`` outside the lock; documents that ``_unblock_children`` re-promotes both ``PENDING`` and ``BLOCKED_ON_FAILED_DEPENDENCY`` children so the cascade→teardown→respawn→complete recovery path lights up. - Per-slice branch helpers section documents the defense-in-depth ``re.fullmatch(r"slice-[0-9]+", slice_id)`` re-validation in ``ConcurrentPhaseExecutor.get_worktree_branch`` and ``get_slice_integration_branch``. - Stacked-PR reconciler section: documents ``GatewayClient.rebase_onto`` as the production binding for the reconciler's ``rebase_onto`` callable, including the canonical argv shape, the existing per-agent ``/api/v1/git`` endpoint reuse, and the no-new-privileged-endpoint invariant (decision-15). - New "Planner & plan-reviewer prompt updates" section covers the three task_planner additions (slice-sizing guidance, hard forest constraint, auto-serialization rule + worked example, ``slices:`` yaml key) and the two reviewer_plan additions (forest-violation NACK on populator-stashed errors, slice-sizing advisory tone scaling with magnitude per HITL decision-6 opt-2). [documenter]

…e run loop Per HITL decision-20 (operator chose opt-2 — complete the run-loop wire- up in this PR), connect the previously library-only slice DAG building blocks to the orchestrator's implement-phase run loop. Previously the SliceScheduler / stacked-PR reconciler / create_slice_pr / rebase_onto helpers shipped as unit-tested library code but the run loop still spawned a single monolithic team. This commit closes that gap. Changes: 1. ConcurrentPhaseExecutor accepts an optional ``slice_id``. When supplied: - ``spawn_all`` registers the BRC tracker under the nested ``{pipeline_id}/{slice_id}`` key (refine-phase decision-14 hybrid: per-slice CONSENSUS_* state isolated; HEARTBEAT / OVERSEER_ALERT keep flowing through the bare pipeline-id). - ``_spawn_agent`` resolves per-role branches via ``get_worktree_branch(role, slice_id=...)`` so commits land on ``egg/issue-N/{slice_id}/{role}/work`` instead of the shared pipeline branch. - ``check_consensus`` looks up the slice-scoped tracker first. 2. ``_run_concurrent_phase`` accepts ``slice_id`` and forwards it to the executor + ``_handle_brc_consensus_timeout``. The sandbox env ``EGG_PIPELINE_ID`` is overridden to ``{pipeline_id}/{slice_id}`` so agent CLIs send CONSENSUS_* messages keyed on the slice's tracker scope; ``EGG_SLICE_ID`` is exported as an advisory hint. 3. ``_handle_brc_consensus_timeout`` propagates ``slice_id`` so the timeout / stuck-phase handler operates on the correct tracker. 4. New ``_run_implement_phase_slices()`` drives the SliceScheduler iteration: - Loads the contract, constructs a SliceScheduler from ``contract.slices``, computes execution waves. - For each ready slice: persists ``Slice.parent_branch_at_creation`` on the contract (the reconciler reads this for orphan detection — TASK-4-2 / TASK-5-3 plumbing), marks the slice spawned, calls ``_run_concurrent_phase(slice_id=...)`` and waits for that slice's BRC consensus. - On consensus reached, opens a per-slice PR via ``GatewayClient.create_slice_pr`` with ``base`` resolved from the slice's DAG parent (root → pipeline branch; child → parent slice's integration branch). - On failure, calls ``record_failure`` so the 60s grace window arms and the cascade fires for downstream descendants. - Drains ``poll_cascades`` between waves so BLOCKED siblings are visibly marked. - Tears down per-slice trackers via ``remove_peer_consensus_tracker(pipeline_id, slice_id)`` after each slice completes. 5. New ``_start_stacked_pr_reconciler()`` schedules the periodic reconciler as a daemon thread for the lifetime of the slice loop. Cadence reads from ``EGG_ORCH_STACKED_PR_RECONCILER_INTERVAL_SECONDS`` (default 30). The list-callables (``list_open_prs`` / ``list_extant_branches``) are stubbed pending the gateway-side helpers in a follow-up; the ``rebase_onto`` callable already routes through ``GatewayClient.rebase_onto`` which forwards to the existing per-agent ``/api/v1/git`` endpoint (refine-phase decision-15: no new privileged orchestrator role). 6. ``_run_pipeline`` gates the implement phase on multi-slice contracts. When ``current_phase == "implement"`` AND ``len(contract.slices) > 1``, the loop dispatches to ``_run_implement_phase_slices``. Single-slice and no-slice contracts continue to use the legacy monolithic path so existing pipelines are unaffected. The gateway-side ``list_open_prs`` / ``list_remote_branches`` helpers needed by the reconciler to actually find orphan PRs ship in a follow-up — the daemon currently sees no orphans and is a clean no-op on each tick. The wire-up itself (start / stop, deterministic shutdown via Event) is exercised by the slice loop's lifecycle. All 103 slice-DAG tests still pass: - test_slice_scheduler.py (28 tests) - test_stacked_pr_reconciler.py (11 tests) - test_slice_execution.py (13 tests) - test_slice_pr_creation.py (7 tests) - test_concurrent_executor.py (44 tests) Lint clean (ruff check + format). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Combines: 1. Prior tester surface (TASK-1-4 / 2-5 / 3-5 / 4-5 / 5-4) — 99 tests covering schema rename, forest validation, scheduler state machine, slice-aware branch naming, BRC tracker namespacing, orphan-PR detection. 2. New tester surface for the run-loop wire-up (coder commit 36d34da) — 49 tests covering _start_stacked_pr_reconciler daemon lifecycle, _run_implement_phase_slices DAG iteration, _run_concurrent_phase slice_id env override, _handle_brc_consensus_timeout slice_id propagation, gateway-side rebase argv canonicality (TASK-5-2), and orchestrator-side rebase_onto bridge (TASK-5-2). Files: - orchestrator/tests/test_slice_scheduler.py (28 tests) - orchestrator/tests/test_slice_branch_naming.py (13 tests) - orchestrator/tests/test_stacked_pr_reconciler.py (13 tests) - orchestrator/tests/test_slice_run_loop_integration.py (20 tests) - orchestrator/tests/test_gateway_client_rebase_onto.py (13 tests) - gateway/tests/test_build_rebase_onto_args.py (16 tests) - shared/egg_contracts/tests/test_slice_migration.py (24 tests) - shared/egg_contracts/tests/test_validate_forest.py (14 tests) - shared/egg_contracts/tests/test_plan_parser_dependencies.py (9 updated) 148 net-new tests + 9 updated; ruff + format clean; mypy clean on shared/gateway. Validates the schema rename, forest validation, slice scheduler state machine + iterator, slice-aware branch naming, BRC tracker namespacing, orphan PR reconciliation, orchestrator run-loop slice integration, per-slice PR creation, and the rebase argv allowlist invariants. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Tester v1 (commit 00ab572) drew a NACK from reviewer_code_holistic flagging three coder-side blocking issues that the test surface did not catch: 1. _run_implement_phase_slices opens the slice PR with head= egg/issue-N/slice-M (the integration branch) but never merges/pushes the per-role agent branches into that integration branch — gh pr create silently fails on the empty head. 2. _start_stacked_pr_reconciler ships with _list_open_prs / _list_extant_branches stubbed to empty collections, so the reconciler is permanently a no-op despite the daemon thread running cleanly. 3. (out-of-scope for tester role boundary): docs/architecture/ slice-dag.md drift — coder/documenter territory. Per the tester role boundary I cannot fix the underlying production code; instead this commit pins the post-fix invariants as ``pytest.mark.xfail(strict=True)`` tests so they (a) fail today (the bug is present), (b) don't count as red, and (c) become regression guards once the coder lands the fix — at which point they pass and ``strict=True`` flags the XPASS as a signal to drop the marker. New xfail tests: * orchestrator/tests/test_slice_run_loop_integration.py TestCoderGapsSurfacedByHolisticReview: * test_integration_branch_pushed_before_create_slice_pr — asserts spawner.gateway.push_worktree_branch is called before create_slice_pr (holistic NACK #1). * test_reconciler_detects_real_orphans_not_no_op — asserts the list_open_prs callable threaded into reconcile_once delegates to the gateway helper (holistic NACK #2). Per reviewer_code's non-blocking observations on tester v1 (which coincide with the coder's open NACKs), this commit also pins: * shared/egg_contracts/tests/test_validate_forest.py TestCycleDetection: * test_two_cycle_rejected — slice-1 -> slice-2 -> slice-1 must surface an error (xfail until coder wires has_cycle into validate_forest). * test_self_loop_rejected — slice-1 -> slice-1 must surface an error (xfail until coder wires has_cycle into validate_forest). * orchestrator/tests/test_slice_run_loop_integration.py test_rebase_onto_callable_bridges_to_gateway: now asserts the ``repo_path`` positional matches the value the production wiring currently passes, with a comment flagging that the assertion needs an update once the coder switches to a real repo path (reviewer_code non-blocking #4). Test-suite shape after v2: * 20 passed, 2 xfailed in test_slice_run_loop_integration.py * 11 passed, 2 xfailed in test_validate_forest.py * All other slice tests unchanged. ruff + format clean; mypy clean on shared/gateway. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

… on v4 Addresses 8 of the 10 blocking findings from reviewer_code (commit 185a08a7) and all 4 blocking findings from reviewer_contract (commit cff1bb8e) on v4 (HEAD=36d34da9612). Two reviewer_code findings (EGG_PIPELINE_ID env routing, record_cycle wiring) are documented trade-offs scoped to the #2199 follow-up. ### Blocking findings closed in v5 - **TASK-2-2 — HTTP 422 surface wired** (reviewer_contract #1): ``orchestrator/routes/phases.py`` ``populate_contract`` now branches on the ``ForestValidationError`` class name (avoids import cycle) and returns the structured ``to_response()`` body with ``status_code=422``. Acceptance test "route returns HTTP 422 with the structured error body when a multi-parent slice is ingested" is now mechanically satisfiable. - **TASK-4-2 — Slice integration branch creation** (reviewer_contract #2): new ``GatewayClient.create_slice_integration_branch(...)`` pushes ``parent_branch:refs/heads/integration_branch`` through the existing per-agent ``/api/v1/git/push`` allowlist (no new privileged endpoint, decision-15). The slice loop calls it before spawning containers and surfaces a clear error log when creation fails. - **TASK-4-4 — Wave parallelism** (reviewer_code #3, reviewer_contract #3, decision-5 hard requirement): ``_run_implement_phase_slices`` now drives the inner loop through ``concurrent.futures.ThreadPoolExecutor(max_workers=len(ready_batch))`` so every slice in a wave spawns simultaneously. The ``max_parallel_slices`` cap from ``iter_ready`` already bounds ``ready_batch``. The previous "future iterations can lift this" comment is gone; ``_run_one_slice`` is the per-slice worker function (load contract → write parent_branch → create integration branch → spawn → wait → create_slice_pr → record_complete). - **TASK-5-3 — Reconciler list helpers** (reviewer_code #1, reviewer_contract #4): ``GatewayClient.list_open_prs(repo)`` and ``GatewayClient.list_remote_branches(repo_path)`` are now implemented and wired into ``_start_stacked_pr_reconciler``. ``list_open_prs`` routes through ``/api/v1/gh/execute`` with ``args=["pr","list",...,"--json","number,headRefName,baseRefName"]`` (``pr list`` is on ``READONLY_GH_COMMANDS`` allowlist — ``gateway/github_client.py:54``). ``list_remote_branches`` routes through the existing ``/api/v1/git/fetch`` route with ``operation=ls-remote --heads``. Both return empty on transport error (the reconciler treats this as "see no orphans this tick" which is safe). - **#2 — repo_path bug** (reviewer_code): ``_start_stacked_pr_reconciler`` now accepts ``worktree_repo_path: Path`` keyword and passes the filesystem path to ``gateway.rebase_onto`` rather than the branch-name string. Fixes the "every rebase attempt 4xx at the gateway" failure mode. - **#5 — State lock** (reviewer_code): the contract load → mutate ``parent_branch_at_creation`` → save and the post-CONFIRMED ``create_slice_pr`` re-load are both wrapped in ``with get_pipeline_state_lock(pipeline_id):`` so concurrent tester / documenter contract writes can't lose data. - **#6 — Cycle detection in validate_forest** (reviewer_code + tester xfail): new ``_detect_cycles`` DFS in ``shared/egg_contracts/plan_parser.py`` runs alongside the multi-parent check. ``slice-1 → slice-2 → slice-1`` is now rejected with ``"Slice DAG contains a cycle: ..."``. Closes the silent-deadlock failure mode where ``compute_waves`` sets ``waves=[]`` on cycles and the run loop spins forever. - **#7 — Scheduler revalidates forest at construction** (reviewer_code): ``SliceScheduler.__init__`` now calls ``validate_forest(contract.slices)`` and raises ``ValueError`` with the structured errors if the contract bypassed plan-ingestion validation. Defense-in-depth for legacy state-branch restores and manual ``egg-contract`` edits. - **#8 — build_rebase_onto_args ref shape validation** (reviewer_code): ``branch`` / ``new_base`` / ``old_base`` are now rejected if they start with ``-`` (flag-shaped), contain whitespace / NUL, or fail the ``[A-Za-z0-9._/+-]+`` ref-shape regex. Closes the seam where ``--abort`` would slip through ``validate_git_args`` (it's on the rebase allowlist). ### Cascade emission (TASK-3-4 path) ``_run_implement_phase_slices`` now emits an ``OVERSEER_ALERT`` through the in-process ``message_store`` after each cascade fires, with metadata ``{anomaly: slice-cascade-block, priority: high, failed_slice_id, blocked_subtree}``. The orchestrator log line remains the always-on fallback. ### Trade-offs documented in code (deferred to #2199) - **EGG_PIPELINE_ID nested-form env override** (reviewer_code #4): the agent CLI uses one env var for every outbound signal, so HEARTBEAT and OVERSEER_ALERT also route to the slice tracker rather than the pipeline tracker. CONSENSUS_* isolation works as intended; cross-slice telemetry is per-slice today. The always-on fallback is the orchestrator-side log line + ``slice-cascade-block`` OVERSEER_ALERT emission. Pipeline-level fan-out for HEARTBEAT requires a CLI-side message-type-aware router (substantial change to ``shared/egg_orchestrator/client.py`` and the agent CLI) — tracked alongside the per-slice MCP control verbs in #2199. - **record_cycle two-tier max_cycles wiring** (reviewer_code #9): ``_run_implement_phase_slices`` records failures via ``record_failure`` directly (single-attempt-per-slice today). The ``EGG_ORCH_SLICE_LOCAL_MAX_CYCLES`` / ``EGG_ORCH_SLICE_GLOBAL_MAX_CYCLES`` knobs are read by the scheduler but not yet exercised in production. Wiring ``record_cycle`` into the BRC re-proposal seam inside ``_run_concurrent_phase`` is the natural next step but requires threading the max_cycles trip-flag through the inner BRC loop — scoped for a #2199 follow-up. ### Tests All 326 pre-existing slice tests still pass (267 previously + 59 from the in-tree run-loop integration tests landed by tester in commit 00ab572 / 1163736). The 4 XPASS(strict) "failures" are tester xfail markers that flip to PASS because this commit closes the gaps they pin (#6 cycle detection, #1 reconciler stubs, #2 repo_path). The tester will drop the markers in their next iteration. ruff check + ruff format clean on all 6 production files. Tasks satisfied (added / strengthened in v5): TASK-2-2 (HTTP 422 wiring), TASK-4-2 (slice integration branch creation), TASK-4-4 (wave parallelism), TASK-5-3 (reconciler list helpers + functional reconciliation). Reviewer-readiness: - closes reviewer_code v4 NACK findings #1, #2, #3, #5, #6, #7, #8 - closes reviewer_contract v4 NACK findings #1, #2, #3, #4 - defers reviewer_code v4 #4 (EGG_PIPELINE_ID env), #9 (record_cycle) to #2199 with documented trade-off Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Critical fix: in slice mode, agents now share the slice's integration branch ``egg/issue-N/slice-M`` instead of per-role siblings ``egg/issue-N/slice-M/{role}/work``. Without this fix the per-slice PR opened by ``create_slice_pr(head=integration_branch, base=parent_branch)`` shows an empty diff because the integration branch points at the parent's tip while agent commits live on per-role sibling branches GitHub doesn't see in the PR. The slice work was on origin but invisible to reviewers. Adopts holistic NACK option (a) "drop per-role branches in slice mode": - ``ConcurrentPhaseExecutor.get_worktree_branch(role, slice_id=...)`` now returns ``egg/issue-N/slice-M`` (no per-role suffix) when ``slice_id`` is supplied. Babysit-pr per-role staging is unchanged. Within a slice, all agents collaborate on one history — the same shared-branch model the non-slice flow has always used, just scoped per slice. The slice is the unit of isolation; cross-slice isolation is preserved by the per-slice integration branch. Silent-fallback fixes (holistic non-blocking notes): - ``_run_one_slice``: on ``create_slice_integration_branch`` failure (return False or exception), now ``record_failure(slice_id)`` and return early instead of silently spawning agents that would push to a missing parent. The cascade machinery surfaces the missing-parent error to the operator via OVERSEER_ALERT. - ``_run_one_slice``: on ``create_slice_pr`` failure, now ``record_failure(slice_id)`` instead of ``record_complete(slice_id)`` so an empty / failed PR doesn't masquerade as a successful slice. HITL escalates instead of the cascade machinery thinking everything is fine. - ``_run_implement_phase_slices``: scheduler construction now wrapped in ``try/except ValueError`` so a contract that bypassed plan ingestion validation surfaces as a structured error in the run-loop return path rather than crashing the loop. Lock-scope fix (reviewer_code v5 non-blocking #1): - ``_run_one_slice``: per-pipeline state lock now only covers the contract read for the slice-PR data snapshot; the gateway HTTP round-trip happens after the lock is released so a slow gateway can't serialise other contract writers for ~30 s. Known follow-up housekeeping (tester role boundary): - 3 tests in ``test_slice_branch_naming.py`` assert the old per-role suffix shape (``egg/issue-N/slice-M/{role}/work``) and now fail against v6's shared-branch shape. Tester needs to update those assertions to ``egg/issue-N/slice-M`` to reflect the v6 contract. Same fix shape as the XPASS(strict) markers from v4 → v5. Tasks affected: TASK-4-1 (branch naming) — implementation now correctly matches the slice-as-unit-of-isolation design promised by the architecture doc and the per-slice PR diff invariant. Reviewer-readiness: closes reviewer_code_holistic v5 NACK #1 (empty-diff slice PR), addresses non-blocking observations (silent-fallback in create_slice_pr / create_slice_integration_branch / scheduler ValueError swallow + state-lock-around-HTTP). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…er coder v5 Coder v5 (commit 7f42034) closed the three blocking findings reviewer_code_holistic flagged on the v1 cycle, plus reviewer_code's non-blocking gaps that the tester surface had pinned as xfail-strict. This commit promotes those markers to regular regression guards now that the underlying production code is correct. Promoted xfail → regular guard: * orchestrator/tests/test_slice_run_loop_integration.py TestCoderFixesForHolisticReview (was TestCoderGapsSurfacedByHolisticReview): - test_integration_branch_created_before_create_slice_pr (was test_integration_branch_pushed_before_create_slice_pr): retargeted at the actual coder seam ``GatewayClient.create_slice_integration_ branch`` (the v1 xfail named ``push_worktree_branch`` which was the closest existing primitive at NACK time). Now asserts the call-order invariant: integration branch is created BEFORE the per-slice PR is opened, so gh pr create finds a populated head. - test_reconciler_detects_real_orphans_not_no_op: xfail removed — coder v5 implemented ``GatewayClient.list_open_prs`` and ``list_remote_branches`` and wired them into the reconciler. * shared/egg_contracts/tests/test_validate_forest.py TestCycleDetection: - test_two_cycle_rejected: xfail removed — coder v5 wired ``_detect_cycles`` (DFS) into ``validate_forest``; 2-cycles and self-loops now produce structured errors at plan ingestion. - test_self_loop_rejected: xfail removed. Other fixes: * test_rebase_onto_callable_bridges_to_gateway: now passes ``worktree_repo_path=Path("/tmp/test-worktree")`` to ``_start_stacked_pr_reconciler`` and asserts the value flows through to ``gateway.rebase_onto`` as the second positional. This locks in the coder v5 fix for reviewer_code non-blocking #4 (was passing the branch string as repo_path; now passes the real filesystem path). Test-suite shape after v3: * test_slice_run_loop_integration.py: 22 passed (up from 20+2 xfailed). * test_validate_forest.py: 13 passed (up from 11+2 xfailed). * All other slice tests unchanged. Total: 326 tests, 0 xfails, all green. ruff + format clean; mypy clean on shared/gateway. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…ailed Coder v6 (commit 97de106) lands two behaviour changes that the tester surface needs to track: 1. **Shared per-slice branch (TASK-4-1 fix for holistic v5 NACK #1):** ``ConcurrentPhaseExecutor.get_worktree_branch(role, slice_id=...)`` now returns ``egg/issue-N/slice-M`` for every role in the slice instead of the per-role ``egg/issue-N/slice-M/{role}/work`` shape. This eliminates the empty-diff per-slice PR failure mode where each role's commits sat on a separate branch the per-slice PR never referenced. 2. **PR creation failure now marks the slice failed:** the slice loop's ``record_complete()`` is now gated on successful PR creation; an exception from ``create_slice_pr`` causes ``record_failure(slice_id)`` and a non-zero overall exit code. This closes the silent-fallback non-blocking observation from earlier reviews. Tester surface updates: * ``test_slice_branch_naming.py::TestSliceAwareWorktreeBranch``: - ``test_slice_aware_branch_for_canonical_id`` / ``test_bare_integer_slice_id_normalised`` / ``test_falls_back_to_issue_number_when_no_branch`` now assert the shared-branch shape ``egg/issue-N/slice-M``. - New ``test_role_does_not_affect_branch_name_when_slice_set`` samples coder/tester/documenter and asserts every role in slice-2 returns the same branch — locks in the v6 fix invariant against future per-role-suffix regression. * ``test_slice_run_loop_integration.py::TestRunImplementPhaseSlices``: - ``test_pr_creation_failure_does_not_abort_loop`` renamed to ``test_pr_creation_failure_marks_slice_failed`` and inverted: PR creation failure must now surface as non-zero exit, not the previous silent best-effort behaviour. Sibling slice still runs (decision-2 sibling-independence preserved). Test-suite shape after v4: * test_slice_branch_naming.py: 14 passed (up from 13). * test_slice_run_loop_integration.py: 22 passed (one renamed). * All other slice tests unchanged. * Total slice-related: 327 tests, 0 xfails, all green. ruff check + format clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Address all 10 blocking findings + 3 non-blocking notes from reviewer_code's NACK on commit 5d3ab58. The doc was authored before coder v4 (run-loop wire-up), v5 (8/10 reviewer_code blockers closed), and v6 (per-slice shared-branch collapse) shipped, so it described a deferred / library-only state that no longer matches the code on disk. Blocking #1 — Status banner: rewritten to reflect HITL decision-20 opt-2 ("require wire-up to land here"). The slice loop is live, the reconciler is functional with live `list_open_prs` / `list_remote_branches` helpers, integration branches are created on origin before agents spawn, and per-slice PRs open on consensus reach. Two trade-offs are called out explicitly: the EGG_PIPELINE_ID nested-form override that also scopes HEARTBEAT/OVERSEER_ALERT to the slice tracker (decision-14 hybrid honoured partially), and the deferred `record_cycle` two-tier wiring. Both are scoped to #2199. Blocking #2 — Per-slice branches & BRC trackers: rewrote the section for the v6 shared-branch shape `egg/issue-N/slice-M`. The earlier per-role suffix `egg/issue-N/slice-M/{role}/work` shape produced empty per-slice PR diffs and was deliberately removed. Doc now says "the slice is the unit of isolation, not the role within the slice" and surfaces the multi-agent push attribution dependency on `gateway/git_client.py:get_attributed_changed_files_in_push` so the security model is explicit. Notes that the slice run loop creates the integration branch on origin via `GatewayClient.create_slice_integration_branch` *before* agents spawn, and on creation failure calls `record_failure` to arm the cascade timer rather than silently spawning agents. Blocking #3 + #4 — Two-tier max_cycles section: added "Status: deferred to #2199" callout. The `record_cycle` invocation point is not yet wired into the slice run loop; the env knobs are read but the trip path is dead code today. Configuration knobs table now annotates `EGG_ORCH_SLICE_LOCAL_MAX_CYCLES` / `EGG_ORCH_SLICE_GLOBAL_MAX_CYCLES` as "(currently inert)" so operators don't tune them expecting an effect. Blocking #5 — Stacked-PR reconciler: documented the live `GatewayClient.list_open_prs` (gh pr list --json) and `GatewayClient.list_remote_branches` (git ls-remote --heads) helpers and confirmed both flow through existing per-agent allowlists (decision-15 invariant preserved). The reconciler is no longer a no-op. Blocking #6 — Plan Parser & Forest Validation: added "Cycle detection" subsection covering the new `_detect_cycles` DFS that rejects cyclic chains (e.g. `slice-1 → slice-2 → slice-1`) at plan ingestion. Cited the structured error format showing the full cycle chain and noting that multi-parent + cyclic violations are reported in the same returned list. Blocking #7 — `SliceScheduler.__init__` constructor revalidation: added new "Constructor-time forest revalidation" subsection. The constructor calls `validate_forest` and raises `ValueError` on multi-parent / cyclic violations so contracts that bypass plan ingestion (legacy state-branch restores, manual `egg-contract` edits, in-process fixtures) still hit the gate before the run loop spins. Blocking #8 — Cascade OVERSEER_ALERT emission: added a paragraph in the "Failure cascade" section documenting the orchestrator-side emission via the in-process `message_store`. Body shape and metadata fields (anomaly, priority, failed_slice_id, blocked_subtree, phase) are documented. Notes explicitly that this is the always-on safety net under the v4/v5/v6 EGG_PIPELINE_ID override, since agent-emitted overseer alerts route to the slice tracker and would otherwise be invisible at the pipeline level. Blocking #9 — Wave parallelism: new "Implement-phase run loop" section documents the wave-parallel slice spawn via `concurrent.futures.ThreadPoolExecutor(max_workers=len(ready_batch))`. The pool's max-workers mirrors the `EGG_ORCH_MAX_PARALLEL_SLICES` budget that `iter_ready` already enforces, so the executor cap and env knob agree. Walks through the run-loop state machine (construct scheduler → start reconciler thread → wave loop with parallel `_run_one_slice` workers → `poll_cascades` after each wave → loop until `all_done` → tear down). Blocking #10 — TASK-3-4 cascade alert path: covered by #8's orchestrator-side emission paragraph in the Failure cascade section. Non-blocking notes: - Out of scope (#2137) section now lists the EGG_PIPELINE_ID hybrid trade-off and the `record_cycle` deferral as explicit carve-outs rather than burying them in inline notes. - Per-slice MCP control verbs entry tightened to enumerate `restart_slice`, `restart_agent` w/ slice_id, `get_slice_status`, and `list_slices` plus the slice-addressable hooks (`teardown_slice`, `respawn_slice`, `get_slice_status`) that the follow-up will wrap. - Resolved design decisions section adds decision-20 ("operator chose opt-2 — wire it up here") with citations to commits 36d34da, 7f42034, 97de106. [documenter]

- Update stale /api/v1/git reference in TestEndToEndOrphanHeal docstring to /api/v1/git/execute (matches the corrected endpoint asserted by the test body). - Drop dead local variables (extant, extant_only_slice3, the second never-orphaned PR record) from test_orphan_detected_on_producer_shape; the test always passed set() to find_orphaned_child_prs so the locals were unused churn. - Replace silent except: pass around delete_session in GatewayClient.rebase_onto's finally block with a logger.debug so leaked sessions are diagnosable. Authored-by: egg

egg-reviewer

No agent-mode design concerns in the delta since 85df7378.

The single new commit (d3ec0a1) is purely cosmetic cleanup of the three non-blocking nits flagged in the prior round:

Docstring fix /api/v1/git → /api/v1/git/execute in integration_tests/test_slice_pipeline_e2e.py:257.
Dead local variables (extant, extant_only_slice3, redundant PR record) removed from test_orphan_detected_on_producer_shape.
Silent except: pass around delete_session at orchestrator/gateway_client.py:1487 replaced with a logger.debug carrying pipeline_id and error so leaked sessions are diagnosable.

None of this touches agent prompts, agent invocation, output formats, model IDs, or the Agent SDK. The reconciler still routes through the existing per-agent /api/v1/git/execute, /api/v1/git/push, and /api/v1/gh/pr/edit endpoints rather than introducing a privileged orchestrator-role surface, preserving the refine-phase decision-15 alignment.

— Authored by egg

james-in-a-box

Contract Verification — PR #2220 (issue #2137) — Re-review at `d3ec0a1`

Verdict: approve.

Re-reviewed against the prior approval at 85df737. Delta is one commit, d3ec0a1 ("PR #2220: address non-blocking review nits") — pure cleanup of advisory items flagged in the prior round.

Delta inspected

85df737..d3ec0a1
 integration_tests/test_slice_pipeline_e2e.py | 28 +++-------------------------
 orchestrator/gateway_client.py               |  8 ++++++--
 2 files changed, 9 insertions(+), 27 deletions(-)

Findings

Dead-variable cleanup in test_orphan_detected_on_producer_shape (integration_tests/test_slice_pipeline_e2e.py:185-211). The previous body declared extant, extant_only_slice3, and a second PR record (slice-3 with a still-extant base), then never used them — find_orphaned_child_prs(contract, only_orphan, set()) was already called with a literal set() and the single-orphan list only_orphan. The new body drops the dead locals; the assertions are unchanged (pr_number == 4242, branch == "egg/issue-2137/slice-2", intended_new_base == "egg/issue-2137"). This is a behaviour-preserving simplification, not a coverage reduction.
Docstring fix (integration_tests/test_slice_pipeline_e2e.py:257). 1. local rebase via /api/v1/git (canonical argv shape). → 1. local rebase via /api/v1/git/execute (canonical argv shape). matches the endpoint actually asserted by the test body and by the gateway change in 85df737. No-op for the test logic; pure docstring drift fix.
Diagnosable session-cleanup failures (orchestrator/gateway_client.py:1483-1492). except: pass around delete_session(session_token) in rebase_onto's finally block is replaced with except Exception as exc: logger.debug("rebase_onto: session cleanup failed", pipeline_id=..., error=str(exc)). The exception is still swallowed (correct — cleanup failure must not mask the rebase result the caller already returned), but operators now have a debug-level breadcrumb when sessions leak. The structured-log key shape (pipeline_id, error) matches the surrounding logger.warning site at line 1476-1481 in the same method.

Regression check on previously-verified behaviour

Re-ran integration_tests/test_slice_pipeline_e2e.py (all 6 tests, including TestReconcilerOnProducerShape::test_orphan_detected_on_producer_shape and TestEndToEndOrphanHeal::*) — all pass. The end-to-end heal coverage that grounded the prior approval (rebase via /api/v1/git/execute → --force-with-lease push via /api/v1/git/push → PR retarget via /api/v1/gh/pr/edit) is intact.

Contract state

current_phase: refine, acceptance_criteria: [], phases: [] — the contract for #2137 has no formal acceptance-criteria entries to mark. No verify-criterion calls are required for this delta; verification stands on the inline regression check above.

Approve rationale

All three changes are advisory cleanup (dead code removal, docstring fix, swallowed-exception observability) with no functional effect on the orphan-heal three-step path.
Production wiring (reconcile_once → find_orphaned_child_prs → rebase_onto → gh pr edit) is untouched.
Test suite stays green.
No prior approval criterion is at risk.

— Authored by egg

egg-reviewer

Re-review at `d3ec0a1`

The single new commit since 85df737 is pure cosmetic cleanup of the three non-blocking nits flagged at 85df737. All three land cleanly and don't change behavior.

Verification of nits

1. Stale docstring /api/v1/git → /api/v1/git/execute (integration_tests/test_slice_pipeline_e2e.py:257). Fixed. Greppable confirmation: no remaining "/api/v1/git" literals across orchestrator/, gateway/, or integration_tests/ — every call-site / assertion uses /api/v1/git/execute.

2. Dead locals in test_orphan_detected_on_producer_shape. Removed extant, extant_only_slice3, and the second never-orphaned PR record. The setup was confused: it built a 2-PR list, mutated unused locals, then discarded everything except producer_prs[0] before calling find_orphaned_child_prs(contract, only_orphan, set()). The "PR with extant base must be skipped" path the deleted code appeared to be testing is already covered properly by orchestrator/tests/test_stacked_pr_reconciler.py:127 (test_child_with_extant_base_is_not_orphan) — so no coverage is lost. The cleanup also makes the test assertion (intended_new_base == "egg/issue-2137") the only behavior the test is responsible for, which is what the test name promises.

3. Silent except: pass around delete_session (orchestrator/gateway_client.py:1487). Replaced with except Exception as exc: logger.debug("rebase_onto: session cleanup failed", pipeline_id=pipeline_id, error=str(exc)). Diagnosable now. pipeline_id is a method parameter, so it's always in scope when the finally fires. The # noqa: BLE001 matches the existing except Exception as exc block 14 lines above (logger.warning("rebase_onto: gateway request failed", ...)) — same broad-except pattern, justified by the finally semantics.

Tests

Suite	Result
`integration_tests/test_slice_pipeline_e2e.py -m integration`	6/6 pass
`orchestrator/tests/test_stacked_pr_reconciler.py`	19/19 pass
`orchestrator/tests/test_gateway_client_rebase_onto.py`	17/17 pass
`gateway/tests/test_reconciler_push_wiring.py`	5/5 pass

Not addressed (intentionally deferred)

The third nit from my last review — "No Flask-level test for /api/v1/git/execute with a rebase payload" — is still outstanding. That was filed as worth-a-follow-up, not a blocker, and stays appropriate for a follow-up PR. The push-side now has real-handler coverage; the rebase-side is still mock-only at the gateway-client boundary.
Earlier reviewer suggestion to rename test_slice_pipeline_e2e.py (since it's mock-heavy rather than truly e2e) was not taken. Also non-blocking; the module docstring is now explicit that external side-effects are stubbed at the gateway-client boundary, so a careful reader won't be misled.

Pre-existing observation (not a regression, not introduced here)

The structlog-style kwarg pattern in this module (logger.debug(msg, pipeline_id=..., error=...)) only works against the real egg_logging.get_logger. The try/except ImportError fallback at gateway_client.py:31-37 returns a plain logging.Logger, which would raise TypeError on the unrecognized kwargs. This applies to every structlog-style call in the file (the existing logger.warning("rebase_onto: gateway request failed", ...) 14 lines above included), so the new logger.debug doesn't make it worse — just consistent with the rest of the module. If the fallback is reachable in practice, it's broken across the board, not by this commit.

Summary

LGTM. Cleanup commit only; no behavior change; no regression. All previously flagged blockers from the earlier audit remain fixed.

— Authored by egg

james-in-a-box · 2026-04-29T01:13:46Z

egg is addressing review feedback...

james-in-a-box · 2026-04-29T01:16:44Z

egg feedback addressed. View run logs

27 previous review(s) hidden.

* docs: update docs for slice-DAG implement phase (#2137) [doc-updater] Update documentation to reflect changes from d1a2139 (PR #2220): - README.md: describe slice-DAG model in the Implement phase step - STRUCTURE.md: add slice_scheduler.py and stacked_pr_reconciler.py entries; update dependency_graph.py to mention its generification - orchestrator-cli.md: add the five new EGG_ORCH_*SLICE* and EGG_ORCH_STACKED_PR_RECONCILER_* env vars to the reference table - sdlc-pipeline.md: introduce slice-DAG paragraph in Multi-Agent Orchestration section with link to architecture/slice-dag.md - concurrent-execution.md: correct "sharing the pipeline branch" claim (implement now uses per-slice integration branches) Triggered by: #2220 Authored-by: egg * docs: resolve slice-DAG/shared-branch contradictions raised in PR #2231 Address review feedback from egg-reviewer: - concurrent-execution.md: qualify "shared branch" claims at L49 (top-level branch model paragraph), L862 (per-agent worktree architecture bullet), L867 (push coordination step), and L877 (reviewer worktree sync) so the whole guide consistently teaches the slice model for the implement phase rather than contradicting the new top-of-file note. - sdlc-pipeline.md: rewrite the "Shared Pipeline Branch" subsection (now "Branch Model") and the "Commit conflicts" troubleshooting bullet to match the slice-DAG model introduced earlier in the same file. - orchestrator-cli.md: add "API live, not yet wired in the run loop — see [Slice-DAG Implement Phase]; #2199" caveat to the EGG_ORCH_SLICE_LOCAL_MAX_CYCLES and EGG_ORCH_SLICE_GLOBAL_MAX_CYCLES rows so the reference table doesn't oversell current behavior. Authored-by: egg --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

…h merged-PR signatures (#2290) * Fix #2224: pipeline-watcher OVERSEER_ALERT on branch divergence with merged-PR signatures Defense-in-depth follow-on to #2222 (Phase 4, item 3 of 3). When a pipeline branch is more than 20 commits ahead of base AND those ahead-commits contain merged-PR subject signatures (`(#NNNN)`), the pipeline-watcher emits an `OVERSEER_ALERT` so operators see the contamination shape from #2222 in real time rather than only at PR open. A real pipeline branch grows by refine/plan/implement/state-file commits authored by agents — none of those carry a `(#NNNN)` suffix in the subject. When that signature appears alongside material divergence, the branch has absorbed merged-main commits, which is the exact failure mode that produced #2220 (the contaminated PR for #2137). The signature heuristic is intentionally cheap and false-positive-tolerant; the issue explicitly prefers over-alerting to missing another contaminated PR. The detector runs inside the existing `_health_monitor_poll` thread (every 30s), reads pipeline state fresh each tick so mid-pipeline branch updates are picked up, and uses a per-pipeline SHA dedupe set so each offending commit produces at most one alert per pipeline run. All errors are logged-and-swallowed — observability must never block the pipeline. Adds `BRANCH_DIVERGENCE_THRESHOLD = 20`, `_check_branch_divergence_for_alert`, and `_publish_branch_divergence_alert` near the existing `_publish_consensus_timeout_alert` (#2264). * Address #2290 review feedback: dedupe rev-list, reset-on-clear, polling-tick tests Addresses non-blocking observations from the egg-reviewer comment: - Dedupe git rev-list --count between helper and caller (#2): the _check_branch_divergence_for_alert helper now returns (ahead_count, offenders) so the polling caller no longer re-runs rev-list to populate the alert body. Removes the racy second subprocess and the duplicated git boilerplate, and obviates the unchecked returncode (#3). - Reset dedupe set when contamination clears (#4): when the offender list goes empty, divergence_alerted_shas is cleared so a re-introduced SHA (e.g. agent re-runs a bad rebase) re-fires per the issue's 'rather over-alert than miss' stance. - Extract _branch_divergence_tick (#5): pulls the polling-thread integration block out of the _health_monitor_poll closure into a standalone helper, making the dedupe + reset + per-tick re-load behavior unit-testable. New TestBranchDivergenceTick class exercises 8 scenarios: first-tick publish, dedupe across ticks, partial overlap, empty-offenders reset, re-introduction re-fires, missing branch/base skip, load_pipeline exception swallow, and per-tick re-load. - Test naming + assertion nits (#6): renamed test_returns_empty_when_below_threshold -> test_returns_empty_when_at_threshold (the body exercised the at-threshold case), added a real below-threshold test, and fixed the no-op slice ('abc1234'[:12]) in the publisher body assertion. - Document phase-boundary detection latency (#1): added a paragraph to the module-level comment noting that the polling thread does not fetch, so contamination introduced mid-phase is detected at the next phase boundary's fetch (not within 30 s). Skipped per reviewer's own framing: - ImportError fallback uncovered (#7): reviewer explicitly noted this is acceptable to track with the consensus-timeout publisher. - BRANCH_DIVERGENCE_THRESHOLD config override (#8): reviewer noted the current default is fine per the issue's over-alert stance. Tests: 22 divergence-alert tests + 23 pipelines-routes tests pass. * Clarify dedupe-reset behavior on transient git errors The reset-on-empty branch in _branch_divergence_tick also fires when git invocations in _check_branch_divergence_for_alert fail (count returncode != 0, parse error, log timeout) and surface as (0, []). The next reader of this code reasonably assumes the reset only fires on genuinely-cleared contamination, so add an inline note pointing at the issue's over-alert posture as the rationale. Per egg-reviewer non-blocking observation on PR #2290. --------- Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

…ushes (#2370) * Fix #2368: synthetic-session exemption for slice integration-branch pushes Multi-slice implement phases were stranded because two correct-in-isolation behaviours collided: * #2028 (gateway): every pipeline-session push without `consensus_push=true` is rejected unless the target is in `INFRASTRUCTURE_BRANCHES`. * #2220 (orchestrator): `create_slice_integration_branch` registers a synthetic pipeline session and pushes `parent:refs/heads/<integration>` via `/api/v1/git/push` so the slice PR's diff is non-empty before agents spawn. Result: every multi-slice pipeline 403'd on the per-slice integration push and 15 slices failed before any agent ran (latent since #2220, only unmasked once #2337 stopped silently demoting multi-slice contracts to monolithic implement). Fix: add a path + flag exemption to the gateway's infrastructure-push bypass — a session whose `synthetic` flag is `True` (only settable by the launcher, since `/api/v1/sessions/create` is gated on `require_launcher_auth`) targeting `egg/<base>/(slice|phase)-N` is treated as orchestrator infrastructure. No new orchestrator-role push surface is introduced; agent BRC enforcement is unchanged. Bonus: the slice integration-branch name now derives from `pipeline.branch` directly, so a qualifier suffix (`-v3`, `-backend`) is preserved. Two qualified pipelines for the same issue would otherwise collide in the `egg/issue-N/slice-M` namespace. Tests cover the four parent shapes (issue / qualified-issue / JIRA / legacy `phase-N`), confirm a non-synthetic session is still blocked on slice-shaped branches (the synthetic flag is load-bearing), and add an orchestrator regression assert that `create_slice_integration_branch` precedes `_run_concurrent_phase` so a future refactor can't re-introduce the spawn-then-push ordering bug. * Address #2370 review: fix qualifier-dropping orphan no-op + nits The blocking fix: stacked_pr_reconciler.find_orphaned_child_prs derived issue_branch from contract.issue.number, producing 'egg/issue-N' even when the contract carried a qualifier (e.g. pipeline_id='issue-N-v3'). Since this PR's bonus fix made create_slice_integration_branch preserve the qualifier ('egg/issue-N-v3/slice-M'), the reconciler's per-slice lookup never matched and orphan detection silently no-op'd for every qualified pipeline. Switch to 'egg/{contract.contract_key}' which returns the canonical pipeline-id for all three shapes (issue-driven, qualified, JIRA). Non-blocking nits from the review: - Tighten _SLICE_INTEGRATION_BRANCH_RE to single-segment bases (drop / from the second character class). - Replace dead Python-2-shaped 'except E1, E2' try/except in the slice-loop test helper with a direct PipelineConfig kwarg construction; refactor the new qualified-pipeline test to reuse the helper instead of duplicating the boilerplate. - Document the intentional dual audit emission (push_slice_integration_exempt + push_infrastructure_exempt with exempt_type=slice_integration_branch) inline so operators don't conclude the latter was an infra push. Tests: regression coverage for the qualifier-preservation bug (qualified-pipeline orphan detection + walk-up resolver) and for the tightened regex (multi-segment base rejected). * Drop pre-#2137 framing from reconciler comment Reviewer noted the historical reference is misleading — the issue-number-based derivation that this comment documents was introduced and removed in the same PR (#2370), not a pre-#2137 artefact. Re-frame as describing the prior ternary directly. Cosmetic; no behaviour change. --------- Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

…otFoundError) (#2901) * Fix sliced implement phase crash: dual-import SliceScheduler _run_implement_phase_slices imported SliceScheduler with a bare `from orchestrator.slice_scheduler import SliceScheduler` and no ImportError fallback. In the deployed orchestrator pod the code runs from /app/ with top-level modules (routes.pipelines, slice_scheduler), so the orchestrator.* prefix does not resolve and the implement phase crashes on entry with ModuleNotFoundError: No module named 'orchestrator'. Every other sibling import in pipelines.py already uses the dual try/except pattern (and slice_scheduler.py itself does too). Apply the same fallback here so the import resolves in both the repo-root (orchestrator.slice_scheduler) and pod (slice_scheduler) contexts. This crash blocked every sliced-implement pipeline on entry, latent since #2220 landed the slice-DAG implement phase (2026-04-28). Verified: both import paths resolve; file byte-compiles. * Guard remaining orchestrator.* imports in slice loop Apply the same dual-import try/except pattern to the three sibling imports at lines 16046-16048 (global_slice_admit, peer_consensus, state_store) and the impasse_routing import at line 17123 inside _run_concurrent_phase_with_impasse_retry. Without these the pod runtime (PYTHONPATH=/app, flat module layout) still hits ModuleNotFoundError after the original SliceScheduler fix — the slice loop calls every one of these helpers on entry. * Add static regression guard for slice-loop orchestrator.* imports AST-based check that every `from orchestrator.X import Y` inside _run_implement_phase_slices and _run_concurrent_phase_with_impasse_retry is wrapped in a try/except that catches ImportError (directly or via Exception/BaseException). A bare import inside either function raises ModuleNotFoundError in the pod runtime (PYTHONPATH=/app, flat layout) and crashes the implement phase on entry — this guard fails fast at test time so the recurrence pattern can't slip in unnoticed. --------- Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

james-in-a-box Bot added egg agent:orchestrator labels Apr 28, 2026

This comment has been minimized.

Sign in to view

egg and others added 27 commits April 28, 2026 15:19

Persist statefiles after refine phase

dee02e6

Persist statefiles after refine phase

f4d55ea

Persist HITL resolution after refine phase gate

faf08f3

Persist statefiles after plan phase

bd9f359

Persist statefiles after implement phase

075e6e7

This comment has been minimized.

Sign in to view

egg-reviewer Bot approved these changes Apr 29, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

james-in-a-box Bot commented Apr 29, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

egg-reviewer Bot reviewed Apr 29, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

jwbron approved these changes Apr 29, 2026

View reviewed changes

jwbron merged commit d1a2139 into main Apr 29, 2026
35 of 36 checks passed

james-in-a-box Bot mentioned this pull request Apr 29, 2026

docs: update docs for slice-DAG implement phase [doc-updater] #2231

Merged

jwbron mentioned this pull request Apr 29, 2026

Part of #2224 (3/3): OVERSEER_ALERT on pipeline-branch divergence with merged-PR signatures #2290

Merged

4 tasks

This was referenced Apr 30, 2026

Multi-slice implement phase fails: create_slice_integration_branch blocked by pipeline-session push enforcement #2368

Closed

Fix #2368: synthetic-session exemption for slice integration-branch pushes #2370

Merged

jwbron mentioned this pull request May 31, 2026

Fix sliced implement-phase crash: dual-import SliceScheduler (ModuleNotFoundError) #2901

Merged

Navigation Menu

Slice the implement phase into a DAG of independent units - #2220

Slice the implement phase into a DAG of independent units#2220
jwbron merged 35 commits into
mainfrom
egg/issue-2137

james-in-a-box Bot commented Apr 28, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

egg-reviewer Bot left a comment

Uh oh!

This comment has been minimized.

james-in-a-box Bot left a comment

Uh oh!

This comment has been minimized.

egg-reviewer Bot left a comment

Uh oh!

This comment has been minimized.

james-in-a-box Bot commented Apr 29, 2026

Uh oh!

james-in-a-box Bot commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

james-in-a-box Bot commented Apr 28, 2026

Test Plan

Manual Steps

Pipeline Context

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

egg-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

james-in-a-box Bot left a comment

Choose a reason for hiding this comment

Contract Verification — PR #2220 (issue #2137) — Re-review at d3ec0a1

Delta inspected

Findings

Regression check on previously-verified behaviour

Contract state

Approve rationale

Uh oh!

This comment has been minimized.

egg-reviewer Bot left a comment

Choose a reason for hiding this comment

Re-review at d3ec0a1

Verification of nits

Tests

Not addressed (intentionally deferred)

Pre-existing observation (not a regression, not introduced here)

Summary

Uh oh!

This comment has been minimized.

james-in-a-box Bot commented Apr 29, 2026

Uh oh!

james-in-a-box Bot commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Contract Verification — PR #2220 (issue #2137) — Re-review at `d3ec0a1`

Re-review at `d3ec0a1`