# API Review

> Use when reviewing HTTP API design in a PR or change set — endpoints, routes, HTTP methods, status codes, idempotency, and input validation. Treats the API as a consumer-facing contract and flags resource-orientation, method-semantics, status-code, and trust-boundary violations. Read-only.

- Skill: `pekral/api-review` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds@latest add pekral/api-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/pekral/api-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Integrations & APIs
- License: MIT
- Author: pekral (https://skillmd.com/u/pekral)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/pekral/api-review

---


## Constraints
- Apply `@rules/api/general.mdc` — this skill is the focused review lens for that rule.
- Apply `@rules/php/core-standards.mdc`
- Apply `@rules/security/backend.md` — for the error-text and authorization-leak surface of API responses (401/403/404 wording, no internal-detail leak).
- If the current project uses Laravel, also apply `@rules/laravel/architecture.mdc` and `@rules/laravel/laravel.mdc` — validation belongs in FormRequest / Data Validator, controllers stay slim.
- Apply `@rules/reports/general.mdc` — when the findings are folded into the **GitHub PR comment** by a CR wrapper they stay in canonical English per the rule's *Exception — technical CR findings on the GitHub PR*; a non-technical mirror on a linked issue / JIRA ticket follows the language of the source assignment. HTTP verbs, status codes, header names, and code identifiers stay verbatim regardless of the surrounding prose language.
- Output findings only — no praise, no summary of what was checked.
- **Read-only skill** — never modify code, never stage / commit / push, and never run any git write operation. Switching to the relevant branch and `git pull` to read the latest diff are allowed; mutating the working tree or pushing is not.

## Use when
- A PR or change set adds or modifies HTTP endpoints, routes, controllers, API Resources, request/response payloads, or status-code handling.
- Run as part of every code review via `@skills/code-review/SKILL.md` (Specialized Reviews → Always run).
- A consumer-facing API contract needs a design check before release.

## Scope
Review only the API surface on the **diff** — never untouched endpoints. Detect the surface from any of: route definitions, controller/`__invoke` request handlers, API Resources / DTOs serialized into responses, FormRequests, `response()` / `abort()` / status-code calls, and `Idempotency-Key` handling. If the diff touches no API surface, return no findings.

## Core Checks
Walk the diff against each pillar of `@rules/api/general.mdc` and raise one finding per match.

### 1. Contract & consumer orientation
- Response leaks internal DB structure — raw column names, surrogate/internal keys, join tables, enum integers, or storage-only fields serialized without a DTO / API Resource boundary.
- Inconsistent contract shape for the same concept across endpoints (field casing, date format, pagination shape, error envelope).

### 2. Resource-oriented REST
- Action/verb in the endpoint path (`/getUser`, `/createUser`, `/users/{id}/delete`, `/doPayment`) instead of a resource noun + HTTP method.
- Singular collection nouns or flat URIs where a sub-resource nesting (`/users/{id}/orders`) reads clearer.

### 3. HTTP methods & idempotence
- Method whose side effects violate its contract — `GET` that mutates state, `PUT`/`DELETE` not idempotent on repetition.
- `PUT` used for a partial update or `PATCH` used for a full replacement.

### 4. Idempotency keys
- Critical, retry-prone, state-changing operation (payment, transfer, order placement) with no `Idempotency-Key` handling, so a client retry can double-execute.

### 5. Status codes
- Imprecise success code — `200` for a creation (`201`), for an async hand-off (`202`), or where `204` (no body) is correct; a body returned alongside `204`; a missing `Location` header on `201`.
- Error code collapsed into a generic one where a narrower code applies (`400`/`401`/`403`/`404`/`409`/`422`/`429`).
- 401-vs-403 inversion — `401` for an authorization failure or `403` for a missing/invalid credential.

### 6. Validation at the trust boundary
- Input reaching business logic or the database before schema/business validation runs (trust-boundary bypass).
- Validation inlined in the action/controller/model instead of the dedicated boundary layer (FormRequest / Data Validator).
- Authorization check missing or running before the input is known to be well-formed.
- Error responses that leak identity, resource existence, or internal detail — defer to `@rules/security/backend.md` *Safe Validation & Error Messages* and do not duplicate a finding `@skills/security-review/SKILL.md` already owns.

## Prioritization
- Focus on contract defects a consumer would feel: double-charges, wrong status branching, breaking payload shapes, bypassed validation.
- Deprioritize purely cosmetic naming nits — keep them as **Minor**.
- Do not propose API features the current scope does not require (YAGNI per `@rules/php/core-standards.mdc`).

## Report
Use the severity scale of `@skills/code-review/SKILL.md` so findings fold cleanly into the code review:

- **Critical** / **Moderate** / **Minor** — apply the severity declared in `@rules/api/general.mdc` *CR Severity Rules*.

Each finding includes:
- location (`file:line`)
- risk/impact (the consumer-facing consequence)
- the cited rule reference (e.g. `@rules/api/general.mdc#Resource-Oriented REST`)
- concrete fix

Each **Critical** and **Moderate** finding additionally includes:
- **Faulty Example** — minimal endpoint / route / payload snippet that reproduces the issue (redact secrets/PII)
- **Expected Behavior** — single assertable statement (status code, response shape, idempotent outcome, rejection before side effect)
- **Test Hint** — one sentence pointing at the test layer (feature/HTTP, integration) and the entry point
- **Suggested Fix** — minimal corrected snippet that complies with `@rules/api/general.mdc`, `@rules/php/core-standards.mdc`, and on Laravel projects `@rules/laravel/architecture.mdc`. Use `n/a — <reason>` only when a snippet adds nothing over the one-line fix.

Minor findings may omit these fields when no behavior change is implied.

These fields exist so `@skills/process-code-review/SKILL.md` can turn each finding into a reproducer test and apply the fix without re-deriving context.

## Functional cross-check (light — issue #737 carve-out)
`@rules/code-review/general.mdc` *Two-part CR output — Technical & Functional review* requires every CR wrapper to render a Functional review, normally built by `@skills/assignment-compliance-check/SKILL.md`. `api-review` is a documented carve-out: it does **not** invoke the full `assignment-compliance-check` — running a second full assignment-conformance engine over the same diff would duplicate the wrapper's checklist. Instead, derive a **light** functional verdict from the Core Checks walk already performed above: does the diff's API surface (routes, status codes, payload shapes, idempotency) match what the linked assignment describes for the API contract? Render one line at the end of the returned markdown — `API contract matches assignment: Yes/No` — where `No` cites whichever Core Checks finding above traces to an unmet API requirement (no duplicated detail). When the diff touches no API surface (per Scope), omit the line entirely — there is nothing to cross-check.

**Render target — standalone runs only.** Render the light verdict line only when `api-review` is invoked as a **standalone** review (not wrapped by another CR skill). When invoked **inline** as a sub-lens inside a full CR wrapper (`@skills/code-review/SKILL.md` Specialized Reviews → Always run), the wrapper's own `assignment-compliance-check` already owns the Functional review for that run — suppress the light verdict line entirely so the two-part output carries exactly one functional signal, not a redundant second one that the wrapper would discard.

## Output Format
Use the template defined in `templates/review-output.md`, appended with the **Functional cross-check** verdict line above only on a standalone run that touches an API surface; suppress the line on an inline sub-lens invocation per the Render target rule above. Omit any severity section that has no findings; never emit `None.` / `n/a` placeholders.

## Done when
- Every API-surface change on the diff has been walked against the six Core Checks.
- Findings are grouped by severity with the mandatory reproducer fields on every Critical and Moderate item.
- No code, git, or remote state was modified (read-only).