API error bodies: inconsistencies found while documenting every route (#264) #272

New issue

Closed

opened 2026-10-13 20:32:31 +02:00 by dominik.polakovics · 0 comments

dominik.polakovics commented

2026-10-13 20:32:31 +02:00

Owner

This was generated by AI during triage.

Summary

Make every API error body a coded message with the right code, the right args and the right owner: one rule for page-path validation, a code for each engine refusal instead of error.unexpected, no composed English or absolute paths inside args, and docs/api.md updated with each change. Found while verifying the docs for #264; every item below is confirmed in the code.

Current behavior

Reproduced against a fresh single-page site (about.md):

Request	Answer today
`GET /api/media/bundle` without `page`	`400 page.pathMissing`, text `missing required parameter "path"`, `args: {}`
`POST /api/element/delete` without `page`	`400 request.missingParam {name: "path"}` (the route reads `page`)
`POST /api/status`	`405 text/plain "method not allowed"` — the only uncoded error body
`POST /api/element/delete` with a stale path `[5]`	`409 error.unexpected {detail: "resolve: path[0] = 5 is out of range (the parent has 0 children)"}`

Confirmed by reading:

Two owners of the page-path rule. The server (resolvePageFile, refuseTranslationPath in internal/server) re-implements what pageops owns (CleanPath, refuseTranslationPath in internal/pageops/paths.go), with different codes: an empty path is request.missingParam {name: "path"} in the server but page.pathMissing in pageops; a translation file's path is editor.translationPath in the server but page.translationPath in pageops.
The server's missing-parameter message always says path, even on routes whose key is page (page/save, every element route, element/body, element/refs, element/delete-plan, move-refs, the form routes).
content.MutateError (stale path, index out of range, a node that cannot hold children, move into itself) and content.Parse/content.Serialize failures reach the wire as error.unexpected {detail} at 409 or 422 (element insert/remove/duplicate/move, element transfer, pageAndTree, serializePage).
A media reference that escapes the media root answers 400 error.unexpected in the server (media.ResolvePath error wrapped by writeError) and in mediaops resolveFile, although library.invalidRef {ref} exists and is used by media delete.
mediaops folderName puts the English text the library root into args.folder of media.folderNotFound.
translation.appeared {path} (pageops translate) carries the absolute filesystem path of the file that appeared.
A failed publish job's args is omitempty on publishJobState, so it vanishes when empty; every other error body carries args: {}.
field.unknown's args.field is the bare name for an item-row key: items[0].foo comes back as foo (the form checker in internal/server/forms.go passes the row name on), while the other 400s echo the key as sent.
Stale doc comments: handlePublishAll says "requireGit's 409/503" (requireGit only writes 409); pageResponse.Translation and translationInfo say translation appears only on GET page, page/save and element/enabled (it is on every page answer in a non-default language); transferResponse's comment on Target and handleElementInsert's response comment do not mention uid.

Desired behavior

Every error body on every /api/ route, including a wrong method on GET /api/status, is JSON {error, code, args} with args always an object.
Page-path validation (empty, non-local, translation file) is evaluated in exactly one place, pageops, and the server calls it. Each refusal has exactly one code on the wire: page.translationPath for a translation file's path, request.missingParam {name} for an empty path, where name is the key the route actually reads (page or path, as docs/api.md documents per route). editor.translationPath and page.pathMissing no longer exist in the registry or the catalog.
An engine refusal has a code of its own: one per content.MutateError kind (stale or out-of-range path, a node that cannot hold children, a move into itself or an empty path), page.parseFailed {detail} when a page does not parse, page.serializeRefused {detail} when the serializer refuses the result. Status codes stay as they are (409 for a stale request, 422 for refused content). The English text keeps the detail it has today.
A media reference that escapes the media root answers 400 library.invalidRef {ref} from every route that resolves one, with the rule evaluated once.
args never contains composed English: media.folderNotFound carries the folder as sent, and the root case has a code of its own (or no such error can arise for the root; decide from what resolveFolder can actually fail on, and document the answer).
translation.appeared {path} carries the content-root-relative path of the translation file (about.de.md), never an absolute path.
A failed publish job's args is on the wire as {} when the message names nothing.
field.unknown's args.field is the key as sent (items[0].foo), like every other field error.
The five stale doc comments say what the code does.
docs/api.md describes every changed shape: the "Errors" and "Coded messages" conventions, the per-route error lists that name the affected codes, the publish job state, and the GET /api/status 405.

Decisions

One issue, one PR — every item is a small edit to an error body plus its line in docs/api.md; splitting would cost two catalog regenerations for no independent value.
pageops owns page-path validation; the server calls it — CLAUDE.md "Every rule has exactly one owner"; the owner already exists and is tested there. Moving the owner is part of the change if the server needs something pageops does not export today.
"The param is missing" stays a request-shape check with the route's own key — request.missingParam {name} is what every other missing parameter answers; a page-specific spelling was the second copy, so page.pathMissing is retired rather than given a name arg.
Engine refusals get codes; internal/content does not import apimsg — the engine stays free of the API vocabulary. MutateError carries a kind the server maps to a code in one function; that function is the only place the mapping lives.
lastSnapshotError and unsyncedReason stay uncoded — ADR 0020 and docs/api.md "Coded messages" list them as deliberately uncoded git text, and the git state's reason already carries a coded message beside it. Not in scope.
Codes are retired, not kept as aliases — the SPA's catalog has no code that switches on any affected code (verified: no web/src TypeScript references them), so the only client change is the regenerated servermessages.json and the en.json/de.json entries.

Key interfaces

internal/apimsg — the registry: new codes for the engine refusals and the parse/serialize failures, editor.translationPath and page.pathMissing removed. TestCallSites enforces literal args at every call site; TestExport -update regenerates web/src/i18n/servermessages.json, and en.json/de.json need the matching entries (German included).
content.MutateError — gains a kind (an enum, not a string match on the message) the server reads; its Error() text stays.
resolvePageFile / refuseTranslationPath (internal/server/server.go) — call pageops' validation; the function takes, or its callers supply, the request key so the missing-param message names it.
writeError/writeOpError (internal/server) — the one place a MutateError kind and a parse/serialize failure become a code.
media.ResolvePath (internal/media/library.go) and mediaops resolveFile — the escaping-ref refusal is coded once; both the server's media routes and mediaops answer library.invalidRef.
mediaops folderName (internal/mediaops/paths.go) — no longer composes English into args.
publishJobState.Args (internal/server/publish.go) — no omitempty.
The form checker's item-row path (internal/server/forms.go, checkField) — the unknown-field error echoes the full key.
docs/api.md — owner of every wire shape above. docs/architecture.md only if its error narrative names a retired code (verify).
Not affected: scaffold/site/AGENTS.md, the scaffolded skills, docs/harness.md — none names an API error code (verified by grep).

Acceptance criteria

A wrong method on GET /api/status answers 405 with a JSON request.method body, like every other route.
GET /api/media/bundle without page and POST /api/element/delete without page answer 400 request.missingParam {name: "page"}; GET /api/page without path answers {name: "path"}.
A translation file's path answers 400 page.translationPath on every route that resolves a page, server and pageops alike; editor.translationPath and page.pathMissing are absent from internal/apimsg, servermessages.json, en.json and de.json.
The server no longer implements page-path cleaning or translation-path refusal itself; a test in internal/pageops covers the rule and the server tests cover the status and code only.
A stale element path, an out-of-range index, a child of a non-container, and a move into itself each answer 409 with their own code and args; error.unexpected no longer appears for any content.MutateError.
A page that does not parse answers 422 page.parseFailed {detail}; a serializer refusal answers 422 page.serializeRefused {detail}.
A media ref that escapes the root answers 400 library.invalidRef {ref} from every media route and from mediaops.
No args value anywhere is composed English text; media.folderNotFound's args.folder is the folder as sent.
translation.appeared's args.path is content-root-relative.
A failed publish job serializes "args": {} when the message names nothing.
field.unknown for items[0].foo carries args.field: "items[0].foo".
The five listed doc comments are correct.
docs/api.md names every new code at the routes that answer it and no retired code anywhere; docs/docs_test.go and the route test in internal/server pass.

How to verify

go test ./... and make check (or the repo's equivalent lint/test target) pass, including TestCallSites, TestExport and the SPA's catalog parity test under web/.
Repeat the four requests in the reproduction table against a fresh site and compare the bodies with the acceptance criteria.
grep -rn "editor.translationPath\|page.pathMissing" internal web/src docs finds nothing.
grep -rn "error.unexpected" docs/api.md names only underlying failures (filesystem, git, decoder), none of the engine refusals.

Assumptions to check

That no route other than the ones listed reaches content.Parse/Serialize or a MutateError through a different write helper; grep for writeError( with a content error before trusting the list.
That media.folderNotFound can arise for the root at all; if it cannot, the root case needs no code.
That docs/architecture.md does not name a retired code.

Out of scope

Coding lastSnapshotError, unsyncedReason, Hugo output or sidecar load errors (deliberately uncoded, ADR 0020).
Changing any HTTP status, any English text beyond what a new code requires, or any success shape.
Translating anything server-side.

Blocked by

None - can start immediately

Original report

Verifying docs/api.md against the handlers for #264 turned up these error-body inconsistencies. The docs describe the code as it is; each is a candidate fix.

One rule, two owners (CLAUDE.md): an empty page path is request.missingParam {name: "path"} in the server (resolvePageFile) but page.pathMissing in pageops (pageops/paths.go:48); a path naming a translation file is editor.translationPath in the server (refuseTranslationPath) but page.translationPath in pageops (paths.go:260).
resolvePageFile (server.go ~709) names the missing parameter path even on routes whose key is page (page/save, every element route, element/body, element/refs, element/delete-plan, move-refs, the form routes). GET /api/media/bundle without page answers page.pathMissing, whose text says "path".
GET /api/status answers a wrong method with plain text via http.Error (server.go ~354) — the only uncoded error body.
content.MutateError (stale path, index out of range, move into itself) and content parse/serialize errors surface as error.unexpected {detail} at 409/422 (element.go:159,230,319,439; element_transfer.go:284; server.go:806,1166) rather than codes of their own.
An escaping media reference answers 400 error.unexpected (media.go:354, :882, mediaops resolveFile) although library.invalidRef exists.
mediaops folderName puts the English text "the library root" into args.folder of media.folderNotFound — untranslatable text inside args.
translation.appeared {path} (pageops/translation.go:323) carries the absolute filesystem path.
A failed publish job's args is omitempty (publish.go:136), so it disappears when empty, unlike every other error body's args: {}.
field.unknown's args.field is the bare name for items[<i>].<unknown> (forms.go:407-412), while the other 400s echo the key as sent.
lastSnapshotError and unsyncedReason (git/state, git/sync) are server/git text with no code beside them.
Stale doc comments: handlePublishAll says "requireGit's 409/503" (requireGit only writes 409); pageResponse.Translation (server.go:488) and translationInfo (translation.go:41) say translation appears only on GET page, page/save and element/enabled (it is on every page answer in a non-default language); transferResponse on Target.Uid; handleElementInsert omits uid.

> *This was generated by AI during triage.* ## Summary Make every API error body a coded message with the right code, the right args and the right owner: one rule for page-path validation, a code for each engine refusal instead of `error.unexpected`, no composed English or absolute paths inside `args`, and `docs/api.md` updated with each change. Found while verifying the docs for #264; every item below is confirmed in the code. ## Current behavior Reproduced against a fresh single-page site (`about.md`): | Request | Answer today | | --- | --- | | `GET /api/media/bundle` without `page` | `400 page.pathMissing`, text `missing required parameter "path"`, `args: {}` | | `POST /api/element/delete` without `page` | `400 request.missingParam {name: "path"}` (the route reads `page`) | | `POST /api/status` | `405 text/plain "method not allowed"` — the only uncoded error body | | `POST /api/element/delete` with a stale path `[5]` | `409 error.unexpected {detail: "resolve: path[0] = 5 is out of range (the parent has 0 children)"}` | Confirmed by reading: - **Two owners of the page-path rule.** The server (`resolvePageFile`, `refuseTranslationPath` in `internal/server`) re-implements what pageops owns (`CleanPath`, `refuseTranslationPath` in `internal/pageops/paths.go`), with different codes: an empty path is `request.missingParam {name: "path"}` in the server but `page.pathMissing` in pageops; a translation file's path is `editor.translationPath` in the server but `page.translationPath` in pageops. - The server's missing-parameter message always says `path`, even on routes whose key is `page` (page/save, every element route, element/body, element/refs, element/delete-plan, move-refs, the form routes). - `content.MutateError` (stale path, index out of range, a node that cannot hold children, move into itself) and `content.Parse`/`content.Serialize` failures reach the wire as `error.unexpected {detail}` at 409 or 422 (element insert/remove/duplicate/move, element transfer, `pageAndTree`, `serializePage`). - A media reference that escapes the media root answers `400 error.unexpected` in the server (`media.ResolvePath` error wrapped by `writeError`) and in mediaops `resolveFile`, although `library.invalidRef {ref}` exists and is used by media delete. - mediaops `folderName` puts the English text `the library root` into `args.folder` of `media.folderNotFound`. - `translation.appeared {path}` (pageops translate) carries the absolute filesystem path of the file that appeared. - A failed publish job's `args` is `omitempty` on `publishJobState`, so it vanishes when empty; every other error body carries `args: {}`. - `field.unknown`'s `args.field` is the bare name for an item-row key: `items[0].foo` comes back as `foo` (the form checker in `internal/server/forms.go` passes the row name on), while the other 400s echo the key as sent. - Stale doc comments: `handlePublishAll` says "requireGit's 409/503" (requireGit only writes 409); `pageResponse.Translation` and `translationInfo` say `translation` appears only on GET page, page/save and element/enabled (it is on every page answer in a non-default language); `transferResponse`'s comment on `Target` and `handleElementInsert`'s response comment do not mention `uid`. ## Desired behavior - Every error body on every `/api/` route, including a wrong method on `GET /api/status`, is JSON `{error, code, args}` with `args` always an object. - Page-path validation (empty, non-local, translation file) is evaluated in exactly one place, pageops, and the server calls it. Each refusal has exactly one code on the wire: `page.translationPath` for a translation file's path, `request.missingParam {name}` for an empty path, where `name` is the key the route actually reads (`page` or `path`, as `docs/api.md` documents per route). `editor.translationPath` and `page.pathMissing` no longer exist in the registry or the catalog. - An engine refusal has a code of its own: one per `content.MutateError` kind (stale or out-of-range path, a node that cannot hold children, a move into itself or an empty path), `page.parseFailed {detail}` when a page does not parse, `page.serializeRefused {detail}` when the serializer refuses the result. Status codes stay as they are (409 for a stale request, 422 for refused content). The English text keeps the detail it has today. - A media reference that escapes the media root answers `400 library.invalidRef {ref}` from every route that resolves one, with the rule evaluated once. - `args` never contains composed English: `media.folderNotFound` carries the folder as sent, and the root case has a code of its own (or no such error can arise for the root; decide from what `resolveFolder` can actually fail on, and document the answer). - `translation.appeared {path}` carries the content-root-relative path of the translation file (`about.de.md`), never an absolute path. - A failed publish job's `args` is on the wire as `{}` when the message names nothing. - `field.unknown`'s `args.field` is the key as sent (`items[0].foo`), like every other field error. - The five stale doc comments say what the code does. - `docs/api.md` describes every changed shape: the "Errors" and "Coded messages" conventions, the per-route error lists that name the affected codes, the publish job state, and the `GET /api/status` 405. ## Decisions - **One issue, one PR** — every item is a small edit to an error body plus its line in `docs/api.md`; splitting would cost two catalog regenerations for no independent value. - **pageops owns page-path validation; the server calls it** — CLAUDE.md "Every rule has exactly one owner"; the owner already exists and is tested there. Moving the owner is part of the change if the server needs something pageops does not export today. - **"The param is missing" stays a request-shape check with the route's own key** — `request.missingParam {name}` is what every other missing parameter answers; a page-specific spelling was the second copy, so `page.pathMissing` is retired rather than given a `name` arg. - **Engine refusals get codes; `internal/content` does not import `apimsg`** — the engine stays free of the API vocabulary. `MutateError` carries a kind the server maps to a code in one function; that function is the only place the mapping lives. - **`lastSnapshotError` and `unsyncedReason` stay uncoded** — ADR 0020 and `docs/api.md` "Coded messages" list them as deliberately uncoded git text, and the git state's `reason` already carries a coded message beside it. Not in scope. - **Codes are retired, not kept as aliases** — the SPA's catalog has no code that switches on any affected code (verified: no `web/src` TypeScript references them), so the only client change is the regenerated `servermessages.json` and the `en.json`/`de.json` entries. ## Key interfaces - `internal/apimsg` — the registry: new codes for the engine refusals and the parse/serialize failures, `editor.translationPath` and `page.pathMissing` removed. `TestCallSites` enforces literal args at every call site; `TestExport -update` regenerates `web/src/i18n/servermessages.json`, and `en.json`/`de.json` need the matching entries (German included). - `content.MutateError` — gains a kind (an enum, not a string match on the message) the server reads; its `Error()` text stays. - `resolvePageFile` / `refuseTranslationPath` (`internal/server/server.go`) — call pageops' validation; the function takes, or its callers supply, the request key so the missing-param message names it. - `writeError`/`writeOpError` (`internal/server`) — the one place a `MutateError` kind and a parse/serialize failure become a code. - `media.ResolvePath` (`internal/media/library.go`) and mediaops `resolveFile` — the escaping-ref refusal is coded once; both the server's media routes and mediaops answer `library.invalidRef`. - mediaops `folderName` (`internal/mediaops/paths.go`) — no longer composes English into args. - `publishJobState.Args` (`internal/server/publish.go`) — no `omitempty`. - The form checker's item-row path (`internal/server/forms.go`, `checkField`) — the unknown-field error echoes the full key. - `docs/api.md` — owner of every wire shape above. `docs/architecture.md` only if its error narrative names a retired code (verify). - Not affected: `scaffold/site/AGENTS.md`, the scaffolded skills, `docs/harness.md` — none names an API error code (verified by grep). ## Acceptance criteria - [ ] A wrong method on `GET /api/status` answers `405` with a JSON `request.method` body, like every other route. - [ ] `GET /api/media/bundle` without `page` and `POST /api/element/delete` without `page` answer `400 request.missingParam {name: "page"}`; `GET /api/page` without `path` answers `{name: "path"}`. - [ ] A translation file's path answers `400 page.translationPath` on every route that resolves a page, server and pageops alike; `editor.translationPath` and `page.pathMissing` are absent from `internal/apimsg`, `servermessages.json`, `en.json` and `de.json`. - [ ] The server no longer implements page-path cleaning or translation-path refusal itself; a test in `internal/pageops` covers the rule and the server tests cover the status and code only. - [ ] A stale element path, an out-of-range index, a child of a non-container, and a move into itself each answer `409` with their own code and args; `error.unexpected` no longer appears for any `content.MutateError`. - [ ] A page that does not parse answers `422 page.parseFailed {detail}`; a serializer refusal answers `422 page.serializeRefused {detail}`. - [ ] A media ref that escapes the root answers `400 library.invalidRef {ref}` from every media route and from mediaops. - [ ] No `args` value anywhere is composed English text; `media.folderNotFound`'s `args.folder` is the folder as sent. - [ ] `translation.appeared`'s `args.path` is content-root-relative. - [ ] A failed publish job serializes `"args": {}` when the message names nothing. - [ ] `field.unknown` for `items[0].foo` carries `args.field: "items[0].foo"`. - [ ] The five listed doc comments are correct. - [ ] `docs/api.md` names every new code at the routes that answer it and no retired code anywhere; `docs/docs_test.go` and the route test in `internal/server` pass. ## How to verify - `go test ./...` and `make check` (or the repo's equivalent lint/test target) pass, including `TestCallSites`, `TestExport` and the SPA's catalog parity test under `web/`. - Repeat the four requests in the reproduction table against a fresh site and compare the bodies with the acceptance criteria. - `grep -rn "editor.translationPath\|page.pathMissing" internal web/src docs` finds nothing. - `grep -rn "error.unexpected" docs/api.md` names only underlying failures (filesystem, git, decoder), none of the engine refusals. ## Assumptions to check - That no route other than the ones listed reaches `content.Parse`/`Serialize` or a `MutateError` through a different write helper; grep for `writeError(` with a `content` error before trusting the list. - That `media.folderNotFound` can arise for the root at all; if it cannot, the root case needs no code. - That `docs/architecture.md` does not name a retired code. ## Out of scope - Coding `lastSnapshotError`, `unsyncedReason`, Hugo output or sidecar load errors (deliberately uncoded, ADR 0020). - Changing any HTTP status, any English text beyond what a new code requires, or any success shape. - Translating anything server-side. ## Blocked by None - can start immediately ## Original report Verifying docs/api.md against the handlers for #264 turned up these error-body inconsistencies. The docs describe the code as it is; each is a candidate fix. - **One rule, two owners** (CLAUDE.md): an empty page path is `request.missingParam {name: "path"}` in the server (`resolvePageFile`) but `page.pathMissing` in pageops (pageops/paths.go:48); a path naming a translation file is `editor.translationPath` in the server (`refuseTranslationPath`) but `page.translationPath` in pageops (paths.go:260). - `resolvePageFile` (server.go ~709) names the missing parameter `path` even on routes whose key is `page` (page/save, every element route, element/body, element/refs, element/delete-plan, move-refs, the form routes). `GET /api/media/bundle` without `page` answers `page.pathMissing`, whose text says "path". - `GET /api/status` answers a wrong method with plain text via `http.Error` (server.go ~354) — the only uncoded error body. - `content.MutateError` (stale path, index out of range, move into itself) and content parse/serialize errors surface as `error.unexpected {detail}` at 409/422 (element.go:159,230,319,439; element_transfer.go:284; server.go:806,1166) rather than codes of their own. - An escaping media reference answers `400 error.unexpected` (media.go:354, :882, mediaops `resolveFile`) although `library.invalidRef` exists. - `mediaops` `folderName` puts the English text "the library root" into `args.folder` of `media.folderNotFound` — untranslatable text inside args. - `translation.appeared {path}` (pageops/translation.go:323) carries the absolute filesystem path. - A failed publish job's `args` is `omitempty` (publish.go:136), so it disappears when empty, unlike every other error body's `args: {}`. - `field.unknown`'s `args.field` is the bare name for `items[<i>].<unknown>` (forms.go:407-412), while the other 400s echo the key as sent. - `lastSnapshotError` and `unsyncedReason` (git/state, git/sync) are server/git text with no code beside them. - Stale doc comments: `handlePublishAll` says "requireGit's 409/503" (requireGit only writes 409); `pageResponse.Translation` (server.go:488) and `translationInfo` (translation.go:41) say `translation` appears only on GET page, page/save and element/enabled (it is on every page answer in a non-default language); `transferResponse` on Target.Uid; `handleElementInsert` omits `uid`.

dominik.polakovics added the

needs-triage

bug

labels

2026-10-13 20:32:31 +02:00

dominik.polakovics referenced this issue

2026-10-13 20:43:26 +02:00

docs: complete and verify the split documents — every route, CLI, features, architecture and 22 ADRs #274

dominik.polakovics referenced this issue

2026-10-13 21:39:52 +02:00

translate-plan and translate evaluate the translatable rule separately; plan approves what translate refuses #275

dominik.polakovics added