← Volver al Blog

Conventional Comments: 1 de 9 bloquea el merge

Cuando adopté Conventional Comments en mi flujo de review, la primera decisión que tuve que tomar no fue cómo escribir los comentarios. Fue cuáles de las 9 etiquetas de la especificación bloquean el merge y cuáles no.

La especificación oficial no responde eso. En la sección de decoraciones habla de “organizaciones que consideran los comentarios bloqueantes por defecto” y de “organizaciones que los consideran no bloqueantes por defecto” como dos modos válidos — o sea, te deja la decisión a ti. Si no la tomas tú, cada revisor y cada autor de PR la toman por su cuenta cada vez que leen un comentario. Y ahí vuelve el peaje de “¿esto hay que arreglarlo antes del merge, o puede quedar como tarea posterior?”.

La decisión que terminé poniendo en mi harness es la más simple que se me ocurrió: solo issue: bloquea el merge. Las otras 8 pasan a feedback, pregunta o información. El resto de este post es cómo quedó la configuración, por qué descarté usar las decoraciones (blocking) / (non-blocking), y en qué se me rompió el gate la primera vez que lo intenté.

Las 9 etiquetas de la spec, agrupadas por rol

La especificación de conventionalcomments.org lista 9 etiquetas principales más 3 variantes expresivas (typo, polish, quibble). Agrupadas por el rol que tienen en una review, se ven así:

Bloqueante (1):

  • issue: — problema concreto en el código, corrección obligatoria antes del merge.

Pide respuesta (3):

  • todo: — cambio pequeño pero pendiente a resolver antes de aceptar el PR.
  • question: — una duda sobre la razón o la intención. No es pedido de cambio, pero sí pide respuesta.
  • chore: — tarea rutinaria (actualizar CHANGELOG, firmar CLA) que la spec dice que debe estar hecha antes de aceptar.

Informativa (5):

  • suggestion: — propuesta de mejora, no obligatoria.
  • nitpick: — detalle trivial de estilo, no bloqueante por naturaleza.
  • thought: — reflexión abierta que surgió al revisar, también no bloqueante por naturaleza.
  • praise: — reconocer algo que quedó bien.
  • note: — información de contexto que el lector debería tener en cuenta.

Las tres variantes expresivas (typo / polish / quibble) son sinónimos tonales de todo / suggestion / nitpick. En mi harness no las uso porque multiplicar sinónimos sin añadir semántica solo hace que el gate tenga que mantener más reglas.

Las 9 etiquetas de Conventional Comments agrupadas en 3 grupos: 1 bloqueante (issue), 3 que piden respuesta (todo, question, chore), 5 informativas (suggestion, nitpick, thought, praise, note)

Al mirar el primer y el segundo grupo juntos, la tentación es tratar a issue:, todo: y chore: todos como bloqueantes, porque la spec habla de “obligatorio antes de aceptar” para los tres. Lo que me pasó cuando intenté esa regla en el CI es que los revisores (humanos y la IA de CodeRabbit) terminaban usando issue: para cualquier cosa que quisieran marcar como obligatoria, porque era la etiqueta mejor definida en la cabeza de todos. todo: y chore: quedaban como huérfanos: nadie se acordaba de que también eran bloqueantes, los autores los trataban como opcionales, y el gate de CI dejaba pasar PRs con todo: sin resolver.

En vez de pelear contra eso, cambié la regla: solo issue: bloquea. Si algo tiene que bloquear y no es un bug claro, se escribe como issue: con una frase que explique por qué es obligatorio. todo: y chore: quedan como etiquetas que piden respuesta pero no frenan el merge, útiles para dejar tareas visibles sin convertir cada PR en un bloqueo.

Por qué no uso las decoraciones (blocking) / (non-blocking)

La especificación agrega una capa extra: cualquier etiqueta puede llevar la decoración (blocking), (non-blocking) o (if-minor). Así:

suggestion (blocking): este endpoint tiene que validar el input antes de persistir.
nitpick (if-minor): este nombre de variable podría ser más corto.
question (non-blocking): ¿la elección de ORM fue por preferencia personal?

Para un equipo con mucho rodaje, las decoraciones son geniales porque le dan a cada comentario un dial propio sin forzar la etiqueta. El problema aparece cuando el gate automático las tiene que leer. “¿Un suggestion (blocking): cuenta como bloqueante?” es una pregunta sensata, y la respuesta correcta es que sí. Pero poner esa lógica en un workflow de GitHub Actions significa parsear el prefijo del comentario con una regex tipo /^\s*(issue|todo|chore)(\s*\(blocking\))?:/ o /^\s*\w+\s*\(blocking\):/, y después mantener esa regex contra 12 variantes de etiqueta y 3 decoraciones. Cuando la regla es “solo issue: bloquea”, el filtro cabe en una línea: body.trimStart().startsWith('issue:').

La elección es entre expresividad del comentario y simplicidad del gate. Para un harness solo, elegí la simplicidad del gate. Si algún día un comentario suggestion: tiene que bloquear, lo reescribo como issue: y pongo en el cuerpo “esto nació como suggestion, pero decidí tratarlo como obligatorio porque…”. Es un pequeño impuesto de escritura a cambio de un gate que no se discute.

La configuración de CodeRabbit que impone el vocabulario

El primer sitio donde entra la regla es .coderabbit.yaml. Esto le dice al revisor de IA que todo comentario que escriba lleve etiqueta de Conventional Comments, y define el subset que mi harness espera leer:

# .coderabbit.yaml
reviews:
  profile: default
  auto_review:
    enabled: true
  path_instructions:
    - path: "**"
      instructions: |
        Pon una etiqueta de Conventional Comments al inicio de cada comentario.

        Etiquetas que usamos:
        - praise: reconocer una buena implementación (úsalo sin miedo)
        - issue: problema de corrección obligatoria (merge-block)
        - suggestion: propuesta de mejora (opcional)
        - nitpick: mejora minúscula (opcional, no tratar como bloqueante)
        - question: pregunta sobre razón o intención
        - thought: reflexión para abrir discusión de diseño
        - todo: tarea pequeña pendiente (no bloqueante)
        - chore: tarea de proceso (no bloqueante)
        - note: información de referencia (no bloqueante)

        Solo issue: bloquea el merge. Cualquier otro comentario que
        tenga que bloquear hay que reescribirlo como issue: con la razón.

        En comentarios issue, escribe siempre la razón y la alternativa.

El schema reviews.path_instructions está documentado en docs.coderabbit.ai y tiene exactamente dos campos: path (glob) + instructions (string multi-línea). CodeRabbit lee eso en cada PR y adapta sus comentarios a la convención. En paralelo, los humanos del equipo (en mi caso, solo yo) ven la misma lista cuando abren el AGENTS.md del repo, donde copio la misma tabla.

Un detalle que me rompió la primera vez: si pones la lista de etiquetas sin la línea Solo issue: bloquea el merge, CodeRabbit empieza a emitir issue: para todo lo que considera “importante”, incluyendo nitpicks tonales. La frase explícita del dial lo calibra. Sin ella, el revisor de IA actúa como un revisor humano ansioso: todo le parece bloqueante.

El gate del CI: donde se me cayó la REST API

La idea del gate es simple: si hay algún comentario issue: sin resolver, el workflow falla y el merge queda bloqueado. La primera versión que escribí en el capítulo 7 de mi libro en PT sobre revisión de código usaba la REST API de GitHub:

# .github/workflows/review-gate.yml (versión incorrecta — no detecta resolve)
name: Review Gate
on:
  pull_request:
    types: [review_requested, synchronize]

jobs:
  check-unresolved-issues:
    runs-on: ubuntu-latest
    steps:
      - name: Check unresolved CodeRabbit issues
        uses: actions/github-script@v7
        with:
          script: |
            const comments = await github.rest.pulls.listReviewComments({
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: context.payload.pull_request.number
            });

            const unresolvedIssues = comments.data.filter(c =>
              c.body.startsWith('issue:') && !c.line
            );

El filtro !c.line del final estaba apoyado en la suposición de que, cuando alguien marca el thread de un comentario como resolved en la UI de GitHub, el campo line desaparece de la respuesta de la REST API. Esa suposición es falsa.

El schema del endpoint GET /repos/{owner}/{repo}/pulls/{pull_number}/comments tiene estos campos de respuesta: url, id, node_id, pull_request_review_id, diff_hunk, path, position, original_position, commit_id, original_commit_id, in_reply_to_id, user, body, created_at, updated_at, html_url, pull_request_url, author_association, _links, start_line, original_start_line, start_side, line, original_line, side, subject_type, reactions, body_html, body_text.

Ningún campo expone resolución. No hay resolved, no hay isResolved, no hay state. El line apunta a la línea del diff y no se anula cuando el thread pasa a resolved. Para confirmar que no era yo interpretando mal la doc, busqué el pedido oficial: en community/discussions/9175 el título termina en “lack of status”, y la respuesta más votada apunta a GraphQL como el único camino actual. El tópico sigue abierto como pedido de feature hace años.

Lo que hace !c.line es evaluar a true cuando el campo line es nulo. Según el schema de la REST API, line puede llegar nulo para comentarios de tipo file o para comentarios cuya posición en el diff se volvió outdated (el commit del PR cambió y la línea original ya no existe). Esos son los que el filtro mantiene dentro de unresolvedIssues. Los que descarta son los comentarios con line definido — y ahí caen tanto los issue: abiertos sobre una línea concreta como los issue: que ya fueron marcados como resolved en la UI. La REST API no expone el estado de resolución del thread, así que el filtro no tiene con qué distinguir esos dos casos y los descarta a los dos por igual.

El resultado es que ese gate pasa silenciosamente. En una review normal, los comentarios issue: viven sobre líneas específicas del diff, siempre tienen line definido y siempre son descartados por el filtro. unresolvedIssues.length queda en 0, el workflow termina en verde y el merge se habilita aunque el issue: siga abierto. El único caso en que el gate llega a fallar es cuando aparece un issue: como comentario file-level o uno queda outdated por un rebase — casos borde que casi no ocurren. Peor que no tener gate: te da la sensación de que estás protegido.

La versión que sí funciona: GraphQL con isResolved

El estado de resolución es una propiedad del review thread, no del comment, y vive en el tipo PullRequestReviewThread de la GraphQL API. El campo que necesitamos es isResolved: Boolean!.

La query equivalente, bajando al primer comentario de cada thread para leer la etiqueta, es:

query ($owner: String!, $repo: String!, $pr: Int!) {
  repository(owner: $owner, name: $repo) {
    pullRequest(number: $pr) {
      reviewThreads(first: 100) {
        nodes {
          isResolved
          comments(first: 1) {
            nodes { body }
          }
        }
      }
    }
  }
}

Y el workflow completo, ya corregido:

# .github/workflows/review-gate.yml (versión corregida)
name: Review Gate
on:
  pull_request:
    types: [opened, synchronize, review_requested]

jobs:
  check-unresolved-issues:
    runs-on: ubuntu-latest
    steps:
      - name: Check unresolved issue: threads via GraphQL
        uses: actions/github-script@v7
        with:
          script: |
            const query = `
              query ($owner: String!, $repo: String!, $pr: Int!) {
                repository(owner: $owner, name: $repo) {
                  pullRequest(number: $pr) {
                    reviewThreads(first: 100) {
                      nodes {
                        isResolved
                        comments(first: 1) {
                          nodes { body }
                        }
                      }
                    }
                  }
                }
              }
            `;
            const data = await github.graphql(query, {
              owner: context.repo.owner,
              repo: context.repo.repo,
              pr: context.payload.pull_request.number,
            });
            const threads = data.repository.pullRequest.reviewThreads.nodes;
            const unresolved = threads.filter(t =>
              !t.isResolved &&
              t.comments.nodes[0]?.body.trimStart().startsWith('issue:')
            );
            if (unresolved.length > 0) {
              core.setFailed(
                `${unresolved.length} thread(s) con "issue:" sin resolver`
              );
            }

Tres cambios respecto a la versión original:

  1. github.graphql en vez de github.rest.pulls.listReviewComments. El cambio de superficie es lo que arregla el bug principal.
  2. El filtro ahora es a nivel de thread, no de comment. !t.isResolved lee la propiedad real; el !c.line de antes leía una propiedad imaginada.
  3. trimStart() antes de startsWith. Muchas respuestas de CodeRabbit vienen con un espacio o salto de línea al inicio del body. Sin el trim, el gate deja pasar comentarios que empiezan con issue: o \nissue:.

También cambié el trigger de [review_requested, synchronize] a [opened, synchronize, review_requested]. En la versión del libro, el gate solo se activaba cuando alguien pedía review explícitamente, lo que dejaba la primera apertura del PR sin protección.

Cómo probar que el gate falla cuando tiene que fallar

Un gate que nunca falla es indistinguible de un gate que siempre pasa. La única manera de saber cuál de los dos tienes es un test que espera el failure. Esto no estaba en el capítulo 7 original, y por eso la versión REST pasó meses viviendo tranquila en el repo.

El test manual más corto:

  1. Abre un PR a propósito, uno chico (un cambio de typo en el README sirve).
  2. Deja tú mismo un comentario de review en una línea con el body issue: dejar este sin resolver para probar el gate y publícalo.
  3. No lo marques como resolved.
  4. Mira la pestaña Checks del PR: el workflow Review Gate tiene que estar en rojo con el mensaje 1 thread(s) con "issue:" sin resolver.
  5. Marca el thread como resolved en la UI de GitHub.
  6. Vuelve a disparar el workflow (basta con un push vacío al branch del PR).
  7. El workflow ahora tiene que estar en verde.

Si cualquiera de los dos pasos (4 o 7) no se cumple, el gate tiene un bug. Mejor descubrirlo ahora que el día que un issue: real se cuele.

Lo que quedó aprendido

Dos cosas prácticas de este ejercicio:

1. El código de ejemplo en un libro técnico es una afirmación, no decoración. La línea // resolved comments lose line del capítulo 7 era una afirmación sobre el comportamiento de la REST API de GitHub. Nunca verifiqué contra la doc oficial. El YAML pasaba lint, el workflow compilaba, el comentario parecía razonable. Nada de eso prueba que la afirmación sea cierta.

2. “Resolve de review” es una propiedad de thread, no de comment. En todas las APIs modernas de GitHub. Si tu filtro depende de “este comentario fue resuelto”, tienes que subir un nivel a reviewThreads en GraphQL. La REST solo devuelve comentarios individuales, sin contexto de resolución.

La propuesta del libro no cambia — un gate automático que trata issue: como “obligatorio antes de merge” sigue siendo útil. Lo que cambia es la plomería. Y, siendo honesto, haber publicado la plomería rota y haberlo notado meses después es un recordatorio de que verificar la doc después de que el YAML se vea bonito ya es tarde.

Referencias


ken imoto · WebRTC & Voice AI engineer · kenimoto.dev · TabNews

Harness Engineering Libro relacionado Harness Engineering Harness Engineering desde cero — cinco interpretaciones (OpenAI, Anthropic, LangChain, Martin Fowler, academia) unificadas en un solo sistema para ingenieros en producción Ver la página del libro →