Repository navigation
feat(api): generate the server from the document instead of wiring it by hand - #237
Merged
Merged
Conversation
kvaps
approved these changes
Sep 19, 2026
Member
There was a problem hiding this comment.
Confirmed the middleware chain still wraps the whole mux and dispatches on the path prefix, so limitBody, the action grouping, the access, as-of and setup gates keep firing over the generated handlers. The repeated-key 400 does not reach the UI: viewquery.ts joins selected teams into one comma value, and the team param is a single string the server splits, so two selected teams still work on the listing and the watch. PATCH /people is 413 like every other route now.
Action SHA pins are kept and CI gains a regenerate-and-diff step.
LGTM.
kvaps
force-pushed
the
feat/openapi-spec
branch
from
September 19, 2026 09:00
2d34c24 to
9984e95
Compare
The routes were listed twice: once in api/openapi.yaml, once as a list of mux.HandleFunc lines, and nothing held the two together beyond a test comparing names. The generated server closes that gap by construction — a route exists because the document describes it — so the document is the description of the surface rather than a copy of it. The operations are registered by code generated with oapi-codegen; the handlers behind them are still the ones written against the raw request, reached through an adapter that goes away as each is converted. Three patterns stay hand-registered: the index, whose operation under a server of /api/v1 would be the path "/" — registered as the subtree pattern "GET /api/v1/", which outranks the catch-all and would answer every unknown GET beneath it instead of the 404; the watch, which upgrades the connection and needs the raw writer; and the catch-all itself. One behaviour change comes with the binding, and it is the binder's rather than a handler's: a repeated query key (?team=a&team=b) is refused with 400 invalidParameter, where reading the query by hand took the first value and dropped the rest. A selector is one value — team unions a comma-separated set inside it — so the refusal is the honest answer; TestARepeatedQueryKeyIsRefused pins both halves. getOpenAPI leads the conversion: it was hand-registered on a pattern the generator also emits, and two registrations of one pattern panic the mux at start-up, so it is a strict method and the manual route is gone. It answers the converted document as it stands rather than through the map its bare-object schema would otherwise decode it into. newService loses its request parameter. No implementation read it — what a visitor may do rides the context the middleware stamped — and a strict method is handed no request to pass on. Four edits to the document. Three are Go-only extensions that leave the wire contract alone: x-go-name on the View schema, because the parameter of that name would generate a second type and the generator refuses to choose, and x-go-type on the two required numbers — a person's capacity and a team's points — so each keeps a pointer, since a plain int cannot tell an absent field from a zero and zero MEANS something there, it takes a set number back, which would have put the 400 that says the field is required out of reach. The fourth is a wire addition: invalidParameter joins the Problem.code enum, because the binder's refusal is a code a client can now be given, and the TypeScript schema generated from the same document carries it. make generate rebuilds both halves now — the Go server and that schema — and CI diffs both, because one document with two generated artefacts drifts the moment only one of them is refreshed. It stages what the generators wrote before diffing, because a generator that began emitting a NEW path would leave it untracked and a bare diff does not see those. The contributor-facing notes in CLAUDE.md said the opposite, that CI does not regenerate and a stale copy passes unnoticed; that was true until this step existed. The line beside it listing what CI runs named four steps against six — one of the two it missed is this one, the other is the Windows cross-check and predates this branch. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Twelve operations addressed at /cards/{uid} — the read, the patch, the
delete and the nine actions — are strict methods now. Each takes the
request already bound and answers with a response object the document
names, so a status or a body it does not describe no longer compiles.
The request structs they parsed by hand are gone with them: the patch,
its dates fragment and the placement body are the schemas from
api/openapi.yaml, which is where the shape of a body was supposed to be
written down and was instead written down three times.
The patch keeps the order it applied fields in, assignees last, because
that order is a rule rather than an accident: ungrouping hands an
ownerless child the parent's person, and a patch that also names
assignees means the caller decided who owns it.
One thing a caller sees changes, and it is the generated writers': a
JSON answer carries Content-Type: application/json where it carried
application/json; charset=utf-8. JSON is UTF-8 by definition (RFC 8259),
and the browser client in this repository reads the status and the body
and never the media type.
The three value parsers answer with the problem they used to write,
which is what a strict method can return, and the one handler still
reading them writes it as before.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
…ment The listing, the board, the sprint pointers, the create and the four presses made ON a board are strict methods now, and so is the catalog of boards. The view helpers move with them: viewOf, selectorOf and gestureOn answer with the problem they used to write, which is what a strict method can return. The selectors are still read from the raw query rather than from the bound parameters. apiserver.ParseViewSelector is the only query-to-Selector parse in the tree and the hand-registered watch goes through the same function, so reading the typed copy here would leave a second implementation of that parse beside the one the watch still needs; it also refuses an explicit `view=` query, which no binder can do because the generated parameters model none. The document still declares the parameters, which is what a client reads. The ×'s intentless form gets the pin it never had: no body at all hands the card back to a home it still has. That tolerance is the document's rather than the handler's — removeCard is the one operation whose requestBody is `required: false`, and that word is what makes the generated server forgive an empty body instead of refusing it with 400. Matching it to its forty neighbours compiles and leaves the rest of the suite green, so the case that notices had to exist. One ordering change, in the two places the body was read after a gate rather than before it. The generated server decodes first, so a malformed body sent to a board that does not draw the gesture answers 400 invalidBody where it answered 404, and the same for a create into a board nobody has. Both answers are true of such a request; TestTheBodyIsReadBeforeTheBoardIsChecked pins which one arrives, beside the well-formed request that still gets the 404. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
…ument
The card's notes and its activity feed, the day feed, the links read out
of a description, the sprint pointers and the carry-over, a person's
capacity and the team roster's four writes are strict methods now.
PATCH /people/{login} gains the answer its neighbours already gave. It
decoded straight off the body instead of through the shared helper, so a
body over the cap was reported as invalid JSON (400) where every other
route said 413 bodyTooLarge. There is one decode for the whole surface
now, so the special case cannot come back;
TestAnOversizedBodyIsTooLargeOnEveryRoute pins it beside two routes that
already answered that way.
The two required numbers — a person's capacity and a team's points —
stay distinguishable from an absent field. The document says both are
required and the generator would otherwise give them a plain int, where
a body without the field reads as zero and zero MEANS something: it
takes a set number back. An x-go-type on each keeps the pointer, so the
400 that says the field is required is still reachable.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The last group — the Project board's columns and projects, the processes
and their tasks, the deadline lines and the shared presence mark — leaves
the raw request behind, and with it the adapter that let converted and
unconverted operations share one mux.
Every operation the generator emits is a method on one type now, and the
compiler says so: `var _ apiv1.StrictServerInterface = surface{}` fails
to build if one of them has no method. The watch is the exception the
configuration names, and it stays registered by hand. The hand-written
body decoder goes with the bridge; there is one decode for the whole
surface, in the generated server, reporting through one hook.
One invariant the bridge was hiding is now pinned. Every handler here
reads req.Body without checking it for nil, and what keeps it non-nil is
one word per operation in the document: only a body left `required:
false` makes the generated wrapper tolerate an empty one. Widening that
set makes a handler dereference nil, which surfaces today as a panic in
a test about something else. The set is pinned where the edit would be
made instead.
The twelve roster writes shared a paragraph of comment about reading the
board through a snapshot that may be minutes old. They share a function
instead, which is where that reason belongs: these writes read the board
to check a name and to carry its ids into the write, and blocking on a
full reload made adding a column feel broken on a big board.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The behaviour matrix gains the rule and the four answers that changed with it; the surface's own page says which two routes are not generated and why a generated operation cannot be either; and the list of documents that must move with the code says the Go server is built from the description as well as held against it. ADR 0004 records the decision ADR 0003 forecast, and corrects the one consequence 0003 listed that this change replaced: a route is added to the document now, not to registerAPI, where the test that guards the hand-registered set refuses additions outright. docs/embedding.md gains what the generator costs an importer — nothing in the binary, version floors in the module graph. Three rows named handlers this branch renamed — the patch's field order, the create judged by its team, the listing, the document's own route — and a citation that no longer resolves reads as coverage that is not there. They name the methods that replaced them. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
kvaps
force-pushed
the
feat/openapi-generated-server
branch
from
September 19, 2026 09:03
e5f77e5 to
902ebef
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The step before this wrote
api/openapi.yamland proved the server obeys it. This one makes the server come from it.internal/server/apiv1/api.gen.gois generated by oapi-codegen, every operation is a typed method, and the hand-written routing and body decoding are gone.registerAPIis four lines now. Three routes stay hand-registered because they cannot be generated: the index would be the path/under a server of/api/v1, which Go's mux turns into a subtree pattern answering every unknown GET beneath it; the watch upgrades the connection instead of answering, so it needs the raw writer; and the catch-all is what makes an unserved path a problem rather than a page. Everything else is one call binding the generated handler onto the same mux with the same URL patterns.That last part is the claim everything else rests on. Re-run it rather than taking my word for it. The middlewares above these handlers key on
r.URL.Path, so if one pattern changed spelling they keep compiling and quietly stop matching. The check is a set comparison: pull the patternsregisterAPIwired by hand out ofgit show <base>:internal/server/api.go, pull the ones the generated handler registers out ofapi.gen.go, add the three that stay by hand, diff the two sets both ways. 63 against 63, empty both ways. Match whole pattern strings, not aMETHOD /pathshape: the catch-all is registered without a method, and a method-anchored extraction drops it without saying so.var _ apiv1.StrictServerInterface = surface{}fails the build if the document grows an operation nobody implements.decodeJSON,decodeJSONAllowingEmptyand every hand-written request struct are deleted.Behaviour changes
A repeated query key is now refused by the generated routes and still accepted by the watch.
.../cards?team=a&team=banswers 400invalidParameterwhere it used to takeaand dropb, whichTestARepeatedQueryKeyIsRefusedpins;.../watch?team=a&team=bstill takesaand opens a stream scoped to it, which nothing pins. The watch is hand-registered and reads selectors throughParseViewSelector'sq.Get, so it never reaches the generated binder. Before this change both doors took the first value and agreed, so the divergence is this change's. Nothing in the tree produces the input:web/src/viewquery.tsjoins teams into one comma-separated value, for the listing and the watch alike. Routing the watch's query through the same parse is a change to a hand-registered route and belongs in its own change.PATCH /people/{login}with an oversized body answers 413 like every other route. It answered 400, because it was the one handler decoding its body without the shared helper.A malformed body on a view gesture answers 400 rather than 404. The old handler checked whether the board draws the gesture before parsing, the generated one parses first. Both are defensible; the order moved.
A JSON response now carries
Content-Type: application/jsonwithout; charset=utf-8. Cosmetic, and deliberately unpinned: the index and the document route still send the charset, and pinning the generated form would turn a future generator bump into a build failure for nothing.One keyword was load-bearing and nothing tested it
removeCardis the only one of forty-one request bodies markedrequired: false, and that is what makes the generator emit anio.EOFtolerance branch instead of the unconditional decode its neighbours get. That branch is what lets the×work with no body at all. Every existing test of that route sends a body, so nothing covered the case the word exists for.It is pinned now, and the pin was earned by flipping the word to
true, regenerating, and confirming the code still compiles while exactly one test goes red. The comment says why the word matters, because in Go it reads as a redundant nil check and in the document it reads as an inconsistency worth tidying.Changes to the document
Three are Go-only and produce no change in the generated TypeScript, which is how they were checked rather than asserted:
x-go-nameon theViewschema, because the schema and the path parameter both wanted the Go nameView, andx-go-type: '*int'on the two capacity fields. The se 8000 cond avoids a real trap. A required integer generates a plainint, so a body without the field arrives as zero, and zero means something there: it takes a set number back. Without the pointer the refusals for a missing number become unreachable code, and a test already pins one of them.invalidParameterjoins theProblem.codeenum, because the generated parameter binder needs a code to refuse with.Generated code is committed, and CI now says so
None of the build paths runs a generator: the Dockerfile copies the tree and builds, CI runs
make backend, external importers compilepkg/directly.make generaterebuilds both artefacts, and CI regenerates and diffs them, so an edit to the document without regeneration fails instead of shipping a stale artefact. The diff is staged before comparing, because a generator that began emitting a new path would leave it untracked where a baregit diffcannot see it.Verification
Beyond what CI shows: generation is reproducible,
GOOS=windows go vetis clean,go mod tidyis a no-op, andkin-openapistays out of the binary. Each of the six commits was verified on its own, not only the head.