api-contract-reviewer

name	api-contract-reviewer
version	1.0.0
description	Reviews REST + gRPC contracts for stability, versioning, completeness, backwards compatibility. References rest-api-architect / protobuf-architect / grpc-architect for rules; runs `buf breaking` / `openapi-diff`. Severity-keyed findings. Use when reviewing a new endpoint, proto change, or before a breaking-change release.

API Contract Reviewer

Reviews contracts — REST OpenAPI specs and protobuf .proto files — for stability, versioning hygiene, and completeness. Catches the contract issues architects encoded rules against before they ship to clients.

1. When to invoke

• A PR changes .proto files, OpenAPI YAML, or any handler that affects the wire contract.
• Before publishing a new major version of an API.
• Before deprecating a field or method.
• Periodic audit of an existing API for drift between spec and implementation.

2. Output format

Same shape as security-reviewer §2 — findings table with severity, then a one-line summary.

| Severity | Rule | Location | Evidence | Fix |
|---|---|---|---|---|
| Critical | Breaking change to v1 | `proto/orders/v1/order.proto:23` | Field `status` type changed `string` → `int32` | Revert; introduce `status_v2` as a new field in v1, deprecate `status`, or bump to v2 |
| High | Versioning mismatch | `openapi.yaml` | Endpoint `/orders` lacks `/v1/` prefix | Add `/v1/` prefix per rest-api-architect §4 |
| Medium | Missing OpenAPI example | `openapi.yaml:42` | `CreateOrderRequest` has no `example:` | Add a realistic example — drives SDK gen + docs |
| Low | Inconsistent error shape | `openapi.yaml` | 404 returns `{detail: ...}` while 422 returns RFC 7807 | Standardize on RFC 7807 per rest-api-architect §7 |

Severity guide:

• Critical — wire-breaking change in a stable version (clients break on deploy).
• High — convention violation that's expensive to fix later (versioning, error shape).
• Medium — completeness gap that hurts client experience (missing examples, undocumented errors).
• Low — polish (inconsistent casing, missing descriptions).

3. Review approach

1. Mechanical pass (REST: openapi-diff; gRPC: buf breaking) — surfaces wire-breaking changes.
2. Convention pass — read the spec / proto files against the architect rules in §4.
3. Completeness pass — every endpoint, every model, every error has the metadata clients need (examples, descriptions, types).

4. What to check — by category

Versioning

Per rest-api-architect §4 and protobuf-architect §4:

• REST: every endpoint under /v1/, /v2/, etc. Not in headers, not in query params.
• gRPC / proto: version is part of the package path (acme.shop.orders.v1), never appended to message names (OrderV1 is wrong).
• Additive changes don't bump the version — new optional field, new endpoint, new enum value, new RPC method. Stay in the existing version.
• Breaking changes always bump the major version — field removal, type change, semantic change, required-field tightening. New vN package + run side-by-side.
• Deprecate before remove — add [deprecated = true] (proto) or deprecated: true (OpenAPI), set Deprecation / Sunset headers (RFC 9745 / 8594) for REST.

Field & method hygiene (proto)

Per protobuf-architect §3:

• Field numbers never reused. Removed field → reserved N; + reserved "name";.
• Field type never changed. int32 → string is wire-breaking even if the runtime value fits both.
• Field numbers 1–15 reserved for fields read on every request (1-byte wire encoding).
• Enum: *_UNSPECIFIED = 0 mandatory. Enum value names UPPER_SNAKE_CASE prefixed with the enum name.
• No primitive wrappers (StringValue, Int32Value) — use optional instead.

Error contracts

Per rest-api-architect §7 for REST and grpc-architect §2 for gRPC:

• REST: every error returns application/problem+json (RFC 7807) — type, title, status, detail, instance, correlation_id. Never {"detail": "..."} and {"errors": [...]} mixed in one API.
• gRPC: status.Error with a standard code. Domain-error → code mapping is centralized; no handler invents its own.
• type URLs are stable once published — clients switch on them.
• 422 validation errors include the structured field list per rest-api-architect §7.
• 5xx responses always include correlation_id ties to server logs.

Idempotency and concurrency

Per rest-api-architect §8 and §9:

• Idempotency-Key mandatory on POST/PATCH. Missing → 400. Documented in OpenAPI as a required header.
• ETag + If-Match mandatory on PUT/PATCH. Stale → 412. Documented.
• Cursor pagination, not offset. Documented next_cursor and limit in response.

JSON shape & encoding

Per rest-api-architect §12:

• snake_case JSON field names.
• ISO 8601 timestamps, string-encoded with timezone.
• Money as strings ("99.99"), never JSON numbers.
• UUIDs as canonical hex with dashes, UUID v7 preferred.
• null ≠ missing — both are documented behaviors in PATCH.

OpenAPI completeness (REST)

• Tag, summary, description on every operation. They drive docs and SDK code-gen.
• responses: documents non-default codes (401, 403, 404, 409, 412, 422).
• examples: on every request / response model. SDKs render them; integration tests use them.
• requestBody.required: true when the body is mandatory — default is false, easy to miss.
• securitySchemes declared (Bearer / OAuth2) and referenced per-endpoint.
• servers: and info.contact set — these aren't FastAPI defaults but matter for published specs.
• info.version matches the API major version (1.0.0, not 0.1.7).

gRPC service hygiene

Per grpc-architect §1:

• One service per file.
• Every RPC takes a <Verb><Noun>Request and returns <Verb><Noun>Response. Never google.protobuf.Empty as input.
• ListXRequest/ListXResponse use cursor pagination matching REST conventions.
• google.protobuf.Empty only for fire-and-forget responses with no useful return.
• Streaming pattern justified in proto comments (server-stream vs client-stream vs bidi).

Documentation drift

The spec is the contract; drift between code and spec is a contract failure:

• OpenAPI generated from code, not hand-written. Per rest-api-architect §15.
• Snapshot test in CI: the spec is asserted against a committed snapshot file. Any change is reviewed.
• gRPC equivalent: generated code is committed under gen/ per protobuf-architect §6. PR shows the generated diff alongside the proto diff.

5. Tooling

Run these on the diff before the read pass; their output goes into the report.

Tool	Catches
buf breaking --against '.git#branch=main,subdir=proto'	Wire-breaking changes in .proto files
buf lint	proto3 style + Buf-style package naming
openapi-diff <old> <new>	Wire-breaking changes in OpenAPI specs (additions, removals, type changes)
swagger-cli validate openapi.yaml (or redocly lint)	OpenAPI 3.1 validity + completeness rules
Snapshot diff in CI	assert(app.openapi() == snapshot) per fastapi-architect §10

These run in CI per rest-api-architect §15 and protobuf-architect §8 — review they're actually wired and failing builds on findings.

6. What this skill does NOT do

• Performance review. Slow queries, blocking I/O — see performance-reviewer.
• Security review. Auth, injection, secrets — see security-reviewer.
• Architecture review. Whether the API surface is shaped right at the boundary level — see improve-codebase-architecture.

This skill is about whether the contract is stable and complete, not whether the underlying implementation is fast or safe.

7. Cross-skill ties

• rest-api-architect — REST rules this skill verifies.
• protobuf-architect — proto rules + buf breaking / buf lint.
• grpc-architect — gRPC service-definition conventions + error codes.
• fastapi-architect / gin-architect / nethttp-architect — implementation skills; reviewer confirms code matches the contract.
• security-reviewer — when contract issues are also security issues (tokens in query params, sensitive data in URLs), promote severity.
• commit-author — breaking changes get the BREAKING CHANGE: footer in the commit.