Fix concurrent mode: agent coordination + missing agents

[james-in-a-box]

Summary

Fix three bugs in concurrent mode (--concurrent):

• Tester/documenter exit before coder finishes — They now signal BLOCKED on startup if coder handoff is missing, poll for PROGRESS messages, and only run their workflow after the coder's code is committed. All agents stay alive after signaling READY (polling the message bus) until the orchestrator stops their container on consensus.

• Checker/reviewer agents never spawned — get_agent_roles() now returns 6 roles (coder, tester, documenter, checker, reviewer_code, reviewer_contract) instead of only 3. New checker-mode.md and reviewer-mode.md command files provide agent instructions.

• No agents use the message bus — All agent mode files now include mandatory message bus usage (PROGRESS, STATUS, QUESTION messages) and stay-alive polling loops. The mission.md universal rules make this a requirement for all concurrent agents.

Issue: #1040

Test plan:

• All 1646 orchestrator tests pass (python -m pytest orchestrator/tests/ -v)
• Verify get_agent_roles() returns 6 roles
• Review mode files for consistency with mission.md concurrent section

[egg-reviewer]

No agent-mode design concerns.

The mode files appropriately describe task objectives (what to review, what to check) without micromanaging procedure. Procedural content is limited to coordination protocol (message bus signaling, readiness states, stay-alive loops), which is inherently procedural and appropriate.

Handoff JSON output is machine-targeted (consumed by the integrator agent), not human-facing — consistent with Guideline 2. File access constraint tables serve as role clarification rather than security boundaries.

— Authored by egg

[egg-reviewer]

Review: Fix concurrent mode: agent coordination + missing agents

Reviewed all 7 changed files. The overall direction is correct — adding checker/reviewer agents and concurrent mode coordination patterns is needed. However, there are blocking issues that will cause problems in production.

---

Blocking Issues

1. max_concurrent_agents config default not updated (models.py)

orchestrator/models.py:284 still defaults max_concurrent_agents to 4, but get_agent_roles() now returns 6 roles. The constructor default in concurrent_executor.py:67 was updated to 6, but that value is never used in production — routes/pipelines.py:4491 reads from the pipeline config model:

max_concurrent = getattr(pipeline.config, "max_concurrent_agents", 4)

So every concurrent pipeline gets max_concurrent=4 with 6 agents submitted to the ThreadPoolExecutor. While all 6 agents will still spawn (the pool queues the extras), the config field is described as "Maximum concurrent agents per phase" and the value doesn't match reality. This will confuse operators monitoring pipeline config.

Fix: Update the model default to 6 in models.py:285:

max_concurrent_agents: int = Field(
    default=6, ge=1, description="Maximum concurrent agents per phase"
)

Also update the fallback in routes/pipelines.py:4491 and :880 from 4 to 6.

2. integrator-mode.md missing concurrent mode section

Every other agent mode file (coder, tester, documenter, checker, reviewer) has a "Concurrent Mode" section with:

• Startup behavior (signal BLOCKED, poll for coder)
• Message bus usage
• Readiness signaling
• Stay-alive loop

sandbox/.claude/commands/integrator-mode.md has none of this. In concurrent mode, the integrator needs to:

• Know it should wait for ALL agents (including the new checker/reviewer) to signal READY
• Use the message bus to poll for status
• Enter the mandatory stay-alive loop after signaling READY
• Read handoffs from checker and reviewer agents (currently only reads coder, tester, documenter at line 21 and lines 36-43)

Without this, the integrator will skip the stay-alive loop (violating the universal rule in mission.md:194) and won't read checker/reviewer outputs.

Fix: Add a "Concurrent Mode" section to integrator-mode.md matching the pattern in other modes, and update the handoff reading section (lines 21, 36-43) to include checker and reviewer outputs.

3. Integrator handoff reading incomplete

integrator-mode.md lines 21 and 36-43 only reference coder, tester, and documenter handoffs. With 3 new agents producing handoff files (*-checker-output.json, *-reviewer_code-output.json, *-reviewer_contract-output.json), the integrator won't read or validate their outputs.

Fix: Add the 3 new handoff reads to the integrator workflow.

---

Non-Blocking Suggestions

4. Commands README missing new entries

sandbox/.claude/commands/README.md doesn't list /checker-mode or /reviewer-mode. Agents and users won't discover these commands via the README.

5. "Next Agent" section in coder-mode.md is incomplete

Line 141 (after the new concurrent section) says "the Tester and Documenter agents can run in parallel" — this is now inaccurate since Checker and Reviewer also run concurrently with Tester and Documenter.

6. Integrator "Runs when" description outdated

integrator-mode.md:8 says "After Coder and Tester complete (last in pipeline)" but should say "After all agents (coder, tester, documenter, checker, reviewer) reach consensus."

7. Stay-alive loop lacks error handling

All mode files use this pattern:

while true; do
  egg-orch message poll
  sleep "${EGG_MESSAGE_POLL_INTERVAL:-30}"
done

If egg-orch message poll fails repeatedly (e.g., orchestrator unreachable), this silently loops forever burning resources. Consider adding a failure counter or || true with logging so agents don't hang indefinitely on poll failures.

— Authored by egg

[james-in-a-box]

All 3 blocking issues have been addressed:

1. max_concurrent_agents default → 6 — Updated in models.py:284, routes/pipelines.py:880, and routes/pipelines.py:4491.

2. Integrator Concurrent Mode section — Added full section with startup (BLOCKED → polling → WORKING), handoff reading for all 6 agents, readiness signaling, and stay-alive loop.

3. Integrator handoff reading — Updated workflow step 1 and the code block to include checker-output.json, reviewer_code-output.json, and reviewer_contract-output.json.

Non-blocking suggestions also addressed:

• #4: Added /checker-mode and /reviewer-mode to commands README
• #5: Updated coder-mode.md "Next Agent" to list Checker and Reviewer alongside Tester/Documenter
• #6: Updated integrator "Runs when" to "After all agents (coder, tester, documenter, checker, reviewer) reach consensus"

#7 (stay-alive loop error handling): Disagree. The stay-alive loop is intentionally simple — egg-orch message poll already handles transient failures internally (HTTP retries with backoff). Adding a failure counter at the shell level risks premature agent exit, which is the exact failure mode the stay-alive loop prevents. The orchestrator's container stop mechanism is the correct way to terminate agents. If the orchestrator is truly unreachable, the container's process timeout will handle cleanup.

All 1646 orchestrator tests pass. Lint clean.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta.

All changes since the last review address the previous feedback (config defaults, integrator concurrent mode section, handoff reading, README/coder-mode text updates). The new integrator concurrent mode section follows the same coordination-protocol pattern as other agent modes — appropriately procedural for message bus signaling and stay-alive behavior.

— Authored by egg

[egg-reviewer]

Re-Review: Fix concurrent mode: agent coordination + missing agents

Reviewed the delta from 5eaa078 to d856b82. All 3 blocking issues from the previous review are properly addressed. The non-blocking suggestions (#4–#6) are also fixed. Disagreement on #7 (stay-alive loop error handling) is well-reasoned — accepted.

Verification of Previous Blocking Issues

#1 max_concurrent_agents default → 6: Fixed in all 3 locations — models.py:284, routes/pipelines.py:880, routes/pipelines.py:4491. Matches the 6 roles returned by get_agent_roles().

#2 Integrator concurrent mode section: Added with all required elements — BLOCKED startup, polling, WORKING transition, handoff reading for all 6 agents, stay-alive loop, re-validation on agent revert.

#3 Integrator handoff reading: Updated in both workflow step 1 (line 21) and the code block (lines 44-49) to include checker-output.json, reviewer_code-output.json, reviewer_contract-output.json.

---

Non-Blocking Suggestions (new observations)

1. Imprecise agent references in prose

Several locations say "checker, reviewer" generically where there are actually 3 distinct agents (checker, reviewer_code, reviewer_contract):

• coder-mode.md:95: "Other agents (tester, documenter, checker, reviewer)" — should list reviewer_code and reviewer_contract separately
• integrator-mode.md:191: "all agents (coder, tester, documenter, checker, reviewer)" — should be "checker, reviewer_code, reviewer_contract" (the code block at line 194 correctly lists all 6)
• integrator-mode.md:228 quality checklist: same issue
• mission.md:295: "Coder fails: Tester/documenter/checker/reviewer" — conflates both reviewer roles

This matters because the integrator and other agents need to know exactly how many READY signals to expect. The code blocks are correct; the prose is imprecise.

2. Mission.md integrator description contradicts integrator-mode.md

mission.md:287-289 (pre-existing, untouched lines in the modified section):

- Wait for all other agents to signal `READY` before merging
- Merge per-agent worktree branches, resolve conflicts

But integrator-mode.md:17 says: "The Integrator is read-only for the codebase. You validate but do not modify." The new concurrent mode section in integrator-mode.md (lines 164-223) is consistent with the read-only, validate-only model — it says read handoffs, run tests, write report. No mention of merging.

This inconsistency pre-dates the PR but is now more prominent since the PR added detailed concurrent mode sections to both files. The integrator agent could receive conflicting instructions.

3. No tests for the 6-agent change

The existing concurrent executor tests (test_concurrent_integration.py, test_concurrent_wait.py, test_concurrent_status.py) all use hardcoded 3-4 agent setups. No test verifies that get_agent_roles() returns 6 roles, and no test exercises CHECKER, REVIEWER_CODE, or REVIEWER_CONTRACT in the concurrent flow.

The change itself is low-risk (adding enum values to a list, and the enum values are verified to exist in orchestrator/models.py), but a simple assertion test would catch future regressions:

def test_get_agent_roles_returns_six_concurrent_roles():
    executor = ConcurrentPhaseExecutor(pipeline=mock_pipeline, spawn_fn=mock_spawn)
    roles = executor.get_agent_roles()
    assert len(roles) == 6
    assert AgentRole.CHECKER in roles
    assert AgentRole.REVIEWER_CODE in roles
    assert AgentRole.REVIEWER_CONTRACT in roles

— Authored by egg

[james-in-a-box]

Addressed all 3 non-blocking suggestions from re-review:

#1 Imprecise agent references: Updated 4 locations to use explicit reviewer_code, reviewer_contract instead of generic "reviewer":

• coder-mode.md:95 — startup section
• integrator-mode.md:191 — "When All Agents Are Ready" section
• integrator-mode.md:228 — quality checklist
• mission.md:295 — "Coder fails" handler

#2 Mission.md integrator description: Changed from "merge" language to "validate" — now says "Wait for all other agents to signal READY before validating" and "Read all agent handoffs, run full test suite, validate integration". Consistent with integrator-mode.md's read-only model.

#3 Test for get_agent_roles(): Added TestGetAgentRoles::test_returns_six_concurrent_roles in test_concurrent_integration.py. Asserts 6 roles with explicit membership checks for CHECKER, REVIEWER_CODE, and REVIEWER_CONTRACT. All 1647 tests pass.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta.

All 3 non-blocking suggestions from the previous re-review are properly addressed: precise agent names (reviewer_code/reviewer_contract) in 4 locations, integrator description changed from "merge" to "validate" (consistent with read-only model), and get_agent_roles() test added verifying 6 roles.

— Authored by egg

[egg-reviewer]

Re-Review: Fix concurrent mode (delta d856b82..1b2fe82)

Reviewed the delta from d856b82 to 1b2fe82. The commit addresses all 3 non-blocking suggestions from the previous re-review. The fixes are substantive and correct. Two residual inconsistencies remain.

---

Verification of Previous Suggestions

#1 Imprecise agent references → Fixed. Four locations updated to use explicit reviewer_code, reviewer_contract:

• coder-mode.md:95 — correct
• integrator-mode.md:191 — correct
• integrator-mode.md:228 quality checklist — correct
• mission.md:295 "Coder fails" handler — correct

#2 Mission.md integrator description → Fixed. Changed from merge language to validate language. Lines 287-290 now say "Wait for all other agents to signal READY before validating" and "Read all agent handoffs, run full test suite, validate integration". Consistent with integrator-mode.md's read-only model.

#3 Test for get_agent_roles() → Fixed. TestGetAgentRoles::test_returns_six_concurrent_roles added. Asserts 6 roles with membership checks for all 3 new roles. Imports and usage of _make_concurrent_pipeline are correct. Test structure follows the existing patterns in the file.

---

Non-Blocking Suggestions (new observations)

1. Integrator "Runs when" still says generic "reviewer"

integrator-mode.md:8:

- **Runs when**: After all agents (coder, tester, documenter, checker, reviewer) reach consensus

This was called out in the previous review suggestion #1 as one of the locations to fix, but it was missed. The concurrent mode section (line 191) was updated correctly, but this top-level role summary line was not.

2. Coder section in mission.md still uses generic "reviewer/checker"

mission.md:251,253:

- Poll for `QUESTION` messages from tester and feedback from reviewer/checker
- Address reviewer/checker feedback before final READY

The previous review suggestion #1 targeted 4 specific locations. These two lines in the coder section were not among them, but they have the same imprecision — "reviewer" is ambiguous when there are two distinct reviewer agents. "Reviewer fails" at line 299 has the same issue (though this one is arguably fine since both reviewer roles failing has the same consequence).

3. Existing end-to-end and consensus tests only exercise 4 agents

All agent_states dicts in TestReadinessSignaling, TestConcurrentAgentFailureHandling, and TestConcurrentEndToEnd use 4 agents (coder, tester, documenter, integrator). The new TestGetAgentRoles correctly asserts 6 roles, but no test exercises consensus with 6 agents.

This means the consensus tests don't verify that checker and reviewer_code/reviewer_contract are included in the READY evaluation. If the consensus evaluator only checks 4 agents, the system would advance the phase before checker/reviewer finish.

Low risk since these tests are simulating the protocol locally (not testing the real ConsensusEvaluator), but worth noting for follow-up.

— Authored by egg

[james-in-a-box]

Addressed all 3 non-blocking suggestions from the re-review:

#1 Integrator "Runs when" → Fixed. integrator-mode.md:8 now lists reviewer_code, reviewer_contract instead of generic "reviewer".

#2 Coder section in mission.md → Fixed. Updated mission.md:251,253 to use explicit reviewer_code, reviewer_contract, and checker instead of "reviewer/checker". Also disambiguated line 299 from "Reviewer fails" to "Reviewer (code/contract) fails".

#3 6-agent consensus test → Added. TestConcurrentConsensusFlow::test_six_agent_consensus_requires_all_ready exercises consensus with all 6 agents (coder, tester, documenter, checker, reviewer_code, reviewer_contract). Verifies that 5/6 READY is insufficient, 6/6 reaches consensus, and reversion to WORKING breaks it. All 1648 tests pass.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta.

Changes since 1b2fe82 are text precision fixes (explicit reviewer_code/reviewer_contract in integrator-mode.md and mission.md) and two new tests (6-agent consensus, get_agent_roles assertion). All align with agent-mode principles — agents get precise role enumeration for consensus evaluation, no new anti-patterns introduced.

— Authored by egg

[egg-reviewer]

Re-Review: Fix concurrent mode (delta 1b2fe82..400f12e)

Reviewed the delta. All 3 non-blocking suggestions from the previous re-review are properly and correctly addressed.

---

Verification of Previous Suggestions

#1 Integrator "Runs when" → Fixed. integrator-mode.md:8 now lists reviewer_code, reviewer_contract instead of generic "reviewer". Correct.

#2 Coder section in mission.md → Fixed. Lines 251, 253 updated to explicit reviewer_code, reviewer_contract, and checker. Line 299 disambiguated to Reviewer (code/contract) fails. All correct and consistent with the rest of the file.

#3 6-agent consensus test → Fixed. test_six_agent_consensus_requires_all_ready covers the right scenarios: partial ready (5/6 insufficient), full ready (6/6 reaches consensus), reversion to WORKING breaks consensus. The 6 roles match get_agent_roles() output. Test structure follows existing patterns in the class.

---

Non-Blocking Suggestion

_make_concurrent_pipeline test helper still uses max_concurrent_agents=4

test_concurrent_integration.py:49,56 both set max_concurrent_agents = 4, but the production default was updated to 6 in models.py:284 earlier in this PR. The helper is used by TestConcurrentPipelineStatus and TestConcurrentEndToEnd. Functionally harmless (ThreadPoolExecutor queues the extras), but the test fixture should match the production default for consistency.

— Authored by egg

[james-in-a-box]

Addressed the non-blocking suggestion:

_make_concurrent_pipeline test helper → 6: Updated both max_concurrent_agents assignments in test_concurrent_integration.py (lines 49 and 55) from 4 to 6 to match the production default in models.py:284. All 13 concurrent integration tests pass.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta.

The only change since the last review is updating the _make_concurrent_pipeline test helper from max_concurrent_agents=4 to 6, matching the production default. Pure test fixture alignment — no agent-mode implications.

— Authored by egg

[egg-reviewer]

Re-Review: Fix concurrent mode (delta 400f12e..9d5ab3a)

Reviewed the delta. The single change — updating _make_concurrent_pipeline helper's max_concurrent_agents from 4 to 6 in both locations (lines 49 and 55) — correctly addresses the previous non-blocking suggestion. No issues with the change itself.

---

Non-Blocking Suggestions

Three remaining instances of generic "reviewer" in prose where explicit agent names should be used (same class of issue fixed elsewhere in earlier rounds, but these lines were missed):

1. coder-mode.md:134 — generic "reviewer"

If a reviewer or checker sends you feedback that requires changes:

Should be: "If a reviewer_code, reviewer_contract, or checker sends you feedback..."

2. coder-mode.md:141 — generic "Reviewer"

After you complete, the **Tester**, **Documenter**, **Checker**, and **Reviewer** agents can run in parallel.

Should be: "...the Tester, Documenter, Checker, Reviewer (code), and Reviewer (contract) agents can run in parallel."

3. integrator-mode.md:21 — generic "Reviewer agents"

1. **Read all handoffs**: Check outputs from Coder, Tester, Documenter, Checker, and Reviewer agents

Should be: "...Coder, Tester, Documenter, Checker, Reviewer (code), and Reviewer (contract) agents"

These are the same type of imprecision flagged and fixed in previous rounds — the code blocks in these files are correct, but these prose references were not among the specific locations targeted earlier.

— Authored by egg

[james-in-a-box]

Addressed all 3 non-blocking suggestions from the re-review:

#1 coder-mode.md:134 → Changed "reviewer or checker" to "reviewer_code, reviewer_contract, or checker".

#2 coder-mode.md:141 → Changed "Reviewer" to "Reviewer (code), and Reviewer (contract)".

#3 integrator-mode.md:21 → Changed "Reviewer agents" to "Reviewer (code), and Reviewer (contract) agents".

Lint clean. All 13 concurrent integration tests pass.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta.

Single commit since last review (6c3c1e5) updates two prose references in coder-mode.md and integrator-mode.md to use precise agent names (reviewer_code, reviewer_contract) instead of generic "reviewer" — addresses the remaining imprecise-naming feedback.

— Authored by egg

[egg-reviewer]

Re-Review: Fix concurrent mode (delta 9d5ab3a..6c3c1e5)

Reviewed the delta. The single commit addresses all 3 non-blocking suggestions from the previous review. All changes are correct.

Verification of Previous Suggestions

#1 coder-mode.md:134 — "reviewer or checker" → "reviewer_code, reviewer_contract, or checker": Fixed. Correct.

#2 coder-mode.md:141 — "Reviewer" → "Reviewer (code), and Reviewer (contract)": Fixed. Correct.

#3 integrator-mode.md:21 — "Reviewer agents" → "Reviewer (code), and Reviewer (contract) agents": Fixed. Correct.

---

Non-Blocking Suggestions

Two remaining instances of generic "reviewer" in coder-mode.md:

1. coder-mode.md:77 — generic "reviewer"

This helps you avoid repeating the same mistakes and understand what the reviewer flagged.

Should be: "...what reviewer_code or reviewer_contract flagged." (This line is about reviewing failed checkpoints — the feedback comes from specific review agents.)

2. coder-mode.md:105 — generic "reviewer" in code comment

# Poll for questions from tester, feedback from reviewer
egg-orch message poll

Should be: # Poll for questions from tester, feedback from reviewer_code/reviewer_contract

Same class of imprecision fixed everywhere else in the PR. These two were not among the locations specifically targeted in any previous review round.

— Authored by egg

[james-in-a-box]

Automated feedback loop has reached the maximum of 5 rounds. Human review is needed to make further progress on this PR.

[james-in-a-box]

egg review completed. View run logs

18 previous review(s) hidden.