Loading
docs(specs): correct S01 post-revert (#89)
Summary
S01 was reviewed against current main and LabKit v2.4.0 in preparation
for re-implementation under #89 (closed). This MR records the 13 corrections
that came out of that review. No implementation changes — spec only.
Refs #89 (closed).
What changed
Each row maps to one defect ID surfaced during the review.
| # | Where | Fix |
|---|---|---|
| D1 | Line 3 | Status: Draft → Approved (catches up to commits afaa3577 / e62de5bc) |
| D2 | AC #11, middleware row #3 (closed) | path → uri, status_code → status to match LabKit v2.4.0's actual field names (fields/fields.go lists status_code as deprecated) |
| D4 | §Health endpoints | Rewrote intro: health endpoints bypass all middleware including LabKit's (httpserver/server.go:167-187); operational consequences spelled out |
| D5 | Middleware row #7 (closed) | Dropped "and access logging" — LabKit's AccessLogger ignores SetRoutePattern (always logs r.RequestURI) |
| D6 | Middleware row #8 (closed) | Rewrote the "on entry" claim — r.Pattern is set by the mux during dispatch, not knowable to any wrapping middleware at entry |
| D7 | Route topology /-/ row |
Owner → "LabKit v2 httpserver.Server (endpoints); S01 + #58 contribute readiness checks" (S03 narrowed by 731610d) |
| D8 | AC #16 (closed) | Added request_id to the asserted envelope shape and example |
| D9 | §Router envelope-rewrite | Switched to Content-Type-based trigger (zero-allocation; survives stdlib message changes) |
| D10 | §Shutdown sequencing | Documented pre-shutdown delay preemption (WARN, return nil) |
| D11 | §Shutdown sequencing + new AC | Composition root derives app.Config.ShutdownTimeout; invariant holds by construction |
| D12 | Proto + YAML + tables + AC #15 | Dropped max_header_size and timeouts.read_header — LabKit v2.4.0 exposes neither. Tags 2 and 1 held via skip-and-comment (matches the existing Config.database = 3 precedent in config.proto); fields land back when LabKit #10 ships |
| D13 | AC #18 (closed) | LabKit dispatch-before-ServeMux clarification (Go 1.22+ HEAD→GET auto-routing doesn't apply) |
AC count: 19 → 21 (D10 + D11 add one each).
Out of scope
- The S01 re-implementation itself — separate plan MR will follow per project guardrails.
Test plan
-
npx markdownlint-cli2 docs/specs/S01-http-server-and-routing.mdpasses locally - CI: markdownlint, vale, lychee all green
- Visual review: each row in the "What changed" table matches the diff
- No other spec or dev doc still references
max_header_sizeortimeouts.read_headeras live fields
Edited by Suleimi Ahmed