docs: add api-v2-routes skill and freeze /api/v1
Adds the api-v2-routes agent skill (CRUD + non-CRUD shapes, descriptions required) and marks /api/v1 frozen in AGENTS.md/CLAUDE.md so new routes go to the Huma-backed /api/v2.
This commit is contained in:
kolaente
kolaente
parent
057b2e5439
commit
d2a3186b3e
|
|
@ -0,0 +1,168 @@
|
|||
---
|
||||
name: api-v2-routes
|
||||
description: Use when adding or changing a resource on the Huma-backed /api/v2 API (new endpoints, porting a v1 resource, editing pkg/routes/api/v2/). Covers per-operation Huma handlers, the shared envelopes, error/auth bridging, REST verb conventions, and what's automatic.
|
||||
user-invocable: true
|
||||
---
|
||||
|
||||
# Adding /api/v2 routes for a CRUDable resource
|
||||
|
||||
`/api/v2` is served by [Huma v2](https://github.com/danielgtaylor/huma) mounted on an Echo group via the vendored `pkg/modules/humaecho5` adapter. Unlike v1's generic `WebHandler`, each operation is a typed Huma handler registered explicitly. The handlers are thin: they pull auth off the context, call the same `pkg/web/handler.Do*` functions v1 uses, and translate domain errors into RFC 9457 responses.
|
||||
|
||||
**Reference implementation:** `pkg/routes/api/v2/labels.go` is the canonical example — copy its shape. Shared envelopes live in `pkg/routes/api/v2/types.go`; the auth/error bridge in `pkg/routes/api/v2/errors.go`; config in `pkg/routes/api/v2/huma.go`.
|
||||
|
||||
## Prerequisite: the model must be CRUDable
|
||||
|
||||
v2 handlers call `handler.DoReadAll/DoReadOne/DoCreate/DoUpdate/DoDelete`, which invoke the model's `Can*` methods. If the model isn't already a working v1 resource, do the model work first — invoke the **`crudable`** skill. Permissions are enforced at the model level; **never** re-check them in a v2 handler.
|
||||
|
||||
**Every exposed model field needs a `doc:` tag.** v2's schema is reflected from struct tags at runtime; Huma cannot read the Go doc comments swaggo uses for v1. A field without `doc:"..."` ships with no description in the spec. Add the tag alongside the existing comment (keep both — swaggo still reads the comment for v1, and they should stay in sync):
|
||||
|
||||
```go
|
||||
// The title of the label. You'll see this one on tasks associated with it.
|
||||
Title string `json:"title" minLength:"1" maxLength:"250" doc:"The title of the label. You'll see this one on tasks associated with it."`
|
||||
```
|
||||
|
||||
These model edits are safe for v1 — swaggo, XORM, and govalidator all ignore the `doc` tag. (Huma *does* read validation tags like `minLength`/`maxLength`/`enum`/`format`, so those carry over without a `doc` tag.) As with operations, a `doc` tag earns its place when it says something the field name and type don't: a format hint ("hex, 6 chars"), a read-only note ("set by the server; ignored on write"), units, or allowed values. "The label description." on a `Description` field is filler. See `pkg/models/label.go` for the reference.
|
||||
|
||||
## Steps
|
||||
|
||||
### 1. Create `pkg/routes/api/v2/<resource>.go`
|
||||
|
||||
Define the list-response body, a `Register<Resource>Routes(api huma.API)` function, and one handler per operation. Mirror `labels.go` exactly:
|
||||
|
||||
```go
|
||||
// Element type matches what models.<Model>.ReadAll returns; extra fields
|
||||
// tagged json:"-" keep the wire shape identical to the plain model.
|
||||
type fooListBody struct {
|
||||
Body Paginated[*models.Foo]
|
||||
}
|
||||
|
||||
func RegisterFooRoutes(api huma.API) {
|
||||
tags := []string{"foos"}
|
||||
|
||||
Register(api, huma.Operation{
|
||||
OperationID: "foos-list",
|
||||
Summary: "List foos",
|
||||
Description: "Returns the foos the authenticated user has access to, paginated.",
|
||||
Method: http.MethodGet, Path: "/foos", Tags: tags,
|
||||
}, foosList)
|
||||
Register(api, huma.Operation{OperationID: "foos-read", Summary: "Get a foo", Description: "...", Method: http.MethodGet, Path: "/foos/{id}", Tags: tags}, foosRead)
|
||||
Register(api, huma.Operation{OperationID: "foos-create", Summary: "Create a foo", Description: "...", Method: http.MethodPost, Path: "/foos", Tags: tags}, foosCreate)
|
||||
Register(api, huma.Operation{OperationID: "foos-update", Summary: "Update a foo", Description: "...", Method: http.MethodPut, Path: "/foos/{id}", Tags: tags}, foosUpdate)
|
||||
Register(api, huma.Operation{OperationID: "foos-delete", Summary: "Delete a foo", Description: "...", Method: http.MethodDelete, Path: "/foos/{id}", Tags: tags}, foosDelete)
|
||||
}
|
||||
```
|
||||
|
||||
Use the package's `Register` wrapper, **not** `huma.Register` directly — it sets `DefaultStatus` from the verb (POST → 201, DELETE → 204). Don't spell out `DefaultStatus` unless you need a non-default code. Don't set `Security:` per operation — it's applied globally in `NewAPI`.
|
||||
|
||||
**Every operation needs a `Summary` and `Description`.** v2's OpenAPI spec is generated from these `Operation` fields at runtime — unlike v1's swaggo, Huma cannot read Go doc comments, so anything you don't put in the `Operation` (or in a `doc:` tag, see below) is simply absent from the spec and the docs UI. An operation without them ships undocumented.
|
||||
|
||||
**Make the description document the non-obvious — don't restate the verb+noun.** "Deletes a label" adds nothing over `DELETE /labels/{id}`. Spend the description on what a consumer *can't* infer from the method/path/schema: permission scope ("only the owner may delete it"; "returns only labels you can see, not a global list"), full-replace vs partial (PUT replaces, PATCH merges), read-only/conditional behavior (ETag → `If-None-Match` → 304), side effects (create sets ownership), non-obvious status codes. If the honest description is just the verb+noun, a short summary alone is fine — don't pad. See `labels.go` for the calibration.
|
||||
|
||||
### 2. Write the handlers
|
||||
|
||||
Every handler: pull auth with `authFromCtx(ctx)`, call the matching `handler.Do*`, wrap returned errors in `translateDomainError`. Use the shared envelopes from `types.go` (`singleBody`, `singleReadBody`, `emptyBody`, `ListParams`, `Paginated`/`NewPaginated`).
|
||||
|
||||
- **List** takes `*ListParams` (gives you `page`/`per_page`/`q` for free) and returns `*fooListBody`. **You must type-assert the `DoReadAll` result to the concrete slice** — `result` is `any`, and a blind cast or a generic wrapper silently serialises `[]` (the "generic-any silent-empty trap"). Return a hard error on mismatch:
|
||||
```go
|
||||
items, ok := result.([]*models.Foo)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("foos.ReadAll returned unexpected type %T", result)
|
||||
}
|
||||
return &fooListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil
|
||||
```
|
||||
- **Read** embeds `conditional.Params` in its input, builds an ETag from `id + Updated.UnixNano()`, calls `in.PreconditionFailed(etag, label.Updated)` when `in.HasConditionalParams()`, and returns `*singleReadBody[Model]` with the **quoted** ETag (`"`+etag+`"`).
|
||||
- **Create / Update** take a `Body Model` input and return `*singleBody[Model]`. Update sets `in.Body.ID = in.ID` (URL wins over body).
|
||||
- **Delete** returns `*emptyBody`.
|
||||
|
||||
### 3. Wire it into the group
|
||||
|
||||
In `pkg/routes/routes.go`, `registerAPIRoutesV2`, add the registration **before** `EnableAutoPatch(api)`:
|
||||
|
||||
```go
|
||||
apiv2.RegisterFooRoutes(api)
|
||||
// ... other resources ...
|
||||
apiv2.EnableAutoPatch(api) // MUST stay last — walks registered GET+PUT pairs
|
||||
```
|
||||
|
||||
That's the only edit outside the v2 package for a standard CRUD resource.
|
||||
|
||||
## REST verb conventions (v2 inverts v1)
|
||||
|
||||
| Operation | v1 | v2 |
|
||||
|---|---|---|
|
||||
| create | PUT | **POST** |
|
||||
| update | POST | **PUT** (and PATCH) |
|
||||
| read / read-all / delete | GET / GET / DELETE | same |
|
||||
|
||||
## Non-CRUDable / custom routes
|
||||
|
||||
Not everything is plain CRUD — bulk operations, custom actions (`POST /tasks/{id}/duplicate`), sub-resource toggles, RPC-ish endpoints. These still go through Huma and reuse most of the machinery, but two responsibilities move **into your handler** because there's no `handler.Do*` doing them for you:
|
||||
|
||||
1. **Permission enforcement is now yours.** This is the one place the "never check permissions in the handler" rule inverts. With no generic `Do*` to call the model's `Can*`, the handler must do it explicitly — load the relevant entity and call its permission method, then refuse on denial. Mirror the v1 custom-handler shape (`pkg/routes/api/v1/task_attachment.go`):
|
||||
```go
|
||||
func tasksDuplicate(ctx context.Context, in *struct{ ID int64 `path:"id"` }) (*singleBody[models.Task], error) {
|
||||
a, err := authFromCtx(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
t := &models.Task{ID: in.ID}
|
||||
can, err := t.CanUpdate(s, a) // or whichever Can* gates this action
|
||||
if err != nil {
|
||||
_ = s.Rollback()
|
||||
return nil, translateDomainError(err)
|
||||
}
|
||||
if !can {
|
||||
return nil, huma.Error403Forbidden("forbidden")
|
||||
}
|
||||
// ... do the work against s ...
|
||||
if err := s.Commit(); err != nil {
|
||||
return nil, translateDomainError(err)
|
||||
}
|
||||
return &singleBody[models.Task]{Body: t}, nil
|
||||
}
|
||||
```
|
||||
2. **Session / transaction management is now yours.** The `Do*` helpers open and commit their own `xorm.Session`; custom handlers open one with `db.NewSession()`, `defer s.Close()`, and `Commit`/`Rollback` explicitly for anything that writes.
|
||||
|
||||
Otherwise the same rules apply: register with the `Register` wrapper, pull auth via `authFromCtx`, route every error through `translateDomainError`, and reuse the `types.go` envelopes — or define a small body struct when none fits (don't bend a custom response into `singleBody` if it's awkward).
|
||||
|
||||
**Verb choice:** pick by semantics, not the CRUD table. Non-idempotent actions are `POST`. AutoPatch only synthesises PATCH for GET+PUT *pairs*, so standalone custom routes are never touched.
|
||||
|
||||
**Token permissions still automatic, but mind the derived name:** `collectRoutesForAPITokens` keys a route off its prefix-stripped path, so `POST /api/v2/tasks/{id}/duplicate` lands under the `tasks` group as a `duplicate` permission. Single-segment custom paths fall into the `other` group. Name the path so the derived `(group, permission)` reads sensibly — that string is what users grant tokens against.
|
||||
|
||||
## What's automatic — do NOT hand-roll
|
||||
|
||||
- **PATCH** — `EnableAutoPatch` synthesises a JSON-Merge-Patch PATCH for every GET+PUT pair. Don't register PATCH yourself.
|
||||
- **API token permissions** — `collectRoutesForAPITokens` walks the Echo router after registration, so your new routes land in the v2 token table automatically under the same `(group, permission)` keys as their v1 names. PATCH is intentionally not stored; `CanDoAPIRoute` accepts it as an alias for the stored PUT (see `pkg/models/api_routes.go`).
|
||||
- **Security schemes** — `JWTKeyAuth` + `APITokenAuth` are declared globally in `NewAPI`. For a public endpoint, set `Security: []map[string][]string{}` on that operation and add its path to `unauthenticatedAPIPaths` in `routes.go`.
|
||||
- **Error shape** — `translateDomainError` maps any `web.HTTPErrorProcessor` (e.g. `ErrFooDoesNotExist`) onto Huma's status error, producing RFC 9457 `application/problem+json`. Errors without HTTP semantics become 500.
|
||||
- **OpenAPI spec / Scalar docs / `$schema` URLs** — handled in `huma.go`. Leave `Servers` alone (the relative entry must stay at index 0).
|
||||
|
||||
## Anti-patterns (these get flagged)
|
||||
|
||||
- Re-checking permissions in the handler instead of trusting `handler.Do*` → the model's `Can*`.
|
||||
- Blind `result.([]*models.Foo)` without the `ok` check, or returning the `any` straight into the envelope — silent empty lists.
|
||||
- `huma.Register` instead of the package `Register` wrapper (loses the verb-based status).
|
||||
- Per-operation `Security:` lines (now global) or registering a manual PATCH (AutoPatch does it).
|
||||
- Returning a raw model error instead of routing it through `translateDomainError` → leaks a 500 instead of the right code.
|
||||
- Unquoted ETag in the response header.
|
||||
- Operations without `Summary`/`Description`, or model fields without `doc:` tags — they ship undocumented because Huma can't read Go comments.
|
||||
|
||||
## Tests (mandatory)
|
||||
|
||||
Mirror the v1 webtest shape so v2 parity is readable side-by-side. Use the `webHandlerTestV2` harness in `pkg/webtests/integrations.go` — it takes the same `urlParams` map as v1's `webHandlerTest`. See `pkg/webtests/huma_label_test.go`:
|
||||
|
||||
- One `Test<Resource>` covering list/read/create/update/delete, positive + negative (forbidden, nonexistent), mirroring the v1 model test.
|
||||
- v2-only behaviour (ETag/304, PATCH merge-patch) goes in separate top-level `Test<Resource>_*` funcs using the `humaRequest`/`humaTokenFor` helpers in `pkg/webtests/huma_helpers_test.go`.
|
||||
- The RFC 9457 error-body shape is asserted **once** globally in `TestHuma_ErrorShapeIsRFC9457` — don't re-assert the full problem+json shape per resource, just the status code.
|
||||
|
||||
Run with `mage test:filter Test<Resource>` while iterating. Save output to a file per the project test-output rule.
|
||||
|
||||
## Related
|
||||
|
||||
- `crudable` skill — the model-layer prerequisite
|
||||
- `pkg/routes/api/v2/labels.go` — reference resource
|
||||
- `pkg/routes/api/v2/{types,errors,huma}.go` — shared envelopes, bridge, config
|
||||
- `pkg/web/handler/core.go` — the `Do*` functions handlers call
|
||||
28
AGENTS.md
28
AGENTS.md
|
|
@ -11,12 +11,24 @@ The project consists of:
|
|||
- `desktop/` – Electron wrapper application
|
||||
- `docs/` – Documentation website
|
||||
|
||||
## API Version Policy — new work goes to /api/v2
|
||||
|
||||
**`/api/v1` is effectively deprecated and frozen.** It still runs and is fully supported for existing clients, but it should not grow.
|
||||
|
||||
- **Every new route goes on `/api/v2`** (the Huma-backed API in `pkg/routes/api/v2/`). This includes new CRUDable entities, new custom/non-CRUD endpoints, and new actions on existing resources.
|
||||
- **Before adding any v2 route, invoke the `api-v2-routes` skill** — it covers both CRUD and non-CRUD shapes.
|
||||
- **Touch `/api/v1` only to:** fix a bug, or port an existing resource to v2. Do not add net-new functionality there.
|
||||
- Models in `pkg/models/` are shared by both APIs — a new entity still gets its model + `Can*` methods (invoke `crudable`); only the HTTP surface differs (v2, not v1).
|
||||
|
||||
If a task says "add an endpoint for X" without naming a version, it means v2.
|
||||
|
||||
## Skills
|
||||
|
||||
Before writing code in these areas, invoke the matching skill with the `Skill` tool. They are short checklists derived from recurring review feedback — loading them up front avoids rework.
|
||||
|
||||
- Adding or modifying a model in `pkg/models/` (new CRUD, new or changed `Can*` methods, anything touching permissions): invoke `crudable`.
|
||||
- Creating or editing any file under `pkg/migration/`: invoke `migration`.
|
||||
- Adding **any** new API route (new entity, custom action, or porting from v1) — all new routes go on the Huma-backed `/api/v2`, editing `pkg/routes/api/v2/`: invoke `api-v2-routes`. See the API Version Policy above.
|
||||
|
||||
## Plans and Worktrees
|
||||
|
||||
|
|
@ -172,11 +184,10 @@ Modern Vue 3 composition API application with TypeScript:
|
|||
### Adding New Features
|
||||
|
||||
**Backend Changes:**
|
||||
1. Create/modify models in `pkg/models/` with proper CRUD and Permissions interfaces as required
|
||||
2. Add database migration if needed: `mage dev:make-migration <StructName>`
|
||||
1. Create/modify models in `pkg/models/` with proper CRUD and Permissions interfaces as required (invoke the `crudable` skill)
|
||||
2. Add database migration if needed: `mage dev:make-migration <StructName>` (invoke the `migration` skill)
|
||||
3. Create/update services in `pkg/services/` for complex business logic
|
||||
4. Add API routes in `pkg/routes/api/v1/` following existing patterns
|
||||
5. Update Swagger annotations
|
||||
4. Add API routes on **`/api/v2`** in `pkg/routes/api/v2/` — invoke the `api-v2-routes` skill. Do **not** add new routes to `/api/v1`; it is frozen (see API Version Policy above)
|
||||
|
||||
**Frontend Changes:**
|
||||
1. Create TypeScript interfaces in `src/modelTypes/` matching backend models
|
||||
|
|
@ -192,10 +203,11 @@ Modern Vue 3 composition API application with TypeScript:
|
|||
4. Update TypeScript interfaces in frontend `src/modelTypes/`
|
||||
|
||||
### API Development
|
||||
- All API endpoints follow RESTful conventions under `/api/v1/`
|
||||
- Use generic web handlers in `pkg/web/handler/` for standard CRUD operations
|
||||
- Implement proper permissions checking using the Permissions interface
|
||||
- Add Swagger annotations for automatic documentation generation
|
||||
- **New endpoints go on `/api/v2`** (Huma-backed, `pkg/routes/api/v2/`). `/api/v1` is frozen — see the API Version Policy near the top. Invoke the `api-v2-routes` skill before writing v2 routes.
|
||||
- v2 verb conventions differ from v1: POST creates, PUT/PATCH update (v1 used PUT to create, POST to update).
|
||||
- Both versions reuse the generic `pkg/web/handler/` `Do*` functions for standard CRUD, which enforce permissions via the model's `Can*` methods.
|
||||
- Implement permission checks at the model level via the Permissions interface — never in the route handler (the exception: non-CRUD v2 actions must call `Can*` explicitly; the skill covers this).
|
||||
- v2 generates its OpenAPI spec from Go types automatically — no Swagger annotations. v1's swaggo annotations stay as-is but no new ones are needed.
|
||||
|
||||
### Testing
|
||||
- Backend: Feature tests alongside source files, web tests in `pkg/webtests/`
|
||||
|
|
|
|||
Loading…
Reference in New Issue