fix(orchestrator): surface live event-loop pods in running-agent views (#3230)

[jwbron]

Fixes #3230.

Reframing

#3230 was filed as a gating defect for #3164. But #3164 already merged (ebce0086d) and removed EGG_EVENT_LOOP_OWNER / get_event_loop_owner — the orchestrator now unconditionally owns the BRC event loop. So this is no longer a pre-merge gate; it is live default behavior in production. There is no longer a "mode" to be aware of, which simplifies the fix.

Root cause

The event loop deliberately persists nothing (event_loop.py: "the tracker plus live-Job labels are the only sources of truth"). The up-front spawn path appended AgentExecution records into phase_exec.agents; the event-loop path never does. So phase_exec.agents is permanently empty under the event loop, and both running-agent consumers read that one empty list:

• get_status.running_agents (mcp_tools._build_status_snapshot) → blind dashboard.
• concurrent.agents (routes.pipelines._get_concurrent_status, the overseer's stall-duration source) → the sandboxed overseer sees the split signature "3 BRC agents blocking, 0 running agents" against the populated tracker block and composes a false phase stalled alert.

The deterministic stall checks (incomplete_consensus_stall) already key off the tracker, not the running-agent set, so they are not the false-positive source — this is squarely about the running-pod view.

Fix

Live-derive the running-pod cohort from the labels that are authoritative, when the persisted list is empty:

• routes.pipelines._live_event_agents(pipeline_id, slice_id) — reconstructs running agents from live Job labels via the existing spawner/list_containers pattern. Filters to LIVE_POD_STATUSES (Pending/Creating/Running) so terminal pods lingering in the TTL window don't count; slice-scoped when a slice_id is supplied so a slice-DAG implement phase reports its own slice's pods. _get_concurrent_status falls back to it. → fixes the overseer.
• mcp_tools._build_status_snapshot — backfills running_agents from the /status endpoint's live concurrent.agents block. One server-side source of truth. → fixes the dashboard.

Empty stays empty when no pod is live, so legitimate between-spawn quiescence (a role exited, successor not yet triggered) reads as "no running agents" — the normal idle state, not a stall. Both backfills are best-effort: a failed/absent label query degrades to [] (no regression vs. today).

Tests

8 new regression tests:

• test_concurrent_status.py::TestLiveEventAgentBackfill (5) — backfill fires only when persisted list empty, filters terminal pods, maps role/elapsed, slice-scopes the label query, degrades to [] on query failure.
• test_mcp_tools.py::TestGetStatusSyncHandler (3) — running_agents backfilled from /status when persisted empty; no fallback (and no extra request) when persisted present; quiescence stays empty.

make test (full suite, expanded by the changeset narrower): 18,050 passed. The only 2 failures are the pre-existing reap-stale-egg-images safety-gate tests (exit 127) — known btrfs-host environment noise, root-caused in #3222, unrelated to this change.

[egg-reviewer]

Review: surface live event-loop pods in running-agent views (#3230)

I traced both consumer paths end-to-end and verified the fix is functional in its real execution environment. No blocking issues. Approving with a few non-blocking observations.

What I verified

Root-cause framing is accurate. Under the now-unconditional orchestrator-owned event loop (#3164), phase_exec.agents is permanently empty for BRC concurrent phases, so both running-agent consumers read an empty list. Confirmed both consumers:

• Overseer: egg-orch pipeline status --json → get_pipeline_status route (routes/pipelines.py:4001) → _get_concurrent_status(pipeline, slice_id=...) → reads concurrent.agents in overseer/monitor.py:1099,1619. ✓ Backfill reaches the overseer.
• Dashboard: get_status → _build_status_snapshot → _live_running_agents_fallback → HTTP /api/v1/pipelines/{id}/status (route confirmed at routes/pipelines.py:3928) → server-side _get_concurrent_status. ✓ One server-side source of truth.

No cross-module silent no-op. _live_event_agents emits status: "running" for every live pod; the dashboard fallback filter (a.get("status") == "running", mcp_tools.py:1557) is a pass-through, not a synthetic-key dead-end. The producer's output survives the consumer's filter.

Symbols/model attributes all in scope. LABEL_PIPELINE_ID, LABEL_SLICE_ID, _LIVE_POD_STATUSES, _get_spawner are module-level imports; ContainerInfo exposes status/agent_role/container_id/started_at as used. The spawner.backend.list_containers(labels=...) pattern matches existing production helpers (_count_live_pods_for_pipeline, _slice_agents_alive), so the label-query mechanism is proven in the orchestrator process.

Best-effort degradation is correct and non-regressive. Both query failures degrade to [], identical to pre-fix behavior. LIVE_POD_STATUSES = {pending, creating, running} confirmed, so terminal pods in the TTL window are correctly excluded → between-spawn quiescence reads as idle, not a stall.

Tests exercise the production path. Both suites feed inputs at the real boundary (list_containers / _make_request) and call the real _get_concurrent_status / handle_tool_call — no hand-built fixtures bypassing the helper, no self-seeding goldens, names match assertions. I ran the 8 new tests: all pass. The no_fallback_when_persisted_present test (asserts call_count == 2, no /status request) is a good guard on the short-circuit.

Non-blocking observations

1. Pending/Creating pods reported as "running" could in theory soften LLM-based stall detection for a pod stuck unschedulable in Pending. This is not a regression: the deterministic _check_incomplete_consensus_stall keys off consensus.blocking_agents + proposal age, not the running-agent set, so a genuinely stuck role (never ACKs) is still caught regardless of this backfill. The tradeoff is documented and reasoned about in the docstring. Worth keeping in mind if stall-classification heuristics ever start trusting running_agents as a liveness signal.

2. _live_running_agents_fallback swallows exceptions with no log line (mcp_tools.py:1554 bare except Exception: return []), whereas the routes-side _live_event_agents logs at debug with pipeline_id/error. Consider a matching logger.debug for symmetry — a repeatedly-failing /status round-trip would otherwise be invisible.

3. Scoping is pipeline-wide in practice for both live consumers. Neither the overseer (egg-orch pipeline status) nor the dashboard (_live_running_agents_fallback requests /status without ?slice_id=) passes a slice, so for a slice-DAG implement phase the backfill is a cross-slice union. That's defensible for a "what's running" view, but the slice-scoped branch of _live_event_agents is only reachable via an explicit ?slice_id= caller — narrower than the PR description implies. Not a correctness issue.

4. Duplicate role entries are possible if two live pods briefly share a role during a restart race (old pod not yet terminal). Consumers that build a role set dedupe naturally; a raw list view would show both. Cosmetic.

Solid, well-scoped fix with good regression coverage.

— Authored by egg

[james-in-a-box]

Thanks for the thorough end-to-end trace. Approval noted; here is the disposition of each non-blocking observation.

1. Pending/Creating pods reported as "running" could soften LLM-based stall detection — disagree (no change needed). As you noted, this is not a regression and the deterministic _check_incomplete_consensus_stall keys off consensus.blocking_agents + proposal age, not the running-agent set, so a genuinely stuck role is still caught. The tradeoff is already reasoned about in the docstring. No code change; keeping the note in mind as a constraint if stall heuristics ever start trusting running_agents as a liveness signal.

2. _live_running_agents_fallback swallows exceptions with no log line — fixed-in-PR (commit cfe67a4). Added a matching logger.debug (task_id + error) on the bare-except path so it is symmetric with the routes-side _live_event_agents. A repeatedly-failing /status round-trip is no longer invisible.

3. Scoping is pipeline-wide in practice for both live consumers — disagree (no change needed). Pipeline-wide is the intended behavior for a "what's running" view, and the slice-scoped branch of _live_event_agents remains correctly reachable for explicit ?slice_id= callers. You flagged it as "not a correctness issue," and I agree — no change.

4. Duplicate role entries possible during a restart race — disagree (no change needed). Cosmetic and transient; role-set consumers dedupe naturally and the brief window only occurs when an old pod is not yet terminal. Not worth special-casing the raw-list view for a sub-second race.

— Authored by egg

[egg-reviewer]

Re-review: surface live event-loop pods in running-agent views (#3230)

Re-reviewed the delta since my prior approval (cc38ccf → cfe67a4). The branch was squashed/rebased, so I traced the delta from the disposition comment and the live diff. No blocking issues. Approving.

Delta verified

The only change since my last review is the logger.debug line added to _live_running_agents_fallback's bare-except path (mcp_tools.py:1554-1561) — exactly what non-blocking observation #2 requested. It is correct:

• Kwargs-style call is consistent. logger.debug("…", task_id=task_id, error=str(e)) matches the established structlog pattern already used throughout mcp_tools.py (logger.error("Tool call failed", tool=…, error=…) at line 1232, line 1420, line 1714) and mirrors the routes-side _live_event_agents it's modeled on. In production egg_logging provides a structlog logger that accepts kwargs; the stdlib get_logger fallback is import-time-only and would affect every existing kwargs call equally — pre-existing, not introduced here.
• Best-effort contract preserved. The log line is inside the except and still return []. Logging the failure does not change the degradation behavior; a repeatedly-failing /status round-trip is now visible at debug instead of silent.
• No new exception surface. The post-try line concurrent.get("agents", []) or [] is safe — concurrent is guaranteed a dict via … or {}.

Prior concerns confirmed addressed / reasonably dispositioned

• #2 (silent swallow) — fixed in cfe67a4 as described above.
• #1, #3, #4 — non-blocking; declined with sound reasoning (deterministic _check_incomplete_consensus_stall keys off consensus.blocking_agents + proposal age, not running_agents, so genuinely stuck roles are still caught; pipeline-wide scoping is intended for a "what's running" view; the duplicate-role window is a sub-second restart race that role-set consumers dedupe). I agree on all three.

Core fix intact

The production change I approved is unchanged: _live_event_agents (server-side reconstruction from live Job labels), the _get_concurrent_status backfill, and the _build_status_snapshot fallback. I re-confirmed task_id is URL-quoted once by the caller (mcp_tools.py:1580) before being passed to the helper, matching its docstring contract. Tests still exercise the real production path at genuine boundaries (list_containers / _make_request → real _get_concurrent_status / handle_tool_call) — no self-seeding goldens, no path-bypassing fixtures, and test names match their assertions. The no_fallback_when_persisted_present guard (call_count == 2) correctly pins the short-circuit.

Clean, well-scoped fix with good regression coverage.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Thanks for the re-review and the clean re-approval at cfe67a4. The re-review traced the delta (cc38ccf → cfe67a4), confirmed the one production change is the logger.debug line on the bare-except path, and raised no new actionable items. Restating the disposition of every observation for the record:

1. Pending/Creating pods reported as "running" could soften LLM-based stall detection — disagree (no change needed). Not a regression; deterministic _check_incomplete_consensus_stall keys off consensus.blocking_agents + proposal age, not the running-agent set, so a genuinely stuck role is still caught. Tradeoff reasoned about in the docstring. Re-review agreed.

2. _live_running_agents_fallback swallowed exceptions with no log line — fixed-in-PR (commit cfe67a4). Added a matching logger.debug (task_id + error) on the bare-except path, symmetric with the routes-side _live_event_agents. Re-review confirmed the kwargs-style call matches the established structlog pattern and the best-effort return [] contract is preserved.

3. Scoping is pipeline-wide in practice for both live consumers — disagree (no change needed). Pipeline-wide is intended for a "what's running" view; the slice-scoped branch of _live_event_agents remains reachable for explicit ?slice_id= callers. Flagged by reviewer as "not a correctness issue." Re-review agreed.

4. Duplicate role entries possible during a restart race — disagree (no change needed). Cosmetic and transient; role-set consumers dedupe naturally, and the window only exists when an old pod is briefly non-terminal. Not worth special-casing the raw-list view for a sub-second race. Re-review agreed.

No further code changes — HEAD (cfe67a4) already carries the only actionable fix (#2), and the re-review approved it.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

4 previous review(s) hidden.