Cloonar/deckle

Fork 0

Minor API behaviour inconsistencies found while documenting every route (#264) #273

New issue

Closed

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

dominik.polakovics commented

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

Owner

This was generated by AI during triage.

Summary

Fourteen small API behaviours that the #264 docs pass found and documented as they are, each either an inconsistency between sibling routes or a second evaluation of a rule that already has an owner. Fix every one, add the test it lacks, and make docs/api.md describe the corrected behaviour.

Current behavior

All fourteen reproduce on main at a0c4ce7 by reading the handlers; no test covers any of them. Locations are starting hints, not instructions.

GET /api/link/resolve (handleLinkResolve, internal/server/linktargets.go) answers external and media values before opsRoot validates lang, so an unknown lang gets a 200 for those kinds and a 404 for the others. docs/api.md documents the quirk.
POST /api/element/duplicate (handleElementDuplicate, internal/server/element.go) runs placementViolation but never elemops.SlotElementsMsg; insert, move and transfer run both.
GET /api/page/refs (handlePageRefs) on the root _index.md collects every permalink in the tree and skips the "exclude the bundle's own directory" step because the directory is ., so it lists every file linking to any page. The root page cannot be deleted (pageops.Delete), so the delete warning never asks.
GET /api/media/library (handleMediaLibrary, internal/server/media.go) slug-checks folder with validMediaFolder and refuses a legacy non-slug folder with library.folderInvalid, while GET /api/media/tree lists it and every mediaops route addresses it (internal/mediaops/paths.go shape-checks, never slug-checks, on purpose).
GET /api/media/folder/delete-plan normalizes folder with trimFolderSlashes (\ → /, trims /); POST /api/media/folder/delete passes req.Folder raw to mediaops.DeleteFolder and normalizes only the snapshot subject, so a plan can succeed and the delete be refused.
POST /api/media/upload (handleMediaUpload) runs media.Process (decode and re-encode) before checking target and folder.
pageops.Rename and pageops.RenameTranslation refuse a blank title but store it untrimmed; pageops.Create stores it trimmed.
An item row's enabled (elemops.itemDisabled, internal/elemops/save.go) treats any value other than JSON false as enabled, a string included; an element's own enabled goes through element.ReadEnabled, which answers a field error for a non-boolean.
handleSavePage (internal/server/server.go) decodes its body inline, a second copy of decodeBody and its request.malformed answer.
POST /api/form with kind: "create": formRef.Parent is decoded (internal/server/forms.go) and the client's CreateDialog sends it, but the formCreate branch of resolveForm reads only Type and Lang.
In a connected translation, elementL10n (internal/server/translation.go) fills synced with every Appearance param whether or not a source element with that uid and name exists, so describe() (internal/server/forms.go) marks the fields readOnly with note.followsSourceElement when there is nothing to follow.
writeStructural (internal/server/translation_ops.go) snapshots with opSave, so element insert, delete, move and duplicate, and POST /api/element/enabled, all read "save" in the undo catalog although opCreate, opDelete, opMove, opEnable and opDisable exist (internal/server/snapshot_ops.go) and the client's undo.op.<op> catalog translates them.
l10n.UnplacedError (internal/l10n/translate.go) carries a plain message (l10n.beforeSlot, l10n.beforeParent, l10n.beforePage) and a parentFirst one; its only caller, handleTranslateElement, swaps to ParentFirst() when the uid is untranslated, and docs/api.md lists only the …ParentFirst codes.
GET /api/publish/status (handlePublishStatus, internal/server/publish.go) answers {phase: "static", code: "publish.static"} for any commit the status map does not hold, with no shape or existence check.

Desired behavior

GET /api/link/resolve validates lang first: an unknown lang is 404 editor.unknownLang for every kind, external and media included.
POST /api/element/duplicate refuses a copy the destination slot's elements allowlist does not admit, with the same code and status insert answers, by calling elemops.SlotElementsMsg.
GET /api/page/refs on the content root's own page answers 400 with a new coded message page.rootRefs, defined and worded like page.rootRename, page.rootMove and page.rootReorder. Every other page keeps its current answer.
GET /api/media/library lists a legacy non-slug folder exactly as GET /api/media/tree and the mediaops routes address it. An existing folder is accepted by the shape check mediaops already owns; the slug rule applies only where a folder comes into being (folder create, upload into a folder that does not exist yet) and keeps its current codes there. library.folderInvalid stays for a folder that fails the shape check.
POST /api/media/folder/delete and GET /api/media/folder/delete-plan normalize folder identically, through one owner both call, so any folder the plan accepts the delete accepts, and the executed plan equals the dry run for the same input.
POST /api/media/upload refuses a wrong target or an invalid folder before it decodes the file; the codes and statuses do not change.
Rename and translation rename store the trimmed title, exactly as create does, through one trim in pageops that create, rename and translation rename all call. A title that is blank after trimming is still refused with page.titleEmpty.
An item row whose enabled is present and not a JSON boolean is refused with the same field error an element's enabled gets, by calling element.ReadEnabled. Absent stays enabled; true and false keep their meaning. elemops keeps exactly one place that reads a row's enabled state back.
handleSavePage decodes its body through decodeBody; the answer for a malformed body is unchanged.
POST /api/form with kind: "create" uses parent: a parent that does not resolve answers the refusal POST /api/page/create answers for it, and a type the parent's page type does not allow answers the refusal create answers for that, with the same codes and statuses, so the dialog fails before the user fills the form. A valid parent changes nothing in the description.
In a connected translation, when no source element with the translation element's uid and name exists, the Appearance fields are ordinary editable fields: no readOnly, no note, no noteCode, not named in the description's synced list. When a source element exists, the current behaviour is unchanged.
Structural element writes label their WIP snapshots from the existing vocabulary: insert and duplicate create, delete delete, move move, enabled enable or disable by the resulting state. The snapshot subject stays the page. snapshotOps and the exported undo_ops.json do not change.
A test proves that an UnplacedError reaching POST /api/element/translate is always answered with the …ParentFirst code (or shows a case where it is not). If it is unreachable, UnplacedError carries one message, the three plain l10n.before* codes are deleted from internal/apimsg, and nothing else references them. If a reachable case turns up, keep both and document the plain codes in docs/api.md with the case that produces them.
GET /api/publish/status: a commit that is not a 40-character lowercase hex sha answers 400 with a new coded request. message for an invalid parameter value, carrying the parameter name; a well-formed sha the repository does not contain answers 404 with a new publish. code; static is answered only for a commit the repository has and the server never polled.

Every route touched has its docs/api.md section updated to the new behaviour: the error list, the key list and the prose that described the old behaviour (item 1's "answer whatever lang says", item 3's root sentence, item 11's read-only clause, the undo section's op words where relevant). Every new or removed code appears or disappears there.

Decisions

One issue, fourteen fixes — each is a few lines plus a test and a docs line, none depends on another, and a single pull request keeps the docs pass that found them in one review.
Call the owner, never a sibling (CLAUDE.md "Every rule has exactly one owner") — the owners are: opsRoot for the unknown-lang verdict (1); elemops.SlotElementsMsg for a slot's allowlist (2); the folder shape check in mediaops for an existing folder (4); one normalizer for a folder argument that both folder-delete routes call, moved into mediaops if that is where it fits (5); one title trim in pageops (7); element.ReadEnabled for an enabled value (8); decodeBody for a request body (9); elementL10n for what is synced (11). A fix that adds a second evaluation of any of these is wrong.
Root refs is a refusal, not a scoped answer — the root permalink is /, so any prefix scoping still matches every link; and the root cannot be deleted, so no caller needs the list.
Legacy folders are listed, not hidden — mediaops already decided an existing folder is shape-checked and never slug-checked; the library route follows that decision instead of the tree route being bent the other way.
The create form uses parent — the client already sends it, and failing at form time is the same refusal create would give, earlier.
No source element means not synced — a read-only field that follows nothing misleads; a dedicated "source gone" note would be a new wire fact for a state the translation repair path already handles.
Snapshot ops reuse the vocabulary — no new op words, so the client's undo catalog and the exported undo_ops.json stay as they are; a page delete and an element delete share the word delete, which the subject (the page) makes clear enough.
Publish status distinguishes malformed, unknown and unpolled — a mistyped sha should not look like a static-site publish; the existence check is a cheap git object lookup on a path the handler already has a repository for.
Dead codes are deleted only on proof — a test decides item 13, so the catalog is not pruned on a reading of the code alone.

Key interfaces

handleLinkResolve, handleElementDuplicate, handlePageRefs, handleMediaLibrary, handleMediaFolderDelete, handleMediaFolderDeletePlan, handleMediaUpload, handleSavePage, handlePublishStatus, handleElementEnabled in internal/server — the handlers whose order of checks or owner calls change.
writeStructural(w, root, full, page, lang, doc) in internal/server/translation_ops.go — needs to take, or be told, the snapshotOp of the write.
resolveForm / formRef in internal/server/forms.go — the formCreate branch reads Parent.
elementL10n and elementForm.describe() in internal/server — synced depends on srcSC.
pageops.Rename, pageops.RenameTranslation, pageops.Create — share one title trim.
elemops.itemDisabled / elemops.NewItem in internal/elemops/save.go — the one place a row's enabled state is read back; it should answer a field error through element.ReadEnabled.
validMediaFolder in internal/server/media.go and the folder path check in internal/mediaops/paths.go — which routes call which, per decision above.
l10n.UnplacedError, ParentFirst(), l10n.UntranslatedUIDs in internal/l10n — item 13.
internal/apimsg/codes_page.go, codes_server.go, codes_publish.go, codes_l10n.go — new codes page.rootRefs, an invalid-parameter request. code, an unknown-commit publish. code; the three plain l10n.before* codes possibly removed.
publishStatus map and the repository handle in internal/server/publish.go — the existence check.
docs/api.md — one ### section per route; the route drift test in internal/server keeps the heading set, the content is on you.

Acceptance criteria

Each of the fourteen desired behaviours above holds and has a test that fails on main and passes with the fix (item 13's test may instead prove the plain codes unreachable).
No fix adds a second evaluation of a rule listed under Decisions; each calls the named owner.
New coded messages are defined in internal/apimsg and every removed one has no remaining reference in Go, the client or the docs.
docs/api.md describes the new behaviour for every touched route, with the error lists updated.
snapshotOps and internal/server/testdata/undo_ops.json are unchanged.
The client's FormRef for kind: "create" still sends parent; nothing in web/src needs to change for items 10 to 12 unless a test asserts the old behaviour.
scaffold/site/AGENTS.md and the scaffolded skills are untouched (nothing they document changes); if a fix turns out to change a documented sidecar or command behaviour, update them in the same change.

How to verify

go test ./... and go vet ./... green, including the drift tests in internal/server (routes ⇔ docs/api.md headings, TestSnapshotOpsExport), docs/docs_test.go and internal/apimsg.
cd web && npm test green.
Against a scaffolded site (harness init into a temp dir, server started as docs/harness.md describes), exercise with curl: link/resolve?url=https://x&lang=zz → 404; page/refs?path=_index.md → 400 page.rootRefs; media/library?folder=<legacy name> → 200 listing; media/folder/delete-plan and media/folder/delete with a \-spelled folder agree; publish/status?commit=zzz → 400, commit=<40 hex not in repo> → 404.
Insert, delete, move and duplicate an element, then read the undo info: the ops read create, delete, move, create.

Assumptions to check

Item 13's deadness: handleTranslateElement swaps to ParentFirst() only when UntranslatedUIDs contains the uid; verify an UnplacedError can never arise for a uid outside that list before deleting the codes.
Item 14's existence check: confirm the handler can reach a repository handle cheaply (the publish job already opens one); if the server can run without a repository, decide what a missing repository answers and document it.
Item 10: confirm pageops exposes a parent-resolution and type-allowlist check callable at form time without creating anything; resolveParent and the type allowlist in internal/server/pageops.go are the starting points.
Item 4: confirm which mediaops function is the folder shape check and that it is importable by internal/server without a cycle.

Out of scope

Changing the snapshot-op vocabulary or the client's undo catalog.
A new note or wire state for a translation element whose source element is gone.
Hiding legacy folders from GET /api/media/tree.
Any route behaviour not listed in the fourteen items, even if found nearby.
docs/architecture.md, ADRs, scaffold/site/AGENTS.md, the scaffolded skills.

Blocked by

None - can start immediately.

Original report

Verifying docs/api.md against the handlers for #264 turned up these behaviours that look unintended. The docs describe the code as it is; each needs a decision.

GET /api/link/resolve answers external and media values before validating lang, so an unknown lang gets a 200 (linktargets.go:180-224).
POST /api/element/duplicate re-checks placement but not the slot's elements allowlist, unlike insert, move and transfer (element.go:294-312).
GET /api/page/refs on the root _index.md lists every file linking to any page (linktargets.go:376-409).
GET /api/media/library refuses a legacy non-slug folder with library.folderInvalid that GET /api/media/tree lists and mediaops keeps addressable (media.go:220).
GET /api/media/folder/delete-plan turns \\ into /; POST /api/media/folder/delete does not, so a plan can succeed and the delete be refused (media.go:778 vs 802).
POST /api/media/upload decodes and re-encodes the whole file before refusing a wrong target/folder (media.go:157-166).
Rename stores the title untrimmed (pageops/relocate.go:79, pageops/translation.go:100) while create trims it (create.go:193).
An item row's non-boolean enabled is silently enabled (elemops/save.go:335), while an element's enabled gets field.bool.
handleSavePage decodes its body itself instead of decodeBody (server.go:959) — a second owner of the rule.
FormRef parent is decoded but never read for kind create (forms.go:301-327); the client sends it.
In a connected translation, Appearance fields are marked readOnly with note.followsSourceElement even when no source element matches by uid (translation.go:376-380, forms.go:582-591).
Element insert/delete/move/duplicate (writeStructural) and element enabled label their snapshots save, so undoOp cannot tell them from a param save although delete, move, enable, disable ops exist.
Dead branch in translation_ops.go:527: the plain l10n.beforeSlot/beforeParent/beforePage codes never reach the wire.
GET /api/publish/status answers static for any commit it does not know (publish.go:709-712), a mistyped sha included.

> *This was generated by AI during triage.* ## Summary Fourteen small API behaviours that the #264 docs pass found and documented as they are, each either an inconsistency between sibling routes or a second evaluation of a rule that already has an owner. Fix every one, add the test it lacks, and make `docs/api.md` describe the corrected behaviour. ## Current behavior All fourteen reproduce on `main` at a0c4ce7 by reading the handlers; no test covers any of them. Locations are starting hints, not instructions. 1. `GET /api/link/resolve` (`handleLinkResolve`, `internal/server/linktargets.go`) answers `external` and `media` values before `opsRoot` validates `lang`, so an unknown `lang` gets a `200` for those kinds and a `404` for the others. `docs/api.md` documents the quirk. 2. `POST /api/element/duplicate` (`handleElementDuplicate`, `internal/server/element.go`) runs `placementViolation` but never `elemops.SlotElementsMsg`; insert, move and transfer run both. 3. `GET /api/page/refs` (`handlePageRefs`) on the root `_index.md` collects every permalink in the tree and skips the "exclude the bundle's own directory" step because the directory is `.`, so it lists every file linking to any page. The root page cannot be deleted (`pageops.Delete`), so the delete warning never asks. 4. `GET /api/media/library` (`handleMediaLibrary`, `internal/server/media.go`) slug-checks `folder` with `validMediaFolder` and refuses a legacy non-slug folder with `library.folderInvalid`, while `GET /api/media/tree` lists it and every `mediaops` route addresses it (`internal/mediaops/paths.go` shape-checks, never slug-checks, on purpose). 5. `GET /api/media/folder/delete-plan` normalizes `folder` with `trimFolderSlashes` (`\` → `/`, trims `/`); `POST /api/media/folder/delete` passes `req.Folder` raw to `mediaops.DeleteFolder` and normalizes only the snapshot subject, so a plan can succeed and the delete be refused. 6. `POST /api/media/upload` (`handleMediaUpload`) runs `media.Process` (decode and re-encode) before checking `target` and `folder`. 7. `pageops.Rename` and `pageops.RenameTranslation` refuse a blank title but store it untrimmed; `pageops.Create` stores it trimmed. 8. An item row's `enabled` (`elemops.itemDisabled`, `internal/elemops/save.go`) treats any value other than JSON `false` as enabled, a string included; an element's own `enabled` goes through `element.ReadEnabled`, which answers a field error for a non-boolean. 9. `handleSavePage` (`internal/server/server.go`) decodes its body inline, a second copy of `decodeBody` and its `request.malformed` answer. 10. `POST /api/form` with `kind: "create"`: `formRef.Parent` is decoded (`internal/server/forms.go`) and the client's `CreateDialog` sends it, but the `formCreate` branch of `resolveForm` reads only `Type` and `Lang`. 11. In a connected translation, `elementL10n` (`internal/server/translation.go`) fills `synced` with every Appearance param whether or not a source element with that uid and name exists, so `describe()` (`internal/server/forms.go`) marks the fields `readOnly` with `note.followsSourceElement` when there is nothing to follow. 12. `writeStructural` (`internal/server/translation_ops.go`) snapshots with `opSave`, so element insert, delete, move and duplicate, and `POST /api/element/enabled`, all read "save" in the undo catalog although `opCreate`, `opDelete`, `opMove`, `opEnable` and `opDisable` exist (`internal/server/snapshot_ops.go`) and the client's `undo.op.<op>` catalog translates them. 13. `l10n.UnplacedError` (`internal/l10n/translate.go`) carries a plain message (`l10n.beforeSlot`, `l10n.beforeParent`, `l10n.beforePage`) and a `parentFirst` one; its only caller, `handleTranslateElement`, swaps to `ParentFirst()` when the uid is untranslated, and `docs/api.md` lists only the `…ParentFirst` codes. 14. `GET /api/publish/status` (`handlePublishStatus`, `internal/server/publish.go`) answers `{phase: "static", code: "publish.static"}` for any `commit` the status map does not hold, with no shape or existence check. ## Desired behavior 1. `GET /api/link/resolve` validates `lang` first: an unknown `lang` is `404 editor.unknownLang` for every kind, `external` and `media` included. 2. `POST /api/element/duplicate` refuses a copy the destination slot's `elements` allowlist does not admit, with the same code and status insert answers, by calling `elemops.SlotElementsMsg`. 3. `GET /api/page/refs` on the content root's own page answers `400` with a new coded message `page.rootRefs`, defined and worded like `page.rootRename`, `page.rootMove` and `page.rootReorder`. Every other page keeps its current answer. 4. `GET /api/media/library` lists a legacy non-slug folder exactly as `GET /api/media/tree` and the `mediaops` routes address it. An *existing* folder is accepted by the shape check `mediaops` already owns; the slug rule applies only where a folder comes into being (folder create, upload into a folder that does not exist yet) and keeps its current codes there. `library.folderInvalid` stays for a folder that fails the shape check. 5. `POST /api/media/folder/delete` and `GET /api/media/folder/delete-plan` normalize `folder` identically, through one owner both call, so any `folder` the plan accepts the delete accepts, and the executed plan equals the dry run for the same input. 6. `POST /api/media/upload` refuses a wrong `target` or an invalid `folder` before it decodes the file; the codes and statuses do not change. 7. Rename and translation rename store the trimmed title, exactly as create does, through one trim in `pageops` that create, rename and translation rename all call. A title that is blank after trimming is still refused with `page.titleEmpty`. 8. An item row whose `enabled` is present and not a JSON boolean is refused with the same field error an element's `enabled` gets, by calling `element.ReadEnabled`. Absent stays enabled; `true` and `false` keep their meaning. `elemops` keeps exactly one place that reads a row's enabled state back. 9. `handleSavePage` decodes its body through `decodeBody`; the answer for a malformed body is unchanged. 10. `POST /api/form` with `kind: "create"` uses `parent`: a parent that does not resolve answers the refusal `POST /api/page/create` answers for it, and a `type` the parent's page type does not allow answers the refusal create answers for that, with the same codes and statuses, so the dialog fails before the user fills the form. A valid parent changes nothing in the description. 11. In a connected translation, when no source element with the translation element's uid and name exists, the Appearance fields are ordinary editable fields: no `readOnly`, no note, no `noteCode`, not named in the description's `synced` list. When a source element exists, the current behaviour is unchanged. 12. Structural element writes label their WIP snapshots from the existing vocabulary: insert and duplicate `create`, delete `delete`, move `move`, enabled `enable` or `disable` by the resulting state. The snapshot subject stays the page. `snapshotOps` and the exported `undo_ops.json` do not change. 13. A test proves that an `UnplacedError` reaching `POST /api/element/translate` is always answered with the `…ParentFirst` code (or shows a case where it is not). If it is unreachable, `UnplacedError` carries one message, the three plain `l10n.before*` codes are deleted from `internal/apimsg`, and nothing else references them. If a reachable case turns up, keep both and document the plain codes in `docs/api.md` with the case that produces them. 14. `GET /api/publish/status`: a `commit` that is not a 40-character lowercase hex sha answers `400` with a new coded `request.` message for an invalid parameter value, carrying the parameter name; a well-formed sha the repository does not contain answers `404` with a new `publish.` code; `static` is answered only for a commit the repository has and the server never polled. Every route touched has its `docs/api.md` section updated to the new behaviour: the error list, the key list and the prose that described the old behaviour (item 1's "answer whatever lang says", item 3's root sentence, item 11's read-only clause, the undo section's op words where relevant). Every new or removed code appears or disappears there. ## Decisions - **One issue, fourteen fixes** — each is a few lines plus a test and a docs line, none depends on another, and a single pull request keeps the docs pass that found them in one review. - **Call the owner, never a sibling** (CLAUDE.md "Every rule has exactly one owner") — the owners are: `opsRoot` for the unknown-lang verdict (1); `elemops.SlotElementsMsg` for a slot's allowlist (2); the folder shape check in `mediaops` for an existing folder (4); one normalizer for a folder argument that both folder-delete routes call, moved into `mediaops` if that is where it fits (5); one title trim in `pageops` (7); `element.ReadEnabled` for an `enabled` value (8); `decodeBody` for a request body (9); `elementL10n` for what is synced (11). A fix that adds a second evaluation of any of these is wrong. - **Root refs is a refusal, not a scoped answer** — the root permalink is `/`, so any prefix scoping still matches every link; and the root cannot be deleted, so no caller needs the list. - **Legacy folders are listed, not hidden** — `mediaops` already decided an existing folder is shape-checked and never slug-checked; the library route follows that decision instead of the tree route being bent the other way. - **The create form uses `parent`** — the client already sends it, and failing at form time is the same refusal create would give, earlier. - **No source element means not synced** — a read-only field that follows nothing misleads; a dedicated "source gone" note would be a new wire fact for a state the translation repair path already handles. - **Snapshot ops reuse the vocabulary** — no new op words, so the client's undo catalog and the exported `undo_ops.json` stay as they are; a page delete and an element delete share the word `delete`, which the subject (the page) makes clear enough. - **Publish status distinguishes malformed, unknown and unpolled** — a mistyped sha should not look like a static-site publish; the existence check is a cheap git object lookup on a path the handler already has a repository for. - **Dead codes are deleted only on proof** — a test decides item 13, so the catalog is not pruned on a reading of the code alone. ## Key interfaces - `handleLinkResolve`, `handleElementDuplicate`, `handlePageRefs`, `handleMediaLibrary`, `handleMediaFolderDelete`, `handleMediaFolderDeletePlan`, `handleMediaUpload`, `handleSavePage`, `handlePublishStatus`, `handleElementEnabled` in `internal/server` — the handlers whose order of checks or owner calls change. - `writeStructural(w, root, full, page, lang, doc)` in `internal/server/translation_ops.go` — needs to take, or be told, the `snapshotOp` of the write. - `resolveForm` / `formRef` in `internal/server/forms.go` — the `formCreate` branch reads `Parent`. - `elementL10n` and `elementForm.describe()` in `internal/server` — `synced` depends on `srcSC`. - `pageops.Rename`, `pageops.RenameTranslation`, `pageops.Create` — share one title trim. - `elemops.itemDisabled` / `elemops.NewItem` in `internal/elemops/save.go` — the one place a row's enabled state is read back; it should answer a field error through `element.ReadEnabled`. - `validMediaFolder` in `internal/server/media.go` and the folder path check in `internal/mediaops/paths.go` — which routes call which, per decision above. - `l10n.UnplacedError`, `ParentFirst()`, `l10n.UntranslatedUIDs` in `internal/l10n` — item 13. - `internal/apimsg/codes_page.go`, `codes_server.go`, `codes_publish.go`, `codes_l10n.go` — new codes `page.rootRefs`, an invalid-parameter `request.` code, an unknown-commit `publish.` code; the three plain `l10n.before*` codes possibly removed. - `publishStatus` map and the repository handle in `internal/server/publish.go` — the existence check. - `docs/api.md` — one `###` section per route; the route drift test in `internal/server` keeps the heading set, the content is on you. ## Acceptance criteria - [ ] Each of the fourteen desired behaviours above holds and has a test that fails on `main` and passes with the fix (item 13's test may instead prove the plain codes unreachable). - [ ] No fix adds a second evaluation of a rule listed under Decisions; each calls the named owner. - [ ] New coded messages are defined in `internal/apimsg` and every removed one has no remaining reference in Go, the client or the docs. - [ ] `docs/api.md` describes the new behaviour for every touched route, with the error lists updated. - [ ] `snapshotOps` and `internal/server/testdata/undo_ops.json` are unchanged. - [ ] The client's `FormRef` for `kind: "create"` still sends `parent`; nothing in `web/src` needs to change for items 10 to 12 unless a test asserts the old behaviour. - [ ] `scaffold/site/AGENTS.md` and the scaffolded skills are untouched (nothing they document changes); if a fix turns out to change a documented sidecar or command behaviour, update them in the same change. ## How to verify - `go test ./...` and `go vet ./...` green, including the drift tests in `internal/server` (routes ⇔ `docs/api.md` headings, `TestSnapshotOpsExport`), `docs/docs_test.go` and `internal/apimsg`. - `cd web && npm test` green. - Against a scaffolded site (`harness init` into a temp dir, server started as `docs/harness.md` describes), exercise with curl: `link/resolve?url=https://x&lang=zz` → 404; `page/refs?path=_index.md` → 400 `page.rootRefs`; `media/library?folder=<legacy name>` → 200 listing; `media/folder/delete-plan` and `media/folder/delete` with a `\`-spelled folder agree; `publish/status?commit=zzz` → 400, `commit=<40 hex not in repo>` → 404. - Insert, delete, move and duplicate an element, then read the undo info: the ops read `create`, `delete`, `move`, `create`. ## Assumptions to check - Item 13's deadness: `handleTranslateElement` swaps to `ParentFirst()` only when `UntranslatedUIDs` contains the uid; verify an `UnplacedError` can never arise for a uid outside that list before deleting the codes. - Item 14's existence check: confirm the handler can reach a repository handle cheaply (the publish job already opens one); if the server can run without a repository, decide what a missing repository answers and document it. - Item 10: confirm `pageops` exposes a parent-resolution and type-allowlist check callable at form time without creating anything; `resolveParent` and the type allowlist in `internal/server/pageops.go` are the starting points. - Item 4: confirm which `mediaops` function is the folder shape check and that it is importable by `internal/server` without a cycle. ## Out of scope - Changing the snapshot-op vocabulary or the client's undo catalog. - A new note or wire state for a translation element whose source element is gone. - Hiding legacy folders from `GET /api/media/tree`. - Any route behaviour not listed in the fourteen items, even if found nearby. - `docs/architecture.md`, ADRs, `scaffold/site/AGENTS.md`, the scaffolded skills. ## Blocked by None - can start immediately. ## Original report Verifying docs/api.md against the handlers for #264 turned up these behaviours that look unintended. The docs describe the code as it is; each needs a decision. - `GET /api/link/resolve` answers `external` and `media` values before validating `lang`, so an unknown `lang` gets a 200 (linktargets.go:180-224). - `POST /api/element/duplicate` re-checks placement but not the slot's `elements` allowlist, unlike insert, move and transfer (element.go:294-312). - `GET /api/page/refs` on the root `_index.md` lists every file linking to any page (linktargets.go:376-409). - `GET /api/media/library` refuses a legacy non-slug folder with `library.folderInvalid` that `GET /api/media/tree` lists and mediaops keeps addressable (media.go:220). - `GET /api/media/folder/delete-plan` turns `\\` into `/`; `POST /api/media/folder/delete` does not, so a plan can succeed and the delete be refused (media.go:778 vs 802). - `POST /api/media/upload` decodes and re-encodes the whole file before refusing a wrong `target`/`folder` (media.go:157-166). - Rename stores the title untrimmed (pageops/relocate.go:79, pageops/translation.go:100) while create trims it (create.go:193). - An item row's non-boolean `enabled` is silently enabled (elemops/save.go:335), while an element's `enabled` gets `field.bool`. - `handleSavePage` decodes its body itself instead of `decodeBody` (server.go:959) — a second owner of the rule. - FormRef `parent` is decoded but never read for kind `create` (forms.go:301-327); the client sends it. - In a connected translation, Appearance fields are marked `readOnly` with `note.followsSourceElement` even when no source element matches by uid (translation.go:376-380, forms.go:582-591). - Element insert/delete/move/duplicate (`writeStructural`) and element enabled label their snapshots `save`, so `undoOp` cannot tell them from a param save although `delete`, `move`, `enable`, `disable` ops exist. - Dead branch in translation_ops.go:527: the plain `l10n.beforeSlot`/`beforeParent`/`beforePage` codes never reach the wire. - `GET /api/publish/status` answers `static` for any commit it does not know (publish.go:709-712), a mistyped sha included.

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 commented

2026-10-13 23:52:02 +02:00

Author

Owner

This was generated by AI during triage.

Triage Notes

What we've established so far:

All fourteen items reproduce by reading the current main (a0c4ce7); go test of internal/server, pageops, elemops and l10n is green, so none is covered by a test today. Locations as of now (translation_ops.go in the report is internal/l10n/translate.go for item 13; the dead branch's caller is internal/server/translation_ops.go handleTranslateElement):

handleLinkResolve answers external and media before opsRoot validates lang. docs/api.md currently documents the quirk ("external and media answer whatever lang says").
handleElementDuplicate runs placementViolation but never elemops.SlotElementsMsg; insert, move and transfer run both.
handlePageRefs: the root node collects every permalink, and the "exclude the bundle's own directory" step is skipped for dir == ".", so the root _index.md lists every file linking to any page. The root permalink is /, so scoping it to "its own permalink" would match everything too. The root page cannot be deleted (pageops.Delete), so the question is only what the route should answer.
handleMediaLibrary slug-checks folder with validMediaFolder; mediaops (paths.go) deliberately shape-checks, never slug-checks, so a legacy folder is listed by /api/media/tree, addressable by every mediaops route, and refused only by /api/media/library.
handleMediaFolderDeletePlan passes folder through trimFolderSlashes (\ → /, trims /); handleMediaFolderDelete passes req.Folder raw to mediaops.DeleteFolder and only normalizes for the snapshot subject.
handleMediaUpload runs media.Process (decode + re-encode) before checking target and folder.
pageops.Rename and RenameTranslation check TrimSpace(title) == "" but store title untrimmed; Create stores the trimmed title.
elemops.itemDisabled treats any non-false value (a string included) as enabled; an element's own enabled goes through element.ReadEnabled, which answers a field error for a non-boolean.
handleSavePage inlines the json.NewDecoder(...).Decode + request.malformed that decodeBody owns.
formRef.Parent is decoded; the formCreate branch of resolveForm reads only ref.Type and ref.Lang. The client's CreateDialog sends it.
elementL10n: synced is filled with every Appearance param whether or not srcSC was found, so describe() marks those fields readOnly with note.followsSourceElement when no source element with that uid (and name) exists.
writeStructural snapshots with opSave, so insert, delete, move and duplicate of an element, and /api/element/enabled, all show as "save" in the undo catalog; opCreate, opDelete, opMove, opEnable, opDisable exist and the client's undo.op.<op> catalog already translates them.
l10n.UnplacedError carries msg (l10n.beforeSlot / beforeParent / beforePage) and parentFirst; the only caller (handleTranslateElement) swaps to ParentFirst() when the uid is in UntranslatedUIDs, and docs/api.md lists only the …ParentFirst codes. Whether the plain codes are truly unreachable still needs a test that proves it (see decision H).
handlePublishStatus answers {phase: "static", code: "publish.static"} for any commit value the status map does not hold, with no shape or existence check.

Affected surfaces (CLAUDE.md): docs/api.md sections for /api/link/resolve, /api/element/duplicate, /api/page/refs, /api/media/library, /api/media/folder/delete, /api/media/upload, /api/page/rename and /api/translation/rename, /api/page/save (items), /api/form, /api/translate/element, /api/publish/status, and the undo/snapshot vocabulary where it is described. scaffold/site/AGENTS.md and the scaffolded skills: nothing (no sidecar, command, flag or template contract changes). Rule owners the fix must call rather than copy: opsRoot (1), elemops.SlotElementsMsg (2), the folder shape check in mediaops (4), trimFolderSlashes or a mediaops normalizer both routes share (5), one title-trim in pageops (7), element.ReadEnabled (8), decodeBody (9), elementL10n (11).

Recommendation: category bug (as labelled), state ready-for-agent, as one issue: each item is a few lines plus a test and a docs/api.md line, and none depends on another. Items 1, 2, 5, 6, 7, 8, 9 need no decision and would be fixed as described. The rest need one of these answers; each comes with the answer I would write into the brief if nobody objects.

What we still need from the maintainer:

A. Slicing — one issue for all fourteen (recommended), or carve the behaviour changes (3, 4, 10, 11, 12, 14) into a second issue?
B. Item 3, /api/page/refs on the root _index.md — refuse with a 400 page.rootRefs coded like page.rootRename / page.rootMove / page.rootReorder (recommended: the root cannot be deleted, so the delete warning never asks), or answer {pages: []}?
C. Item 4, legacy folders — /api/media/library lists a legacy non-slug folder, using mediaops' shape check for an existing folder and keeping the slug rule only where a folder is created (folder create, upload into a new folder) (recommended), or keep refusing and have /api/media/tree hide legacy folders too?
D. Item 10, parent on a create form ref — use it: the form answers the same refusals /api/page/create would for a missing parent (page.notFound) or a type the parent's page type does not allow, so the dialog fails before the user fills it in (recommended, since the client already sends it); or drop parent from the client's FormRef and formRef and document that a create form is parent-independent?
E. Item 11, no source element by uid — the Appearance fields are plain editable fields (no readOnly, no note, not in synced) (recommended); or keep them read-only with a different note saying the source element is gone?
F. Item 12, snapshot ops — reuse the vocabulary: insert and duplicate → create, delete → delete, move → move, enabled → enable / disable, subject unchanged (recommended: no vocabulary change, no client catalog change); or add element-specific ops (element delete …) so an undo step can tell a page delete from an element delete?
G. Item 14, /api/publish/status — a commit that is not a 40-hex sha is 400 request.invalidParam, a well-formed sha the repository does not have is 404 with a new code, and static is kept only for a real commit the server never polled (recommended); or shape check only?
H. Item 13 — if a test proves the plain l10n.before* codes unreachable, delete them and collapse UnplacedError to one message (recommended); or keep them as a reachable fallback?

> *This was generated by AI during triage.* ## Triage Notes **What we've established so far:** All fourteen items reproduce by reading the current `main` (a0c4ce7); `go test` of `internal/server`, `pageops`, `elemops` and `l10n` is green, so none is covered by a test today. Locations as of now (`translation_ops.go` in the report is `internal/l10n/translate.go` for item 13; the dead branch's caller is `internal/server/translation_ops.go` `handleTranslateElement`): 1. `handleLinkResolve` answers `external` and `media` before `opsRoot` validates `lang`. `docs/api.md` currently documents the quirk ("external and media answer whatever lang says"). 2. `handleElementDuplicate` runs `placementViolation` but never `elemops.SlotElementsMsg`; insert, move and transfer run both. 3. `handlePageRefs`: the root node collects every permalink, and the "exclude the bundle's own directory" step is skipped for `dir == "."`, so the root `_index.md` lists every file linking to any page. The root permalink is `/`, so scoping it to "its own permalink" would match everything too. The root page cannot be deleted (`pageops.Delete`), so the question is only what the route should answer. 4. `handleMediaLibrary` slug-checks `folder` with `validMediaFolder`; `mediaops` (`paths.go`) deliberately shape-checks, never slug-checks, so a legacy folder is listed by `/api/media/tree`, addressable by every mediaops route, and refused only by `/api/media/library`. 5. `handleMediaFolderDeletePlan` passes `folder` through `trimFolderSlashes` (`\` → `/`, trims `/`); `handleMediaFolderDelete` passes `req.Folder` raw to `mediaops.DeleteFolder` and only normalizes for the snapshot subject. 6. `handleMediaUpload` runs `media.Process` (decode + re-encode) before checking `target` and `folder`. 7. `pageops.Rename` and `RenameTranslation` check `TrimSpace(title) == ""` but store `title` untrimmed; `Create` stores the trimmed title. 8. `elemops.itemDisabled` treats any non-`false` value (a string included) as enabled; an element's own `enabled` goes through `element.ReadEnabled`, which answers a field error for a non-boolean. 9. `handleSavePage` inlines the `json.NewDecoder(...).Decode` + `request.malformed` that `decodeBody` owns. 10. `formRef.Parent` is decoded; the `formCreate` branch of `resolveForm` reads only `ref.Type` and `ref.Lang`. The client's `CreateDialog` sends it. 11. `elementL10n`: `synced` is filled with every Appearance param whether or not `srcSC` was found, so `describe()` marks those fields `readOnly` with `note.followsSourceElement` when no source element with that uid (and name) exists. 12. `writeStructural` snapshots with `opSave`, so insert, delete, move and duplicate of an element, and `/api/element/enabled`, all show as "save" in the undo catalog; `opCreate`, `opDelete`, `opMove`, `opEnable`, `opDisable` exist and the client's `undo.op.<op>` catalog already translates them. 13. `l10n.UnplacedError` carries `msg` (`l10n.beforeSlot` / `beforeParent` / `beforePage`) and `parentFirst`; the only caller (`handleTranslateElement`) swaps to `ParentFirst()` when the uid is in `UntranslatedUIDs`, and `docs/api.md` lists only the `…ParentFirst` codes. Whether the plain codes are truly unreachable still needs a test that proves it (see decision H). 14. `handlePublishStatus` answers `{phase: "static", code: "publish.static"}` for any `commit` value the status map does not hold, with no shape or existence check. Affected surfaces (CLAUDE.md): `docs/api.md` sections for `/api/link/resolve`, `/api/element/duplicate`, `/api/page/refs`, `/api/media/library`, `/api/media/folder/delete`, `/api/media/upload`, `/api/page/rename` and `/api/translation/rename`, `/api/page/save` (items), `/api/form`, `/api/translate/element`, `/api/publish/status`, and the undo/snapshot vocabulary where it is described. `scaffold/site/AGENTS.md` and the scaffolded skills: nothing (no sidecar, command, flag or template contract changes). Rule owners the fix must call rather than copy: `opsRoot` (1), `elemops.SlotElementsMsg` (2), the folder shape check in `mediaops` (4), `trimFolderSlashes` or a `mediaops` normalizer both routes share (5), one title-trim in `pageops` (7), `element.ReadEnabled` (8), `decodeBody` (9), `elementL10n` (11). **Recommendation:** category `bug` (as labelled), state `ready-for-agent`, as **one issue**: each item is a few lines plus a test and a `docs/api.md` line, and none depends on another. Items 1, 2, 5, 6, 7, 8, 9 need no decision and would be fixed as described. The rest need one of these answers; each comes with the answer I would write into the brief if nobody objects. **What we still need from the maintainer:** - **A. Slicing** — one issue for all fourteen (recommended), or carve the behaviour changes (3, 4, 10, 11, 12, 14) into a second issue? - **B. Item 3, `/api/page/refs` on the root `_index.md`** — refuse with a 400 `page.rootRefs` coded like `page.rootRename` / `page.rootMove` / `page.rootReorder` (recommended: the root cannot be deleted, so the delete warning never asks), or answer `{pages: []}`? - **C. Item 4, legacy folders** — `/api/media/library` lists a legacy non-slug folder, using mediaops' shape check for an *existing* folder and keeping the slug rule only where a folder is *created* (folder create, upload into a new folder) (recommended), or keep refusing and have `/api/media/tree` hide legacy folders too? - **D. Item 10, `parent` on a `create` form ref** — use it: the form answers the same refusals `/api/page/create` would for a missing parent (`page.notFound`) or a type the parent's page type does not allow, so the dialog fails before the user fills it in (recommended, since the client already sends it); or drop `parent` from the client's `FormRef` and `formRef` and document that a create form is parent-independent? - **E. Item 11, no source element by uid** — the Appearance fields are plain editable fields (no `readOnly`, no note, not in `synced`) (recommended); or keep them read-only with a different note saying the source element is gone? - **F. Item 12, snapshot ops** — reuse the vocabulary: insert and duplicate → `create`, delete → `delete`, move → `move`, enabled → `enable` / `disable`, subject unchanged (recommended: no vocabulary change, no client catalog change); or add element-specific ops (`element delete` …) so an undo step can tell a page delete from an element delete? - **G. Item 14, `/api/publish/status`** — a `commit` that is not a 40-hex sha is `400 request.invalidParam`, a well-formed sha the repository does not have is `404` with a new code, and `static` is kept only for a real commit the server never polled (recommended); or shape check only? - **H. Item 13** — if a test proves the plain `l10n.before*` codes unreachable, delete them and collapse `UnplacedError` to one message (recommended); or keep them as a reachable fallback?

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-10-13 23:56:33 +02:00

dominik.polakovics referenced this issue from a commit

2026-10-14 08:56:53 +02:00

fix(api): settle the fourteen API inconsistencies the route docs pass found

dominik.polakovics referenced this issue from a pull request that will close it,

2026-10-14 08:57:08 +02:00

fix(api): settle the fourteen API inconsistencies the route docs pass found #280