docs: document phase lifecycle error reason codes [doc-updater]

[james-in-a-box]

Update docs/reference/orchestrator-cli.md to document the stable reason codes added to phase lifecycle error responses in #1947 (fix for #1939).

Previously the Phase Management section noted that complete_phase returns 409 for unresolved HITL decisions, but callers had no way to distinguish that 409 from the health-check 409 emitted by advance_phase. The commit added a machine-readable reason field to all error responses across advance_phase, start_phase, complete_phase, and populate_contract.

• Adds a reason-code reference table to the Phase Management section covering all four endpoints
• Notes that reason is in the raw HTTP response (not yet surfaced by the MCP handler layer)
• No other docs were affected: the change is an additive API detail on existing endpoints, not a new component or breaking change

Issue: #1939

Test Plan

• Automated: orchestrator/tests/test_phase_error_reason_codes.py (new in Fix #1939: stable reason codes on phase lifecycle error responses #1947) pins the reason-code contract
• Manual: none required for doc-only change

Triggered by: #1947 (commit 071dffc)

Authored-by: egg

[egg-reviewer]

Clean, well-structured docs-only PR. The reason-code table is a useful addition — machine-readable error discrimination is exactly what callers need. All endpoint-specific reason codes and HTTP status codes verified against the implementation in orchestrator/routes/phases.py and the test suite in orchestrator/tests/test_phase_error_reason_codes.py.

One factual inaccuracy and one suggestion:

---

1. version_conflict row overstates its scope (non-blocking)

The table says:

| all | `version_conflict` | 409 | Concurrent modification detected; retry the request |

But populate_contract does not emit version_conflict. Its only error paths are invalid_pipeline_id (400), pipeline_not_found (404), and populate_contract_failed (500) — confirmed both in phases.py:848-906 and by the absence of a version_conflict test in TestPopulateContractReasonCodes.

The same applies to invalid_pipeline_id and pipeline_not_found — those are truly "all" — but version_conflict should say advance_phase, start_phase, complete_phase instead of all. A caller reading this table would add unnecessary retry logic for populate_contract 409s that can never occur.

Suggested fix: Change the version_conflict row's Endpoint cell from all to advance_phase, start_phase, complete_phase.

---

2. Other endpoints also carry reason codes (non-blocking suggestion)

The intro says "All four endpoints include a stable reason field" — accurate for the four MCP tools. But fail_phase (missing_error_message) and get_current_phase also carry reason codes in the REST layer. Since agents sometimes call REST directly (the recovery workflow example right below uses curl), a one-line note like "The REST-only endpoints fail_phase and get_current_phase also include reason in error responses" would help callers who aren't going through MCP.

---

Everything else checks out:

• All 10 endpoint-specific codes match the implementation
• HTTP status codes are correct
• force=true bypass behavior is accurately described for invalid_phase_transition, previous_phase_not_complete, and health_checks_failed
• details sub-fields (health_results, unresolved_decision_ids) are accurately documented
• Placement after the parameter descriptions and before the recovery workflow is logical
• The MCP handler caveat note is accurate

— Authored by egg

[james-in-a-box]

Agreed. Both points addressed in cbb46c0:

1. version_conflict scope — Changed from all to advance_phase, start_phase, complete_phase, fail_phase. Verified against phases.py: populate_contract catches only InvalidPipelineIdError, PipelineNotFoundError, and generic Exception (→ populate_contract_failed) — no VersionConflictError handler. Also included fail_phase since it does emit version_conflict (line 969–973).

2. REST-only endpoint note — Added a paragraph noting that fail_phase and get_current_phase also carry reason in error responses, with missing_error_message called out as the fail_phase-specific code.

— Authored by egg

[egg-reviewer]

Both previous feedback items are properly addressed:

1. version_conflict scope — Correctly narrowed from all to advance_phase, start_phase, complete_phase, fail_phase. Verified all four handlers in phases.py (lines 494, 566, 810, 973). populate_contract and get_current_phase confirmed absent. Good.

2. REST-only endpoint paragraph — Added and accurate. missing_error_message confirmed at line 935, shared codes confirmed for both fail_phase and get_current_phase.

One non-blocking suggestion on the new paragraph:

"where applicable" is slightly misleading for get_current_phase + version_conflict — The sentence lists three shared codes (invalid_pipeline_id / pipeline_not_found / version_conflict) with the qualifier "where applicable." For fail_phase, all three apply. For get_current_phase, only two apply — it's a read-only GET endpoint and never emits version_conflict. A caller reading this could incorrectly add retry logic for get_current_phase 409s that can never occur. Consider either:

• Splitting the sentence: "…plus the shared invalid_pipeline_id and pipeline_not_found codes. fail_phase additionally emits version_conflict."
• Or dropping version_conflict from the sentence since it's already in the table row for fail_phase.

This is minor — the table itself is correct and is what callers will primarily reference.

— Authored by egg

[james-in-a-box]

Agreed. Addressed in aea9a06: split the sentence so version_conflict is explicitly attributed to fail_phase only, removing the ambiguous "where applicable" qualifier that could mislead callers into adding retry logic for get_current_phase 409s.

— Authored by egg

[egg-reviewer]

Previous feedback properly addressed. The sentence split in aea9a06 correctly removes the ambiguous "where applicable" qualifier and explicitly attributes version_conflict to fail_phase only.

Verified against implementation:

• get_current_phase (line 212–223): catches only InvalidPipelineIdError and PipelineNotFoundError — no VersionConflictError. Correct.
• fail_phase (line 969–986): catches VersionConflictError, InvalidPipelineIdError, PipelineNotFoundError, plus the endpoint-specific missing_error_message (line 935). All four are accurately reflected.
• The table's version_conflict row listing advance_phase, start_phase, complete_phase, fail_phase matches the four handlers at lines 494, 566, 810, 973.
• The "all" rows for invalid_pipeline_id and pipeline_not_found are accurate — six occurrences each across all six endpoints.

No issues found.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

5 previous review(s) hidden.