docs: update concurrent branch model and coordinator spawn docs [doc-updater] - #1091
Conversation
…updater] Update documentation to reflect changes from 476c39d (PR #1085): - concurrent-execution.md: Replace "per-agent worktree isolation" section with "shared pipeline branch" — agents now share `egg/issue-{N}` instead of having per-role branches (`egg/issue-{N}/{role}`) - coordinator.md: Document two new 409 spawn validation cases: contract enforcement (no contract in implement/pr phase) and dependency ordering (reviewer/downstream roles blocked until primary dependencies complete) - README.md: Sync concurrent execution mode description to match Triggered by: #1085 Authored-by: egg
There was a problem hiding this comment.
Review: docs update for concurrent branch model and coordinator spawn
The documentation changes in concurrent-execution.md and coordinator.md are accurate and well-written. I verified the new content against the actual implementation in orchestrator/routes/coordinator.py, orchestrator/concurrent_executor.py, and shared/egg_contracts/agent_roles.py — all claims match the code.
However, the PR misses stale references in sdlc-pipeline.md that now directly contradict the updated docs.
Blocking: Stale per-agent worktree documentation in sdlc-pipeline.md
docs/guides/sdlc-pipeline.md:1417-1432 — The "Per-Agent Worktrees" section still describes the old per-role branch model with explicit examples:
egg/issue-999/coder ← coder's work
egg/issue-999/tester ← tester's work
egg/issue-999/documenter ← documenter's work
...
This directly contradicts the updated concurrent-execution.md which now states: "All agents operate on the pipeline's shared branch." A reader of sdlc-pipeline.md will get the wrong information.
docs/guides/sdlc-pipeline.md:1505-1508 — The troubleshooting entry also references "merge per-agent worktree branches":
After all concurrent agents reach consensus, the integrator runs in a separate step to merge per-agent worktree branches.
Fix: Update the "Per-Agent Worktrees" section header and content to match the new shared branch model (similar to the "Shared Pipeline Branch" section in concurrent-execution.md). Update the troubleshooting entry to remove the per-agent merge reference.
Non-blocking: Dense 409 error code description
docs/guides/coordinator.md:143 — The 409 bullet packs three distinct error conditions into a single semicolon-separated sentence:
Phase advancement blocked (no contract before implement/pr); or agent spawn rejected because the pipeline has no contract in implement/pr phase; or role's dependencies have not yet completed
Consider breaking this into sub-bullets for scannability — this line is doing a lot of work. Not blocking since the information is correct.
Verified accurate
concurrent-execution.mdintro, line 37 (shared branch), and the new "Shared Pipeline Branch" section all matchConcurrentPhaseExecutor.get_worktree_branch()which returnspipeline.branchorf"egg/issue-{issue}"(no per-role suffix).coordinator.mddependency enforcement docs match the spawn endpoint validation atcoordinator.py:229-260—testerdepends oncoderperagent_roles.py, status check usess.status == "complete", response includesmissing_dependencies.coordinator.mdcontract enforcement docs matchcoordinator.py:212-227— 409 whencurrent_phase in (IMPLEMENT, PR) and not pipeline.contract_synced.- Troubleshooting entries for the new 409 cases are clear and actionable.
README.mdchange is consistent with the other updates.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Agreed. All feedback addressed in 97a48d3:
Blocking — Stale per-agent worktree docs in sdlc-pipeline.md:
- Replaced "Per-Agent Worktrees" section (lines 1417-1432) with "Shared Pipeline Branch" section matching the updated
concurrent-execution.md— describes all agents operating on the shared branch with message bus coordination. - Updated integrator description to "validate integration" rather than "merge per-agent worktree branches."
- Updated troubleshooting entry (lines 1505-1508) from "Merge conflicts at integration" to "Commit conflicts" with guidance on pull/rebase/retry for shared branch conflicts.
Non-blocking — Dense 409 error description in coordinator.md:
- Broke the single semicolon-separated sentence into sub-bullets for scannability.
— Authored by egg
There was a problem hiding this comment.
No agent-mode design concerns.
Documentation-only change reflecting infrastructure decisions (shared branch model, dependency/contract enforcement at spawn time). The new constraints are enforced at the orchestrator API level (HTTP 409), not via prompt instructions — well aligned with "the sandbox is the constraint."
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: docs update for concurrent branch model and coordinator spawn
The previous feedback has been properly addressed. The sdlc-pipeline.md stale references and the dense 409 error description in coordinator.md are both fixed correctly.
However, there is one remaining stale reference that falls within the PR's scope.
Blocking: Stale per-agent worktree reference in orchestrator/README.md
orchestrator/README.md:87 — Still describes the old per-agent branch model:
Per-agent worktrees — Each concurrent agent gets an isolated worktree branch (e.g.,
egg/issue-999/coder,egg/issue-999/tester). The integrator merges these at the end.
This directly contradicts the "Shared Pipeline Branch" sections now in concurrent-execution.md and sdlc-pipeline.md. The PR's purpose is to update documentation for the shared branch model — this user-facing README was missed. Same category of issue as the sdlc-pipeline.md fix in this commit.
Fix: Replace the bullet with something like:
- **Shared pipeline branch** — All concurrent agents operate on the pipeline's shared branch
(e.g., `egg/issue-999`). Agents coordinate commits via the message bus.
Verified: Previous feedback properly addressed
sdlc-pipeline.md"Per-Agent Worktrees" section → Replaced with "Shared Pipeline Branch" describing the shared branch model with message bus coordination. Content matchesconcurrent-execution.md.sdlc-pipeline.mdtroubleshooting entry → Updated from "Merge conflicts at integration" to "Commit conflicts" with pull/rebase/retry guidance appropriate for shared branch.coordinator.md409 error codes → Broken into sub-bullets for scannability.
Verified: New content accuracy
HANDOFFis a validMessageTypedefined inshared/egg_orchestrator/types.py:63andorchestrator/message_store.py:23. References in the updated docs are correct.ConcurrentPhaseExecutor.get_worktree_branch()(concurrent_executor.py:90-100) returnspipeline.branchorf"egg/issue-{issue}"— ignoring theroleparameter. All agents get the same branch. Documentation is accurate.- Dependency and contract enforcement docs in
coordinator.mdmatch the implementation atcoordinator.py:212-260.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Agreed. Fixed in f71b381:
Blocking — Stale per-agent worktree reference in orchestrator/README.md:
- Replaced "Per-agent worktrees" bullet at line 87 with "Shared pipeline branch" — now describes all concurrent agents operating on the shared branch with message bus coordination, matching
concurrent-execution.mdandsdlc-pipeline.md.
— Authored by egg
There was a problem hiding this comment.
No agent-mode design concerns.
Delta since last review: single commit fixing stale per-agent worktree reference in orchestrator/README.md to match the shared pipeline branch model. Documentation-only change, no agent-mode implications.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: docs update for concurrent branch model and coordinator spawn
The blocking issue from the previous review — stale per-agent worktree reference in orchestrator/README.md — has been properly fixed.
Verified: Previous feedback addressed
orchestrator/README.md:87— "Per-agent worktrees" bullet replaced with "Shared pipeline branch" describing all agents operating on the shared branch with message bus coordination. Wording is consistent withconcurrent-execution.mdandsdlc-pipeline.md.
Verified: No remaining stale references
Searched all documentation files (docs/, orchestrator/, CLAUDE.md, README.md) for residual per-agent worktree/branch references. The only remaining mentions are in .egg-state/drafts/ historical planning documents (1027-plan.md, 1028-analysis.md), which are out of scope — they describe the model as originally planned and are readonly during the implement phase.
Verified: Content accuracy
orchestrator/README.mdnew text matchesConcurrentPhaseExecutor.get_worktree_branch()which returnspipeline.branchorf"egg/issue-{issue}"(no per-role suffix).- All five updated docs (
README.md,concurrent-execution.md,sdlc-pipeline.md,coordinator.md,orchestrator/README.md) use consistent language: "shared branch", "pipeline's shared branch", "coordinate commits via the message bus".
Non-blocking: Minor wording inconsistency in branch example
The example branch names vary slightly across files:
concurrent-execution.md:37usesegg/issue-123sdlc-pipeline.md:1419usesegg/issue-999orchestrator/README.md:87usesegg/issue-999
This is cosmetic and doesn't affect correctness — just noting that concurrent-execution.md uses a different example issue number. Not worth changing.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
|
egg feedback addressed. View run logs 7 previous review(s) hidden. |
Update documentation to reflect changes from PR #1085 (476c39d).
Concurrent execution branch model changed: Agents no longer get per-role branches (
egg/issue-{N}/{role}). They now all share the pipeline's single branch (egg/issue-{N}orpipeline.branch). Updatedconcurrent-execution.mdto replace the "Per-Agent Worktree Isolation" section with "Shared Pipeline Branch", and updated the startup protocol description and intro paragraph. UpdatedREADME.mdto match.Coordinator spawn now enforces two new validations: The
/coordinator/spawnendpoint now returns HTTP 409 for (1) spawning an agent in implement/pr phase when no contract exists, and (2) spawning a role whose declared dependencies have not yet completed (e.g.,testerbeforecoder). Updatedcoordinator.mderror codes table, spawn endpoint docs, and troubleshooting section.Issue: none
Test plan:
concurrent-execution.mdno longer mentions per-role branchescoordinator.mddocuments the 409 spawn rejection cases and troubleshooting stepsTriggered by: #1085
Authored-by: egg