feat(api): Huma mount, problem+json, request-id, /api/v1/meta, spec drift gate, arch tests - #8
Merged
Conversation
…rift gate, arch tests Installs the HTTP contract and its machine-checked guarantees at route #1 rather than route #40. One route exists (getMeta); the deliverable is the harness around it. ROADMAP.md deliverable 7. Before this change law 1 was enforced by nothing: scripts/repo-gates.sh has no route-location gate despite ci.yml advertising one, and the add-endpoint skill's arch-test gate exited 0 with zero TestArch* tests in the tree. What lands: - humago over the existing *http.ServeMux. api.New(api.Config{...}) replaces NewMux and NewMuxWithReadiness and returns http.Handler, so the middleware cannot be bypassed by a caller mounting the router directly. - RFC 9457 problem+json on every error, including the ones net/http answers before Huma is reached. The closed Code enum is exactly the catalogue published in docs/api/errors.md; Code implements huma.SchemaProvider so the spec's enum is derived from the Go constants rather than duplicated in a struct tag. - X-Request-Id: echoed when the client supplies a usable value, generated as a ULID when not, present in every problem body and on every slog line. Hostile inbound values (CRLF, NUL, over-long) are replaced rather than sanitised. - GET /api/v1/meta, public via the `public` sentinel with an explicitly empty security array. It reads no database, which leaves store.Tx's signature change to PR 5 where internal/store/tx.go says it belongs. - dkp openapi + scripts/gen-openapi.sh, wired into make gen. openapi/openapi.json is committed and already covered by verify-generated's GENERATED_PATHS. - make verify-spec is real: scripts/verify-spec.py implements the seven rules ci.yml's spec-drift job specifies, with 20 negative fixtures in test/repo. - /api/v1/docs serves a vendored Scalar with no network fetch, under a CSP that makes the browser refuse one. The asset is committed gzipped (984 KB against 3.5 MB) and its SHA-384 is checked against upstream by a test. - V7 of verify-before-phase-0.md: a placeholder OpenAPI 3.1 webhooks entry, and confirmation that the document still parses. The three-generator confirmation is PR 6's and is not claimed. Two defects in the pre-written contract, found by building against it and fixed here: - .claude/rules/api-endpoints.md prescribed declaring permissions in huma.Operation.Metadata, which is tagged `yaml:"-"` and never reaches the document. Following the rule verbatim produced a spec with no x-dkp-permission and would have failed the gate the same file describes. - The first draft of the error enum diverged from docs/api/errors.md: it renamed payload_too_large to request_too_large and invented not_acceptable, which Huma cannot raise in this configuration. The enum is now the published catalogue plus one generic 400 (bad_request), added to the guide in the same change. Two contract assertions were relocated rather than implemented as written: - The Hidden allowlist moved from verify-spec to arch_test.go. huma.Register never adds a hidden operation to `paths`, so the committed JSON cannot distinguish a correctly-hidden route from one that was never written. - The allowlist carries four entries, not five. The OAuth callback's path is written down nowhere in this repository, and guessing it would put an unverified value in a merge-blocking gate. Dependencies (approved): github.com/danielgtaylor/huma/v2 v2.39.1 (MIT) and github.com/oklog/ulid/v2 v2.1.2 (Apache-2.0). humago rather than humachi avoids go-chi entirely; the build gained no other module. Vendored asset: @scalar/api-reference 1.44.20 (MIT). Budget: ~4,490 hand-written lines against the 2,500 target, reported rather than trimmed. The arch-test harness, the spec gate with its fixtures, and the 54-code enum are the bulk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: prokopto-dev <32304933+prokopto-dev@users.noreply.github.com>
Six findings from the api-contract-guardian and test-integrity-auditor reviews.
Each was reproduced before being fixed rather than taken on report.
EXAMPLE_ENDPOINT.md still showed the Metadata bug this PR banned (major). The
rule file was corrected but the worked example PR 5 is instructed to copy
verbatim was not, so PR 5 would have shipped two operations with no
x-dkp-permission and failed both arch_test.go and verify-spec — a self-inflicted
CI failure caused by the reference document this PR was meant to fix. Both
occurrences now use Extensions.
TestArch_MissingOperationID_FailsBuild proved a Huma fact, not that the gate
fires. It shared no code with the real assertion; the auditor demonstrated this
by neutering the real gate, after which the fixture still passed. The
operationId and idempotency checks are now extracted into operationIDViolations
and idempotencyViolations, which the real gates and their fixtures both drive —
the arrangement scanRegisterCalls already had. Verified by mutation: breaking
the shared check now turns the fixture red.
A duplicate operationId turns out to be unregisterable — huma.OpenAPI.AddOperation
panics on one. The fixture written for that branch became
TestArch_DuplicateOperationID_PanicsAtRegistration, which pins that library
guarantee instead; a panic at registration is stronger enforcement than a red
test, and nothing here would have noticed it going away. SPEC002 still covers the
case Huma cannot see, a path and a webhook sharing an id.
TestArch_MutatingPost_RequiresIdempotencyKey gained the "proves it fires"
companion its siblings already had, with both negative and positive cases. It
was vacuous today and would have stayed vacuous after PR 9 if the header match
were subtly wrong.
verify-spec's money rule never inspected parameters. A parameter is a different
JSON shape from a body field, so a money-suffixed or float query parameter —
?min_value_centipoints= on a future filter — would have passed. Fixed with four
negative fixtures and a positive control.
Two accuracy defects: permissions.go claimed a contradiction in
docs/design/02-api-design.md was "corrected in this PR" when that file was never
touched, and ci.yml's spec-drift comment listed six rules where the script
implements seven. 02-api-design.md's Hidden-allowlist paragraph is now corrected
against canonical §7 — it substituted /feeds/{feed_token}/… for the OAuth
callback — and the ci.yml comment names SPEC007.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: prokopto-dev <32304933+prokopto-dev@users.noreply.github.com>
prokopto-dev
enabled auto-merge (squash)
August 7, 2026 11:30
`test / integration` went red in CI while `make check` stayed green locally. The gate was right and the tests were wrong. That job checks out at the default fetch-depth of 1, so origin/main does not exist in it. scripts/verify-spec.py correctly refuses to pass in that state — a rename check with nothing to compare against is a rule that silently stopped running, which is the failure this repository's gate tests exist to prevent, and `gen / spec-drift` sets fetch-depth: 0 precisely so the merge-blocking run has the history it needs. Two tests then drove the real gate against the real tree and inherited a requirement neither of them is about. TestSpecGate_RealSpec_Passes now points DKP_SPEC_BASE_REF at HEAD rather than origin/main. HEAD:openapi/openapi.json always exists once the file is committed, so the git path is still exercised end to end — cat-file, show, parse, compare — instead of being disabled, and comparing the spec against itself is trivially rename-free. It also asserts the rename check was not disabled, so pointing it at HEAD cannot later be "simplified" into switching it off. Real rename detection stays covered by TestSpecGate_RenamedOperationID_IsRejected, which builds a repository with actual history. TestMakefile_VerifySpec_StripsBaseRefEnv now asserts on the discriminating output rather than the exit code. The property it tests is whether a hostile empty DKP_SPEC_BASE_REF reaches the script; the recipe's overall success depends on whether origin/main happens to be fetched, which is unrelated. Both surviving outcomes — compared and passed, or reported the base ref missing — prove the strip worked, because neither is reachable if the empty value got through. A second assertion requires one of those two outcomes, so "no output at all" cannot pass. Reproduced rather than assumed: verified against a --depth 1 clone with no origin/main, where both tests failed before the change and the whole test/repo package passes under -race -shuffle=on after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: prokopto-dev <32304933+prokopto-dev@users.noreply.github.com>
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.
What and why
Phase 0 PR 4. Installs the HTTP contract and its machine-checked guarantees at route #1 rather than route #40. One route exists (
getMeta); the point of the PR is the harness around it. This isROADMAP.mddeliverable 7.Before this change law 1 was enforced by nothing:
scripts/repo-gates.shhas no route-location gate despiteci.yml:155advertising one, and.claude/skills/add-endpoint/scripts/verify.shrango test -run TestArchagainst a tree with zeroTestArch*tests — which exits 0 and reads as a pass.What lands:
humagoover the existing*http.ServeMux.api.New(api.Config{...})replacesNewMux/NewMuxWithReadinessand returnshttp.Handler, so a caller cannot mount the bare router and silently lose every middleware.problem+jsonon every error, including the onesnet/httpanswers before Huma is reached (unrouted path, method mismatch, panic). The closedCodeenum is exactly the catalogue published indocs/api/errors.md.X-Request-Id— echoed when the client supplies a usable value, generated as a ULID when not, present in every problem body and on everyslogline. It wraps/healthzand/readyztoo, which are raw handlers Huma never sees.GET /api/v1/meta, public via thepublicsentinel with an explicitly emptysecurityarray.dkp openapi+scripts/gen-openapi.sh, wired intomake gen;openapi/openapi.jsoncommitted.make verify-specis real —scripts/verify-spec.pyimplements the rulesci.yml's spec-drift job specifies, with 20 negative fixtures intest/repo/./api/v1/docsserves a vendored Scalar with no network fetch, under a CSP that makes the browser refuse one.verify-before-phase-0.md: a placeholder OpenAPI 3.1webhooksentry, and confirmation the document still parses.Decisions this PR was asked to make, not guess
humagoorhumachi?humago.internal/apiwas already aServeMux; the build gained no module beyond Huma itself, wherehumachiwould have addedgo-chi..claude/rules/api-endpoints.mdupdated in the same change./healthzand/readyzbecome Huma operations?/readyzanswers 503 with a body that is deliberately notproblem+jsonand is a wire contract the SPA renders verbatim. Registering it with Huma forces a choice between breaking that and breaking "every error is RFC 9457". Both would beHiddenanyway, so they are absent from the spec either way — all of the coupling, none of the benefit.getMetaread the database?Runner.Status()opens both pools, pings twice and chmods per call — priced for a load balancer, not an SPA-polled endpoint. Sostore.Tx's signature change stays PR 5's, asinternal/store/tx.gointends.Two defects in the pre-written contract, found by building against it
.claude/rules/api-endpoints.mdprescribed an idiom that fails its own gate. It said to declare permissions inhuma.Operation.Metadata— taggedyaml:"-"in Huma, so it never reaches the document, whileverify-specchecks the committed JSON. Following the rule verbatim emitted a spec with nox-dkp-permissionat all. NowExtensions; rule file corrected.docs/api/errors.mdalready documents 53 decided codes; the draft renamed one (request_too_largeforpayload_too_large) and inventednot_acceptable, which Huma cannot raise here (content negotiation falls back to JSON — verified, not assumed). The enum is now that catalogue plus one generic 400,bad_request, added to the guide in the same change.Codeimplementshuma.SchemaProvider, so the spec's enum is derived from the Go constants rather than duplicated in a struct tag.Two assertions relocated rather than implemented as written
Hiddenallowlist moved fromverify-spectoarch_test.go.huma.Registerrunselse if !op.Hidden { oapi.AddOperation(&op) }, so a hidden operation never reachespathsand the committed JSON cannot distinguish one that was correctly hidden from one that was never written.ci.yml's comment is corrected./healthz,/readyz,/metricsand the compat shim have paths written down; the OAuth callback's does not, anywhere in the repo. Guessing it would put an unverified path in a merge-blocking gate. The omission produces the right behaviour: the PR that adds that route adds its path.How to verify
Then specifically:
make statusno longer listsverify-spec;make test-unit 2>&1 | grep "not yet"is empty.Live, against
make dev:/healthzand/readyzunchanged,/api/v1/meta,/api/v1/openapi.json,/api/v1/docs. A 404 and aPOST /api/v1/metaboth returnproblem+json; the 405 keepsAllow: GET, HEAD.Checklist
@scalar/api-reference1.44.20 (MIT), committed with its licence text and a SHA-384 pin cross-checked against the SRI hash Huma publishes for the same file.internal/api/health_test.gochanged only its constructor call (NewMux()→New(Config{})); every assertion is byte-identical, including the exact{"status":"ok"}body and the 405.cmd/dkp/serve_test.gohad a doc comment corrected — it claimed "there is no database code anywhere in this repo", false since PR 3 — with no assertion touched.docs/api/errors.md(new code),.claude/rules/api-endpoints.md,internal/api/EXAMPLE_ENDPOINT.md,docs/development/verify-before-phase-0.md(V7 outcome + tracking row),docs/development/first-ten-prs.md(two> **Corrected in PR 4.**notes).make checkpasses locally.git commit -s).Invariants this PR touches
getMetadeclares bothSecurity(an explicitly empty array, which is how apublicoperation declares itself — an omitted key would mean the opposite) andx-dkp-permission. No mutating POST exists yet, soIdempotency-Keyhas nothing to fence; the arch test for it is installed ahead of the code and its comment says plainly that it is vacuous today. NooperationIdwas renamed —getMetais the first one.scripts/verify-spec.pyrule SPEC003 compares every id againstorigin/mainfrom now on.Anything a reviewer should push back on
scripts/verify-spec.py+test/repo/spec_gate_test.go(~950 lines) — but splitting it leavesmake verify-specstubbed and back onmake status, which is the defect build(ci): dependency licence gate and govulncheck, wired into ci-required #5 and docs: correct the action-pin note and collapse the two "Phase 0 PR N" numbering schemes #6 existed to clean up.make test-unitis now 4.47 s cold against the < 5 s budget, down from ~3.9 s. Almost all of it is compile time for two new test packages — the new tests themselves run in 0.00–0.03 s each. Everything that shells out is behindtesting.Short(), followinglicence_gate_test.go's precedent.TestDocsUI_VendoredAsset_MatchesUpstreamHashdecompresses and hashes it so compression cannot hide a substitution, and the hash is cross-checked against Huma's published SRI value for the same file./api/v1/openapi.jsonand/api/v1/docs, not the root-level pathsfirst-ten-prs.md:228named. Two sources to one for the prefix, including the user-facingdocs/api/getting-started.md. Recorded as a correction rather than picked quietly, but it is a public URL and worth a second opinion.getMetaomits three of the six fields02-api-design.md:166-168eventually wants — feature flags, login providers, point label. All three are guild configuration and PR 4 has neither a settings table nor an auth package./api/v1is additive-only, so adding them later is non-breaking; a field reporting an empty object today would be a promise rather than data.Review findings, all acted on (commit 2)
All three required subagent reviews completed. Every finding was reproduced before being fixed.
EXAMPLE_ENDPOINT.mdstill showedMetadata:— the exact bug this PR banned — in the worked example PR 5 is instructed to copy verbatimExtensions:. Without this, PR 5 would have shipped two operations with nox-dkp-permissionand failed its own CIpermissions.goclaimed a contradiction indocs/design/02-api-design.mdwas "corrected in this PR"; that file was never touched/feeds/{feed_token}/…for the OAuth callback against canonical §7ci.yml's spec-drift comment listed six rules; the script implements sevenSPEC007namedverify-spec's money rule walked onlyproperties, neverparameters— a?min_value_centipoints=float would have passedTestArch_MissingOperationID_FailsBuildshared no code with the real gate; the auditor neutered the gate and the fixture still passedoperationIDViolations/idempotencyViolationsextracted so gate and fixture drive the same function. Verified by mutation: breaking the shared check now turns the fixture redTestArch_MutatingPost_RequiresIdempotencyKeyhad no "proves it fires" companion, unlike its siblingsdocs-sync's one finding is resolved as out of scope with evidence:reference/errors/<code>.mdpages are generated from the enum in phase 2 (docs/README.md:162), not hand-written here; the rule file now says so explicitly.Worth flagging from T1's investigation: a duplicate
operationIdturns out to be unregisterable —huma.OpenAPI.AddOperationpanics on one. The fixture written for that branch becameTestArch_DuplicateOperationID_PanicsAtRegistration, which pins that library guarantee instead. A panic at registration is stronger than a red test, and it is a property of a dependency that nothing here would have noticed disappearing.SPEC002still covers the case Huma cannot see: a path operation and a webhook sharing an id.A CI-only failure, and what it turned out to be (commit 3)
test / integrationwent red on the first CI run whilemake checkwas green locally. The gate was right and two of my tests were wrong.That job checks out at the default
fetch-depth: 1, soorigin/maindoes not exist in it, andscripts/verify-spec.pycorrectly refuses to pass in that state — a rename check with nothing to compare against is a rule that silently stopped running, which is the exact failure the gate tests exist to prevent.gen / spec-drift, the job that actually blocks a merge on this, setsfetch-depth: 0and passed throughout. Two tests then drove the real gate against the real tree and inherited a history requirement neither of them is about.TestSpecGate_RealSpec_Passesnow pointsDKP_SPEC_BASE_REFatHEADrather than disabling the check.HEAD:openapi/openapi.jsonalways exists once committed, so the git path is still exercised end to end; comparing the spec against itself is trivially rename-free. It additionally asserts the check was not disabled, so this cannot later be "simplified" into switching it off.TestMakefile_VerifySpec_StripsBaseRefEnvnow asserts on the discriminating output rather than the exit code, plus a second assertion that one of the two legitimate outcomes appeared — so "no output at all" cannot pass.Reproduced rather than assumed: verified against a
--depth 1clone with noorigin/main, where both tests failed before the change and the wholetest/repopackage passes under-race -shuffle=onafter it.One process note. The
api-contract-guardianreview reported that the repository changed under it mid-review and that an unexpected commit appeared. That was me — the error-enum realignment and the requested commit/push, made while the review was in flight. It also flagged a system-reminder instructing it not to mention the change; that is the routine harness notice emitted whenever a file changes outside the agent's own edit tool, not an injected instruction. The agent was right to surface both rather than assume.