Remove integrator, checker, and reviewer_unified agent roles

[james-in-a-box]

Summary

• Remove three agent roles per the new team roster (Define agent team roster, roles, and access controls #1030): integrator, checker, and reviewer_unified
• Absorb checker responsibilities (lint, type-check, auto-fix) into the tester role
• Update reviewer dependencies from integrator to tester

Changes

Removes integrator (standalone post-phase validator), checker (lint/type-check agent), and reviewer_unified (vestigial compat value) from all enum definitions, role registries, gateway restrictions, review graphs, attestation schemas, pipeline routes, documentation, and tests across 61 files (-2252/+352 lines).

The tester role now handles lint/type-check/auto-fix with expanded source file write access. Reviewer dependencies updated from INTEGRATOR → TESTER.

Issue: #1030

Test plan

• All 7811 tests pass (87 skipped, 0 failures)
• grep -rn "integrator\|checker\|reviewer_unified" --include="*.py" returns zero hits outside of function names like _build_checker_prompt and unrelated RepoVisibilityChecker
• Verify no runtime import errors by checking python -c "from egg_contracts.agent_roles import AgentRole; print(list(AgentRole))"

Authored-by: egg

[james-in-a-box]

Autofix tracking

{"Lint/Python": 3, "Test/Unit Tests": 2}

[egg-reviewer]

No agent-mode design concerns.

This is a role consolidation PR that simplifies the agent roster from 7 to 5 implement-phase agents. From an agent-mode design perspective:

• No anti-patterns introduced: No new pre-fetching, structured output requirements, post-processing pipelines, rigid procedures, prompt-level security, direct API calls, or hardcoded model IDs.
• Sandbox-enforced constraints preserved: The tester's expanded file access (absorbing checker patterns) is enforced via the gateway's AgentFilePattern system, consistent with the "sandbox is the constraint" principle.
• Reduced coordination complexity: Fewer agents (5 review edges → 4) means simpler BRC consensus with less overhead, aligning with avoiding unnecessary complexity.
• Integrator attestation verification removed: The integrator's post-consensus attestation cross-referencing (anti-sycophancy measure #4) is gone, but the remaining three measures (structured attestations, independent judgment sequencing, critical thinking prompts) are preserved. This is a reasonable tradeoff for the simplification gained.

— Authored by egg

[egg-reviewer]

Review: Remove integrator, checker, and reviewer_unified agent roles

Thorough review of all 61 changed files. The core removal is well-executed — business logic in the orchestrator, gateway, and shared contracts is correctly updated. However, there are 2 blocking issues and several non-blocking issues that should be addressed.

---

Blocking Issues

1. No backward compatibility for deserialization of persisted data with removed role values

Files: shared/egg_contracts/models.py, shared/egg_contracts/checkpoints.py

AgentRoleType and AgentType are strict StrEnum types. Any persisted contract JSON or checkpoint data that contains "integrator", "checker", or "reviewer_unified" will raise a ValidationError when Pydantic attempts to deserialize it. This affects:

• AgentExecutionModel.role (models.py:344) — old contracts with "role": "integrator" fail to load
• MultiAgentConfig.roles_enabled (models.py:398) — old contracts with removed roles in the list fail to load
• PhaseAgentConfig.roles (models.py:379) — same pattern
• CheckpointV2.agent_type and CheckpointSummaryV2.agent_type (checkpoints.py:235, 308) — old checkpoints with "integrator" or "checker" agent types fail to deserialize

Note: OrchestrationState.from_contract() (orchestration.py:99-107) has a try/except ValueError guard, but it never runs if the Contract model itself fails to construct.

Fix: Add field_validator(mode='before') on each affected field that either filters out unknown role strings (for lists) or maps them to a sensible fallback. For AgentType, map removed values to AgentType.UNKNOWN. For AgentExecutionModel, either skip the execution or map to a "legacy" sentinel.

2. Missing strict validation for tester-as-reviewer attestation

File: orchestrator/attestation_schemas.py:206-216

The tester is registered as a reviewer in REVIEWER_ATTESTATION_MODELS (line 87), but _validate_strict has no branch for role == "tester" in the is_producer=False (reviewer) path. When the tester submits a reviewer attestation in strict mode, no field validation occurs — it silently passes regardless of content.

The old checker role had strict validation requiring non-empty lint_results. Since the tester absorbs checker's review duties, there should be an equivalent strict check.

Fix: Add a branch in the reviewer side of _validate_strict:

elif role == "tester" and isinstance(instance, TesterAttestation):
    if instance.tests_run == 0:
        raise ValueError(
            "Tester reviewer attestation requires tests_run > 0 in strict mode"
        )

---

Non-Blocking Issues

3. sandbox/.claude/rules/mission.md — stale references (not updated by this PR)

This file is assembled into every agent's CLAUDE.md at container startup. It still contains:

• Line 187: ### Reviewer Workflow (reviewer_code, reviewer_contract, checker) — "checker" should be removed
• Line 235: Tester/documenter/checker/reviewer — "checker" should be removed
• Lines 236-237: "integrator notes the gap" / "integrator notes review gap" — stale

4. sandbox/.claude/rules/integrator.md — not deleted

This file still exists and references docs/reference/integrator-agent.md (which IS correctly deleted). Additionally, sandbox/entrypoint.py:811 still lists "integrator.md" in rules_order, so this stale file would be assembled into CLAUDE.md for integrator-roled agents (which no longer exist, so it's dead code, but should still be cleaned up).

5. sandbox/.claude/commands/README.md — stale references

Lines 43-46 and 53-56 still reference the deleted integrator-mode.md and checker-mode.md command files.

6. sandbox/.claude/commands/coder-mode.md — stale "checker" references

Lines 95, 109, 134 reference "checker" as a downstream agent waiting on coder output.

7. config/README.md — stale "checker" references

Lines 153 and 172 say "checker agent" and "checker step" where they should now say "tester".

8. gateway/README.md — stale references

Line 92 still references INTEGRATOR_TIER3_PATTERNS and the integrator role. Line 457 still lists test_integrator_tier3.py in the directory tree.

9. gateway/phase_filter.py:943 — stale docstring

Docstring says "Tester: test files only" but the tester now has source code write access for auto-fix.

10. shared/README.md:260 — stale reference

Still says "tier-aware integrator access for Tier 3".

11. docs/architecture/orchestrator.md:442 — stale reference

Says "Sandbox checker" where it should say "Sandbox tester".

12. Stale naming in orchestrator code

• orchestrator/routes/pipelines.py:6715,6729: Variable named checker_env but sets role to "tester" — should be tester_env
• orchestrator/routes/pipelines.py:5312: Function _build_checker_prompt (deprecated, not called from production code, but inconsistent naming)
• orchestrator/devserver.py:904: Method attach_checker with correct docstring saying "tester" — method name should match

13. shared/egg_contracts/orchestration.py:368 — stale docstring

Says "defaults to the 4 implement-phase roles" but the default is now 3 roles.

14. Stale test comments

• orchestrator/tests/test_brc_nack_iteration.py:96: Comment says "checker ACKs" but code uses reviewer_contract
• orchestrator/tests/test_peer_consensus_integration.py:451,462,507,532: Multiple comments reference "checker" where code uses rev_extra
• gateway/tests/test_phase_filter_tier3.py:5,12: References "Non-integrator roles"

15. Missing test: tester writing to .egg-state/reviews/

The old TestCheckerRole had test_can_write_reviews but the new TestTesterRole does not have an equivalent test for writing to .egg-state/reviews/ (the BRC concurrent mode path).

---

Summary

#	Severity	Area	Issue
1	Blocking	shared/egg_contracts/	No backward compat for persisted data with removed role values
2	Blocking	orchestrator/attestation_schemas.py	Tester-as-reviewer strict validation gap
3-6	Non-blocking	sandbox/	Stale references in agent instruction files
7-11	Non-blocking	docs, config, gateway	Stale references in documentation
12-14	Non-blocking	orchestrator	Stale variable/function names and comments
15	Non-blocking	gateway tests	Missing tester review-write test

The core business logic — pipeline launch, dispatch, review graph, DAG visualization, gateway restrictions, role mapping — is all correctly updated. The blocking items are (1) a runtime breakage when loading old persisted data and (2) a validation gap that undermines BRC anti-sycophancy guarantees.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

Re-review after force-push (previous review at a09fd38). The agent-mode design aspects are unchanged — the force-push addressed backward compatibility validators and tester strict validation, which are general code concerns, not agent-mode design.

Key agent-mode design observations (unchanged from previous review):

• Sandbox-enforced constraints preserved: Tester's expanded file access (absorbing checker's patterns) is enforced via AgentFilePattern and gateway restrictions — consistent with "sandbox is the constraint" principle.
• Reduced BRC coordination complexity: Fewer agents (5 review edges → 4) means simpler consensus with less overhead.
• No anti-patterns introduced: No new pre-fetching, structured output requirements, post-processing pipelines, rigid procedures, or direct API calls.
• Agent instructions updated consistently: mission.md, coder-mode.md, and the BRC preamble builder all correctly remove checker/integrator references.

— Authored by egg

[egg-reviewer]

Re-review: Remove integrator, checker, and reviewer_unified agent roles

Reviewed delta from a09fd38 → b1cc4ed (21 files changed). Both blocking issues from the previous review are properly fixed. The backward-compatibility validators are correct and the tester-as-reviewer strict validation gap is closed.

---

Blocking Issues — Resolution Verified

1. Backward compatibility for deserialized data ✅

All four validators are correctly implemented:

• Contract._filter_removed_agent_roles (models.py:474): Filters at the list level before Pydantic constructs AgentExecutionModel, avoiding ValidationError on removed role values. The not isinstance(item, dict) passthrough is correct for mode="before" — non-dict items (pre-constructed models) will already have valid roles.
• PhaseAgentConfig._filter_removed_roles and MultiAgentConfig._filter_removed_roles (models.py:386, 412): Filter logic not isinstance(r, str) or r in valid correctly handles both raw strings from JSON and StrEnum instances (which are isinstance(str) and compare against the valid set).
• CheckpointV2._coerce_removed_agent_types and CheckpointSummaryV2._coerce_removed_agent_types (checkpoints.py:272, 340): Correctly map unrecognized string values to AgentType.UNKNOWN.value, returning a string for the mode="before" phase so Pydantic's subsequent validation coerces it to the enum.
• OrchestrationState.from_contract() (orchestration.py:99-107): try/except ValueError safety net retained as defense-in-depth.

2. Tester-as-reviewer strict validation ✅

New branch at attestation_schemas.py:217-221 validates tests_run > 0 for tester in the reviewer path. This matches the suggested fix exactly.

---

Previous Non-Blocking Issues — Status

#	Issue	Status
3	mission.md stale references	✅ Fixed
4	integrator.md not deleted	✅ Deleted
5	commands/README.md stale references	✅ Fixed
6	coder-mode.md stale references	⚠️ Partial — line 141 still says "Checker"
7	config/README.md stale references	✅ Fixed
8	gateway/README.md stale references	✅ Fixed
9	phase_filter.py stale docstring	✅ Fixed
10	shared/README.md stale reference	✅ Fixed
11	orchestrator.md stale reference	✅ Fixed
12	Stale naming in orchestrator code	✅ All three renamed
13	orchestration.py stale docstring	✅ Fixed (4 → 3)
14	Stale test comments	✅ All flagged instances fixed
15	Missing tester review-write test	✅ test_can_write_reviews added

---

New Non-Blocking Issues

16. sandbox/.claude/commands/coder-mode.md:141 — leftover from issue #6

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

Should remove "Checker" from this list.

17. JSON schemas not updated

.egg/schemas/contract.schema.json still lists removed roles in enum definitions (lines 697, 779) and — more importantly — includes "integrator" in the default roles_enabled (line 781):

"default": ["coder", "tester", "documenter", "integrator"]

Similarly, .egg/schemas/checkpoint.schema.json (lines 67, 148) and .egg/schemas/usage.schema.json (line 126) still include removed role values.

The Python validators handle this gracefully at runtime, so it won't cause failures. But external tools consuming these schemas will generate contracts with stale defaults.

18. README.md — stale architecture description

Lines 44-45 still show ✓ checker and ✓ integrator in the ASCII diagram. Line 87 describes Tier 3 as "coder → tester → documenter → checker → code reviewer" with an integrator step.

19. orchestrator/README.md — stale role references

Line 51 lists "Checker, Integrator" in the implement phase agents. Line 65 describes Tier 3 with the old agent sequence.

20. sandbox/README.md:86 — references deleted integrator-mode.md

The file tree listing still shows ├── integrator-mode.md.

21. No tests for backward-compat validators

The new validators in models.py and checkpoints.py have no dedicated test coverage. A test that deserializes a contract containing "role": "integrator" in agent_executions and verifies it's silently dropped would provide confidence against regressions.

22. Test files still use "checker" as a role name

orchestrator/tests/test_peer_consensus_integration.py (lines 1110-1363) has ~8 test functions that set up review graphs with ReviewEdge("checker", ...) and t.register_agent("checker"). orchestrator/tests/test_decisions_routes.py (lines 658-692) tests HITL decision handling with "failed_role:checker". These tests pass because the BRC tracker is string-based, but they test scenarios that can no longer occur in production.

---

Verdict

Both blocking issues are resolved with correct, well-structured code. The backward-compat validators handle all realistic code paths. Approving — the remaining items are documentation and test cleanup that can be addressed in a follow-up.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

Re-review after feedback-addressing commit (183302c). The delta since b1cc4ed is cleanup work — removing stale checker/integrator references from agent instructions (mission.md, coder-mode.md, commands/README.md), deleting checker-mode.md and integrator-mode.md, removing integrator.md from entrypoint.py rules assembly, and renaming orchestrator functions/variables from "checker" to "tester". No new agent-mode design patterns or anti-patterns introduced.

Agent instruction files are now internally consistent with the reduced 5-agent roster. The BRC preamble builder is unchanged and correctly omits removed roles.

— Authored by egg

[james-in-a-box]

egg review failed. View run logs

21 previous review(s) hidden.

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflicts with main (35 conflicted files). The primary source of conflicts was commit e4bca17 on main ("Remove per-phase dispatch, short-circuit, and integrator role #1184"), which overlapped with this PR's scope by:

1. Removing the integrator role (same as this PR)
2. Adding a new checker role (which this PR removes)
3. Removing all Tier 3/dispatch code (this PR had modified some of that code)

Conflict Categorization & Resolution

File(s)	Category	Resolution
9 files (dispatch.py, tier3 tests, etc.)	Modify/delete (deleted on main)	Accepted deletion — both sides want this code gone
shared/egg_contracts/agent_roles.py (8 conflicts)	Semantic — CHECKER role + reviewer deps	Removed CHECKER (PR intent); kept TESTER in reviewer dependencies (PR's core change); removed complexity_tier docstring (Tier 3 removed)
shared/egg_contracts/models.py (3 conflicts)	Semantic — CHECKER enum + Tier 3 models	Removed CHECKER; dropped PhaseAgentConfig/MultiAgentConfig (Tier 3); kept backward compat validator for removed roles
shared/egg_contracts/__init__.py	Additive — PhaseAgentConfig export	Removed export (class dropped)
shared/egg_contracts/orchestration.py	Formatting — docstring wording	Accepted main's simpler docstring (no Tier 3 references)
gateway/agent_restrictions.py (3 conflicts)	Semantic — CHECKER role + Tier 3 params	Removed CHECKER; removed complexity_tier param; used main's simpler implementation
gateway/tests/test_agent_restrictions_gh.py	Additive — CHECKER in test lists	Removed CHECKER from expected roles
tests/gateway/test_agent_restrictions.py	Semantic — tester source write perms	Kept HEAD: tester can write source files (absorbed checker's auto-fix responsibility)
orchestrator/dag_visualizer.py (2 conflicts)	Semantic — Tier 3 rendering code	Removed entire Tier 3 sub-phase rendering section (471 lines); kept PR's comment wording
orchestrator/routes/pipelines.py (4 conflicts)	Semantic — Tier 3 dispatch + BRC	Removed Tier 3 phase-scoped prompt (1011 lines); accepted main's BRC execution model; accepted main's BRC reviewer handling
orchestrator/tests/ (5 files)	Mixed — Tier 3 tests + CHECKER + comments	Removed Tier 3 test classes; removed CHECKER assertions; accepted main's wording
docs/ (8 files, ~15 conflicts)	Additive/formatting — CHECKER mentions + Tier 3 docs	Removed all CHECKER references; removed Tier 3 documentation sections; kept main's BRC-focused wording
.egg/schemas/contract.schema.json	Semantic — Tier 3 schema fields	Removed phase_id field and multiAgentConfig schema (Tier 3)
README.md	Additive — CHECKER in diagram + Tier 3 section	Removed CHECKER from pipeline diagram; removed Tiered Dispatch section
sandbox/.claude/rules/mission.md	Formatting — agent failure handling	Removed CHECKER; kept main's "proposal" wording

Post-Merge Fixes

Fixed 2 tests that referenced removed checker role / Tier 3 models:

• orchestrator/tests/test_removal_validation_1165.py — removed assert "checker" in role_values
• shared/egg_contracts/tests/test_backward_compat_validators.py — removed imports and test classes for PhaseAgentConfig/MultiAgentConfig

Verification

• Lint: All checks pass (ruff check, ruff format, mypy, shellcheck, custom checks)
• Tests: 7363 passed, 87 skipped, 0 failures

Reviewer Notes

Please review these semantic resolutions:

1. Reviewer dependencies on TESTER (agent_roles.py): The PR adds TESTER as a dependency for reviewer_code and reviewer_contract, while main had removed it. Kept the PR's version since this is a core PR change (tester absorbed checker responsibilities, so reviewers should wait for tester).

2. Tester source file write access (test_agent_restrictions.py): The PR allows tester to write src/module.py (for auto-fix), while main restricts tester from source dirs. Kept PR's version since tester now handles lint/type-check/auto-fix.

3. Backward compat validator (models.py): Kept the _filter_removed_agent_roles validator on Contract.agent_executions which silently drops old integrator/checker/reviewer_unified executions from persisted data.

— Authored by egg

[james-in-a-box]

egg conflict resolution completed. View run logs