| name | go-review-pr |
| description | Review all changes in the current branch against GOB project conventions before opening a PR. Diff-aware: only analyzes what changed, not the entire codebase. Triggers: "review pr", "revisar pr", "pre-pr check", "review branch", "revisar branch", "checar antes do pr", "quality gate". |
CSTK Adaptation
Adapted from JotJunior/cstk skill go-review-pr at commit 35922cf.
Use this as a Codex skill; follow local repository instructions and Codex tool names when source text mentions Claude-specific commands or slash commands.
Go Review PR
Perform a diff-aware quality review of ALL changes in the current branch before opening a PR. This is the final quality gate — it checks everything that go-review-service checks, but scoped to the diff only.
Arguments
the user's request should specify:
- Base branch (optional, default:
main) — the branch to compare against
Step 1: Gather the Diff
BASE=${1:-main}
git diff --name-only $BASE...HEAD
git diff $BASE...HEAD
git log --oneline $BASE..HEAD
Categorize changed files:
- Go source files (*.go, excluding *_test.go)
- Go test files (*_test.go)
- Migration files (migrations/*.sql)
- Frontend files (*.ts, *.tsx)
- Config files (*.json, *.yaml, *.toml, go.mod, go.sum)
- Documentation (*.md)
Step 2: Per-Service Analysis
For each service with changed files, read the changed files and run these checks:
Check A: Compilation
cd services/{service} && go build ./...
Check B: Tests Pass
cd services/{service} && go test ./... -count=1 -timeout 60s
Check C: Lint
cd services/{service} && golangci-lint run ./... 2>&1 || true
Step 3: Convention Checks (on diff only)
For each changed Go file, verify:
3.1 Code in English
- All new variable names, function names, comments, error messages are in English
- FAIL if Portuguese found in code (excluding string literals for user-facing messages)
3.2 JSON Tags snake_case
- All new struct fields with
json:"" tags use snake_case
- FAIL if camelCase or PascalCase found in json tags
3.3 DB Tags Match Columns
- All new struct fields with
db:"" tags use snake_case matching column names
- Cross-reference with migration files if available
3.4 Schema Prefix in SQL
- All SQL queries in repository files use
{schema}.{table} format
- FAIL if any bare table name found (e.g.,
FROM members instead of FROM member.members)
3.5 Route Registration Order
- In handler files: static routes BEFORE parameterized
/:id routes
- Sub-groups BEFORE
/:id catch-all
- FAIL if order is wrong (Fiber trie conflict)
3.6 Error Patterns
- Service layer: sentinel errors (
var ErrXxxNotFound), no HTTP codes
- Handler layer:
errors.Is() dispatch, dto.ErrorResponse returns
- Repository layer:
sql.ErrNoRows → nil, nil
3.7 Import Hierarchy
Verify no import cycles:
domain → (nothing)
dto → domain (if needed)
repository → domain
service → repository + domain
handler → service + dto + domain
factory → repository + postgres
FAIL if handler imports repository directly, or service imports handler, etc.
3.8 Context First Parameter
- All repository and service methods have
context.Context as first parameter
- FAIL if any public method is missing context
3.9 Middleware Order (if main.go changed)
Verify order: recover → requestid → nrfiber → logging → cors → [dryrun] → audit → endpoints
3.10 Test Coverage
- Every new .go file (excluding main.go, migrations, mocks) should have a corresponding _test.go
- New service methods should have test cases for: happy path, not found, validation error, repo error
Step 4: Migration Checks (if migrations changed)
4.1 Naming Convention
- Format:
{NNN}_{descriptive_name}.up.sql / {NNN}_{descriptive_name}.down.sql
- Number is sequential (no gaps, no duplicates)
- Up and down files exist as pairs
4.2 Schema Prefix
- CREATE TABLE uses
{schema}.{table_name}
- All references use schema prefix
4.3 Rollback Safety
- Down migration has
DROP TABLE IF EXISTS ... CASCADE or appropriate reversal
- Down migration reverses ALL changes in up migration
4.4 CIM Format
- Any CIM values in seed data use 7-digit zero-padded format (e.g.,
'0051522')
- Uses
LPAD if converting from integer sources
4.5 Idempotency
- Uses
IF NOT EXISTS for CREATE TABLE/INDEX
- Uses
DO $$ ... IF NOT EXISTS for enum types
Step 5: Frontend Checks (if frontend files changed)
5.1 API Client Usage
- Uses
apiClient (not raw fetch/axios/ky)
- snake_case in request bodies (backend expects it)
- FormData uploads use
{ body: data } (not { json: data })
transformResponse camelCase conversion accounted for in types
5.2 Portuguese Accents
- UI text uses proper Portuguese accents (é, ã, ç, ô, í)
- Lodge display format: "No {number} - {name}" (not just name)
5.3 Type Safety
- No
any types where a proper type could be used
- Response types match backend DTOs
- Query key factories pattern used (
fooKeys.all/lists/detail)
5.4 Shared Components
- Uses
PageHeader for page titles (not custom headers)
- Uses
ServerPagination for tables (not custom pagination)
- Uses
useConfirmDialog for destructive actions (not inline confirm)
- CRUD forms on dedicated pages, NOT modals
5.5 Navigation
- Parent nav items use
end: true to prevent highlight on child routes
- New routes registered in
src/config/navigation.ts
Step 6: Cross-Cutting Checks
6.1 go.mod Consistency
- No replace directives (unless justified —
go-commons replace is expected)
go mod tidy was run (no extra/missing deps)
6.2 Submodule State
- If changes span multiple services, submodule pointers are updated
6.3 AGENTS.md Updates
- If new routes/endpoints added, check if routing table in AGENTS.md needs update
- If new service added, check if service table needs update
6.4 Storage API (if S3/storage used)
- Uses
storage.Storage interface (not direct S3 calls)
- Key pattern:
{service}/{category}/{entityID}/{uuid}-{filename}.ext
- Upload uses
Upload(ctx, key, reader, size, contentType) — NOT Put
- Presigned URL uses
PresignedGetURL(ctx, key, expiry) — NOT GetPresignedURL
6.5 RabbitMQ Events (if publisher/consumer changed)
- Publisher uses
AMQPPublisher + NoopPublisher (graceful degradation)
- Consumer uses
ExchangeDeclarePassive for foreign exchanges (not ExchangeDeclare)
- Channel reopened after passive declare failure
- Return
nil for unrecognized event routing keys (not error)
6.6 Inter-Service Clients (if client/ changed)
- Authenticated with
X-Internal-Key header
- Key from ETCD
INTERNAL_API_KEY or SHARED_INTERNAL_API_KEY
- Graceful degradation: service continues if external service unavailable
Output Format
# PR Review: {branch-name}
Date: {date}
Base: {base-branch}
Changes: {N} files across {M} services
## Build & Test
| Service | Build | Tests | Lint |
|---------|-------|-------|------|
| gob-xxx-service | PASS | PASS (12/12) | PASS |
| gob-yyy-service | FAIL | - | - |
## Convention Checks
| # | Check | Status | Details |
|---|-------|--------|---------|
| A | Code in English | PASS | |
| B | JSON tags snake_case | FAIL | dto/member.go:45 — `firstName` should be `first_name` |
| C | Schema prefix | PASS | |
| ... | ... | ... | ... |
## Migration Checks
| # | Check | Status | Details |
|---|-------|--------|---------|
| 1 | Naming | PASS | |
| 2 | Rollback | WARNING | 015_add_column.down.sql missing |
| ... | ... | ... | ... |
## Summary
- **PASS**: X checks
- **FAIL**: Y checks (must fix before merge)
- **WARNING**: Z checks (review recommended)
## Required Actions
1. [List of things that MUST be fixed]
## Recommendations
1. [List of things that SHOULD be fixed]
Important Notes
- This skill is read-only — it does NOT modify any files
- Always provide
file:line references for FAIL and WARNING results
- If the branch has no Go changes, skip Go-specific checks
- If the branch has no migration changes, skip migration checks
- Run Build & Test checks first — if build fails, skip convention checks for that service
- Be thorough but avoid false positives — when uncertain, use WARNING not FAIL
Source License
This skill includes material adapted from JotJunior/cstk, licensed under MIT. The copyright and permission notice are included in references/cstk-license.md.
