Improvements

plugins/protobuf/skills/protobuf/SKILL.md

@@ -33,8 +33,9 @@ If none exists, ask the user what style should be used or an existing library to
 
 ### 2. Write Proto Code
 
-- Apply universal best practices from [best_practices.md](references/best_practices.md):
-- For service templates, see [assets/](assets/).
+- Apply universal best practices from [best_practices.md](references/best_practices.md)
+- Add [protovalidate](references/protovalidate.md) constraints to every field—this is not optional for production APIs
+- For service templates, see [assets/](assets/)
 
 ### 3. Verify Changes
 
@@ -59,6 +60,7 @@ Fix all errors before considering the change complete.
 | buf CLI, buf.yaml, buf.gen.yaml | [buf_toolchain.md](references/buf_toolchain.md) |
 | Migrating from protoc | [migration.md](references/migration.md) |
 | Lint errors, common issues | [troubleshooting.md](references/troubleshooting.md) |
+| Proto API review checklist | [review_checklist.md](references/review_checklist.md) |
 
 ## Project Setup
 
@@ -76,7 +78,8 @@ Fix all errors before considering the change complete.
    ```
 
 2. Use `assets/buf.yaml` as starting point
-3. Use `assets/buf.gen.*.yaml` for code generation config
+3. Add `buf.build/bufbuild/protovalidate` as a dependency in `buf.yaml` and run `buf dep update`
+4. Use `assets/buf.gen.*.yaml` for code generation config
 
 ### Code Generation Templates
 
@@ -102,7 +105,7 @@ Located in `assets/proto/example/v1/`:
 ### Add a new field
 
 1. Use next sequential field number
-2. Add appropriate semantic validation
+2. Add [protovalidate](references/protovalidate.md) constraints: every field should have validation appropriate to its type (format validators, length bounds, numeric ranges, enum constraints, etc.)
 3. Document the field
 4. Run `buf format -w && buf lint`
 
@@ -115,17 +118,24 @@ Located in `assets/proto/example/v1/`:
    ```
 2. Run `buf breaking --against '.git#branch=main'` to verify
 
-### Add semantic validation
+### Add protovalidate constraints
 
-See [protovalidate.md](references/protovalidate.md) for constraint patterns:
-- Required fields: `(buf.validate.field).required = true`
-- String formats: `.string.uuid`, `.string.email`, `.string.uri`
-- Numeric bounds: `.int32.gt`, `.uint32.lte`
+Every field in a production API should have appropriate validation.
+See [protovalidate.md](references/protovalidate.md) for the full reference.
+
+Common constraints:
+- String formats: `.string.uuid`, `.string.email`, `.string.uri`, `.string.pattern`
+- String bounds: `.string.min_len`, `.string.max_len`
+- Numeric bounds: `.int32.gte`, `.uint32.lte`
+- Enum validation: `.enum.defined_only`, `.enum.not_in = 0`
 - Repeated bounds: `.repeated.min_items`, `.repeated.max_items`
+- Required fields: `(buf.validate.field).required = true`
+- Oneof required: `(buf.validate.oneof).required = true`
 
 ## Verification Checklist
 
 After making changes:
+- [ ] Every field has appropriate protovalidate constraints
 - [ ] `buf format -w` (apply formatting)
 - [ ] `buf lint` (check style rules)
 - [ ] `buf breaking --against '.git#branch=main'` (if modifying existing schemas)

plugins/protobuf/skills/protobuf/assets/proto/example/v1/book_service.proto

@@ -41,39 +41,48 @@ message GetBookResponse {
 }
 
 message ListBooksRequest {
-  // Maximum number of books to return. Default is 20, max is 100.
-  int32 page_size = 1 [(buf.validate.field).int32 = {
-    gte: 0
-    lte: 100
-  }];
+  // The list order.
+  enum Order {
+    ORDER_UNSPECIFIED = 0;
+    // Order by create_time newest to oldest.
+    ORDER_CREATE_TIME_DESC = 1;
+    // Order by create_time oldest to newest.
+    ORDER_CREATE_TIME_ASC = 2;
+    // Order by update_time newest to oldest.
+    ORDER_UPDATE_TIME_DESC = 3;
+    // Order by update_time oldest to newest.
+    ORDER_UPDATE_TIME_ASC = 4;
+  }
+
+  // The maximum number of items to return.
+  //
+  // The default value is 20.
+  uint32 page_size = 1 [(buf.validate.field).uint32.lte = 100];
 
-  // Token for pagination. Empty for first page.
-  string page_token = 2;
+  // The page to start from.
+  //
+  // If empty, the first page is returned.
+  string page_token = 2 [(buf.validate.field).string.max_len = 4096];
 
   // Filter by genre.
+  //
+  // If not specified, all genres are returned.
   Genre genre = 3 [(buf.validate.field).enum.defined_only = true];
 
-  // Order results by field.
-  OrderBy order_by = 4 [(buf.validate.field).enum.defined_only = true];
-
-  // Ordering options.
-  enum OrderBy {
-    ORDER_BY_UNSPECIFIED = 0;
-    ORDER_BY_TITLE_ASC = 1;
-    ORDER_BY_TITLE_DESC = 2;
-    ORDER_BY_PUBLICATION_YEAR_ASC = 3;
-    ORDER_BY_PUBLICATION_YEAR_DESC = 4;
-    ORDER_BY_CREATE_TIME_ASC = 5;
-    ORDER_BY_CREATE_TIME_DESC = 6;
-  }
+  // The order to return results.
+  //
+  // If not specified, defaults to ORDER_CREATE_TIME_DESC.
+  Order order = 4 [(buf.validate.field).enum.defined_only = true];
 }
 
 message ListBooksResponse {
-  // Books in this page.
-  repeated Book books = 1;
+  // The next page token.
+  //
+  // If empty, there are no more pages.
+  string next_page_token = 1 [(buf.validate.field).string.max_len = 4096];
 
-  // Token for the next page. Empty if no more results.
-  string next_page_token = 2;
+  // The books.
+  repeated Book books = 2 [(buf.validate.field).repeated.max_items = 100];
 }
 
 message CreateBookRequest {

plugins/protobuf/skills/protobuf/references/best_practices.md

@@ -3,6 +3,8 @@
 Universal best practices for designing `.proto` files.
 For buf CLI configuration, see [buf_toolchain.md](buf_toolchain.md).
 
+**Validation:** Use [protovalidate](protovalidate.md) on every field—it makes the schema the single source of truth for both structure and constraints.
+
 ## Contents
 
 - [File Structure](#file-structure)
@@ -76,6 +78,7 @@ package acme.user.v1;
 - Use `snake_case`: `user_id`, `created_at`
 - Avoid abbreviations: `message` not `msg`
 - Pluralize repeated fields: `repeated User users`
+- Add [protovalidate](protovalidate.md) constraints to every field—format validators, length/range bounds, enum constraints, and required markers are all part of the field definition
 
 ### Nesting
 
@@ -183,29 +186,36 @@ Avoid grouping comments above multiple values; comments only attach to the first
 
 ### Required Enum Fields
 
-For enum fields that must have a meaningful value (not UNSPECIFIED), use both constraints:
+For enum fields that must have a meaningful value (not UNSPECIFIED), always use **both** `not_in = 0` and `defined_only = true`:
 
 ```protobuf
 Status status = 1 [
-  (buf.validate.field).required = true,        // rejects zero/UNSPECIFIED
+  (buf.validate.field).enum.not_in = 0,        // rejects UNSPECIFIED
   (buf.validate.field).enum.defined_only = true // rejects unknown values
 ];
 ```
 
+Using only one leaves a gap: `defined_only` alone allows UNSPECIFIED; `not_in = 0` alone allows unknown values.
+Optional enum fields where zero means "no preference" should use only `defined_only = true`.
+See [protovalidate.md](protovalidate.md#enum-rules) for the full pattern table.
+
 ## Oneof
 
 Use `oneof` when exactly one of several fields should be set:
 
 ```protobuf
 message SearchQuery {
   oneof query {
+    option (buf.validate.oneof).required = true;
     string text = 1;
     int64 id = 2;
     EmailFilter email = 3;
   }
 }
 ```
 
+**Validation:** Add `(buf.validate.oneof).required = true` for required choices. See [protovalidate.md](protovalidate.md#oneof-rules).
+
 **Behavior:** Setting any member clears all others. Cannot distinguish "not set" from "set to removed field" across versions.
 
 **Evolution:** Adding fields to existing oneof is safe. Moving existing fields into a oneof or removing fields is unsafe.
@@ -252,6 +262,7 @@ Prefer standard types from `google/protobuf`:
 - Name as `MethodNameRequest` and `MethodNameResponse`
 - Each RPC should have unique request/response types (enables future evolution)
 - Avoid reusing request types across RPCs
+- Every request field should have protovalidate constraints—requests are the primary system boundary where validation matters most
 
 ```protobuf
 service UserService {
@@ -265,14 +276,14 @@ service UserService {
 
 ### Adding Fields
 
-Add new fields with the next available field number:
+Add new fields with the next available field number and appropriate protovalidate constraints:
 
 ```protobuf
 message User {
-  string id = 1;
-  string email = 2;
-  string name = 3;
-  string phone = 4;  // New field
+  string id = 1 [(buf.validate.field).string.uuid = true];
+  string email = 2 [(buf.validate.field).string.email = true];
+  string name = 3 [(buf.validate.field).string.min_len = 1];
+  string phone = 4;  // New field - add validation constraints
 }
 ```
 
@@ -357,22 +368,35 @@ message User {
 - Note error conditions on RPCs
 - Skip comments on request/response messages (names are self-documenting); document fields within them
 
+### Consistency
+
+- Pick one form of a term and use it everywhere ("shortname" vs "short name" — pick one)
+- Use "ID" not "id" or "Id" in comments
+- Watch article/vowel mismatches: "an Environment" not "a Environment"
+
+### Cross-Reference Consistency
+
+When the same identifier appears in multiple messages, all validation constraints must be identical.
+See [protovalidate.md](protovalidate.md#cross-reference-consistency) for examples.
+
 ## Common Patterns
 
-### Pagination
+### Pagination and List Requests
 
 ```protobuf
 message ListUsersRequest {
-  int32 page_size = 1;
-  string page_token = 2;
+  uint32 page_size = 1 [(buf.validate.field).uint32.lte = 250];
+  string page_token = 2 [(buf.validate.field).string.max_len = 4096];
 }
 
 message ListUsersResponse {
-  repeated User users = 1;
-  string next_page_token = 2;
+  string next_page_token = 1 [(buf.validate.field).string.max_len = 4096];
+  repeated User users = 2 [(buf.validate.field).repeated.max_items = 250];
 }
 ```
 
+See `assets/proto/example/v1/book_service.proto` for a complete example with ordering and filtering.
+
 ### Partial Updates with Field Masks
 
 ```protobuf