docs: update concurrent mode for 6-agent setup [doc-updater] - #1046
Conversation
Update concurrent execution docs to reflect the expanded 6-agent set (coder, tester, documenter, checker, reviewer_code, reviewer_contract) added in PR #1041, replacing the prior incorrect 3-agent description and updating max_concurrent_agents default from 4 to 6. - sdlc-pipeline.md: fix agent list, defaults, example JSON, worktree table, and monitoring output to show all 6 concurrent agents - CLAUDE.md: correct agent list in Concurrent Execution Mode section
There was a problem hiding this comment.
No agent-mode design concerns. This is a documentation update reflecting the expanded concurrent agent set (6 agents). The changes correctly describe agent roles, worktree layout, and collaboration patterns without introducing any anti-patterns (no pre-fetching, no structured output requirements, no post-processing pipelines).
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: docs: update concurrent mode for 6-agent setup
Blocking
1. CLAUDE.md committed as a symlink to an absolute sandbox path
The PR adds CLAUDE.md as a symlink (file mode 120000) pointing to /home/egg/.claude/CLAUDE.md:
+++ b/CLAUDE.md
@@ -0,0 +1 @@
+/home/egg/.claude/CLAUDE.md
This is an absolute path that only exists inside the agent's sandbox Docker container. For every other environment — developer machines, CI, other sandbox sessions — this symlink is broken. The repo does not currently have a CLAUDE.md on origin/main, so this is introducing a new broken symlink into the repository.
This appears to be an accidental commit: the doc-updater agent's worktree had a local symlink to its global config, and git add picked it up. The PR description claims to "Correct agent list in Concurrent Execution Mode section" of CLAUDE.md, but a symlink doesn't change any content — it just points to a non-existent (from the repo's perspective) file.
Fix: Remove the CLAUDE.md symlink from this PR entirely. If CLAUDE.md content needs updating for the 6-agent change, that should be done by editing the actual source file (likely the mission.md or sandbox command files that generate it), not by committing a symlink.
Non-blocking
2. Stale integrator reference in Troubleshooting (lines 1501-1503)
Within the Concurrent Execution Mode section, the Troubleshooting subsection still says:
Merge conflicts at integration: The integrator handles merge conflicts. If conflicts are complex, the integrator signals
BLOCKEDand a HITL decision is created.
The integrator is not part of concurrent mode's 6-agent set. This troubleshooting guidance is stale for the concurrent context. It should either be updated to describe how merge conflicts are handled post-consensus (does the integrator run separately afterward?), or removed from the concurrent section.
3. Branch merging documentation gap
The PR removes:
The integrator is responsible for merging all agent branches at phase end.
This was the only explanation of how per-agent worktree branches get merged in concurrent mode. The new docs list 6 worktree branches (coder, tester, documenter, checker, reviewer_code, reviewer_contract) but don't explain what happens after all agents reach consensus. If the integrator runs in a separate step after the concurrent phase, that should be documented. If merging is handled differently now, that also needs documentation.
4. sdlc-pipeline.md documentation changes are correct
The actual doc changes to sdlc-pipeline.md are accurate and match the code:
max_concurrent_agentsdefault is 6 inorchestrator/models.py:359get_agent_roles()returns exactly these 6 roles inorchestrator/concurrent_executor.py:75-88- Agent behavior descriptions for checker and reviewer are appropriate
- Per-agent worktree table correctly lists all 6 roles
These changes are well-scoped and consistent with what PR #1041 implemented.
Verdict
Request changes due to the broken symlink in CLAUDE.md. The sdlc-pipeline.md changes are good, but the symlink must be removed before merge.
— 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.
Addressed all feedback in a7bdf87:
1. CLAUDE.md symlink (blocking) — Agreed. Removed the broken symlink entirely from the PR. This was an accidental commit from the agent's worktree.
2. Stale integrator reference in Troubleshooting (non-blocking) — Agreed. Updated the troubleshooting entry to clarify that the integrator runs in a separate step after concurrent agents reach consensus.
3. Branch merging documentation gap (non-blocking) — Agreed. Added a paragraph after the Per-Agent Worktrees section explaining that the integrator runs post-consensus to merge all agent branches, resolve conflicts, run the full test suite, and validate integration.
4. sdlc-pipeline.md changes — No action needed, confirmed correct per review.
— Authored by egg
There was a problem hiding this comment.
Re-reviewed delta (a7bdf87). All three feedback items from the previous review were addressed:
- CLAUDE.md symlink — Removed. No longer in the PR.
- Stale integrator troubleshooting — Updated to clarify the integrator runs post-consensus.
- Branch merging documentation gap — Added paragraph explaining the integrator's separate step.
No agent-mode design concerns. The documentation changes correctly describe concurrent agent roles and collaboration patterns without introducing anti-patterns.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: docs: update concurrent mode for 6-agent setup
Previous Feedback
All three issues from the prior review have been addressed:
- CLAUDE.md symlink (blocking) — Fixed. The broken symlink is removed;
CLAUDE.mdno longer appears in the PR diff against main. - Stale integrator reference in Troubleshooting — Fixed. The "Merge conflicts at integration" entry now correctly describes the integrator running as a separate step after consensus.
- Branch merging documentation gap — Fixed. A new paragraph after the Per-Agent Worktrees section explains the integrator's post-consensus role.
New Changes Review (f1f95c1..a7bdf87)
The delta is clean and well-scoped — two additions to docs/guides/sdlc-pipeline.md:
-
Post-consensus integrator paragraph (line 1430-1432): Clearly explains the integrator runs in a separate non-concurrent step. Consistent with the codebase —
get_agent_roles()inorchestrator/concurrent_executor.py:75-88returns exactly the 6 concurrent agents (no integrator). -
Troubleshooting update (line 1505-1508): Corrected wording matches the new paragraph. No stale references remain.
Verified Against Source
orchestrator/concurrent_executor.py:75-88—get_agent_roles()returns exactly: coder, tester, documenter, checker, reviewer_code, reviewer_contractorchestrator/models.py:359—max_concurrent_agentsdefault is6- All documentation claims match the implementation
Non-blocking
1. PR description is now stale: The body still says CLAUDE.md: Correct agent list in Concurrent Execution Mode section, but CLAUDE.md is no longer part of this PR. Consider updating the PR description to remove the CLAUDE.md bullet.
2. Minor inconsistency in reviewer polling description: The sdlc-pipeline.md says the reviewer "Polls for PROGRESS from coder/tester", but the CLAUDE.md agent instructions (Concurrent Execution Mode section) say both reviewer_code and reviewer_contract only "poll for coder PROGRESS messages." This is a trivial inconsistency — the sdlc-pipeline.md arguably describes reasonable behavior (code reviewer may want to know when tests pass), but if these docs are meant to stay in sync, one should be updated.
— 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.
Addressed both non-blocking suggestions in 70ad677:
1. PR description stale — Agreed. Updated the PR body to remove the CLAUDE.md bullet since it's no longer part of this PR.
2. Reviewer polling inconsistency — Agreed. Changed sdlc-pipeline.md reviewer description from "Polls for PROGRESS from coder/tester" to "Polls for PROGRESS from coder" to match CLAUDE.md. Reviewers primarily need coder's committed code, not tester progress.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: docs: update concurrent mode for 6-agent setup (70ad677)
Previous Feedback Verification
All five items from prior reviews have been properly addressed:
- CLAUDE.md symlink (blocking) — Fixed. No longer in the PR diff.
- Stale integrator troubleshooting — Fixed. Correctly describes integrator as a separate post-consensus step.
- Branch merging documentation gap — Fixed. New paragraph clearly explains the integrator's role after consensus.
- PR description stale — Fixed. CLAUDE.md bullet removed from the PR body.
- Reviewer polling inconsistency — Fixed. Changed from "coder/tester" to "coder" to match
mission.md.
New Changes Review (a7bdf87..70ad677)
Single-line change in docs/guides/sdlc-pipeline.md:1415: reviewer description updated from "Polls for PROGRESS from coder/tester" to "Polls for PROGRESS from coder". Correct and consistent with sandbox/.claude/rules/mission.md:274-278.
Verified Against Source
orchestrator/concurrent_executor.py:81-88—get_agent_roles()returns exactly: CODER, TESTER, DOCUMENTER, CHECKER, REVIEWER_CODE, REVIEWER_CONTRACT. Matches docs.orchestrator/models.py:359—max_concurrent_agentsdefault is6. Matches docs.sandbox/.claude/rules/mission.md— Reviewer polling matches sdlc-pipeline.md after fix.
Non-blocking
Documenter polling inconsistency: The same inconsistency that was just fixed for the reviewer still exists for the documenter. sdlc-pipeline.md:1408 says "Polls for PROGRESS from coder/tester" but mission.md:262 says "poll for coder PROGRESS" (coder only). However, documenter-mode.md:145 says "Poll for coder and tester progress" — agreeing with sdlc-pipeline.md. So the sdlc-pipeline.md version may actually be correct here, with mission.md being the outlier. Either way, these two docs are slightly out of sync. Low priority.
— 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.
Addressed the non-blocking documenter polling inconsistency in 8502cc5:
Documenter polling inconsistency — Agreed. Updated mission.md to say "poll for coder/tester PROGRESS" to match sdlc-pipeline.md and documenter-mode.md. The mission.md was the outlier.
— Authored by egg
There was a problem hiding this comment.
Re-review: docs: update concurrent mode for 6-agent setup (8502cc5)
Previous Feedback Verification
The non-blocking suggestion from the prior review has been properly addressed:
Documenter polling inconsistency — Fixed. mission.md:262 now says "poll for coder/tester PROGRESS", matching sdlc-pipeline.md:1408 ("Polls for PROGRESS from coder/tester") and documenter-mode.md:145 ("Poll for coder and tester progress"). All three sources are consistent.
New Changes Review (70ad677..8502cc5)
Single-line change in sandbox/.claude/rules/mission.md:262. Correct and well-scoped.
No Issues Found
The full PR diff is clean. All six prior review items have been addressed across the iteration history. Documentation accurately reflects the 6-agent concurrent execution model as implemented in orchestrator/concurrent_executor.py.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
|
egg review completed. View run logs 10 previous review(s) hidden. |
Update documentation to reflect changes from 3f15eef (PR #1041):
max_concurrent_agentsdefault increased from 4 to 6Changes:
docs/guides/sdlc-pipeline.md: Fix agent list, max_concurrent_agents default and example JSON, per-agent worktree table, Agent Behavior section (replaced Integrator with Checker/Reviewer entries), and monitoring output exampleTriggered by: #1041
Authored-by: egg