Le review-manager : anatomie d'un orchestrateur de code review

La review de code générée par IA a un problème structurel qui est rarement nommé. Ce n’est pas que les modèles écrivent du mauvais code. C’est que quand on leur demande ensuite de l’évaluer, ils tendent vers l’approbation. Pas parce qu’ils sont complaisants — parce qu’un modèle qui a construit une représentation interne d’un bout de code pour le produire l’évalue depuis cette même représentation. Les biais sont identiques. Les angles morts aussi.

L’article opencode-team-lead pose le diagnostic : un agent qui review son propre code ne fait pas de la review, il se valide. La solution qu’il décrit — déléguer la review au review-manager — mérite qu’on rentre dedans. Parce que “déléguer à un autre agent” ne suffit pas si cet autre agent fait la même chose de manière moins structurée.

Cet article est un deep dive dans la mécanique du review-manager. On va lire le code des agents.

Le double rôle : orchestrateur, jamais reviewer

La première chose à comprendre sur le review-manager, c’est ce qu’il n’est pas. Il n’est pas un reviewer amélioré. Il ne lit pas le code pour l’évaluer. Son mandat est explicite dans son system prompt :

You never review code yourself. You read enough to understand what changed 
and select the right reviewers. Then you delegate. Your job is reviewer 
selection, prompt crafting, verdict synthesis, and disagreement arbitration.

Il lit juste assez pour choisir qui envoyer. C’est une distinction qui compte : entre “lire pour comprendre le changement” et “lire pour juger la qualité”. La première informe la délégation. La seconde, c’est déjà de la review — avec tous les biais que ça implique.

Les permissions du review-manager reflètent exactement ça. Il a accès à task pour déléguer, et à question pour interroger le team-lead si la mission est ambiguë. C’est tout. Pas de read, pas d’edit, pas de grep. Il peut recevoir des descriptions de fichiers dans son prompt — mais il ne peut pas aller fouiller la codebase lui-même.

C’est le même principe que le team-lead : la contrainte de permission est plus fiable qu’une instruction dans un prompt. Un prompt peut être mal suivi. Une permission refusée est déterministe.

Anatomie du cycle

Le cycle complet en cinq phases — analyse, sélection, spawn parallèle, confrontation, synthèse :

sequenceDiagram
    participant TL as team-lead
    participant RM as review-manager
    participant RR as requirements-reviewer
    participant CR as code-reviewer
    participant SR as security-reviewer

    TL->>RM: Mission (fichiers modifiés + user request original)
    Note over RM: 1. Analyse le changement
    Note over RM: 2. Sélection des reviewers
    par Spawn parallèle
        RM->>RR: Prompt self-contained
        RM->>CR: Prompt self-contained
        RM->>SR: Prompt self-contained
    end
    RR-->>RM: Verdict + issues
    CR-->>RM: Verdict + issues
    SR-->>RM: Verdict + issues
    Note over RM: 3. Confrontation Protocol
    RM-->>TL: APPROVED | CHANGES_REQUESTED | BLOCKED

Les phases 1 et 2 sont les décisions critiques — une fois les reviewers sélectionnés et leurs prompts envoyés, le review-manager attend. La vraie valeur ajoutée est dans la phase 3 : la confrontation des verdicts. On y reviendra.

Sélectionner les reviewers : un arbre de décision, pas une table

Le mapping type de changement → reviewers ressemble à une table de dispatch dans la documentation. En pratique, c’est un arbre de décision avec des priorités qui se chevauchent.

Les deux axes : taille et risque

Le point de départ n’est pas le type de changement — c’est la combinaison taille × risque.

Le cap à 3 reviewers techniques n’est pas arbitraire — c’est le point de diminishing returns documenté. Au-delà, les reviewers supplémentaires répètent des findings déjà remontés ou génèrent du bruit dans la synthèse.

Les high-risk patterns : une surcharge qui passe avant le tableau

Certains types de changements déclenchent automatiquement le security-reviewer, quelle que soit la taille du diff :

• Auth, sessions, tokens
• Requêtes SQL ou appels ORM
• Opérations cryptographiques
• Gestion de permissions ou contrôle d’accès
• Secrets, credentials, API keys
• Appels externes transmettant des données utilisateurs
• Intégration LLM (vecteurs d’injection de prompt)

Si ton changement de deux lignes touche un handler d’authentification, il passe en Trivial + Élevé — pas en fast path. La taille ne réduit pas le risque.

Le fast path : combined reviewer

Pour les changements triviaux à faible risque, spawner trois agents pour reviewer 30 lignes est disproportionné. Le fast path spawn un seul code-reviewer avec un mandat étendu. La seule modification dans son prompt :

## Context
Also verify requirements alignment for this review: does the implementation 
match the original user request stated below?

[reste du prompt standard]

Une seule ligne dans le ## Context, et le code-reviewer absorbe le mandat du requirements-reviewer. C’est la seule exception à la règle d’isolation des reviewers — et elle n’existe que pour les changements où le coût de l’overhead surpasse clairement la valeur de la séparation.

flowchart TD
    A[Analyser le changement] --> B{Docs-only / Formatting ?}
    B -->|Oui| C[Fast-exit\nAPPROVED immédiat]
    B -->|Non| D{High-risk patterns ?}
    D -->|auth / SQL / crypto\n/ secrets / LLM| E[security-reviewer obligatoire]
    D -->|Non| F{Taille du changement ?}
    E --> F
    F -->|Trivial\n1-2 fichiers, moins de 50 lignes| G{High-risk ?}
    G -->|Non| H[Fast path\n1 combined reviewer]
    G -->|Oui| I[requirements + security + code\n3 agents]
    F -->|Normal\n3 à 10 fichiers| J[requirements + code + 1 domaine]
    F -->|Large\n10+ fichiers| K[requirements + code + 2 domaines]
    J --> L{High-risk ?}
    K --> L
    L -->|Oui| M[+ security-reviewer\ncap 4 agents]
    L -->|Non| N[3 agents]

Le requirements-reviewer : hors cap, toujours présent

Le requirements-reviewer est obligatoire sur tout changement non-trivial. La seule exception : les changements de formatting ou de typos purs sans requirement fonctionnel associé.

Et il ne compte pas dans le cap de 3 reviewers techniques. La logique est simple : vérifier que l’implémentation correspond à ce qui était demandé n’est pas une dimension technique parmi d’autres — c’est la condition préalable à toute review. Un code parfait qui implémente la mauvaise feature est un échec complet. Le requirements-reviewer est là pour ça, indépendamment de combien d’autres reviewers sont présents.

Le prompt self-contained : l’isolation délibérée

Chaque reviewer reçoit un prompt structuré en trois sections exactes :

## Context
[Ce qui a changé, par quel agent, et pourquoi. 
La user request originale verbatim pour que le reviewer puisse vérifier l'intent.]

## Changed Files
[Chaque fichier modifié avec un résumé d'une ligne de ce qui a changé.
Inclure les chemins complets.]

## Out of Scope / Trade-offs
[Ce qui a été explicitement exclu. Les trade-offs intentionnellement faits. 
Ce que le reviewer NE DOIT PAS flaguer comme problème.]

Trois points à retenir dans ce format.

La user request originale verbatim. Pas un résumé, pas une paraphrase — le texte exact. Le requirements-reviewer ne peut pas faire son travail sans ça. C’est assez critique pour être inscrit comme règle cardinale dans son system prompt :

If the original requirements are absent from your mission, return BLOCKED immediately:

Verdict: BLOCKED
Reason: Original requirements not provided. Cannot perform functional 
        compliance review.
Action required: review-manager must include the original user request 
                 verbatim before spawning this reviewer.

Ce n’est pas un BLOCKED de code — c’est un BLOCKED de process. Le review-manager le traite différemment des autres BLOCKED (voir la section arbitrage).

L’isolation délibérée. Les reviewers ne savent pas qu’ils sont plusieurs. Ils ne voient pas les verdicts des autres. Cette isolation n’est pas un oubli — c’est une décision de design. Un reviewer qui sait que security-reviewer a déjà approuvé est biaisé vers l’approbation. Un reviewer qui sait que code-reviewer a trouvé des problèmes majeurs va chercher des problèmes supplémentaires pour ne pas paraître moins rigoureux. L’indépendance des verdicts dépend de l’isolation des contextes.

La section Out of Scope. C’est la section la plus sous-estimée. Elle empêche les reviewers de flaguer des trade-offs intentionnels comme des bugs. Sans elle, un reviewer qui voit “pas de tests pour ce module” peut remonter ça comme un issue — alors que c’était une décision délibérée. Cette section donne aux reviewers le contexte pour distinguer “oublié” de “intentionnel”.

Les trois reviewers : périmètres distincts, stance commune

Les trois reviewers partagent une ligne dans leur system prompt :

Your default is skepticism. When you identify an issue, report it — do not 
rationalize it away. If something looks wrong, flag it even if uncertain. 
The review-manager arbitrates severity; your job is to surface, not to filter.

C’est une séparation de responsabilité fondamentale. Les reviewers n’arbitrent pas — ils remontent. C’est le review-manager qui décide si un finding mérite d’être dans le rapport final et à quelle sévérité. Cette séparation évite deux travers classiques : le reviewer qui minimise ses propres findings pour ne pas paraître trop sévère, et le reviewer qui escalade tout en critique pour paraître rigoureux.

Leurs périmètres respectifs sont strictement définis et non-chevauchants.

requirements-reviewer

Question unique : l’implémentation correspond-elle à ce qui était demandé ?

Pas de jugement de qualité. Pas de sécurité. Uniquement la conformité fonctionnelle. Son workflow est littéral : il extrait chaque requirement discret de la user request, les décompose en items atomiques, et mappe chacun sur l’implémentation.

| Requirement | Covered? | Evidence                   |
|-------------|----------|----------------------------|
| [exigence]  | Yes      | src/api/users.ts:42        |
| [exigence]  | Partial  | happy path ok, edge case X manquant |
| [exigence]  | No       | not found                  |

Quatre catégories de findings : feature manquante, mauvaise interprétation, implémentation partielle, scope creep. Ce dernier est particulièrement intéressant — le reviewer flag explicitement quand l’implémentation fait quelque chose qui n’était pas demandé, même si c’est techniquement une amélioration. Une feature non demandée peut modifier un comportement existant — c’est un risque réel.

code-reviewer

Question unique : ce code est-il techniquement sain ?

Logique, error handling, edge cases, API design, patterns, maintenabilité, test coverage. Son system prompt contient une checklist concrète, pas une liste de principes abstraits :

- [ ] Null/undefined not guarded where inputs are uncontrolled
- [ ] Errors swallowed silently (catch with empty body or generic log)
- [ ] Off-by-one in loops, index access, range checks
- [ ] Missing validations on inputs (type, range, presence)
- [ ] Async errors not awaited or not caught
- [ ] Functions doing too many things (single-responsibility violation)
- [ ] Dead code or unreachable branches
- [ ] Naming that doesn't match behavior (isValid that throws, get that mutates)
- [ ] Inconsistent patterns vs. the rest of the codebase
- [ ] Missing test for new logic (when tests exist in the project)

Pas de sécurité, pas de conformité fonctionnelle. Quand il croise un problème d’injection SQL, il le laisse au security-reviewer. Une feature manquante, c’est le territoire du requirements-reviewer. Le code-reviewer qui déborde sur les périmètres des autres génère des duplications de findings que le review-manager doit ensuite dédupliquer.

security-reviewer

Question unique : ce changement introduit-il un risque de sécurité ?

Avant de regarder quoi que ce soit, il mappe la surface d’attaque :

Does it handle user input?
Does it interact with auth, sessions, or tokens?
Does it read/write to a database or filesystem?
Does it call external services?
Does it handle secrets or credentials?
Does it expose new API endpoints or modify existing ones?

Si la réponse à toutes ces questions est non, le changement est low-risk. Si plusieurs sont oui, scrutiny complet. La checklist couvre injection (SQL, shell, prompt), auth/authz, data exposure, input validation, secret handling, supply chain, et infra misconfigs.

Une règle spécifique sur l’auth : si le changement touche l’authentication ou la cryptographie, le reviewer doit l’expliciter dans ses Positive Notes, même s’il n’a rien trouvé :

"Reviewed auth/token handling — no issues detected."

Un silence n’est pas une validation. L’absence de finding doit être explicite — sinon le review-manager ne peut pas savoir si la zone a été couverte ou oubliée.

Le protocole de confrontation : arbitrer les désaccords

C’est la phase la plus intéressante. Après que tous les reviewers ont rendu leurs verdicts, le review-manager doit les synthétiser en un verdict unique. Sur les cas d’accord unanime, c’est trivial. Sur les désaccords, c’est là que la valeur réelle est produite.

Les heuristiques d’arbitrage sont hiérarchiques :

flowchart TD
    A[Verdicts reçus — désaccord] --> B{requirements-reviewer\nflagge non-conformité ?}
    B -->|Oui| C{Vraie non-conformité\nou mauvaise interprétation ?}
    C -->|Non-conformité réelle| D[BLOCKED\nimplémentation hors-sujet]
    C -->|Mauvaise interprétation| E[Sided with approbateur\nDocumenter le raisonnement]
    B -->|Non| F{Issue Critical\nchez n'importe quel reviewer ?}
    F -->|Oui| G[Critical always wins\nmême si les autres approuvent]
    F -->|Non| H{security-reviewer\nflagge quelque chose ?}
    H -->|Oui| I{False positive\névident ?}
    I -->|Oui| J[Sided with approbateur\nDocumenter]
    I -->|Non| K[Security concern\ngagne le tie]
    H -->|Non| L{Désaccord uniquement\nsur des Minor issues ?}
    L -->|Oui| M[Sided with approbateur\nMention optionnelle]
    L -->|Non| N[Jugement sur les mérites\nou escalade au team-lead]

Quelques cas particuliers qui méritent d’être détaillés.

Le process failure du requirements-reviewer

Si le requirements-reviewer retourne BLOCKED avec Reason: Original requirements not provided, c’est un BLOCKED de process — pas un BLOCKED de code. Le review-manager ne propage pas ce BLOCKED au team-lead. Il demande les requirements manquants via question et re-spawne uniquement le requirements-reviewer. L’équipe ne voit pas un faux BLOCKED sur son dashboard — elle voit une review qui a nécessité une clarification.

C’est une distinction importante. Un BLOCKED de code dit “l’implémentation est fondamentalement fausse”. Un BLOCKED de process dit “on n’avait pas les informations pour faire la review”. Les traiter identiquement conduirait à stopper des sessions pour des raisons administratives.

Security wins ties

Quand security-reviewer et code-reviewer sont en désaccord sur un même point — l’un approve, l’autre remonte un problème de sécurité — le security concern gagne par défaut, sauf si c’est clairement un false positive.

“Clairement” est le mot qui compte. En cas de doute, c’est le security concern qui gagne. Le coût asymétrique entre ignorer une vraie vulnérabilité et adresser un faux positif de sécurité plaide pour ce biais.

Les findings dupliqués

Si code-reviewer et security-reviewer remontent tous les deux le même problème de validation d’input — ce qui arrive sur les changements qui touchent des zones de front — le rapport final contient un seul bullet point. Le framing du security-reviewer avec sa sévérité prime, l’attribution indique les deux sources. Un seul item, pas deux doublons qui confusent la priorisation.

Quand la mauvaise chose a été construite

Le cas le plus brutal : le requirements-reviewer retourne BLOCKED parce que l’implémentation résout un problème différent de celui qui était demandé. Pas une nuance — la mauvaise feature.

Sauf si le review-manager identifie que c’est une mauvaise interprétation des requirements (auquel cas il documente son raisonnement et sided with l’approbateur), c’est un BLOCKED qui passe au team-lead sans modification. L’escalade vers l’humain est immédiate.

Les seuils de verdict et le biais par défaut

Trois verdicts possibles, avec des critères précis.

BLOCKED — une issue critique sans chemin safe vers l’avant sans input utilisateur, une implémentation fondamentalement hors-sujet, ou une vulnérabilité critique de sécurité. Le changement ne doit pas avancer.

CHANGES_REQUESTED — des issues major ou minor qui peuvent être corrigées sans refonte architecturale. Les requirements sont atteints mais l’implémentation a des gaps de qualité ou de correction.

APPROVED — tous les reviewers sans issue critique ou major, requirements atteints, aucune question ouverte.

Et le biais structurel inscrit dans les instructions :

When in doubt between APPROVED and CHANGES_REQUESTED: default to 
CHANGES_REQUESTED. The cost of a false approval is higher than the cost 
of an extra fix cycle.

Ce n’est pas de la paranoïa — c’est de l’asymétrie de coût. Un cycle de fix supplémentaire coûte quelques minutes d’exécution d’agent. Un false approval qui passe en production peut coûter beaucoup plus. Le review-manager est calibré pour favoriser le faux négatif sur le faux positif.

Le format de sortie

Le format est strict et fixe — aucune variation tolérée, parce que le team-lead parse ce format pour savoir quoi faire ensuite :

## Review Summary

**Verdict**: APPROVED | CHANGES_REQUESTED | BLOCKED

### Reviewers
- requirements-reviewer — APPROVED — requirements couverts
- code-reviewer — CHANGES_REQUESTED — error handling manquant sur l'endpoint /api/users
- security-reviewer — APPROVED — pas de surface d'attaque introduite

### Issues

#### Critical
- **[titre]** (source: code-reviewer)
  [Description de ce qui est incorrect]
  **Suggested fix:** [Correction concrète]

#### Major
[...]

#### Minor
[...]

### Disagreements
[Expliquer les deux positions, l'arbitrage, et pourquoi on a siding avec l'un.]

### Positive Notes
[Consolidé de tous les reviewers — ce qui a été bien fait.]

Les issues sont groupées par sévérité, pas par reviewer. Le team-lead s’intéresse à “qu’est-ce qui est critique” — pas à “qui a dit quoi”. L’attribution de source reste présente pour tracer si besoin, mais c’est la sévérité qui structure la lecture.

Résilience aux pannes et calibration

Quand un reviewer fail

Un reviewer peut rentrer avec un output incomplet, avoir été compacté en plein milieu de sa review, ou sortir un format inutilisable. Le protocole :

1. Retry once — reformuler le prompt, réduire le scope si le reviewer a compacté, clarifier le format attendu
2. Si le retry fail — continuer sans ce reviewer avec les résultats disponibles
3. Documenter le gap dans le rapport final :

⚠ security-reviewer failed to complete (compaction). Security review not performed.
Recommend a dedicated security pass before merging.

Partial review > no review. Mais pas sans documenter ce qui manque. Le team-lead doit savoir qu’un angle n’a pas été couvert — et potentiellement décider de bloquer manuellement jusqu’à ce qu’il le soit.

Calibrer les reviewers à ta codebase

Les reviewers par défaut ont une stance générique — utile pour démarrer, insuffisante pour une codebase qui a ses propres patterns et anti-patterns spécifiques.

Si les verdicts sont systématiquement trop indulgents ou trop sévères pour ta codebase, la bonne approche est de modifier les system prompts des reviewers individuels avec :

• Anti-patterns nommés explicitement — pas “vérifie les problèmes de performance” mais “cherche les N+1 queries sur les relations Prisma avec include imbriqué dans les handlers de liste”
• Few-shot examples — exemples de bons vs. mauvais verdicts sur des cas réels de ta codebase
• Critères pondérés — si la couverture de tests est critique pour toi, l’indiquer explicitement avec les patterns attendus

Un point sous-estimé : recalibrer après un upgrade de modèle. Un prompt tuné pour une version peut être trop agressif ou trop complaisant sur la suivante. Les comportements de base des modèles changent entre versions, et les reviewers héritent de ces changements. Ce qui était une stance correctement calibrée peut devenir trop strict ou trop laxiste sans que rien n’ait changé dans ton code.

Le review-manager opère en mode subagent dans OpenCode — invisible dans la liste des agents, existant uniquement pour être invoqué. C’est précisément ce qui le rend intéressant à disséquer : toute sa valeur est dans sa mécanique interne. Un orchestrateur qui ne prend aucune décision de production, ne lit aucune ligne pour l’évaluer, et dont la seule raison d’être est de produire des verdicts de review plus fiables que ce qu’un agent généraliste ferait seul.

Le repo est sur GitHub (azrod/opencode-team-lead), la documentation complète sur azrod.github.io/opencode-team-lead.