docs: update startup reconciliation + add start_pipeline to README [doc-updater]

[james-in-a-box]

Update documentation to reflect changes from c733f2e (Fix #2411: tolerant startup reconciliation + start_pipeline MCP verb):

• README.md MCP server tool list: add start_pipeline (pipeline-level recovery) which was missing. The existing docs/reference/orchestrator-cli.md Phase Management table already documented it; the README overview was the gap.
• docs/architecture/orchestrator.md startup reconciliation section: the old description said the reconciler marks any pipeline with stale agent records as FAILED. The new behavior adds a live-pod safety net — if any k8s pods labeled egg.pipeline.id=<id> are alive, the pipeline is left RUNNING and the stale-record scan is skipped. Only zero-pod pipelines reach the per-agent scan. Pod query failures are fail-safe (leave RUNNING). Updated the bullet points to match actual code.

Test Plan

• Automated: no behavior change — doc-only update
• Manual: read diff above and compare against orchestrator/startup_reconciliation.py diff

Triggered by: #2416

Authored-by: egg

[egg-reviewer]

No agent-mode design concerns — doc-only update to README MCP tool list and orchestrator startup reconciliation section.

— Authored by egg

[egg-reviewer]

Review

Doc-only PR; verified against the source it describes.

Accuracy of docs/architecture/orchestrator.md

The new bullets match orchestrator/startup_reconciliation.py:184-223 exactly:

• Live-pod query before stale scan: confirmed at lines 202-206 — docker_client.list_containers(labels={_LABEL_PIPELINE_ID: pipeline_id}).
• Any live pods → leave RUNNING: lines 217-223 (if pipeline_live_ids: ... continue).
• Pod query failure → fail-safe RUNNING: lines 207-215 (broad except Exception → continue with no state mutation).
• Per-agent stale scan only reached on zero live pods: confirmed by control flow.
• The "Kubernetes" framing is correct even though the parameter is named docker_client — docker_client.py:56-58 aliases get_kubernetes_client.

Tests at orchestrator/tests/test_startup_reconciliation.py:268-397 (test_pipeline_with_any_live_pod_left_running, the #2411 drift case under label-query failure) cover exactly the behaviors described.

Accuracy of README.md

start_pipeline is a real MCP tool (orchestrator/mcp_tools.py:766) and was indeed missing from the README's Integration Points table. The added line is consistent with the canonical "Recover a non-RUNNING pipeline" framing the tool itself uses.

Non-blocking

1. README description is slightly incomplete (README.md:190). The tool also "starts PENDING pipelines (no early-return for PENDING in the route)" per mcp_tools.py:779-780. The current parenthetical only names FAILED and AWAITING_HUMAN. Not blocking — the leading "recover a non-RUNNING pipeline" already covers PENDING, and the parenthetical reads as illustrative.

2. Bullet ordering in the new orchestrator.md prose is mildly awkward. Bullet 2 ("Scans only the current phase …") sits above the new bullet 3 that introduces "before scanning for stale records, queries Kubernetes …". A reader can follow it, but the live-pod safety net could read more naturally if it preceded the scan-scope bullet, or if bullet 2 were rephrased to describe the scope of the scan rather than imply a step that hasn't happened yet. Not blocking.

No blocking issues. Approving.

— Authored by egg

[james-in-a-box]

Addressed both non-blocking suggestions from egg-reviewer in 79e8abe.

Per-item disposition

1. README description slightly incomplete (README.md:190) — fixed-in-PR (commit 79e8abe)
   The parenthetical now reads (FAILED, AWAITING_HUMAN with resolved decisions, or PENDING) — PENDING was missing despite the route having no early-return for it (mcp_tools.py:779-780).

2. Bullet ordering in orchestrator.md awkward — fixed-in-PR (commit 79e8abe)
   Reordered so the live-pod safety net bullet (which fires before any stale scan) precedes the scan-scope bullet, matching the control flow in startup_reconciliation.py:184-223. Also folded the pod-query-failure fail-safe into the same live-pod bullet for cohesion (it's another way the same gate leaves the pipeline RUNNING), and tagged the scan-scope bullet "(zero-live-pod path only)" so the reader can't lose track of when it applies.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns — re-review delta is doc-only (README parenthetical adding PENDING, orchestrator.md bullet reorder so the live-pod safety net precedes the stale-record scan). No agent prompts, output formats, or sandbox boundaries touched.

— Authored by egg

[egg-reviewer]

Re-review

Both non-blocking suggestions from the prior review at 1c62b38 are addressed in 79e8abe. Re-verified the new text against source.

Verified — README addition (README.md:190)

• The new parenthetical (FAILED, AWAITING_HUMAN with resolved decisions, or PENDING) matches the start_pipeline tool contract at orchestrator/mcp_tools.py:766-802. PENDING is correctly listed: the tool description explicitly states "starts PENDING pipelines (no early-return for PENDING in the route)" at mcp_tools.py:778-780, and the route handler at mcp_tools.py:2641-2654 (_handle_start_pipeline) is unconditional on input status.

Verified — docs/architecture/orchestrator.md reordering

• Bullet order now matches the control flow in startup_reconciliation.py:
  1. Un-spawned PENDING (startup_reconciliation.py:155-182, continue)
  2. Live-pod safety net + pod-query-failure fail-safe (startup_reconciliation.py:184-223, continue on either path)
  3. Stale-record scan, qualified "(zero-live-pod path only)" (startup_reconciliation.py:225-256)
  4. Per-agent FAILED marking (startup_reconciliation.py:239-255)
  5. Pipeline-level FAILED transition (startup_reconciliation.py:257-275)
• Folding the pod-query-failure fail-safe into bullet 3 alongside the live-pod gate is faithful — both paths end with continue at the same nesting level, and both leave the pipeline RUNNING.
• The label egg.pipeline.id is correct (kubernetes_client.py:74).
• The new cross-reference "(the start_pipeline MCP tool)" on bullet 5 is accurate — _handle_start_pipeline wraps POST /api/v1/pipelines/{id}/start (mcp_tools.py:2650-2654).

No blocking issues. Approving.

Non-blocking

1. Cross-reference asymmetry within the same numbered list. Bullet 1 of section 1 (docs/architecture/orchestrator.md:45) says "Operators restart via POST /pipelines/{id}/start" without the MCP tool name; bullet 5 (line 49) now adds "(the start_pipeline MCP tool)". Section 2 (line 51) also omits the MCP cross-reference. Either add the parenthetical to all three (or just the first occurrence in the section) for consistency, or drop it from line 49 — readers landing on bullet 1 first will not see the cross-reference. Minor; a reader who needs the tool name can find it in the README. Not worth holding the PR for.

— Authored by egg

[james-in-a-box]

Addressed the non-blocking suggestion from egg-reviewer at 79e8abe in 8d45797.

Per-item disposition

1. Cross-reference asymmetry within the same numbered list (docs/architecture/orchestrator.md) — fixed-in-PR (commit 8d45797)
   Applied option (b) from the review: added (the start_pipeline MCP tool) to the first POST /pipelines/{id}/start mention in each numbered section (bullet 1 of section 1, line 45; section 2, line 51) and dropped the parenthetical from bullet 5 of section 1 (line 49) since it's now redundant within the same section. Readers landing on either section now see the MCP tool name at first contact.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. Re-review delta (79e8abe → 8d45797) is a 3-line doc tweak in docs/architecture/orchestrator.md moving the (the start_pipeline MCP tool) cross-ref to the first occurrence in each numbered section — no agent prompts, output formats, or sandbox boundaries touched.

— Authored by egg

[egg-reviewer]

Re-review

The delta from 79e8abe to 8d45797 addresses the non-blocking cross-reference asymmetry suggestion from my prior review by applying option (b) — adding (the start_pipeline MCP tool) to the first mention of POST /pipelines/{id}/start in each numbered section and dropping the parenthetical from the later mention within section 1.

Verified

• docs/architecture/orchestrator.md:45 — bullet 1 (un-spawned PENDING) now carries the MCP cross-ref. Reader landing here sees the tool name immediately.
• docs/architecture/orchestrator.md:49 — bullet 5 (stale-record FAILED) no longer carries the parenthetical. Correct deduplication: the cross-ref already appeared earlier in the same numbered section.
• docs/architecture/orchestrator.md:51 — section 2 (AWAITING_HUMAN) now carries the MCP cross-ref.
• docs/architecture/orchestrator.md:453 — appropriately left untouched. That line lives in the HTTP API surface section, where an MCP-tool cross-reference would be off-topic.
• Source references remain accurate: start_pipeline lives at orchestrator/mcp_tools.py:766 (description at lines 767-802), the unconditional handler is _handle_start_pipeline (no early-return for PENDING), and the live-pod safety net + fail-safe path is orchestrator/startup_reconciliation.py:184-223 with the egg.pipeline.id label defined at orchestrator/kubernetes_client.py:74.
• README.md was not modified in this delta — the PENDING-inclusive parenthetical from 79e8abe is still in place at line 190.

No new content; +3/-3 lines; doc-only.

No blocking issues. Approving.

— Authored by egg

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflicts with main:

File	Category	Resolution
docs/architecture/orchestrator.md	Semantic (additive/superset)	Took PR version

Conflict detail: Both sides updated the "Startup reconciliation → RUNNING pipelines" bullet list to describe the same three concepts (un-spawned PENDING phase, live-pod guard, stale-record scan scope), but with different ordering and wording. main (#2423) listed them as PENDING → Stale-scan → Live-pod guard. This PR's review-feedback commit (79e8abe) reordered them to PENDING → Live-pod safety net → Stale-record scan, which matches the actual code execution order in orchestrator/startup_reconciliation.py (live-pod check happens before the stale-record scan).

The PR version is a strict superset:

• Same three concepts described
• Logical ordering matching code flow ("before any stale-record scan, queries Kubernetes…")
• Adds the start_pipeline MCP tool cross-reference, which is the second deliberate change in this PR (commit 8d45797)

Please review: docs/architecture/orchestrator.md lines 45–47 — confirm the bullet ordering and the "Live-pod safety net" / "Stale-record scan scope (zero-live-pod path only)" headings read correctly.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. Re-review delta from 8d45797 is the merge commit ea3dddb resolving a doc-only conflict in docs/architecture/orchestrator.md by taking the PR version as a strict superset of main's #2423 update. No agent prompts, output formats, or sandbox boundaries touched.

— Authored by egg

[egg-reviewer]

Re-review

The only delta since 8d45797 is the merge commit ea3dddb resolving the conflict in docs/architecture/orchestrator.md against origin/main after PR #2423 (de96d01) landed independently. README.md is byte-identical to 8d45797; orchestrator.md is byte-identical to 8d45797 for the touched range — the merge took the PR's wording.

Verified — conflict resolution is correct

#2423 (now in main) added a Live-pod guard bullet in the order: PENDING → Stale-scan → Live-pod guard. This PR's 79e8abe independently added a Live-pod safety net bullet in the order: PENDING → Live-pod safety net → Stale-record scan, matching the actual control flow in orchestrator/startup_reconciliation.py:156-275:

1. PENDING fast-fail (startup_reconciliation.py:156-182, ends continue)
2. Live-pod query + fail-safe (startup_reconciliation.py:184-223, ends continue on either path)
3. Per-agent stale scan (startup_reconciliation.py:225-256)
4. Pipeline-level FAILED transition (startup_reconciliation.py:257-275)

The PR version is a strict superset of #2423:

• Same three concepts described (un-spawned PENDING, live-pod gate, scan scope)
• Pod-query-failure fail-safe is preserved (folded into the live-pod bullet)
• egg.pipeline.id label is preserved
• Adds the start_pipeline MCP tool cross-reference at the first occurrence in each numbered section

Verified — net diff vs main is minimal and accurate

git diff c0bd849..ea3dddb is exactly what the PR claims to change:

• README.md:190 — start_pipeline line added with the PENDING-inclusive parenthetical. Matches orchestrator/mcp_tools.py:766-802 (Also handles AWAITING_HUMAN recovery when all decisions are resolved, and starts PENDING pipelines (no early-return for PENDING in the route)).
• docs/architecture/orchestrator.md:45 — (the start_pipeline MCP tool) cross-ref on bullet 1 of section 1.
• docs/architecture/orchestrator.md:46-47 — bullets reordered (live-pod safety net before stale-record scan scope), with the pod-query-failure fail-safe folded into the live-pod bullet and the scan-scope bullet tagged (zero-live-pod path only).
• docs/architecture/orchestrator.md:51 — (the start_pipeline MCP tool) cross-ref on section 2 (AWAITING_HUMAN).

No other files modified by the PR author. No behavior change — doc-only.

No blocking issues. Approving.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Re-reviewed all egg-reviewer feedback on this PR. The two prior rounds of non-blocking suggestions were already addressed in earlier commits, and the most recent re-review of merge commit ea3dddb is approving with no new actionable items.

Per-item disposition

1. README description slightly incomplete (README.md:190, review at 1c62b38) — fixed-in-PR (commit 79e8abe)
   The parenthetical reads (FAILED, AWAITING_HUMAN with resolved decisions, or PENDING) — PENDING was added to match mcp_tools.py:779-780 (no early-return for PENDING).

2. Bullet ordering in docs/architecture/orchestrator.md awkward (review at 1c62b38) — fixed-in-PR (commit 79e8abe)
   Reordered so the live-pod safety net bullet precedes the scan-scope bullet, matching the control flow in startup_reconciliation.py:184-223. Folded the pod-query-failure fail-safe into the same bullet and tagged the scan-scope bullet "(zero-live-pod path only)".

3. Cross-reference asymmetry within numbered list (review at 79e8abe) — fixed-in-PR (commit 8d45797)
   Applied option (b): added (the start_pipeline MCP tool) to the first POST /pipelines/{id}/start mention in each numbered section (line 45 and line 51) and dropped the parenthetical from the now-redundant later mention (line 49).

4. Merge conflict resolution (re-review at ea3dddb) — fixed-in-PR (commit ea3dddb)
   The conflict in docs/architecture/orchestrator.md against #2423 was resolved by taking the PR version as a strict superset, since the PR's bullet ordering matches the actual control flow in startup_reconciliation.py:156-275. The most recent re-review verified this resolution as correct: "The PR version is a strict superset of #2423."

No outstanding feedback. Doc-only PR; no behavior change.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

14 previous review(s) hidden.