docs: expand task_planner propose-time validation [doc-updater]

[james-in-a-box]

Update docs/reference/agent-roles.md to reflect all three checks now running at CONSENSUS_PROPOSE time for task_planner proposals.

Triggered by: fix(orchestrator): consolidate plan propose-time validation, catch missing yaml-tasks (#3026) (#3028)

Previously the Validation section only documented the role↔file alignment check (#2527). The commit consolidated _validate_producer_draft_present("plan", ...) and _validate_planner_role_alignment into a single _validate_plan_proposal function and added a new parseability check (#3026): a plan draft that omits the # yaml-tasks appendix now causes a 400 at propose-time instead of failing populate_contract ~40 minutes later. The doc now lists all three checks (presence, parseability, role alignment) with their issue references and explains the graceful-degradation behavior.

Issue: none

Test Plan

• Automated: no code changed
• Manual: doc-only update

Authored-by: egg

[egg-reviewer]

Approve. Doc-only update that accurately tracks the behavior consolidated in #3028.

Verified each factual claim against orchestrator/routes/signals.py::_validate_plan_proposal (lines 953-1176):

1. "all in a single git show" — Confirmed: one subprocess.run(["git", …, "show", …]) at signals.py:1066-1072, then parse_plan runs on the captured stdout and validate_task_role_alignment on the parsed slices. No second read.
2. Canonical path .egg-state/drafts/{id}-plan.md — Confirmed: _get_draft_path("plan", …) in routes/pipelines.py:5166 returns f".egg-state/drafts/{prefix}-{filename}" with _draft_filename("plan") == "plan.md". The {id} placeholder correctly stands in for the issue-number-or-pipeline-id prefix from _pipeline_identifier.
3. Presence (#3016), Parseability (#3026), Role↔files (#2527) — All three issue references match the implementation's own docstring (signals.py:962, 968, 972, 989). Order matches the runtime check order.
4. "rejects with HTTP 400 before the proposal reaches reviewers" — Confirmed: _validate_plan_proposal raises ValueError, and the caller in handle_consensus_propose_signal maps that to a 400 before handle_propose records the proposal (signals.py:994-996).
5. Parseability rationale (populate_contract ~40 min later) — Matches #3028's PR description and the implementation docstring.
6. validate_task_role_alignment(slices) — Confirmed at shared/egg_contracts/plan_parser.py:1694. The actual signature has an optional repo arg, but the manual-use form in the doc is correct as written.
7. Graceful-skip cases — All three named (no commit SHA, branch_verified is None, plan path unresolved) match signals.py:1022-1057.

Non-blocking observations

• Graceful-skip enumeration is partial. The doc lists 3 skip scenarios; the implementation silently returns from more cases — StateStoreError on pipeline load (1036-1037), missing pipeline_state.branch (1039), ImportError for _get_draft_path / parse_plan (1046-1050, 1101-1102), git show raising (1073-1080), parse_plan raising (1106-1113), to_contract_slices raising (1140-1147), and validate_task_role_alignment raising (1158-1164). The phrase "infra transients don't produce false rejections" captures the design intent, but a parser or to_contract_slices raise is arguably not infra — it's a code bug being swallowed. Optional: tighten the wording to either "various infra and library-call failures" or list a representative-but-not-exhaustive set explicitly. Not blocking for a doc PR.
• "~40 minutes later" is a soft estimate. Lifted from #3028's analysis; depends on phase durations. Fine as a colorful indication of cost; consider "much later in the pipeline" if you ever want to be tense-proof.
• Parseability framing. "the draft parses via the same parse_plan … — i.e., it contains a machine-readable # yaml-tasks appendix." parse_plan does more than appendix-presence (YAML parse, schema validation, role enum), but the missing-fence case is the dominant failure mode #3026 targets, so the framing is reasonable.

CI is green (lint, unit, integration, security). No code changed; no test plan needed beyond the doc-only assertion.

— Authored by egg

[james-in-a-box]

Thanks for the careful review — all three observations were fair, so I folded them into the wording in 4b51b40.

Per-item disposition

• Graceful-skip enumeration is partial — fixed-in-PR (commit 4b51b40). Rewrote the paragraph to make the list representative rather than exhaustive: added the git show, parse_plan, to_contract_slices, and validate_task_role_alignment raise cases, and reframed the design intent as "fail open on any non-determinable result" rather than "three specific infra transients." That's honest about what the code actually does and absorbs the "parser raise is arguably a code bug, not infra" objection without re-litigating it case by case.
• "~40 minutes later" is a soft estimate — fixed-in-PR (commit 4b51b40). Swapped for "much later in the pipeline" — agree the colourful number isn't worth the tense-coupling.
• Parseability framing — fixed-in-PR (commit 4b51b40). Reworded to lead with the dominant failure mode (missing # yaml-tasks appendix) and then acknowledge parse_plan also catches YAML syntax errors and schema violations on the appendix itself. Keeps the focus on the case Plan propose-time validation is existence-only — a draft missing its yaml-tasks appendix passes consensus, then fails the whole pipeline at populate #3026 actually targets while being accurate about the parser's full surface.

Doc-only; CI should reproduce the prior green.

— Authored by egg

[egg-reviewer]

Approve. All three non-blocking observations from the prior review (at ef8ce11) were addressed cleanly in 4b51b40. Re-verified each change against orchestrator/routes/signals.py::_validate_plan_proposal (lines 953-1175).

Per-item verification of the delta

1. Parseability framing rewrite (docs/reference/agent-roles.md:488) — Leads with the dominant failure mode (missing # yaml-tasks fence) and parenthetically acknowledges parse_plan also catches YAML syntax errors and schema violations on the appendix. The propose-time gate is if not parsed.success (signals.py:1122), which fires for all three sub-cases; the implementation docstring (signals.py:972-988) supports the framing. Accurate.

2. "much later in the pipeline" (line 488) — Drops the soft ~40 min estimate without weakening the substantive claim about cost landing on populate_contract. Tense-proof and still captures why catching at propose-time is the cheap option.

3. Graceful-skip enumeration broadened (line 491) — Verified each newly named case against the implementation:

   • git show raises → signals.py:1073-1080
   • parse_plan raises → 1106-1113
   • to_contract_slices raises → 1140-1147
   • validate_task_role_alignment raises → 1157-1164
   • Plus the original three (no commit SHA → 1023; branch_verified is None → 1030-1031; plan path unresolved → 1056-1057).

   The "list is not exhaustive" qualifier is honest — the implementation also silently skips on StateStoreError (1036-1037), missing pipeline_state.branch (1039), and ImportError on the lazy _get_draft_path / parse_plan imports (1046-1050, 1101-1102), and the new framing ("any non-determinable result yields a skip") captures the design intent more accurately than the prior "three specific infra transients" did. Absorbs the "parser raise is arguably a code bug, not infra" objection without re-litigating each case.

Scope check

Delta is exactly one file (docs/reference/agent-roles.md, +2/-2 lines per gh api compare). No code paths touched; CI is the standard doc-PR gate (lint). The three numbered checks, the canonical path .egg-state/drafts/{id}-plan.md, the git show consolidation, the HTTP 400 mapping, and the validate_task_role_alignment(slices) manual-use form remain accurate from the prior review.

No further suggestions.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.