Land wave: phrase-map zero-row guard (#195), array-uint32 400 golden (#196), atomic trustedProxies cache (#197)#198
Merged
Merged
Conversation
… 400 (#196) Add battery case tickets/list_category_over_uint32 — GET /api/v1/tickets?category_id=4294967296 with read:tickets auth — and record its golden. The repeated categoryId[] filter's array items declare maximum: 4294967295, pinned structurally by TestSpec_Uint32ParamsBounded, but libopenapi-validator does not enforce numeric maximum on array items and no golden replayed the binder's response-level 400 for an over-ceiling value. This case witnesses status 400, code 3, and the binder's "parsing list \"category_id\": ... value out of range", closing the declare+replay coverage gap for the array surface (the repeated-filter complement of the scalar list_modified_since_over_uint32 case). Coverage only — no binder behavior change; the structural spec test is untouched. This was generated by AI
Wrap the trusted-proxy cache in atomic.Pointer[[]*net.IPNet] behind an unexported loadTrustedProxies accessor. InitTrustedProxies now publishes via an atomic Store rather than a direct assignment; clientIP loads via the accessor. This turns the "write once before any listener serves, read-only after" invariant from a comment-only convention into a structural one and removes the theoretical data race surfaced in the #194 type-design review. Behavior is identical: empty/unset still trusts nothing, a non-empty malformed value is still a fatal startup error (made fatal by the composition root), and a read before init loads the zero value (nil) -> trust-nothing, the safe direction. resolveClientIP stays pure and unchanged. InitTrustedProxies keeps its exported `() error` signature; servers/server.go is untouched and its wiring guard still passes. Adjust the test helper that poked the cache directly to use the atomic API, and add a concurrent-reads-during-publish test the race detector exercises. This was generated by AI
…e family (#195) loadPhraseMap returned (emptyMap, nil) whenever the xf_phrase query succeeded but matched zero rows, so the reference cache warmed a fully blank status/priority map and every statusName/priorityName resolved to "" on healthy 200s with nothing in logs, metrics, or errors — the exact silent-degradation mode #189 fixed at the title-prefix level, still latent one layer down (any future drift: another add-on phrase rename, a collation/charset change, a language_id regression). Introduce phraseFamily{prefix, name, mustBePopulated} for the three NF Tickets families (status/priority must be populated; prefix may be legitimately sparse). After the read, warnIfEmpty surfaces a zero-row must-be-populated family at Warn severity, naming the family, and STILL returns the empty map with a nil error — the refresh must not hard-fail on one drifted family and take down the rest of the cache (categories etc.). The healthy populated path is unchanged, and per-id misses inside a populated family still resolve to "" (the guard only fires on the all-or-nothing empty case). The documented status/priority/prefix title asymmetry (the CAUTION block) is preserved, not normalised. Tests: datastores/phrasemap_test.go pins the decision (empty status and priority warn naming the family; empty prefix is silent; any populated map never warns) capturing datastores.Warn. tickets_harness_test.go adds an end-to-end MariaDB-harness test that deletes the seeded status phrases, asserts LoadStatusNames returns an empty map with nil error and emits a Warn naming the family. This was generated by AI
…ly (#195) This was generated by AI
#196) This was generated by AI
This was generated by AI
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
1 participant
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.
Lands the #194 swarm-review follow-up cluster — three independent hardening/coverage items, no behavior change to healthy paths. All three branches are disjoint (
{datastores, referencecache}/{contract}/{rest, servers}) and merged without conflict.What's in the wave
Closes #195 —
datastores: warn on zero-row read of a must-be-populated phrase family.loadPhraseMapnow surfaces a zero-row read of a must-be-populated family (status/priority) loudly via the packageWarnlogger, naming the family, instead of silently warming a blank map that blanksstatusName/priorityNameon healthy 200s. Theprefixfamily stays exempt (may be legitimately sparse). It does not hard-fail the refresh (one drifted family can't take down the rest of the reference cache), genuine query errors still propagate, and per-id miss tolerance is preserved. Covered by a unit test asserting the signal and a live MariaDB harness test.Closes #196 —
contract: replay golden for over-ceiling repeated-uint32 filter400.Adds
tickets/list_category_over_uint32(?category_id=4294967296,read:tickets) witnessing the response-level400 {"code":3,"message":"… value out of range"}on the array binder path (parsing list "category_id", distinct from the scalarparsing field). Closes the replay half of the two-way contract coverage thatTestSpec_Uint32ParamsBoundedonly declared. Structural spec test and binder behavior untouched.Closes #197 —
rest: make the trustedProxies once-write/read-after invariant structural.trustedProxiesbecomes anatomic.Pointer[[]*net.IPNet]published byInitTrustedProxiesand read via an unexported accessor, removing the unsynchronized package-level slice and the theoretical data race.InitTrustedProxies's signature,servers/server.go, the composition-root wiring guard, andresolveClientIP's purity are all unchanged. A concurrency test exercises the publish/read path under-race.Gate (verified locally on the composed tree)
go build ./...,go vet ./...— cleango test ./...— greengo test -race ./...— greenmake test-integration(MariaDB harness) — green (datastores 14.489s, live)go mod tidy— no diff (#197uses stdlibsync/atomic)Review
Each branch was reviewed by a best-fit pr-review-toolkit agent (silent-failure-hunter / pr-test-analyzer / type-design-analyzer) — all APPROVE/ACCEPT, no blocking or should-fix findings. #197 rated 9/9/8/9 with a positive-control proving the old pattern trips
-raceand the new one is clean.This was generated by AI