Claude-skills codex-pr-review
Revisa pull requests en proyectos Drupal 11 (u otro) siguiendo la metodología Codex (lógica de negocio, edge cases de hooks/queries, seguridad, performance, completitud). Genera un informe .md en la carpeta del IDE detectado (.antigravity/, .cursor/, .vscode/ o docs/) con hallazgos por severidad y soluciones accionables. Usar cuando el usuario pida "revisión Codex", "revisión de PR", "revisar PR", "revisar PR #N", "revisión tipo Codex" o indique un número de PR con ramas (ej. develop ← feature/alejandro). Triggers: revisión PR, revisar PR, codex PR, revisión Codex, lint PR.
git clone https://github.com/j4rk0r/claude-skills
T=$(mktemp -d) && git clone --depth=1 https://github.com/j4rk0r/claude-skills "$T" && mkdir -p ~/.claude/skills && cp -r "$T/skills/codex-pr-review" ~/.claude/skills/j4rk0r-claude-skills-codex-pr-review && rm -rf "$T"
skills/codex-pr-review/SKILL.mdRevisión Codex — Pull Request
Primera línea del informe generado: "Español confirmado."
Antes de revisar — pregúntate (framework Codex)
- ¿Qué tipo de cambio es? Hook nuevo, refactor, hotfix, migración, config — el tipo determina qué puntos Codex aplican.
- ¿Cuál es el peor escenario en producción? Si este código falla, ¿qué se rompe? Eso fija la severidad de los hallazgos.
- ¿Hay algo fuera del diff que el cambio asume? Schema, config, dependencias, índices BD, permisos — los olvidos viven en lo que no se ve.
- ¿Es idempotente? Si se ejecuta dos veces (retry, doble clic, re-deploy, re-import config), ¿pasa algo malo?
- ¿Se puede desactivar? ¿Hay kill-switch (config/setting/feature flag) si la feature explota a las 3am sin tiempo de redeploy?
Si no puedes responder a las cinco con confianza, lee el código circundante antes de emitir el informe.
Ejemplo de aplicación (mini-PR)
PR: añade
mymodule_node_update() que calcula un score y lo guarda en una tabla custom vía db_query("INSERT ... VALUES ('" . $title . "')").
- (1) Tipo: hook de entidad + escritura SQL → aplican Codex 1, 3, 4, 9.
- (2) Peor escenario: SQL injection si
viene de input + nodos nuevos sin score (falta$title
)._insert - (3) Asume: que la tabla custom existe (¿hay update hook? ¿schema?).
- (4) Idempotente: ¿qué pasa al re-guardar el nodo? ¿duplica filas o hace UPDATE? Verificar.
- (5) Kill-switch: no hay → hallazgo Media (Codex 11).
Resultado: 4 hallazgos (3 Alta + 1 Media), ninguno fuera del alcance del PR.
Fast path (resumen ejecutable)
1. Leer references/metodologia-codex-completa.md (~70 líneas, completo) 2. Leer references/plantillas-hallazgos.md (~230 líneas, completo) 3. cd a drupal/ si existe, si no raíz del workspace 4. Pedir/confirmar número de PR y ramas (base ← head). Si no se sabe, preguntar. 5. git fetch origin pull/<N>/head:pr-<N> (descarga rama del PR) 6. git diff --name-only origin/<base>...pr-<N> → lista de archivos 7. Aplicar Decision tree para elegir puntos Codex prioritarios 8. Aplicar las 5 preguntas del framework Codex (sección anterior) 9. Revisar archivo por archivo, anotando hallazgos con Severidad 10. Detectar IDE: leer CLAUDE_CODE_ENTRYPOINT (claude-vscode/claude-cursor/claude-antigravity). Solo si no es concluyente, caer a detección por carpeta existente. 11. Escribir informe en <carpeta-IDE>/Revisiones PRs/lint-review-pr<N>.md 12. Auto-verificar contra el Checklist de auto-verificación (final del documento)
Si cualquier paso falla, detente y consulta la sección "Edge cases del propio flujo".
Ubicación fija (no preguntar)
-
Repo git: si existe carpeta
en el workspace, losdrupal/
se ejecutan dentro degit
. Si no, raíz del workspace.drupal/ -
Carpeta de salida (auto-detectada por IDE):
Paso 1 — detectar el IDE por variable de entorno (PRIORITARIO). Ejecuta
(o equivalente) y aplica:printenv CLAUDE_CODE_ENTRYPOINTCLAUDE_CODE_ENTRYPOINTCarpeta a usar claude-antigravity.antigravity/Revisiones PRs/claude-cursor.cursor/Revisiones PRs/claude-vscode.vscode/Revisiones PRs/otros (
, vacío, etc.)clicontinuar al Paso 2 Señales secundarias si
no es concluyente:CLAUDE_CODE_ENTRYPOINT
(__CFBundleIdentifier
→ VS Code,com.microsoft.VSCode
→ Cursor,com.todesktop.*
→ Antigravity), ocom.google.Antigravity
/VSCODE_PID
/CURSOR_*
cuando existan.ANTIGRAVITY_*Si el IDE se identificó por env, crea la carpeta correspondiente aunque no exista todavía. NUNCA caigas a detección por carpeta cuando el env var es claro — eso causa el bug de elegir
solo porque sobrevive de un uso anterior del IDE..cursor/Paso 2 — fallback por existencia de carpeta (solo si no hay señal de env):
- Si existe
en la raíz del workspace →.antigravity/.antigravity/Revisiones PRs/ - Si existe
(y no.cursor/
) →.antigravity/.cursor/Revisiones PRs/ - Si existe
(y no las anteriores) →.vscode/.vscode/Revisiones PRs/ - Si no existe ninguna →
docs/revisiones-prs/
- Si existe
-
Nombre del archivo:
(N = número de PR, sin ceros a la izquierda). Crear la carpeta si no existe. Un archivo por PR (no sobrescribir entre PRs).lint-review-pr<N>.md
Flujo
- Detectar contexto del PR:
- Pedir/confirmar al usuario el número de PR y las ramas exactas (base ← head). Si no se proporcionan, preguntar antes de continuar.
(descarga la rama del PR comogit fetch origin pull/<N>/head:pr-<N>
local).pr-<N>
(asegura que la rama base esté actualizada).git fetch origin <base>
→ lista de archivos.git diff --name-only origin/<base>...pr-<N>- Aplicar el Decision tree (siguiente sección).
- Cargar referencias necesarias (ver sección "Carga de referencias").
- Revisar solo los archivos del PR + dependencias mínimas.
- Emitir el informe con la estructura obligatoria.
Decision tree según contenido del PR
| Contenido del PR | Puntos Codex prioritarios | Foco extra |
|---|---|---|
/ (hooks, services, controllers) | 1, 2, 3, 4, 5, 6, 7, 9, 11, 13 | DI, access, transactions |
solo | 8, 14 | XSS, cache metadata, i18n |
config (, , ) | 11, 15, 18 | Schema completo, overrides |
/ update hooks | 1, 5, 9, 17 | Idempotencia, rollback |
Migrations () | 16 | id_map, file usage |
/ solo | — | Linters, A11y, BigPipe |
| Diff vacío | Reportar "PR sin cambios respecto a la base" y salir | |
| PR ya mergeado | Avisar al usuario; confirmar si se quiere revisar histórico | |
Sin acceso a | Pedir confirmación de número de PR / repo correcto | |
| >200 archivos cambiados | Avisar al usuario y pedir confirmación | |
Solo | Revisar deps añadidas/eliminadas, no líneas |
Edge cases del propio flujo
- Si
falla → verificar acceso al repo, número de PR, o si el repo no es GitHub (GitLab usagit fetch origin pull/<N>/head:pr-<N>
).merge-requests/<N>/head - Si la rama base no existe localmente →
.git fetch origin <base>:<base> - Si el PR tiene >200 archivos cambiados → avisar al usuario y pedir confirmación antes de revisar (puede ser merge ruidoso).
- Si solo hay cambios en
→ revisar deps añadidas/eliminadas, no líneas individuales.composer.lock - Si el PR ya fue mergeado → avisar al usuario y confirmar si se quiere revisar histórico igualmente.
Carga de referencias
Esta skill tiene dos archivos en
references/. Reglas de carga:
- MANDATORY — leer ANTES de citar puntos Codex: lee completo
(~70 líneas, 18 puntos con el PORQUÉ). NUNCA parafrasees los puntos sin haberlo leído. NUNCA uses range limits al leerlo.references/metodologia-codex-completa.md - MANDATORY — leer ANTES de redactar hallazgos: lee completo
(~230 líneas, 14 plantillas con código real). Adapta los snippets al PR real, no inventes código.references/plantillas-hallazgos.md - Si ya las has leído en esta sesión, no recargar — el contexto las conserva.
- Do NOT load ninguna otra documentación externa, README, ni archivos del propio módulo más allá del diff y dependencias mínimas.
NEVER (lecciones aprendidas a las malas)
- NUNCA marcar "Alta" un hallazgo de estilo (typo, espacio, comentario). Por qué: diluye severidad, el equipo deja de leer las Altas reales.
- NUNCA sugerir refactors fuera del diff salvo seguridad crítica o data loss. Por qué: rompe el alcance del PR y genera fricción con el autor.
- NUNCA referenciar ni nombrar otros PRs en el documento. Por qué: el revisor del PR pierde foco y mezcla discusiones.
- NUNCA aprobar
en clases nuevas con el argumento "ya había antes". Por qué: perpetúa deuda y bloquea testing.\Drupal::service() - NUNCA dar por bueno
sin comentarioaccessCheck(FALSE)
en la línea siguiente. Por qué: bypass silencioso de permisos.// accessCheck OK porque... - NUNCA aprobar migración sin verificar
yid_map
(si maneja media). Por qué: rollbacks rotos.file_usage - NUNCA aprobar
en Twig sin verificar que el origen es 100% controlado por el sistema. Por qué: XSS persistente.|raw - NUNCA aprobar
dentro de$query->execute()
sin cache. Por qué: N+1 en cada render.hook_*_alter - NUNCA aprobar nuevo
endependencies:
sin verificar que el módulo está en*.info.yml
. Por qué: deploy roto en CI.composer.json - NUNCA escribir el informe en inglés. Código y comandos en inglés; explicaciones en español.
- NUNCA marcar el informe como "OK" si hay cualquier hallazgo de severidad Alta sin resolver.
- NUNCA citar un punto Codex sin haber leído
en esta sesión.references/metodologia-codex-completa.md - NUNCA aprobar
sin verificar que el field exists primero. Por qué: tras eliminar un field y antes deEntityFieldManagerInterface::getFieldStorageDefinitions()
/cron
, el storage queda zombi y revienta queries.field_purge_batch - NUNCA aprobar Batch API nueva sin
callback que manejefinished
. Por qué: batches que fallan en mitad dejan datos a medias y nadie se entera.$success === FALSE - NUNCA aprobar
sinentityTypeManager->getStorage()->loadMultiple()
vacío como guarda. Por qué:array
devuelve TODAS las entidades — bug clásico de fuga de memoria.loadMultiple([])
Severidades (criterio fijo)
| Severidad | Criterio |
|---|---|
| Alta | Seguridad explotable, data loss, rompe producción, bloquea deploy |
| Media | Bug funcional, incumple estándar del proyecto, deuda inmediata |
| Baja | Estilo, micro-optimización, mejora opcional |
Si dudas entre dos niveles, baja uno. Las Altas deben ser realmente Altas.
Estructura obligatoria del informe
Español confirmado. # Revisión de código — PR #<N> (<base> ← <head>) ## Resumen ejecutivo <2-4 frases: alcance del PR, conteo por severidad, veredicto> ## Hallazgos por categoría ### Seguridad ### Lógica de negocio / Codex ### Estándares / DI ### Performance / Cache ### Accesibilidad / i18n ### Tests / CI ## Riesgos (tabla) | Área | Riesgo | Severidad | Mitigación | ## Sugerencias accionables 1. ... ## Checklist final - [ ] Hallazgos Alta resueltos - [ ] Tests pasan - [ ] Schema config actualizado - [ ] Update hooks idempotentes
Cada hallazgo va con Problema (Severidad), Riesgo y Solución (con código). Adapta las plantillas de
.references/plantillas-hallazgos.md
Idioma y tono
- Español en todo el texto. Inglés en código, nombres de clase, comandos y rutas.
- Tono profesional, directo, simpático con el equipo que aplicará las correcciones.
- Detalle proporcional a la complejidad del hallazgo.
Checklist de auto-verificación (antes de entregar)
Antes de dar por cerrado el informe, comprueba uno por uno:
- Primera línea del informe es exactamente
Español confirmado. - El archivo está en
<carpeta-IDE>/Revisiones PRs/lint-review-pr<N>.md - El título incluye número de PR exacto y las ramas (base ← head)
- He leído
en esta sesiónreferences/metodologia-codex-completa.md - He leído
en esta sesiónreferences/plantillas-hallazgos.md - Cada hallazgo tiene Problema (Severidad), Riesgo y Solución
- Ninguna severidad "Alta" es solo de estilo (typo, espacio, comentario)
- Todas las soluciones de código compilan mentalmente y siguen DI/PSR-12
- No he propuesto cambios fuera del alcance del PR (salvo seguridad crítica)
- No he referenciado ni nombrado otros PRs en el documento
- Todas las explicaciones en español, todo el código en inglés
- He aplicado las 5 preguntas Codex a cada bloque significativo
- El informe incluye Resumen ejecutivo, Hallazgos, Riesgos, Sugerencias y Checklist final
- Si hay hallazgos Alta sin resolver, el veredicto NO es "OK"
Si alguna casilla queda sin marcar, vuelve atrás y arregla antes de entregar.
Recovery — qué hacer si algo falla
| Síntoma | Acción |
|---|---|
no existe | Avisar al usuario, no inventar puntos Codex |
no existe | Generar hallazgos sin plantilla pero con misma estructura |
falla | Verificar número de PR, repo correcto, o si es GitLab () |
| Rama base no existe localmente | y reintentar |
o no se puede crear | Pedir al usuario que cree la carpeta y reintentar |
| PR demasiado grande (>200 archivos) | Pedir confirmación antes de continuar |
| PR ya mergeado | Avisar y confirmar si se revisa histórico igualmente |
| El usuario no proporciona número de PR | Preguntar antes de continuar, no asumir |