Skip to content

Fix #2275: docs follow-up — consensus-timeout HITL → OVERSEER_ALERT - #2277

Merged
jwbron merged 5 commits into
mainfrom
egg/fix-2275-doc-updater-overseer-alert
Apr 29, 2026
Merged

Fix #2275: docs follow-up — consensus-timeout HITL → OVERSEER_ALERT#2277
jwbron merged 5 commits into
mainfrom
egg/fix-2275-doc-updater-overseer-alert

Conversation

@jwbron

@jwbron jwbron commented Apr 29, 2026

Copy link
Copy Markdown
Owner

Summary

Docs leftovers from #2269 (which replaced the consensus-timeout HITL choice with an OVERSEER_ALERT but didn't update every reference). Closes #2275.

  • docs/guides/concurrent-execution.md
    • brc_consensus_progress_gate_seconds table row → references OVERSEER_ALERT.
    • ### Timeout Handling lead sentence updated to match (it had been contradicting the bullets below it, which already describe the alert flow).
  • docs/guides/sdlc-pipeline.md
    • consensus_timeout_minutes and brc_consensus_progress_gate_seconds table rows reference OVERSEER_ALERT.
    • ### Consensus Protocol step 5 rewritten as a non-blocking alert: subject format, /sdlc skill surfacing (Check agent logs / Acknowledge / Cancel pipeline), continued post-timeout polling, and cancel_task / restart_phase / provide_input levers in place of the removed three-option HITL.

No code changes; the runtime behavior already matches the new docs as of #2269.

Test plan

  • git diff --stat — 2 files, +7/-7.
  • Pre-commit hooks pass (Ruff/yaml correctly skipped — no Python or YAML in changeset).
  • Reviewer spot-check: OVERSEER_ALERT wording is consistent with the troubleshooting paragraphs at concurrent-execution.md L738 and sdlc-pipeline.md L1448–1455 that Fix #2264: replace consensus-timeout HITL decision with OVERSEER_ALERT #2269 already updated.

Docs leftovers from #2269 (which replaced the consensus-timeout HITL
choice with an OVERSEER_ALERT but didn't reach every reference).

- concurrent-execution.md: brc_consensus_progress_gate_seconds row
  references OVERSEER_ALERT; Timeout Handling lead sentence updated
  to match (it contradicted the bullets that already described the
  alert flow).
- sdlc-pipeline.md: consensus_timeout_minutes /
  brc_consensus_progress_gate_seconds rows reference OVERSEER_ALERT;
  BRC consensus-protocol step 5 rewritten as a non-blocking alert
  (post-timeout polling continues; cancel_task / restart_phase /
  provide_input replace the removed three-option HITL).
@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

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. Docs-only follow-up to #2269 (terminology alignment from "HITL decision" to OVERSEER_ALERT); no impact on prompts, agent invocations, or workflow design.

— 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.

Stale leftover: docs/guides/concurrent-execution.md L856

The PR's stated scope is "docs leftovers from #2269 (which replaced the consensus-timeout HITL choice with an OVERSEER_ALERT but didn't update every reference)". One leftover in the same file is missed:

847  ### HITL Escalation Paths
848
849  | Scenario | HITL Options |
850  |----------|-------------|
…
856  | Consensus timeout (critical blockers) | Continue waiting, Accept current state, Abort phase |
857  | Consensus timeout (advisory only) | *(no HITL — proceeds automatically)* |

L856 pins the exact three-option HITL choice (Continue waiting, Accept current state, Abort phase) that _handle_brc_consensus_timeout no longer produces. After #2269 the critical-blocker path publishes an OVERSEER_ALERT (high priority); see orchestrator/routes/pipelines.py:9924-9933 and _publish_consensus_timeout_alert at pipelines.py:9735. The phrase "Continue waiting" no longer appears anywhere in the orchestrator (grep -rn "Continue waiting" orchestrator/ is empty); the only remaining Accept current state, Abort phase triple in code is the unresolved-NACK escalation at pipelines.py:11360, 11437, 11464, 11502, 11792 — a different scenario from consensus timeout.

This row directly contradicts the runtime, and contradicts the body of this same file at L723-L738 that the PR just updated, and contradicts the new sdlc-pipeline.md step 5 wording introduced here. It is the canonical place a reader checks for "what HITL fires when consensus times out" — leaving it in is the worst possible miss for a PR whose entire purpose is to remove this contradiction.

Suggested fix: match the shape of L857 (which is already correct):

- | Consensus timeout (critical blockers) | Continue waiting, Accept current state, Abort phase |
+ | Consensus timeout (critical blockers) | *(no HITL — `OVERSEER_ALERT` `priority=high`; orchestrator continues post-timeout polling)* |

(Or similar wording — match whatever you used in step 5 of sdlc-pipeline.md.)

Non-blocking

concurrent-execution.md L36 was not updated

| `consensus_timeout_minutes` | `30` | Consensus timeout before escalation or auto-advance |

The parallel row in sdlc-pipeline.md:1252 was updated by this PR to "Timeout before publishing a consensus-timeout OVERSEER_ALERT". The two tables describing the same field now disagree on what the timeout does. "Escalation or auto-advance" is also a slightly stale framing — post-#2269 the high-priority path publishes an alert and keeps polling, while the advisory-only path proceeds automatically. Worth aligning with the sdlc-pipeline.md wording for consistency.

Verified and correct

  • concurrent-execution.md:725 and :738 updated wording matches _check_brc_progress_gate and _publish_consensus_timeout_alert in orchestrator/routes/pipelines.py.
  • sdlc-pipeline.md:1252-1253 table updates are consistent with the runtime.
  • sdlc-pipeline.md:1321-1323 step 5 rewrite matches the troubleshooting paragraph at sdlc-pipeline.md:1448-1455 (which #2269 already updated) and the alert subject format at pipelines.py:9771.
  • The two other Accept current state, Abort phase references at concurrent-execution.md:746 and :862 are about unresolved NACKs (issue #1693), not consensus timeout — those correctly remain unchanged.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

…R_ALERT

Address PR #2277 review feedback: two stale rows in concurrent-execution.md
contradicted the OVERSEER_ALERT flow established by #2269.

- L36 (PipelineConfig table): consensus_timeout_minutes description now
  matches the parallel sdlc-pipeline.md row.
- L856 (HITL Escalation Paths table): critical-blocker consensus timeout
  no longer lists the removed three-option HITL choice; matches the shape
  of the advisory-only row directly below it.
@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.

@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-review: previous feedback fully addressed

The two issues from the previous review at 837104d have been fixed correctly in commit 5e33e1 (Update concurrent-execution.md HITL/consensus-timeout rows to OVERSEER_ALERT):

Blocking issue resolved

docs/guides/concurrent-execution.md L856 — the stale Continue waiting, Accept current state, Abort phase HITL options for Consensus timeout (critical blockers) are gone. The row now reads:

| Consensus timeout (critical blockers) | *(no HITL — `OVERSEER_ALERT` `priority=high`; orchestrator continues post-timeout polling)* |

This matches the runtime behavior at _handle_brc_consensus_timeout and the new sdlc-pipeline.md step 5 wording, and adopts the same *(no HITL — …)* shape as the advisory-only row directly below it (L857).

Non-blocking issue resolved

docs/guides/concurrent-execution.md L36 — the parallel consensus_timeout_minutes description now matches sdlc-pipeline.md:1252: Timeout before publishing a consensus-timeout OVERSEER_ALERT. The two PipelineConfig tables for the same field are now consistent.

Spot checks

  • grep -n "Continue waiting" docs/guides/ — empty across both files.
  • The two remaining Accept current state, Abort phase references in concurrent-execution.md (L746 and L862) are correctly preserved — they describe the unresolved-NACK escalation (issue #1693), not consensus timeout (issue #2264).
  • Final diff (gh pr diff 2277) shows exactly the four edits expected: concurrent-execution.md L36, L725, L856 + sdlc-pipeline.md L1252-1253, L1321-1323.
  • Wording is consistent with the troubleshooting paragraphs at concurrent-execution.md:738 and sdlc-pipeline.md:1448-1455, and with the alert subject format at pipelines.py:9771.

Docs-only PR, no code paths affected.

— 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. Re-review delta is a single doc-alignment commit (5e33e1) that updates two concurrent-execution.md rows to the OVERSEER_ALERT framing — pure terminology consistency with #2269. No prompts, agent invocations, structured-output contracts, or workflow design touched.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

Copy link
Copy Markdown
Contributor

egg agent-mode-design completed. View run logs

11 previous review(s) hidden.

@jwbron
jwbron merged commit 757bacb into main Apr 29, 2026
29 checks passed
james-in-a-box Bot pushed a commit that referenced this pull request Apr 29, 2026
…urrent-execution.md

Combined the per-phase consensus timeout config (this PR) with the
consensus-timeout HITL → OVERSEER_ALERT migration (#2264 / #2277) in both
guides. The two changes are complementary: per-phase fields stay; descriptions
of the post-timeout escalation surface now reference OVERSEER_ALERT and link
to issue #2264 to match main.
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