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". argument-hint: "[base branch, default: main]" allowed-tools: - Read - Glob - Grep - Bash

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¶

$ARGUMENTS should specify: - Base branch (optional, default: main) — the branch to compare against

Step 1: Gather the Diff¶

# Get the base branch (default: main)
BASE=${1:-main}

# All changed files in this branch vs base
git diff --name-only $BASE...HEAD

# Full diff for context
git diff $BASE...HEAD

# Commits in this branch
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

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 CLAUDE.md Updates¶

If new routes/endpoints added, check if routing table in CLAUDE.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