# Agent Coordination

Quality principles for dispatching and managing subagents.

## Agent Roles

| Role | Agents | Output type | Review needed? |
|------|--------|-------------|---------------|
| **Builder** | builder | Production code + tests via TDD | Yes — quality-code-review after each task |
| **Producer** | architect, doc-writer | Artifacts that downstream work depends on | Yes — errors cascade |
| **Reviewer** | critic, code-reviewer, craft-reviewer, security-reviewer, spec-reviewer | Assessments and verdicts | No — output IS the review |
| **Executor** | e2e-runner | Objective results (pass/fail) | No |
| **Investigator** | tracer | Hypotheses and analysis | No — feeds back to main agent |
| **Recall** | gotcha-hunter | Diff-relevant entries from the gotcha catalogs | No — output is a citation list, not a judgment |

## Pre-Dispatch Requirements

- API contracts defined before dispatching parallel builders or producers
- Provide task objective, scope, and success criteria in the prompt
- Builder agents need architecture artifacts inline in the prompt (`.forge/` is not in version control — worktree checkouts won't have them)
- Define clear expected outputs to verify delivery

## Communication

- Main agent bridges ALL inter-agent communication — subagents never talk directly
- If a subagent discovers a dependency on another's work, STOP and ask the main agent

## Review Gates

Producer output is reviewed in two stages — first `spec-reviewer` for compliance against the source artifact (requirements / architecture / design), then the `quality-code-review` chain (safety → craft → reachability → gotcha-hunter) for code quality. The `critic` agent reviews planning artifacts before they are presented to the user.

## Review agents — quick decision matrix

Dispatch one agent per row. Reviewers stay in their own lane — overlapping concerns are split across passes (e.g. safety vs craft), not duplicated.

| Agent | When to dispatch | What it reviews | What it does NOT review |
|---|---|---|---|
| `code-reviewer` | Safety floor — every code change before merge | Per-line ship-blockers: SQL injection, race conditions, missing auth checks on a controller, secret exposure in code, injection sinks, data-loss risk, unhandled async errors, missing cookie flags | Style / idioms / stubs (handed to `craft-reviewer`); system-level threat modeling, supply chain, auth-model defensibility, crypto primitives, multi-tenant isolation (handed to `security-reviewer`) |
| `craft-reviewer` | Pass 2 of code review — runs after `code-reviewer` passes | Library idiom adherence (Context7-verified), codebase pattern conformance, stub / placeholder detection | Safety vulnerabilities (Pass 1 owns those), spec compliance, plan quality |
| `security-reviewer` | Depth pass on top of the safety floor — code touches auth model, payments, PII, sessions, multi-tenant data, crypto, external integrations, or the dependency tree (or `/security-review` is invoked) | Threat modeling, supply-chain / dependency risk, auth-model defensibility (rotation, hashing algorithm, MFA, account recovery), multi-tenant isolation enforcement, crypto primitives, CSRF / CORS / security headers, deserialization / XXE, logging & monitoring posture — with realistic exploit scenarios | Per-line safety bugs already covered by the floor (one missing auth check, one concatenated query, one hardcoded secret) — flagged as "possible safety-floor miss" rather than duplicated |
| `spec-reviewer` | Producer output (implementation, architecture, requirements) needs to be checked against its source artifact | Completeness and correctness against the spec: missing scope, undocumented features, requirement drift | Code quality (handed off to the `quality-code-review` chain), runtime behavior |
| `prototype-reviewer` | Every ~5 iteration cycles, on demand, or before Phase 5 (codify) starts | Running prototype vs locked wireframe drift: missing screens, undocumented features, interaction-model deviations, convention drift | Production-grade code quality, security; does not fix anything (flag + classify + recommend only) |
| `critic` | Plans, architecture, design artifacts, or code that needs an adversarial second look (broader scope than `code-reviewer`'s safety-floor pass) | Adversarial gap analysis on plans + code: missing requirements, infeasibility, weak assumptions, multi-perspective challenges (executor / skeptic / stakeholder for plans; security / new-hire / ops for code), evidence quality | Routine quality-gate code review (use `code-reviewer` for ship-blocking safety and `craft-reviewer` for idiom/style) |
| `gotcha-hunter` | Session-start (status scope) and as the final pass of `quality-code-review` after the reachability gate | Recall: which entries in project + global gotcha catalogs match the current diff or session | Judgment — does not propose fixes, re-rank severity, or invent new gotchas |

## Model Routing

| Complexity | Model | Examples |
|-----------|-------|---------|
| Simple | Haiku | Formatting, simple refactors |
| Standard | Sonnet | Implementation, tests, code review, docs |
| Complex | Opus | Architecture, multi-perspective critique, cross-system design |

## Worktree Isolation

`isolation: "worktree"` is a **safety mechanism** preventing file interference, not a speed mechanism. Agents MUST commit changes; merge via `git merge --no-ff`, never `cp`. See `build-pr-workflow` for the full protocol.

## Coupling Rules

- If two tasks share a DB table, API endpoint, state, or modify the same file — serialize them
- Independent modules (separate APIs, separate UI components) can run in parallel
- When in doubt, serialize — correctness over speed
- **File-overlap = serialize.** If two tasks in the same phase create or modify the same file, they MUST be in different phases. This is validated during task decomposition. A merge conflict at build time means decomposition failed — resolve the conflict, then re-serialize remaining related tasks in the current and future phases.

## Agent Self-Containment

Agents carry their own execution methodology inline in their definition. They do not load skills via frontmatter (`skills:` field).

- Skills are for human or inline sessions — they define process and can dispatch agents
- Agents are execution units — they receive task scope + artifacts and follow their built-in protocol
- Dispatch prompts pass task + artifacts only, not methodology (the agent already has it)

This prevents circular loading (skill dispatches agent, agent loads skill that dispatches agent) and keeps each agent's context focused on the task.

## Failure Handling

- If a subagent fails, diagnose before retrying
- Review partial results before discarding
- Report failures to the user with context