fix(lifecycle): close leaked goroutines, sessions, and unbounded error reads

[SantiagoDePolonia]

Problem

Third slice of the memory-leak investigation (follow-up to #488 and #489): four lifecycle leaks where goroutines, connections, or buffers survive their request instead of being released. None depend on config — all are active in default deployments.

Changes

1. Pipe-producer goroutine leak (internal/llmclient/client.go)
File uploads and audio transcriptions stream a multipart body into the request through an io.Pipe; the producer goroutine blocks in Write until the transport reads it. When the request died before reaching the transport — circuit breaker open, request build failure — nothing ever read or closed the pipe, so the producer goroutine plus the entire upload buffer leaked until process restart. Trigger pattern: provider outage trips the breaker while clients retry uploads. llmclient now closes RawBodyReader (when it implements io.Closer) on every pre-transport error return, mirroring net/http's Do contract that the body is always closed. Once the request reaches the transport, the transport owns closing it, unchanged.

2. Live-audit orphan on handler panic (internal/auditlog/middleware.go)
Recover() is registered outside the audit middleware, so a handler panic unwound past all the terminal live events — the audit.started snapshot stayed in the broker's active-snapshot state forever and was replayed to every dashboard subscriber. The middleware now publishes audit.removed from a defer when a panic passes through, then re-panics so the recover middleware behaves exactly as before.

3. Realtime websocket heartbeat (internal/realtime/proxy.go)
The relay's two copy loops block in Read; a silently dead peer (NAT timeout, power loss — no RST, no close frame) never returns from Read, leaking two goroutines and both connections per session, indefinitely. The relay now pings both peers every 30s with a 10s timeout — pongs are processed by the always-in-flight reads, so healthy sessions are untouched. One subtlety the new tests caught: a ping failing with net.ErrClosed means teardown is already resolving locally, so the heartbeat waits for the copy loop's definitive close cause instead of racing to report a spurious secondary error.

4. Bounded upstream error-body reads (internal/llmclient/client.go)
DoStream's non-200 path and doRequest's error-status path read the upstream body without a limit; a misbehaving provider answering an error status with an endless body was buffered whole (bounded only by HTTP_TIMEOUT). Both now read through a 64KB LimitReader, matching core's existing audit-capture cap for error bodies. Successful responses are read whole, unchanged — large legitimate results must not be truncated.

Tests

• Reader-close contract: pre-transport build error and breaker-open both close a recording ReadCloser.
• Panic path: audit.started followed by audit.removed, and the panic still propagates.
• Heartbeat: an unresponsive peer (connected but never reading, so never ponging) tears the session down with a heartbeat error; an active session across many heartbeat intervals stays alive and still closes cleanly. Cadence is overridable via a test-only export.
• Also fixes TestProxyRelaysLargeFrame leaking its hijacked session past the test boundary (Accept hijacks the connection, so httptest.Server.Close doesn't wait for relay goroutines) — found because the leaked session raced the new tests' cadence override under -race.

Full suite passes (62 packages); realtime suite stressed 8× under -race.

User-visible impact

Long-running deployments stop accumulating goroutines/connections from dead realtime sessions and breaker-rejected uploads. Realtime sessions over connections that silently die now end within ~40s with a logged heartbeat error instead of living forever. No API or config changes.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Live audit entries are now cleaned up properly when a request handler panics.
  • Requests with upstream error responses now read bounded error payloads and close caller-supplied streaming bodies on early failures.
  • WebSocket relays now include heartbeat monitoring to tear down sessions with unresponsive peers and avoid hanging.
• Tests
  • Added tests covering panic-driven audit removal, pre-transport cleanup on DoRaw failures, and heartbeat behavior for responsive vs. unresponsive proxy sessions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0f55f2b1-fc37-454c-979b-eb8e10c4f382

📥 Commits

Reviewing files that changed from the base of the PR and between e138c28 and 3f52b8b.

📒 Files selected for processing (2)

• internal/realtime/proxy.go
• internal/realtime/proxy_test.go

---

📝 Walkthrough

Walkthrough

This PR adds panic-safe audit cleanup, bounds upstream error-body reads and raw body cleanup in llmclient, and introduces heartbeat-driven teardown for realtime proxy sessions with new regression tests.

Changes

Audit Middleware Panic Cleanup

Layer / File(s)	Summary
Panic recovery publishes audit removal internal/auditlog/middleware.go, internal/auditlog/middleware_test.go	Middleware now publishes LiveEventAuditRemoved on handler panic before re-panicking, and a test verifies the panic and event order.

Bounded Error Bodies and Raw Body Cleanup

Layer / File(s)	Summary
Bounded error body reads internal/llmclient/client.go	Adds maxErrorBodyBytes and limits error-response reads while keeping successful responses fully read.
Raw body cleanup on early failures internal/llmclient/client.go, internal/llmclient/client_test.go	Adds closeRawBodyReader and calls it on pre-transport failures in DoRaw, DoStream, DoPassthrough, and doHTTPRequest; tests confirm the reader is closed when request setup fails or the circuit breaker is open.

Websocket Heartbeat Teardown

Layer / File(s)	Summary
Heartbeat configuration internal/realtime/proxy.go, internal/realtime/export_test.go	Adds heartbeat cadence variables, heartbeat-related imports, and a test helper to override cadence values.
Relay heartbeat loop internal/realtime/proxy.go	Reworks relay shutdown around three goroutines, adds heartbeat pings with timeouts, and returns heartbeat failures or normal close causes after teardown.
Heartbeat proxy tests internal/realtime/proxy_test.go	Updates the large-frame test to wait for proxy completion and adds coverage for unresponsive and responsive heartbeat behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Handler
  participant Middleware
  participant LiveEventEmitter
  Handler->>Middleware: panic
  Middleware->>LiveEventEmitter: publish LiveEventAuditRemoved
  Middleware->>Handler: re-panic

Loading

sequenceDiagram
  participant relay
  participant ClientConn
  participant HeartbeatLoop
  participant UpstreamConn
  relay->>ClientConn: start copyFrames
  relay->>UpstreamConn: start copyFrames
  relay->>HeartbeatLoop: start heartbeat ticker
  loop every heartbeatInterval
    HeartbeatLoop->>ClientConn: ping
    HeartbeatLoop->>UpstreamConn: ping
  end
  HeartbeatLoop-->>relay: heartbeat error or ctx cancellation
  relay->>ClientConn: close both peers
  relay->>UpstreamConn: close both peers

Loading

Poem

I hop through logs with tidy delight,
Catching panics and pinging the night.
Raw bodies are closed,
And dead sockets exposed—
A rabbit’s relay feels just right. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main lifecycle fixes: leaked goroutines, dead sessions, and unbounded error reads.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/lifecycle-leaks

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 86.36364% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/llmclient/client.go	71.42%	4 Missing ⚠️
internal/realtime/proxy.go	92.30%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[greptile-apps]

Confidence Score: 5/5

Safe to merge with low lifecycle-risk impact.

The changes are focused on cleanup paths and preserve existing public behavior. Tests cover pre-transport body closure, panic cleanup, and websocket heartbeat teardown. No blocking correctness or security issues were identified in the changed paths.

No files require special attention.

T-Rex Logs

What T-Rex did

• Ran the normal heartbeat-relay test command for the realtime proxy; the run completed with exit code 0, and the targeted tests TestProxyHeartbeatTearsDownUnresponsivePeer, TestProxyHeartbeatLeavesResponsiveSessionAlive, TestProxyRelaysBidirectionally, and TestProxyRelaysLargeFrame all passed.
• Ran the race-stress heartbeat-relay tests; the run completed with exit code 0 and all four tests passed across all 3 race-stress iterations.

_{Ran code and verified through T-Rex}

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant Proxy as realtime.Proxy relay
participant Upstream
participant HB as heartbeat goroutine

Client->>Proxy: websocket frames
Proxy->>Upstream: copyFrames client to upstream
Upstream->>Proxy: websocket frames
Proxy->>Client: copyFrames upstream to client
loop every heartbeatInterval
    HB->>Client: Ping(timeout)
    HB->>Upstream: Ping(timeout)
    alt peer responds
        Client-->>HB: Pong handled by active Read
        Upstream-->>HB: Pong handled by active Read
    else ping fails or times out
        HB-->>Proxy: heartbeat error
        Proxy->>Client: Close
        Proxy->>Upstream: Close
    end
end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant Proxy as realtime.Proxy relay
participant Upstream
participant HB as heartbeat goroutine

Client->>Proxy: websocket frames
Proxy->>Upstream: copyFrames client to upstream
Upstream->>Proxy: websocket frames
Proxy->>Client: copyFrames upstream to client
loop every heartbeatInterval
    HB->>Client: Ping(timeout)
    HB->>Upstream: Ping(timeout)
    alt peer responds
        Client-->>HB: Pong handled by active Read
        Upstream-->>HB: Pong handled by active Read
    else ping fails or times out
        HB-->>Proxy: heartbeat error
        Proxy->>Client: Close
        Proxy->>Upstream: Close
    end
end

Loading

_{Reviews (1): Last reviewed commit: "fix(lifecycle): close leaked goroutines,..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/realtime/proxy_test.go`:
- Around line 209-233: The heartbeat test for responsive sessions is too tight
and may flake under CI jitter. In
TestProxyHeartbeatLeavesResponsiveSessionAlive, relax the cadence and timeout
used by SetHeartbeatCadenceForTest and extend the overall loop deadline so the
proxy has more slack while still exercising the same responsive-session
behavior. Keep the assertion flow in proxyServer, dialClient, and the client
read/write loop unchanged, but choose wider timing values that reduce the chance
of a delayed ping/read causing a false unresponsive failure.

In `@internal/realtime/proxy.go`:
- Around line 130-140: The shared timeout in pingBoth causes the second
websocket Ping to inherit a nearly expired deadline after the first Ping
succeeds, so update pingBoth to give client and upstream their own full
heartbeatTimeout budget instead of reusing one pingCtx. Keep the same
heartbeatTimeout behavior by creating separate timeout contexts for each Ping
call, and preserve the existing client heartbeat and upstream heartbeat error
wrapping so the caller can still distinguish which peer failed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 99b4fd33-ec00-451f-a236-8dd72cd8c4ed

📥 Commits

Reviewing files that changed from the base of the PR and between 3c81092 and e138c28.

📒 Files selected for processing (7)

• internal/auditlog/middleware.go
• internal/auditlog/middleware_test.go
• internal/llmclient/client.go
• internal/llmclient/client_test.go
• internal/realtime/export_test.go
• internal/realtime/proxy.go
• internal/realtime/proxy_test.go