Maven/docs/plans/24-no-deadline-on-the-turn-path.md

No deadline on the turn path

Last verified: 06-08-2026 @ 60e64dd

All four steps landed on 06-08-2026. What follows describes the defect as it was and the work as it was planned. Two things came out differently. Client.Close read the conn field with no lock while roundtrip re-dialed and dropped it. -race caught that on the new cancellation test. So the conn field now has a mutex of its own, held only across a read or an assignment. And /api/ptt needed nothing: it proxies to the voice port and never touches the shared client, so only /api/chat got the extra connection. The pool inside ipc.Client is still unbuilt and still waiting on a second module measured queueing.

V-638. Sibling of V-607, which is the same class of bug in internal/worker. Reads with docs/offload.md and docs/protocol.md.

What is missing

A chat turn starts in a mavweb HTTP handler and ends at llama-server. Nothing between those two points can be cancelled, and one hop has a timeout.

Four places, all on the same path.

voice.Replier.Reply takes no context (internal/voice/replier.go:41). So llmReplier calls PhraseReply(context.Background(), d) at cmd/mavend/replier_llm.go:42. The turn cannot deadline its own reply. The only bound is phraser.timeout, 60s in deploy.

ipc.Client.roundtrip sets no connection deadline (internal/ipc/client.go:202). A daemon that stops answering parks the caller for as long as the socket stays open.

ipc.Client.call checks the context once, before sending (client.go:149), then blocks in roundtrip. Cancelling mid-call does nothing.

ipc.Server.serveConn dispatches under context.Background() (internal/ipc/server.go:253). A client that hangs up does not cancel the turn, and neither does Server.Close.

And every call queues behind the slowest one

ipc.Client serialises on one connection and one mutex. mavweb routes /api/chat and /api/ptt through the shared client, so one turn blocks all 28 handlers while it runs. Worst case is a 60s page load.

This is understood for exactly one route already. cmd/mavweb/main.go:57 opens a second connection for /models, and the comment there says why. A model swap is a multi-minute call, and sharing the connection would freeze every other page.

The pattern is already in the repo

internal/voice/client.go:101 derives a connection deadline from the caller's context, falls back to 120s, and clears it with a defer. internal/ipc/client.go never learned it. Copy that rather than inventing a second convention.

The work

One commit each.

Context on the reply seam. phraser.Replier.PhraseReply already takes a context and the interface has two implementations, so this is small. Change Reply to take a context, have StubReplier ignore it, and pass it through llmReplier to PhraseReply. Both call sites already hold one: cmd/mavend/voice.go:461 and cmd/mavend/clarify.go:574.

Deadlines and cancellation on the client. Pass the context into roundtrip and set SetDeadline from it. For cancellation mid-call, a watchdog goroutine that calls c.drop() on ctx.Done() is enough. drop exists, and the retry split already separates a lost write from a lost read. So a cancelled call lands in errReadLost and is never retried for a mutation. Check that against internal/ipc/maperr_test.go.

A request context on the server. serveConn should derive from a server-scoped context so Close cancels a dispatch in flight. Server already carries done and a conn registry for this class of problem. The registry comment records what the last version of it cost: eleven days of stale ciphertext.

Stop serialising mavweb. Give /api/chat and /api/ptt their own connection, the way /models has one. Roughly ten lines, and it changes no shared code.

A connection pool inside ipc.Client is the general form and is deliberately not the first step. Each connection is already its own request and response stream. So a pool preserves frame pairing by construction. It still has to keep re-dial on drop, the errWriteLost and errReadLost split, and Close. Do the narrow fix, measure, and reach for the pool only if a second module turns out to queue.

How it is judged

make test stays green. It is green at 06c1cf2.

Nothing here changes routing or recall, so make eval-router and make eval-recall are unchanged rather than re-measured.

By hand: load /dash while a chat turn is in flight. Before the change it waits for the length of the turn.

There is no test today that a cancelled context aborts an in-flight ipc.Client call. That absence is why two of these four went unnoticed, so the test is part of the work.

What is not done here

The store is still SetMaxOpenConns(1) (internal/store/store.go:99) under WAL. WAL is built for concurrent readers against one writer, and the cap makes every read queue. Store.DB(ctx) hands the digestion worker a read transaction on that same connection. This plan does not touch it. It is measurable first and should be measured before it is changed.