Skip to content

Replace chi router with net/http ServeMux#22

Merged
retlehs merged 3 commits intomainfrom
remove-chi
Mar 18, 2026
Merged

Replace chi router with net/http ServeMux#22
retlehs merged 3 commits intomainfrom
remove-chi

Conversation

@retlehs
Copy link
Member

@retlehs retlehs commented Mar 16, 2026

Summary

  • Replace chi with Go 1.22+ net/http.ServeMux — removes the only routing dependency
  • Remove ADMIN_ALLOW_CIDR and RequireAllowedIP (production already had admin_allow_cidr: "")
  • Keep TRUST_PROXY with a stdlib RealIP middleware so clientIP() works behind Caddy for login rate limiting and download dedupe
  • Add appHandler wrapper that handles sitemap-packages prefix routing (can't be expressed as a ServeMux pattern) and custom 404 templates without suppressing stdlib 405 behavior or swallowing handler-generated 404s
  • Add router regression tests covering pattern registration, 405 preservation, handler-generated vs unmatched 404s, and sitemap prefix routing
  • Update docs to remove Tailscale/CIDR references and reflect stdlib router

Test plan

  • go test ./... passes
  • golangci-lint run ./... — 0 issues
  • go vet ./... passes
  • make smoke end-to-end test
  • Verify POST /health returns 405 (not 404)
  • Verify /sitemap-packages-0.xml routes correctly

🤖 Generated with Claude Code

Remove the chi dependency in favor of Go 1.22+ ServeMux patterns
and a small set of stdlib middleware (Recoverer, RealIP, TimeoutHandler).

Drop ADMIN_ALLOW_CIDR and RequireAllowedIP — production already
disabled IP restriction. Admin access relies solely on in-app auth.
Keep TRUST_PROXY so clientIP() works behind Caddy for rate limiting
and telemetry dedupe.

Co-Authored-By: Scott Walkinshaw <scott.walkinshaw@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
retlehs and others added 2 commits March 16, 2026 14:19
RealIP is needed for login rate limiting and telemetry dedupe behind
Caddy. No reason to gate it behind a config flag — the app is always
proxied in production, and in dev the headers aren't present so
RemoteAddr passes through unchanged.

Co-Authored-By: Scott Walkinshaw <scott.walkinshaw@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The global http.TimeoutHandler replaced the ResponseWriter with a
buffering writer that doesn't implement http.Flusher, breaking
/admin/logs/stream in two ways:

1. The SSE handler's w.(http.Flusher) assertion always failed (500)
2. The previous noTimeout(context.WithoutCancel) couldn't bypass
   TimeoutHandler since it replaces the writer, not just the context

Fix: path-based timeout bypass that skips http.TimeoutHandler entirely
for streaming endpoints, preserving the original ResponseWriter. Add
Flush()/Unwrap() to statusRecorder so Flusher propagates through the
custom 404 layer.

Remove the broken context-key noTimeout mechanism.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@retlehs retlehs merged commit f0b3082 into main Mar 18, 2026
6 checks passed
@retlehs retlehs deleted the remove-chi branch March 18, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants