# Gravitee Io Gravitee Access Management Code Review > Code Review - Skill: `tomevault-io/gravitee-io-gravitee-access-management-code-review` (Agent Skill, multi-file: 2 files) - Install (CLI): `npx skillmds@latest add tomevault-io/gravitee-io-gravitee-access-management-code-review` - Raw SKILL.md: https://api.skillmd.com/api/skills/tomevault-io/gravitee-io-gravitee-access-management-code-review/raw - Safety review: pending (external: skill-scanner PASS, skillspector PASS) - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: tomevault-io (https://skillmd.com/u/tomevault-io) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/tomevault-io/gravitee-io-gravitee-access-management-code-review --- # Code Review ## Purpose Perform a systematic code review of the current changeset and report actionable findings. By default this is **review-only**: do not modify code unless explicitly requested. ## When to use Apply this skill: - After implementing changes, **before opening a PR / requesting review / merging** - When explicitly asked to review code - As the "validation" step at the end of a task **Scope:** Identify the current 'staged' changes or the diff between the current branch and the main branch before reviewing. **Priority:** If the changeset is large, prioritize **Global Standards** and **Architecture** findings first. ## Review mode - Review-only by default: identify issues and propose fixes; stop for confirmation before making changes. - **Cite evidence:** For every finding, provide the **line number and a snippet** of the code in question. - Prefer project commands from `@docs/agent-standards/commands.md` (if present); otherwise suggest likely commands and ask before adding/updating commands.md. ## Output format Return results in this structure: 1) **Verdict**: PASS / NEEDS_CHANGES 2) **Top risks** (0–3 bullets) 3) **Findings** - **Blockers** (must fix) - Security, blocking calls, missing audit - **Warnings** (should fix) - Pattern violations, missing tests - **Nits** (optional) - Style, naming, comments 4) **Recommended validation commands** (reference `commands.md` if available) 5) **Next steps (Task List)** - [ ] Task 1: (e.g., Fix the `blockingGet` in `UserService.java:42`) - [ ] Task 2: (e.g., Run `mvn openapi-generator:generate`) - [ ] Task 3: (e.g., Add audit log to `deleteUser`) *Note: I can execute these tasks for you one by one if you say 'Proceed with Step X'.* ## Review checklist ### 1. Global Standards (Apply to ALL changes) **Security & Compliance:** - [ ] No secrets/tokens/PII in logs (prefer structured logs: key=value) - [ ] No full request/response bodies logged - [ ] Authorization/validation fail closed (deny by default) - [ ] RFC compliance verified for OAuth/OIDC/IAM work (RFC 6749, 7519, 7636, 8252, 8707) - [ ] No exposure of internal implementation details in error messages **Diff Integrity:** - [ ] Small, reviewable diffs; no unrelated refactors/reformatting - [ ] Changes focused on stated goal - [ ] No commented-out code or debug statements **Code Quality:** - [ ] UK English for comments, logs, user-facing text - [ ] Method/class visibility not widened without justification - [ ] Error handling preserves business intent (expected 4xx not wrapped into 5xx) - [ ] No redundant null checks (prefer `Optional` or `@NotNull`) - [ ] No log pollution (avoid logging entire DTOs/entities with PII) ### 2. Architecture & Patterns (The "How") **Layering:** - [ ] Respects layering: **Repository → Service → Resource** - [ ] Resources handle HTTP + permissions; Services handle business logic - [ ] No business logic leaking into Resource layer **Reactive Integrity (CRITICAL):** - [ ] **Strictly NO `blockingGet()`** or blocking calls in reactive paths - [ ] RxJava used correctly: `Single`/`Maybe` for I/O, `Completable` for side-effects - [ ] Reactive chains properly composed **Audit Trail (MANDATORY for C/U/D):** - [ ] Audit logging present for **all** CREATE/UPDATE/DELETE operations - [ ] Audit logging implemented in **Service layer** (not Resource) - [ ] Resources **pass authenticated User principal** to service methods - [ ] Uses dedicated `AuditBuilder` per entity type **Pattern Adherence:** - [ ] Follows established patterns (cite similar implementations found) - [ ] No invented endpoints, config keys, or behaviours - [ ] Reuses existing utilities/helpers where applicable ### 3. Data & API Contracts (The "Mechanics") **API Contracts:** - [ ] OpenAPI spec updated when API behaviour/endpoints/DTOs change - [ ] SDK regenerated when required - [ ] Backward compatible OR breaking change explicitly approved - [ ] Contract/API tests cover success + error paths - [ ] HTTP status semantics match existing patterns **Data & Persistence:** - [ ] Repository interfaces extend `CrudRepository` (unless established pattern differs) - [ ] Repository methods are domain-scoped (avoid wide fetch + in-code filtering) - [ ] **If a Repository is changed, check for corresponding implementation in BOTH `mongo/` and `jdbc/` directories** - [ ] Both MongoDB and JDBC implementations provided when applicable - [ ] Liquibase changesets grouped logically (table + indexes + constraints together) - [ ] Mongo indexes follow established initials + sort-order naming convention ### 4. Testing & Validation (The "Proof") **Test Coverage:** - [ ] Success **and** error paths covered - [ ] Bottom-up coverage: **Repository → Service → Resource** - [ ] CREATE/UPDATE/DELETE tests **must verify audit logging** - [ ] Tests actually verify audit events are emitted (not just "audit was called") **Implementation Parity:** - [ ] Both MongoDB and JDBC tested via shared test suite (when applicable) - [ ] Integration tests updated where needed **Validation Commands:** - [ ] Validation commands referenced from `commands.md` or proposed - [ ] Tests pass, lint is clean **Documentation & Traceability:** - [ ] Complex logic explained (JavaDoc or inline comments) - [ ] Files cited that informed implementation (pattern-following) - [ ] Breaking changes clearly called out for PR description ## Self-review prompts (quick) 1) Pattern adherence: what existing pattern was followed, where is it? 2) Search-first: what similar implementations were inspected? 3) Risk: what could break, and which edge cases exist? 4) Reviewability: is the diff focused? 5) Completeness: does it satisfy acceptance criteria? 6) Testing: do tests verify behaviour (not just exercise code)? ## Common issues to catch **Architectural:** - Audit logging implemented in Resource instead of Service layer - Business logic in Resource layer - Blocking calls in reactive paths (`blockingGet()`) **Data:** - Wide queries + in-code filtering instead of scoped queries - Missing Mongo/JDBC parity where required - Repository changes without checking both `mongo/` and `jdbc/` directories **API:** - Missing SDK regeneration after API changes - Breaking changes without migration documentation **Testing:** - Tests that don't verify audit events were emitted - Only testing one storage implementation (Mongo or JDBC, not both) **Code Quality (Agent-isms):** - Redundant null checks when using `Optional` or `@NotNull` - Log pollution: logging entire DTOs/entities instead of IDs - Unrelated refactors mixed into feature changes - Commented-out code or debug statements left behind ## References (optional) - `@docs/agent-standards/commands.md` - `@docs/agent-standards/templates/openapi_change_checklist.template.md` - `@docs/agent-standards/templates/migration_checklist.template.md` ## Exit criteria - Verdict issued with evidence (line numbers and snippets) - Findings prioritized by severity (Blockers > Warnings > Nits) - Actionable task list provided - Suggested validation commands included - No code changes made unless explicitly requested --- > Converted and distributed by [TomeVault](https://tomevault.io/claim/gravitee-io) — claim your Tome and manage your conversions.