Close channel in ChanWriter when RPC stream ends

[phinze]

miren exec (and miren app run, which goes through the same server path) would panic with "close of closed channel" after the command finished. The exec output came back fine, but then the RPC handler blew up — not great for service stability.

There were actually two bugs conspiring here. The first was ChanWriter in pkg/rpc/stream/stream.go — it spawns a goroutine that reads from an RPC stream and forwards values into a Go channel, but when the stream ended it just returned without closing the output channel. Since ChanWriter is the only sender, it owns channel closure by Go convention. Added defer close(ch) and a test that wires up a full RPC pipeline to verify the channel gets closed when the source goes away.

The second (and the one actually producing the panic in the stacktrace) was inlineClient.Close() in pkg/rpc/inline.go. It calls close(c.pool) but never nils out c.pool afterwards. When the RPC framework tears down a streaming call, both the handler's defers and the framework's own cleanup end up calling Close() on the same client — the second call sees c.pool != nil, tries to close it again, and panics. The fix nils out c.pool before closing so the second call is a no-op.

Confirmed the fix by deploying a test app in the dev environment and running sandbox exec repeatedly — zero panics across multiple runs, where before it panicked every time.

Closes MIR-622

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e2f40013-e811-490e-b5de-f7ff52725df8

📥 Commits

Reviewing files that changed from the base of the PR and between 996f9ca and 945644a.

📒 Files selected for processing (1)

• pkg/rpc/inline.go

---

📝 Walkthrough

Walkthrough

Added a defer close(ch) to the ChanWriter goroutine so the provided output channel is closed when the writer finishes. Added a test that sends a value through a ChanReader/ChanWriter pair and asserts the output channel is closed after the stream ends. Refactored inlineClient pool lifecycle: introduced errInlineClientClosed, changed initPool() to return error, added closed-state checks in getStream(), adjusted stream creation/activeCount handling, and updated Close() to drain and close the pool safely.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/rpc/inline.go`:
- Around line 198-201: Guard against races between Close and getStream by
marking the connection as closed before clearing and closing the pool and by
making getStream and initPool check that closed flag; specifically, in Close set
c.closed = true (or otherwise flip a boolean) under the same mutex used around
c.pool, capture the old pool to a local variable, set c.pool = nil, then iterate
over and close that local channel while skipping nil entries; update getStream
to check c.closed before receiving from c.pool and return an error if closed
(avoid using c.pool after it might be nil), and update initPool to early-return
if c.closed is set so it cannot recreate the pool after Close; reference
symbols: Close, getStream, initPool, c.pool, c.closed, conn.enc, conn.stream.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 587d9bc6-f05b-4b5d-9058-b03b8de3c90b

📥 Commits

Reviewing files that changed from the base of the PR and between 4f4bf76 and 996f9ca.

📒 Files selected for processing (1)

• pkg/rpc/inline.go