Skip to content

docs: clarify concurrent consensus protocol details [doc-updater] - #1039

Merged
jwbron merged 2 commits into
mainfrom
egg/doc-update-consensus-protocol
Mar 12, 2026
Merged

docs: clarify concurrent consensus protocol details [doc-updater]#1039
jwbron merged 2 commits into
mainfrom
egg/doc-update-consensus-protocol

Conversation

@james-in-a-box

Copy link
Copy Markdown
Contributor

docs: clarify concurrent consensus protocol details [doc-updater]

Update the Concurrent Execution Mode → Consensus Protocol section in
docs/guides/sdlc-pipeline.md to reflect behavior introduced in
7b4a27d (Add consensus-driven phase advancement for concurrent
execution, #1037).

Before that commit, concurrent phase completion was purely exit-code-
based; the docs described the intended consensus behavior. Now that the
polling loop is implemented, two details were missing from the docs:

  • Objection HITL options: when an agent signals OBJECTING, a HITL
    decision is created — the specific options presented to the human
    ("Override objections", "Wait for resolution", "Abort phase") were
    not documented.
  • Timeout HITL options and fallback: the docs said "timeout triggers
    HITL escalation" but didn't list the decision options ("Continue
    waiting", "Accept current state", "Abort phase") or mention the
    container-exit fallback that takes effect while the decision is pending.
  • Container-exit fallback: if all containers exit before consensus is
    reached, the phase now completes based on exit codes — this path was
    undocumented.
  • 5-second poll interval and immediate stop: added note that the
    orchestrator polls every 5s and stops containers immediately on
    consensus.

Triggered by: #1037

Issue: none

Test plan:

  • Review the updated Consensus Protocol bullet list in
    docs/guides/sdlc-pipeline.md for accuracy against the implementation
    in orchestrator/routes/pipelines.py:_run_concurrent_phase.

Authored-by: egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns. This is a documentation-only change that clarifies existing orchestrator behavior (HITL options for objections/timeouts, container-exit fallback, poll interval). No agent workflow or prompt design implications.

— Authored by egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified every claim against the implementation in orchestrator/routes/pipelines.py:_run_concurrent_phase (lines 4416–4841):

  • 5-second poll interval: poll_interval = 5 (L4568), time.sleep(poll_interval) (L4841) — correct
  • Immediate container stop on consensus: _stop_running_containers() called when is_complete (L4685) — correct
  • Objection HITL options: ["Override objections", "Wait for resolution", "Abort phase"] (L4708) — matches exactly
  • Timeout config and default: getattr(pipeline.config, "consensus_timeout_minutes", 30) (L4569) — correct
  • Timeout HITL options: ["Continue waiting", "Accept current state", "Abort phase"] (L4796) — matches exactly
  • Exit-code fallback after timeout: ThreadPoolExecutor waits for remaining containers, returns based on exit codes (L4802–4838) — correct
  • Container-exit fallback before consensus: len(exited_containers) >= len(active_executions) (L4768–4772) — correct

All documented behavior is accurate. LGTM.

One non-blocking note: lines 3–5 of the updated list are ~150–180 characters, which may wrap awkwardly in narrow viewports. Consider breaking them into a main point + sub-bullet for the mechanism details. Not blocking.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

Agreed. Broke the long consensus protocol lines (3–5) into main point + sub-bullets for better readability in narrow viewports. Pushed in bb9c8e4.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns. The delta since my last review is a formatting-only change (breaking long lines into sub-bullets) that addresses the base reviewer's feedback. No content or design changes.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at bb9c8e4. The previous non-blocking suggestion (break long consensus protocol lines into main point + sub-bullets) has been properly addressed.

Verified all documentation claims against orchestrator/routes/pipelines.py:_run_concurrent_phase:

  • Poll interval: poll_interval = 5 (L4568) — matches
  • Immediate container stop: _stop_running_containers() on consensus (L4685) — matches
  • Objection HITL options: ["Override objections", "Wait for resolution", "Abort phase"] (L4708) — exact match
  • Timeout config: getattr(pipeline.config, "consensus_timeout_minutes", 30) (L4569) — matches
  • Timeout HITL options: ["Continue waiting", "Accept current state", "Abort phase"] (L4796) — exact match
  • Exit-code fallback on timeout: ThreadPoolExecutor waits for containers, returns on exit codes (L4802–4838) — matches
  • Container-exit fallback before consensus: len(exited_containers) >= len(active_executions) (L4768–4772) — matches

No issues found. LGTM.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

egg review completed. View run logs

5 previous review(s) hidden.

@jwbron
jwbron merged commit 5478931 into main Mar 12, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant