Repository navigation
feat: add the v0.3 request inspector - #33
Merged
Merged
Conversation
Design for the v0.3 inspector. Caddy handler module tees each proxied request onto an in-process bus, a single goroutine writes batches to a SQLite ring buffer, and the dashboard reads a live feed off the same bus. Decisions worth calling out: - Metadata capture is on by default and persists. Bodies stay off, as DESIGN.md required. - Header values are redacted against a known list. Best effort, and the docs say so. - The buffer is bounded by rows, by bytes, and by age. Rows alone stops being true once bodies are on. Both volume caps miss the quiet case where old rows never get pushed out, so age is the floor. - The feed is SSE, not WebSocket. It only goes one way, so the extra module buys nothing. DESIGN.md needs updating for this. - Storage stays modernc.org/sqlite. The build is CGO_ENABLED=0 across four targets from one runner and that is worth more than the speed. - Route add and remove is out. It makes the dashboard a write surface and needs its own origin and auth design.
Twelve tasks, each with its own test cycle and commit. Order is config, then the pure record types, the store, the recorder, the Caddy handler, proxy wiring, daemon wiring, the API, the SSE feed, the page, an end to end test, and docs. Two deviations from the spec, both noted in the plan. The handler wraps Caddy's ResponseWriterWrapper rather than ResponseRecorder, because the 101 case needs a WriteHeader hook the recorder does not offer. And the dashboard gets a mux() method with a shared host guard rather than five copies of the same check.
Metadata capture defaults to on, bodies to off. Enabled is a *bool because the default is true and a plain bool cannot tell unset from off. max_age is a duration string so it reads as 168h and not 604800.
Header redaction is a deny-list, so it is best effort and the docs say so. Body capture is a write-through tee rather than a buffer. Buffering a response to capture it would break SSE and stall large downloads.
The original tests only overshot the cap in a single call, so a regression in the room-tracking math would not have been caught. Add exact-cap, straddle, and post-cap passthrough cases for both capWriter and capReader, asserting the captured bytes and the passthrough bytes in each case.
Bounded three ways: age, rows, bytes. Age is enforced on open too, because a daemon that sat idle over a weekend has stale rows and no writes coming to trigger a trim. Row count and byte total are kept in memory and corrected from DELETE ... RETURNING, so trimming after every batch never rescans the table. Byte-cap trimming evicts one row at a time rather than in a fixed batch: a batch larger than the row count the byte cap actually allows wipes the whole buffer on the first breach instead of trimming it back under the limit. Driver is modernc.org/sqlite. The release build is CGO_ENABLED=0 across four targets from one runner and that is worth more than the speed.
Five things review found: Byte-cap trim's no-progress guard: deleteAndAccount now reports how many rows it deleted, and the loop breaks on zero instead of looping forever if the in-memory counters ever drift ahead of what is on disk. That loop runs on the drain goroutine later, so a spin there would hang capture. LastInsertId's error was swallowed, leaving a record with ID == 0 and no signal that anything went wrong. Now it fails the batch. Open's DSN concatenated the data path into a SQLite URI unescaped. A path containing '#', '?', or '%' gets picked up by the driver's own first-'?' split, which quietly breaks parsing of the pragma query after it: confirmed by hand that this reports journal_mode "delete", not "wal", with no error anywhere. Path is now percent-escaped first. Two tests were checking less than they claimed. TestTrimByByteCap's bounds (>0, <40) would have passed with a batch size of 2 or 4, so it did not actually pin one-row-at-a-time eviction; it now asserts the exact count and confirms the in-memory total against a fresh Open. TestTrimByAge's explicit Trim(now) call was dead code, since Insert's own trailing trim had already removed the stale row by wall clock before it ran; MaxAge now starts at zero so the explicit call is the only thing doing the work. Added a test for Query.Before, which had none. Confirmed each fix's test actually fails without it before restoring the fix, rather than trusting the assertions on sight.
Clear ran its DELETE and only afterward took the mutex to zero s.rows/s.bytes. That window let a concurrent Bytes()/Rows() see the old totals against a table that was already empty — exactly the drift the byte-cap trim loop's no-progress guard exists to survive. Insert and deleteAndAccount had the same shape: the write (tx.Commit, or the DELETE in deleteAndAccount) completed, then the mutex was taken separately to update the counters. All three now hold the mutex across the write and the counter update as one step, so no accessor can ever see them out of step. Also added the committed regression test for the no-progress guard itself, kept as a throwaway last round: manufactures drift directly (rows/bytes set past what is on disk) and proves Trim returns instead of spinning.
Submit is a non-blocking channel send. A full buffer drops the record and counts it. Losing a row is a nuisance, adding latency to somebody's dev request is not acceptable. One drain goroutine owns all writing and all fan-out, so subscribers see records only after they have an ID. A subscriber that cannot keep up is dropped rather than slowed down. The feed is SSE, so the browser reconnects and backfills on its own. The recorder lives in a package-level pointer because every config reload re-provisions Caddy's handler instances and the SQLite handle has to outlive that.
A panic in drain skipped subscriber teardown, so a live SSE reader would hang forever and Close() would still report success. Move teardown into drain's deferred cleanup so it runs on every exit, panic or not, and track a closed flag so Subscribe after shutdown hands back an already-closed channel instead of registering one nobody will close. A failed batch write only logged; Dropped() did not count it, so the one number an operator can check understated real loss. Fold it into the existing counter. Insert can return an error that only means its own post-commit Trim failed, not that the batch was lost. Wrap that case in a sentinel in the store and have the recorder still publish to subscribers when it sees it, instead of suppressing a live update for rows that are actually sitting in the store. Also rewrote a subscriber test that asserted close to nothing: cancel ran before any submit, so the channel was already closed when the assertions ran and the test proved nothing about live cancellation.
Times the request, counts the response, hands a record to the recorder with a non-blocking send. Never changes what the client sees. Upgraded connections are recorded when the 101 lands, not when the handler returns. next blocks until the socket dies, so recording on return would mean an HMR websocket appears in the inspector an hour after it opened. Wraps Caddy's ResponseWriterWrapper so Hijack and Flush keep working. Getting that wrong would break every HMR connection on the machine.
Four real behavior gaps from review: ReadFrom was promoted from caddyhttp.ResponseWriterWrapper and wrote straight to the client, skipping Write. Any io.Copy into the response would have recorded nothing. Shadowed it to route through Write. A failed proxy attempt (no upstream, a timeout) never calls WriteHeader, so the status defaulted to 200 instead of the real 502/503/504 carried in the HandlerError. This hid the most common failure a local dev proxy sees. Now reads the status out of the HandlerError first. An HTTP/2 extended-CONNECT websocket (RFC 8441) gets a plain 200 from Caddy, never a 101, so it was recorded only on return, same as the bug the 101 rule exists to prevent. Treat that 200 as an upgrade too when the request is CONNECT with a websocket :protocol pseudo-header. The compile-time guard on watcher checked http.ResponseWriter, which is true regardless of the ResponseWriterWrapper embed. Added a guard on Unwrap, since that's what Flush and Hijack actually depend on. Also documented, without changing behavior: a panic in next produces no record on purpose (recovering would lose the stack and mishandle http.ErrAbortHandler), and turning body capture on also turns header redaction off. Adds tests for all of the above, including one that initially passed for the wrong reason (its source implemented io.WriterTo, which hid a broken ReadFrom) and was corrected after checking it against a deliberately broken build.
Ahead of reverse_proxy on each configured route, and nowhere near the dashboard catch-all. Instrumenting the catch-all would make the inspector record itself, feed included. Disabled means the handler is not in the generated config at all, rather than inserted and told to do nothing.
The recorder is created before the proxy loads, because the handler reads the process-wide pointer on its first request. Every failure is a warning and never a return. A broken inspector must not be the reason the proxy does not start. Reloads can flip enabled either way, so the store opens on the first reload that turns it on. Starting with it off does not mean restarting the daemon to use it. Shutdown order matters: the inspector's cleanup defer is registered first, so it unwinds last, after the proxy, dashboard, and DNS have all stopped serving. Closing the store any earlier could let a request in flight query a database that is already gone.
go test ./... -timeout 300s was going red. Task 7 added two more full daemon boots in the same test process, and Caddy's on-demand cert cache is a package-level variable that Stop() never clears for on-demand-issued certs. A second boot reusing the same domain names got served the first boot's certificate, signed by the first boot's root, and verification failed. Confirmed with a throwaway repro: two sequential proxy.Load/Stop cycles with the same suffix fail identically; a different suffix on the second one does not. That is issue #25, generalized past TestEndToEnd's name to any second full boot. Real production never runs two daemons in one process, so the fix is scoped to the tests: give every cert-verifying test its own suffix (test, internal, localhost - the three reserved ones) so the cache can never hand one test's cert to another. Every test still runs under a plain go test ./...; nothing is gated or skipped. Also: startEnv lost its t.Helper() when it became a thin wrapper, which pointed failures at the wrong line. And the MkdirAll comment promised an error message that was not the one SQLite actually returns.
Disabling the inspector in a reload only stopped proxy.Generate from attaching the handler. The recorder itself, inspect.Current(), the dashboard's stored recorder, and the SQLite handle all stayed alive until the process exited. Once the dashboard grows history endpoints, that means a user who turns capture off still sees the UI serving what was already captured, out of a database nothing ever closed. ensureInspector now tears the recorder down on that transition, same order as shutdown: clear the process-wide pointer, clear the dashboard's, close the store, then drop the local reference. The database file itself is left alone. Turning it back on and reloading reopens the same file, history intact.
List, detail and clear, all behind the same Host guard as the rest of the dashboard. API paths answer a foreign Host with a flat 404 rather than the friendly no-route page: there is no user reading it. Clear is the dashboard's first state-changing endpoint, so it states its rule. POST only, and Origin must be present and match. An absent Origin is refused rather than trusted, which closes the simple-request CSRF hole that loopback alone does not. A reload can close the store while a request is in flight, since the SetInspector swap has no barrier for readers already in flight. Store errors are checked for the driver's "database is closed" text and answered with a clean 503 instead of a 500 carrying raw driver text. Renamed dashboard_test.go's testServer helper to newBasicServer to avoid colliding with the new inspector test helper of the same name.
/api/routes was the one route with no Host check: it read back every route's upstream host:port and a live reachability probe to any page that rebound a domain to 127.0.0.1. Route it through guard like everything else, and cover it in the guarded-route test. Extract hostAllowed so guard, guardPage and handleRoot share one predicate instead of three copies of the same comparison. Add guardPage for /inspect, so a foreign Host gets the friendly no-route page instead of guard's flat 404, matching what a user navigating there directly expects. Declare the route table once as a slice mux() and the guard test both range over, so a route that skips the wrapper fails the test by construction instead of by someone remembering to update two lists. Update the security comments in dashboard.go and inspect.go to say what actually shipped: mutations landed, sameOrigin is the answer, and any future mutating route must call it too. Note sameOrigin's loopback-any-port trust boundary explicitly. Add tests for the query parameters handleInspectRequests wires up beyond q (domain, method, status, before, limit, and their pairings), and extend the no-recorder 503 check to the detail and clear routes.
One way only, so a websocket would buy nothing and cost a dependency. Subscribe happens before the backfill query, and the query happens before any header is written. That order closes two gaps at once: no request lands between the subscription and the backfill, and a closed store still gets a clean storeError response instead of a stream that opens and then silently skips its backfill. A subscriber that falls behind gets its channel closed. EventSource reconnects on its own and backfills, so the worst case is a gap, not a stalled recorder.
The subscribe-race test used n=200 against a store capped at 100 records, so it flaked under low concurrency: records evicted by the trim would only survive if published before the handler read them, which is a real race with nothing wrong on the handler side. Dropped n to 50, under both the store cap and the subscriber buffer, and renamed the test to say what it actually proves: delivery holds up under concurrent submits, not the subscribe/backfill ordering, which no black-box test can force without a hook the recorder does not expose. Restored a loud "streaming unsupported" 500 ahead of any header write when the response writer cannot flush, instead of letting a failed first flush fall through to a silent empty 200. Renamed the channel-receive loop variable so it stops shadowing the recorder variable of a different type thirteen lines up. Documented that a record landing in the subscribe/backfill window arrives twice, once from each path, and that a client can de-duplicate on id since Insert assigns it before publish.
Split pane, live tail with pause, filters by domain, method and status, path search, and clear. Vanilla JS and the existing CSS variables, so it inherits dark mode and adds no build step. Record data is built into the DOM with textContent and dataset, never innerHTML, since paths, headers and bodies come straight from whatever traffic hit the proxy. The page also tells an inspector-off 503 apart from an empty buffer, and clear asks for confirmation first. At most 500 rows in the DOM. Filtering is client side over the last 2000, which is what the live feed can hold anyway.
Address review findings on the inspector page. poll() now tolerates a rejected fetch instead of leaving the page stuck on "no traffic," and it detects the inspector coming back on, not just going off, so a config reload doesn't require a page reload. A 503 from the detail endpoint gets the same off-state message as the list, instead of blaming buffer trimming. show() drops a response that arrived after a newer click superseded it. And an empty list distinguishes "buffer just got cleared" from "filters match nothing."
Boots the daemon, makes a request through the proxy with real TLS, and reads it back off the dashboard's loopback API. Checks path, status, domain, and method against what the upstream actually returned. Also asserts the row count only grows by the marker request when the dashboard itself is loaded through the proxy first. That is the regression that would otherwise be invisible until somebody opened the inspector and found it full of itself. A follow-up request is used as a real synchronization point instead of a fixed sleep, so the check does not race the recorder's async batching. Uses its own domain suffix, since it verifies a certificate and Caddy's on-demand cert cache is shared across daemon boots in this test binary.
Also corrects two things in DESIGN.md. The live feed is SSE, not a websocket, because it only goes one way. And route add/remove is out of v0.3: it makes the dashboard a write surface and needs its own design.
The store runs on one connection, so a connection and s.mu are two locks. Insert took them connection first: it begins a transaction, then locks to commit and update the counters. Clear took them the other way round, and so did deleteAndAccount. Run a batch and a clear at the same time and each side owns what the other waits for. Nothing recovers from that. The drain goroutine is gone for the life of the process, so capture stops and every later record is dropped. The connection is held forever, so history, detail and the stream backfill all block and the inspector page hangs. Recorder.Close waits on the drain goroutine, so the daemon cannot shut down either. Clear and deleteAndAccount now check a connection out first and run their statement on it. Both still hold the mutex across the statement, so the window where the table has changed but the counters have not stays closed. The regression test starts a batch big enough to keep its transaction open, waits for the pool to report the connection checked out, and only then clears. It deadlocks against the old code every time.
http.Server.Shutdown waits for active requests and never cancels their contexts. The SSE handler watched only the client, the record channel and a 30 second ping, so it never learned the server wanted to stop. An inspector left open in a background tab held the whole daemon up: "shutting down" in the log and then nothing, a second Ctrl-C ignored because signal.NotifyContext already owns the handler, no DNS teardown and no WAL checkpoint. Caddy does not rescue it either, since the loopback dashboard bypasses the proxy. The dashboard now carries a done channel. Shutdown closes it before waiting, and the stream handler selects on it. The daemon bounds its own call at five seconds as a backstop, so a future handler that forgets to watch the signal costs a delay rather than a hang. Live stream events also stop carrying bodies. The backfill never had them, so the same record arrived in two shapes depending on its path, and with bodies on that put up to max_body_bytes per record on the wire for a list that never renders one. The page already refetches the full record when a row is opened.
The page's only data source was the live stream, whose backfill is 200 rows. There was no way to ask for more, the before cursor was never sent, and the four filter parameters on the history API were never used at all: filtering happened over the in-memory array. With the default 5,000 record buffer that put about 96% of what was captured out of reach, and made "filter by domain" quietly mean "by domain, within the last 200". A developer who captured a morning of traffic, reloaded and typed a domain saw nothing while the matching rows sat on disk. Filters now go to the server and the view is refetched. A load older button pages backwards with before set to the oldest row on screen. The backend needed no change. Live records still appear while a filter is set. The stream is unfiltered, so the page applies the same predicate the store does before adding one. Records are inserted by id rather than pushed, and an id already present is ignored, so a stream reconnect replaying the last 200 cannot double a row. A record that arrives while a refresh is in flight is held and merged after it, so changing a filter mid-traffic cannot swallow one. The clear button no longer swallows failures. A 503 switches the page into its inspector-off state the way the other calls do, and anything else says so instead of leaving the user believing the buffer was emptied.
The live bodies flag was passed straight into buildRecord on the 101 and extended-CONNECT emit, so its body branches could attach whatever the cap readers were holding at the moment of the upgrade. Bodies are never captured for an upgraded connection. That row now keeps redacted headers too, which is the coherent answer: the reason bodies-on drops redaction is that the secret is already on disk in the body beside it, and here no body ever is. A failing store logged once per failed batch, which is up to ten lines a second for as long as the condition lasts. Repeats inside a minute are now suppressed. Nothing is lost by that: Dropped() still counts every record.
The README listed exactly which headers are redacted, which invites a reader to assume URLs are covered too. They are not. Query strings are stored as sent, so a token in one lands on disk with redaction on. The architecture file listed inspect.db but not inspect.db-wal and inspect.db-shm. That table is what someone reads before deleting things, and both files are present while the daemon runs.
The previous fix passed bodies=false for the upgrade emit, which suppressed the body but also switched that row to redacted headers. Three shipped documents state the bodies-on coupling without qualification, and all three became false: the README, the architecture file and the inspector spec. Both spec rules hold at once. An upgraded row has no body, and bodies-on still means headers are copied as sent. buildRecord now suppresses the body itself for an upgraded row and the live flag is passed through, so the person debugging a handshake with bodies on can still read Cookie and Sec-WebSocket-Protocol. They were the only people who could not. Two tests, one per rule, because a fix for either one alone breaks the other. The deadlock test now skips rather than fails when the insert finishes before the clear can race it. That means the window never opened, which proves nothing either way.
render() cleared the list and built every row again. That reset scrollTop to 0, and it ran from the stream handler on every live event, so a user who clicked load older and scrolled into history was pulled back to the top by the next request through the proxy. On a dev proxy that is continuous. The control only worked while paused, and nothing said so. The same rebuild also had to go for cost. Removing the 500 row draw cap was needed to show paged results, but rebuilding was not: a page left open reaches 5,000 rows on live traffic alone, and each event then tore down and rebuilt about 30,000 nodes on the hot path of this whole feature. Rows are now keyed by record id and reconciled in place. A live record inserts one row and moves nothing else. Paging appends and disturbs nothing above it. The scroll position is held against the topmost visible record rather than a pixel offset, so rows arriving above the viewport no longer move what the user is reading. The load older button is one element for the life of the page, and it only appears when there are rows to page back from. It used to be possible to draw it over an empty list, where clicking it did nothing.
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.
Adds the v0.3 request inspector. Every proxied request is recorded into a bounded SQLite ring buffer and shown live in the dashboard.
Design:
docs/superpowers/specs/2026-08-02-inspector-design.md.What it does
A Caddy middleware sits ahead of
reverse_proxyin each user route. It times the request, learns the status and byte counts, and hands the record to a recorder. One drain goroutine batches those records into SQLite and fans them out to Server-Sent Events subscribers. The dashboard gets a split-pane page at/inspect, plus list, detail, clear, and stream endpoints.Capture never blocks a proxied request. The channel send is non-blocking and a full buffer drops the record and counts it. Losing a row is a nuisance. Adding latency to a dev request is not.
The inspector can never stop the daemon from starting. Every failure path logs a warning and runs with capture off.
Config
The buffer is bounded three ways at once: row count, total bytes, and age. Any of the three can be what trims. Trimming runs on write, on open, and on an hourly tick, so a daemon that sits idle over a weekend still ages rows out.
Bodies are off by default. Turning them on also turns header redaction off. Both effects, not one. Redaction is a deny-list, so it reduces exposure rather than preventing it, and the docs say so. Query strings are stored as sent.
Disabling the inspector by reload closes the recorder and the database, so the endpoints answer 503.
inspect.dbstays on disk, so turning it back on restores the history.Notes
modernc.org/sqlite. It is pure Go, which keepsCGO_ENABLED=0and the four release targets from one runner.DESIGN.mdis updated.Stop(), so a second daemon boot reusing a domain got the first boot's certificate. Cert-verifying tests now use separate domain suffixes.Testing
278 tests pass.
gofmtandgo vetare clean andTHIRD_PARTY_NOTICESis current.An end-to-end test drives a real request through the real proxy and reads it back from the dashboard API. Others cover the failure path still serving traffic with capture off, reload applying new limits, and teardown keeping the file.
Two bugs found by review in the seams between components, both reproduced before fixing:
Inserttook the connection then the mutex,Cleartook the mutex then the connection, on a pool of one. Clicking clear during traffic killed capture for the life of the process and hung shutdown.Shutdownwaits on active requests and the SSE handler never returned.Both have regression tests that fail against the unfixed code.