Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions docs/architecture/orchestrator.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,22 @@ The implement-phase run loop is multi-slice (see [slice-dag.md](slice-dag.md)),

| # | Observed state | Action |
|---|----------------|--------|
| 1 | `IN_PROGRESS`, no commits on integration branch | No-op — scheduler re-yields the slice as `READY` and a fresh team spawns |
| 2 | `IN_PROGRESS` + commits + no consensus tracker | Mark the slice as already spawned so the scheduler does not re-yield it |
| 1 | `IN_PROGRESS`, no commits on integration branch | Reap any orphaned Job, then no scheduler action — the scheduler re-yields the slice as `READY` and a fresh team spawns |
| 2 | `IN_PROGRESS` + commits + no consensus tracker | Reap any orphaned Job, then take no scheduler action so the slice re-yields `READY` and the run loop re-drives it ([#3685](https://github.com/jwbron/egg/issues/3685)) |
| 3 | `IN_PROGRESS` + commits + consensus reached but unrecorded | Mark `COMPLETE`; emit a louder-than-fresh-spawn audit warning |
| 4 | `BLOCKED` | Preserve the BLOCKED state; if no pending HITL decision exists, escalate to HITL via `_escalate_blocked_slice_to_hitl` |
| 5 | Corrupt / unclassifiable state | Escalate to HITL via `_escalate_corrupt_slice_to_hitl` |

Cases 4 and 5 create an unresolved `Decision` on the contract (via the shared `_escalate_layer_c_hitl` helper) with three options — mark complete, restart slice, cancel — and pause the pipeline rather than silently re-yielding as `READY`. Per the plan task body: "silent classification error is worse than an operator pause."

**Why case 2 re-drives instead of resuming in place ([#3685](https://github.com/jwbron/egg/issues/3685)).** Case 2 originally called `scheduler.mark_spawned`, on the reasoning that the surviving agent pods (or a lazy spawn-on-need path) carried the slice forward. Post-[#3164](https://github.com/jwbron/egg/issues/3164) neither exists. The orchestrator-owned BRC event loop is the only thing that dispatches work for a slice; it is process-local, `ConcurrentPhaseExecutor.spawn_all` is the only thing that starts one, and `_run_one_slice` is the only caller of `scheduler.record_complete` for a running slice. `mark_spawned` at bootstrap therefore parked the slice in scheduler-`RUNNING` with no dispatcher and no completer: `iter_ready` never re-yields a `RUNNING` slice, it holds a `max_parallel_slices` slot, and `all_done()` can never turn true, so the pipeline reports `status: running` with zero pods indefinitely while every liveness signal (driver thread present, per-slice tracker reconstructed, slice classified `resumed`) reads healthy. The [#2914](https://github.com/jwbron/egg/issues/2914) live-pod guard only narrowed the wedge to "a pod was still alive at bootstrap", which post-#3164 means "a one-shot event Job that exits within minutes". Re-driving costs little and is what "resume" already means under orchestrator ownership: `spawn_all` spawns no agents up front, it starts the event loop, and the loop's one-shot Jobs pick up the commits already on the slice's integration branch (which resumes in place via `integration_base_sha`, [#2947](https://github.com/jwbron/egg/issues/2947)). Before re-driving, `_reap_orphaned_slice_jobs` force-removes any Job still live for the slice: its loop is gone, so nothing will observe its termination or derive its next event, and left in place it holds the role's worktree while the fresh cohort's Job attaches to the same checkout ([#3337](https://github.com/jwbron/egg/issues/3337)). Per-agent worktrees are deliberately not deleted; the re-driven slice wants them warm. The audit log's `redriven=` field replaces the old `resumed=` / `reclassified_fresh=` pair: a slice id under `redriven` that never appears in a subsequent spawn is a real anomaly, whereas `resumed` was indistinguishable from a wedge. One known cost: the per-slice tracker reconstructed at startup (#2409) is superseded by the fresh one `spawn_all` registers, so the slice replays a BRC round against commits already on its branch.

**Where the reap runs, and why it is not conditioned on the classification.** `_reap_orphaned_slice_jobs` is called for **both** case 1 (`"fresh"`) and case 2 (`"resume"`), above the classification switch rather than inside one of its arms. Both leave the slice `READY` and therefore admissible, and the only thing separating them is whether commits reached the slice's integration branch on origin — a signal orthogonal to whether an agent Job is still live. An orchestrator recycle before the slice's first push, or a transient gateway probe failure (which `_classify_non_complete_slice` deliberately defaults to `"fresh"`), both classify `"fresh"` with the orphan still `Running`; conditioning the reap on `"resume"` would leave the #3337 race open on exactly those paths, and nothing downstream closes it (`_reap_superseded_siblings` documents that it cannot match a same-role key adopted across a restart). Classification decides what prompt the re-driven cohort gets, never whether orphan teardown happens.

**Why the reap waits for observed teardown.** `remove_agent_container(force=True)` only *orders* a foreground delete — `KubernetesClient.remove_container` says in its own docstring that the GC removes pods asynchronously and they are not guaranteed gone when the call returns. Returning there and immediately admitting the slice would re-open the window the reap exists to close: the orphan is still `Running` inside its termination grace period with the role worktree mounted when the fresh cohort's Job attaches and `_clean_reused_worktree` runs `git reset --hard && git clean -fd` under it. So `_await_reaped_jobs_gone` waits on `k8s.wait_for_job_gone` per reaped Job, bounded by a shared `_REAP_TEARDOWN_WAIT_SECONDS` (20 s) deadline — the same "wait for the teardown you requested to be OBSERVED" contract `restart_agent` adopted in [#3597](https://github.com/jwbron/egg/issues/3597). An unobserved teardown (budget exhausted, waiter raised, Job still terminating, or a Pod-UID-only handle that `wait_for_job_gone` cannot address) is logged as such and reported via `teardown_confirmed=false` on the audit line rather than silently claimed; the field under-claims by design, so `false` means "not observed", never "the reap failed". Note this is deliberately **stricter** than `restart_phase` step 4, whose teardown is unwaited: step 4 goes on to delete the per-agent worktrees (step 4b, with salvage), so nothing remains for a lingering pod to corrupt. Here the worktrees are kept warm on purpose, which is exactly what makes the observed teardown load-bearing.

**Why adoption cannot be relied on instead of reaping.** `compute_dedupe_key` *is* deterministic across orchestrator restarts, and that is what lets live-Job reconciliation recognise an in-flight event after a restart — so a matching key is possible, and where it matches the spawner's `_event_dedupe_key_live` adoption would collapse the duplicate on its own. What breaks the guarantee is narrower than "the new process has no tracker" (it does — `startup_reconciliation` reconstructs one, and the classifier consults it): `spawn_all` registers a *fresh* tracker that supersedes the reconstructed one, so the identity the new loop derives comes from zeroed round state. Where the orphan was mid-round, its key differs, adoption cannot see it, and two pods run against one worktree. The orchestrator cannot tell the two cases apart from the outside — it has no view of the dead loop's session or round state — so it reaps unconditionally and accepts that a matching-key orphan is killed slightly early, costing one replayed round. A redundant delete is recoverable; a clobbered worktree is not.

**Case-4 suppression scope.** The case-4 anomaly check (`_slice_has_pending_decision`) returns `True` whenever the contract carries *any* unresolved `Decision`, regardless of which slice or phase it originated in — the contract's `Decision` schema does not carry a structured `slice_id` tag today, so the classifier conservatively treats any unresolved HITL as "potentially the reason this slice is BLOCKED" and skips the missing-HITL escalation. Practical consequence operators should know: a single unrelated pending HITL on the pipeline (e.g. a refine-phase question) silently suppresses every BLOCKED-slice case-4 escalation until that decision resolves. The trade-off favours not double-alerting an operator who is already engaged with a HITL over surfacing a real cross-slice mismatch; a future schema bump that adds slice scoping to `Decision` can tighten this without changing call sites. The classifier itself is extracted to a module-level helper `_classify_non_complete_slice` so tests can fake the gateway probe and consensus-tracker lookup without spinning up the run loop. The classifier's probe-failure default is "fresh, re-yield `READY`" (the safer direction for the scheduler), while the resolver's probe-failure default is "derived parent" (the safer direction for the next push) — the asymmetry is deliberate and each picks the direction least likely to break its own caller.

**Per-slice consensus tracker reconstruction at startup ([#2409](https://github.com/jwbron/egg/issues/2409)).** Documented in the [Startup reconciliation](#pipeline-state-persistence) section above. The signals route (`handle_consensus_confirmed_signal`) also no longer skips reconstruction when `slice_id` is supplied — the strict-equality filter in `reconstruct_tracker_from_messages` is the canonical scope mechanism, so slice-scoped reconstruction is safe at the CONFIRMED handler too.
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/agent-recovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ Restarts are allowed when the pipeline is in `RUNNING`, `AWAITING_HUMAN`, `FAILE
6. Per-agent worktrees and their local branches are **deleted** — unpushed commits are salvaged to `egg/recovered/*` refs on a best-effort basis first (see [Salvaging Unpushed Local Commits](#salvaging-unpushed-local-commits); worktrees with a corrupted `.git` marker may be skipped without salvage). Fresh worktrees then re-fork from `origin/<assigned_branch>` tip, so only commits pushed to the shared work branch survive into respawned agents' trees. For per-worktree retention, use `restart_agent` instead (#3080).
7. All agents for the phase are respawned from scratch

**Resume-after-orchestrator-restart vs. operator-driven phase restart.** The clear above runs when an operator (or the overseer/HITL ladder) calls `restart_phase` to start the phase over. The orthogonal case — an orchestrator-pod recycle mid-phase that should **resume** the in-flight slice DAG rather than start it over — is handled by **Layer-C bootstrap reconciliation** on the next orchestrator startup. Layer C iterates non-`COMPLETE` slices, observes integration-branch commit counts and consensus tracker presence, and applies a 5-way classification: re-yield as `READY` (no commits yet), mark already-spawned (commits but no tracker), mark `COMPLETE` (commits + consensus reached but unrecorded), preserve `BLOCKED` (and escalate to HITL if no pending decision), or escalate corrupt state to HITL. Cases 4 and 5 create an unresolved `Decision` on the contract — silent classification error is worse than an operator pause. See [`Slice/phase restart hardening`](../architecture/orchestrator.md#slicephase-restart-hardening-closes-2409).
**Resume-after-orchestrator-restart vs. operator-driven phase restart.** The clear above runs when an operator (or the overseer/HITL ladder) calls `restart_phase` to start the phase over. The orthogonal case — an orchestrator-pod recycle mid-phase that should **resume** the in-flight slice DAG rather than start it over — is handled by **Layer-C bootstrap reconciliation** on the next orchestrator startup. Layer C iterates non-`COMPLETE` slices, observes integration-branch commit counts and consensus tracker presence, and applies a 5-way classification: re-yield as `READY` (no commits yet), re-yield as `READY` (commits but no tracker; [#3685](https://github.com/jwbron/egg/issues/3685): re-driving the slice is what starts its BRC event loop, and the loop is the only thing that dispatches its work), mark `COMPLETE` (commits + consensus reached but unrecorded), preserve `BLOCKED` (and escalate to HITL if no pending decision), or escalate corrupt state to HITL. Both re-yield cases first reap any Job still live for the slice and wait (bounded) for its teardown to be observed — the reap runs above the classification switch, because commits-on-origin is orthogonal to whether an orphaned Job still holds the role worktree. Cases 4 and 5 create an unresolved `Decision` on the contract — silent classification error is worse than an operator pause. See [`Slice/phase restart hardening`](../architecture/orchestrator.md#slicephase-restart-hardening-closes-2409).

Startup reconciliation also rebuilds per-slice consensus trackers ([#2409](https://github.com/jwbron/egg/issues/2409) closure): for each slice in `contract.slices`, `reconstruct_tracker_from_messages(pipeline_id, graph, slice_id=<slice>)` rebuilds the nested `{pipeline_id}/{slice_id}` tracker from the message store, so a recycled orchestrator pod no longer loses in-flight slice consensus.

Expand Down
4 changes: 3 additions & 1 deletion orchestrator/routes/pipelines/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1280,11 +1280,13 @@ def stream_pipeline(pipeline_id: str) -> Response:
_teardown_phase_overseer,
)
from ._pod_liveness import ( # noqa: E402,F401
_REAP_TEARDOWN_WAIT_SECONDS,
_await_reaped_jobs_gone,
_count_live_pods_for_pipeline,
_get_spawner,
_guard_live_pods_or_force,
_live_event_agents,
_slice_agents_alive,
_reap_orphaned_slice_jobs,
)
from ._populate import ( # noqa: E402,F401
_FOREST_REASON_TO_OUTCOME,
Expand Down
Loading
Loading