feat(client): full-screen terminal UI and local request inspector (TUNNEL-33)

[joaoh82]

Closes the two gaps in TUNNEL-33: the CLI printed a static box and went silent, with no way to see what flowed through a tunnel.

What changed

Part 0 — HTTP visibility. The client forwarded bytes without understanding them, which is why the styled print_request helper had been dead code since it was written. proxy.rs now feeds every copied byte through a passive HTTP/1.x tap (inspect/tap.rs) that parses framing — Content-Length, chunked (with extensions and trailers), EOF-delimited — without modifying, reordering, or delaying the stream. Anything that stops looking like HTTP/1.x (WebSocket upgrades, HTTP/2 prior knowledge, malformed traffic) drops to passthrough and bytes keep flowing untouched. I chose byte-level sniffing over a hyper-based intercepting proxy (Option A in the ticket) precisely to avoid changing tunnel semantics.

Captured exchanges mirror the server's CaptureEvent/CapturedRequest shape, plus headers and 64 KB-capped bodies — which is what makes payload display and replay possible.

Part 1 — Terminal UI (src/tui/). Ratatui alternate screen: session status, region + live keepalive latency, per-tunnel health (the existing health.rs probes were never displayed before), connection/byte/p50/p90 counters with a req/s sparkline, and a scrolling request log. Keys: q/Esc/Ctrl-C quit, ↑↓/jk scroll, f follow, c clear, l log pane.

Part 2 — Web inspector (src/inspect/server.rs). Loopback-only axum server on :4040 — request list, detail (Summary/Headers/Raw), SSE live tail, and replay against the local service. UI is a single self-contained HTML file, no build step or CDN.

Degradation (unchanged behaviour where it matters)

Mode	Behaviour
--json	Byte-identical to before — verified stdout stays pure NDJSON
non-TTY stdout	No TUI; startup box + live request lines
--no-tui	Same as above, on a TTY
--no-inspect	No web server, no port bound
TCP/UDP tunnels	Connection and byte counters only; no HTTP parsing

Bug found and fixed during testing

The inspector bound 127.0.0.1:4040 while the local tunnel server held 0.0.0.0:4040 — macOS/BSD allow the more specific bind, so loopback traffic reached the inspector instead of the server and the client broke its own control connection. bind() now probes for a live listener before claiming a port. Regression test included; this would have hit anyone running the documented local dev setup.

Testing

62 client unit tests (up from 28), workspace green, make check clean.

• Tap: keep-alive pairing, byte-at-a-time splits, chunked + extensions/trailers, HEAD, 100 Continue, 101 upgrade, non-HTTP passthrough, EOF-delimited, oversized head, body truncation, pending-queue cap.
• TUI: rendered via TestBackend and asserted on content; small/awkward terminal sizes don't panic.
• Inspector: port scan, reserved-port skip, wildcard-shadow regression, loopback-only.

End-to-end against a real local server + demo app: all request types captured with correct method/path/status/bodies, chunked decoded, replay re-issued a POST and recorded it, SSE pushed live rows, --json stayed clean, TCP tunnel showed byte counters with zero HTTP entries.

Notes

• Ratatui 0.29, not 0.30 as the ticket suggested — 0.29 is the last single-crate release; 0.30 splits into ratatui-core/-widgets/-backend with breaking API changes. Worth revisiting as a follow-up, not as part of a first cut.
• Docs updated: docs/client-guide.md gets the new flags, a Terminal UI section, and a Request Inspector section (including the local-only security note — captured payloads may contain credentials).

Follow-ups (not in scope here)

• Inspector captures HTTP only; TCP tunnels could get a byte-stream viewer.
• The server's own capture pipeline still stubs bodies/headers (dashboard/capture.rs:124) — the client-side model here is a reference for filling that in.

🤖 Generated with Claude Code

[marvin-agent-rockflow]

Hermes Agent Review

Head: 12a5420 · Files: 16 · +3988 / -75
Verdict: Request changes

Solid TUNNEL-33 delivery: passive HTTP tap (no stream mutation), loopback-only inspector with the macOS/BSD wildcard-shadow bind fix, clean TUI degradation paths (--json / non-TTY / --no-tui), and strong unit coverage on framing edge cases. Docs call out the credential exposure model correctly.

A few correctness/security items should be fixed before merge; CI is also red on an unrelated-looking dashboard scope test.

Critical

• None in the tunnel data path itself (bytes still pass through untouched).

Warnings

• Replay follows redirects by default (inspect/server.rs) — reqwest default policy can chase Location off the local service (link-local/metadata endpoints or external hosts) while still carrying captured Authorization / Cookie headers on same-origin hops. Disable redirects (or restrict to loopback targets) for replay.
• local_addr_for falls back to the first tunnel (inspect/mod.rs) — multi-tunnel sessions can replay against the wrong local service when the label does not match. Prefer hard 409 over silent fallback.
• CI red on get_tunnel_returns_404_for_other_tenant (expected 200, got 404). Client crate tests/clippy passed; this looks like a flaky/unrelated dashboard integration assertion, but the check must be green before merge.

Suggestions

• pump flushes after every 16 KiB chunk (proxy.rs) — extra syscalls vs untapped copy_bidirectional. Consider flush-on-idle or only on partial buffers once capture is on a hot path.
• Request-target absolute-form — if a client sends an absolute-form request-target, replay concatenates the local base with that absolute target and can produce a bad URL. Rare on this path; normalizing to origin-form would harden replay.
• Optional: default --no-inspect under --json for pure automation (stdout is already clean; binding the inspector port is still a side effect).

Looks good

• Port probe before bind + reserved-port skip — excellent regression for the control-plane hijack you found.
• Tap design: keep-alive pairing, chunked/TE, HEAD/1xx/101 upgrade to passthrough, body/pending caps.
• UI XSS hygiene (esc() on untrusted path/header/body text).
• Degradation matrix matches the PR description; health + latency mirrored into UIs cleanly.
• Docs security note for local-only inspector is clear.

---

Automated hourly review by marvin-agent-rockflow (Hermes). Will re-review only if new commits land.

[marvin-agent-rockflow]

Warning — redirect policy on replay: reqwest::Client follows redirects by default (up to 10). A local app that returns a 3xx with a non-loopback Location will be followed from this process. Captured Authorization / Cookie headers are also attached to the initial request.

For an inspector that intentionally re-issues real credentials, prefer:

reqwest::Client::builder()
    .timeout(REPLAY_TIMEOUT)
    .redirect(reqwest::redirect::Policy::none())
    .build()?

Or a custom policy that only allows redirects whose host is loopback / the original local address. Surface the 3xx + Location in the recorded exchange so the UI still shows what happened.

[marvin-agent-rockflow]

Warning — silent multi-tunnel fallback: If tunnel does not match any registered name, this returns the first tunnel local address. On rustunnel start with multiple HTTP services, a label mismatch (or a stale exchange after reconnect) will replay against the wrong backend — potentially mutating the wrong app.

Safer: find by name only (no fallback to first), and let replay_request keep returning 409 when unknown. A unit test with two tunnels where only the second name matches would lock this in.

[marvin-agent-rockflow]

Suggestion — flush cadence: Flushing after every successful write_all of up to 16 KiB is correct for low-latency interactive use, but it is a measurable step up from copy_bidirectional on bulk transfers. If profiling shows it on the hot path, consider flushing only when n < COPY_BUF (likely end of a write burst) or coalescing with a short timed flush — without reintroducing Nagle issues on the local socket (you already set TCP_NODELAY).

[joaoh82]

Thanks — three of these are fixed in ddda910. Notes on each item, including the two I did not change.

Fixed

Replay follows redirects — now redirect::Policy::none(). The fidelity argument turned out to be the stronger one: the new test (replay_records_redirects_instead_of_following_them) fails without the fix by recording 404 instead of 302, because the replay chased the Location all the way out to the real example.com and recorded that response. So the inspector was misreporting what the local service returned, and making an outbound request to a third party in the process. The Location header is preserved in the recorded exchange, so the UI still shows the redirect.

Worth noting for the record: reqwest already strips Authorization/Cookie on cross-host redirects, so the credential-forwarding risk was narrower than described — but same-host redirects did carry them, and the misreporting alone justifies the change.

local_addr_for silent fallback — removed; it now matches by name only, and replay_request returns 409 for an unknown tunnel as you suggested. Test local_addr_matches_by_name_and_never_falls_back uses two tunnels and asserts both that the second resolves correctly and that an unknown name resolves to nothing; replay_refuses_when_the_tunnel_is_unknown covers the 409 path.

Absolute-form request-target — added origin_form(), which reduces http://host/path?q to /path?q before it is appended to the local base. Asterisk-form (OPTIONS *) and anything unrecognised fall back to / rather than building an invalid URL. Covered by a unit test over all the shapes plus an end-to-end replay test.

Not changed

pump flush cadence — the observation is fair but the proposed heuristic is unsafe. Flushing only when n < COPY_BUF stalls whenever a response lands on an exact 16 KiB boundary: the tail sits unflushed in the yamux buffer while the loop blocks on the next read, and if the caller is waiting on that response, it hangs until a timeout. The correct form is flush-when-the-reader-would-block, which is what copy_bidirectional does through poll readiness and would need real plumbing to replicate here.

The cost is also narrower than it looks: TcpStream::flush is a no-op, so only the tunnel direction pays, and only on HTTP connections while capture is enabled. Keeping correctness and interactive latency for now; worth revisiting behind a benchmark rather than a heuristic.

CI red on get_tunnel_returns_404_for_other_tenant — not attributable to this PR, and now disproven: re-running the identical commit went green with no code change. It also passes locally on main, both in isolation and under the full parallel cargo test --workspace against a fresh database. It is a flaky server-side assertion, and note it is the owner check at dashboard_scope.rs:250 ("A still sees their own tunnel" getting a 404), not the cross-tenant assertion the test is named for. Filed separately as TUNNEL-37 with the evidence and repro notes.

Still open for a product decision

Inspector under --json — I agree there is something to fix, but the issue is visibility rather than the bind itself: in --json mode the inspector URL is never reported, so the listening port is an invisible side effect. The options are to emit it as an NDJSON event (keeps the capability for agent workflows) or to default --no-inspect on. Leaving that to @joaoh82.

Client tests: 68 passing (was 62). make check clean.

[joaoh82]

Resolved the last open item in 5855561: the inspector URL is now emitted as an NDJSON event rather than the port being bound silently.

{"event":"inspector_ready","url":"http://127.0.0.1:4040"}
{"event":"tunnel_ready","protocol":"http","public_url":"https://myapp.edge.rustunnel.com","local_port":3000,"local_host":"localhost"}

It fires once at startup before any tunnel_ready, so a script can pick the port up before traffic arrives — which matters because the port is not guaranteed to be 4040 once the scan-forward logic kicks in. With --no-inspect nothing binds and no event is emitted. Verified end-to-end against a dead server so the inspector bind is the only thing exercised; stdout stayed clean NDJSON in both cases.

Chose this over defaulting --no-inspect under --json so agent and script workflows keep access to the capture API — the objection was really about the side effect being invisible, not about it existing.

Client tests: 69.

[marvin-agent-rockflow]

Hermes Agent Review

Head: 5855561 · Files: 17 · +4223 / -76
Verdict: Approve
Note: Re-review after ddda910 (replay hardening) + 5855561 (inspector_ready).

Prior findings — resolved

• Redirect follow on replay — now redirect::Policy::none() with replay_records_redirects_instead_of_following_them.
• Multi-tunnel silent fallback — local_addr_for is name-only; unknown tunnel → 409 CONFLICT, covered by unit + handler tests.
• Absolute-form request-target — origin_form() normalizes proxy-style targets before building the local URL.

Critical

• None.

Warnings

• None blocking.

Suggestions

• inspect/ui.html (replay handler): API returns body_truncated, but the UI never surfaces it. A truncated POST/PUT replay silently sends a short body with a recomputed Content-Length — easy to mutate the wrong state. Show a banner when result.body_truncated is true (and ideally on detail when request_body.truncated).
• proxy.rs flush-after-every-write: Still a latency/throughput trade-off on bulk transfers; optional follow-up to flush only on short reads / idle.
• Replay hop-by-hop set: Consider also stripping Expect (e.g. 100-continue) so replay does not wait on a continue dance the capture path already finished.

Looks good

• Passive tap design (no stream mutation; passthrough on upgrade/non-HTTP) is the right call for tunnel fidelity.
• Port bind probe for macOS/BSD 127.0.0.1 vs 0.0.0.0 shadowing — real footgun, solid regression test.
• Loopback-only inspector, reserved port skip, body/head caps, pending-queue bound.
• UI escapes untrusted path/header/body text before innerHTML.
• Degradation matrix (--json / non-TTY / --no-tui / --no-inspect) and inspector_ready NDJSON event for automation.
• Docs call out credential sensitivity of captures.
• Strong unit coverage across tap framing, bind edge cases, TUI TestBackend, and the new replay hardening.

---

Automated hourly review by marvin-agent-rockflow (Hermes). Will re-review only if new commits land.

[marvin-agent-rockflow]

💡 Suggestion: The replay API returns body_truncated when the captured request body was capped at 64 KiB, but this UI path ignores it and jumps straight to the new row. For mutating methods, a partial replay with a recomputed Content-Length is easy to miss — consider a short error/warning banner when result.body_truncated is true (and/or a note on the detail pane when request_body.truncated).

[marvin-agent-rockflow]

💡 Suggestion (nit): Hop-by-hop stripping correctly drops Transfer-Encoding / Content-Length. Worth also skipping Expect so a captured Expect: 100-continue does not make replay wait on a continue exchange the original connection already completed.

[marvin-agent-rockflow]

💡 Suggestion (perf, non-blocking): Flushing after every successful write_all keeps interactive latency low but is heavier than copy_bidirectional on bulk transfers. Optional follow-up: flush when n < COPY_BUF or on a short idle timer if profiling shows it on the hot path.