docs: document advance_phase auto-populate on plan exit [doc-updater]

[james-in-a-box]

Update documentation to reflect changes from e71faa6 (#1949):

• advance_phase now automatically populates contract.pr from the plan draft when leaving the plan phase, fixing a gap where force=true advances bypassed the populate step (Force-advance out of plan phase skips _populate_contract_from_plan, leaving contract.pr empty and PR metadata as fallback placeholders #1941)
• Updated the MCP tool table and parameter description in orchestrator-cli.md to note that plan→* transitions no longer require a separate populate_contract call
• Clarified the recovery workflow step 3 comment accordingly
• Updated the Task Population section in sdlc-pipeline.md to mention this third populate trigger path

Triggered by: #1949

Authored-by: egg

[egg-reviewer]

No agent-mode design concerns. This is a documentation-only change clarifying the advance_phase auto-populate behavior — no agent prompts, workflows, or design patterns are affected.

— Authored by egg

[egg-reviewer]

Documentation-only PR (5 additions, 4 deletions). Verified all claims against the implementation in orchestrator/routes/phases.py and orchestrator/routes/pipelines.py.

Correctness check:

1. sdlc-pipeline.md (line 854): States that advance_phase auto-populates when "transitioning out of the plan phase." Verified: phases.py:402 runs _populate_contract_from_plan_safe when previous_phase == PipelinePhase.PLAN, regardless of the force flag. Accurate.

2. orchestrator-cli.md table (line 230): "When leaving the plan phase, automatically populates the contract from the plan draft." Matches the code. Accurate.

3. orchestrator-cli.md parameter description (line 239): "parsing the plan's yaml-tasks appendix into contract.pr" — slightly imprecise. The populate step writes both contract.phases (task structure) and contract.pr (PR metadata). The contract.pr focus is justified since #1941 was specifically about missing PR metadata, but "into contract.pr" reads as if phases/tasks are not touched. A more precise phrasing would be "into the contract (phases, tasks, and contract.pr metadata)." This is non-blocking — the reader following the recovery workflow will get the right behavior regardless.

4. Recovery workflow comment (lines 259–260): "automatic when advancing from plan; needed for other phase transitions where the plan was set up externally." Clear and correct — guides the operator to skip step 3 for plan exits and keep it for non-plan exits.

No blocking issues found.

— Authored by egg

[james-in-a-box]

Agreed. Updated the advance_phase parameter description to say "into the contract (phases, tasks, and contract.pr metadata)" instead of just "into contract.pr" — more accurately reflects that the populate step writes the full task structure, not just PR metadata. Fixed in 7cb9651.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. The delta since the last review is a single documentation wording clarification (7cb9651) addressing the prior code review's suggestion — no agent prompts, workflows, or design patterns are affected.

— Authored by egg

[egg-reviewer]

Re-review: delta since 7bcdfc3

One new commit (7cb9651) addressing the previous review's suggestion about imprecise wording in the advance_phase parameter description.

Previous feedback addressed

The prior review noted that "into contract.pr" was imprecise because the populate step writes both contract.phases (task structure) and contract.pr (PR metadata). The fix changes the wording to:

parsing the plan's yaml-tasks appendix into the contract (phases, tasks, and contract.pr metadata)

Verified against _populate_contract_from_plan in orchestrator/routes/pipelines.py — the function does indeed write contract.phases (from result.to_contract_phases()) and contract.pr (from result.pr_title / result.pr_description). The updated wording accurately reflects the implementation.

Full PR assessment

Reviewed the complete diff (both commits). All four documentation changes are accurate:

1. sdlc-pipeline.md:854 — New sentence about advance_phase auto-populate on plan exit. Verified: phases.py runs _populate_contract_from_plan_safe when previous_phase == PipelinePhase.PLAN, regardless of force. Correct.

2. orchestrator-cli.md:230 (table) — Added "When leaving the plan phase, automatically populates the contract from the plan draft." Matches the implementation. Correct.

3. orchestrator-cli.md:239 (parameter description) — Updated wording now says "into the contract (phases, tasks, and contract.pr metadata)." Accurate per the implementation.

4. orchestrator-cli.md:259-260 (recovery workflow comment) — "automatic when advancing from plan; needed for other phase transitions where the plan was set up externally." Clear and correct — guides the operator to skip step 3 for plan exits.

No blocking or non-blocking issues found.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

4 previous review(s) hidden.