From 160700921599df7e6138924b9fc4e12013a2b898 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 22:50:38 +0400 Subject: [PATCH 1/7] the reply seam takes the turn's context (V-638) voice.Replier.Reply had no context, so llmReplier phrased under context.Background() and the only bound on a reply was phraser.timeout, 60s in deploy. Both call sites already held a context. The stub ignores it: it makes no model call. --- cmd/mavend/clarify.go | 2 +- cmd/mavend/replier_llm.go | 8 ++++---- cmd/mavend/replier_llm_test.go | 10 +++++----- cmd/mavend/voice.go | 2 +- internal/voice/replier.go | 9 +++++++-- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index bb87835..1271b8d 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -571,7 +571,7 @@ func (h *reactiveHandler) finishClarified(ctx context.Context, dec router.Decisi } reply := h.applyAction(ctx, dec) if reply == "" { - reply = h.replier.Reply(dec) + reply = h.replier.Reply(ctx, dec) } if reply == "" { // Belt: an empty reply here would be a silent drop. diff --git a/cmd/mavend/replier_llm.go b/cmd/mavend/replier_llm.go index cde52e2..36f0d7f 100644 --- a/cmd/mavend/replier_llm.go +++ b/cmd/mavend/replier_llm.go @@ -22,7 +22,7 @@ func newLLMReplier(c phraser.Completer, block func() string) *llmReplier { // Reply never fails: a clarify, a model error and an unusable generation all // answer from the stub, which is what keeps a turn from breaking on the model. -func (r *llmReplier) Reply(d router.Decision) string { +func (r *llmReplier) Reply(ctx context.Context, d router.Decision) string { if d.Clarify { // The deck, not the stub's single sentence: a clarify she cannot turn // into a question is the line he hears most often when she misses him, @@ -39,14 +39,14 @@ func (r *llmReplier) Reply(d router.Decision) string { // что ты выпел стакан воды" for "я выпил воды". return phraser.FactAck(d.Utterance) } - out, err := r.p.PhraseReply(context.Background(), d) + out, err := r.p.PhraseReply(ctx, d) if err != nil || out == "" { - return r.stub.Reply(d) + return r.stub.Reply(ctx, d) } // The persona checks, on the live path (personaguard.go). A reply that // leaks reasoning or calls him "вы" is worse than a flat one. if _, ok := guardSpoken("reply", out); !ok { - return r.stub.Reply(d) + return r.stub.Reply(ctx, d) } return out } diff --git a/cmd/mavend/replier_llm_test.go b/cmd/mavend/replier_llm_test.go index 027670c..3b3a32c 100644 --- a/cmd/mavend/replier_llm_test.go +++ b/cmd/mavend/replier_llm_test.go @@ -22,7 +22,7 @@ func (s stubCompleter) Complete(_ context.Context, _ llm.Req) (string, error) { func TestLLMReplierPassesTheModelReplyThrough(t *testing.T) { r := newLLMReplier(stubCompleter{out: `{"response":"записала, кофе закончился","mood":"neutral"}`}, nil) - got := r.Reply(router.Decision{Intent: router.IntentNote, Slots: router.Slots{Text: "кофе закончился"}}) + got := r.Reply(context.Background(), router.Decision{Intent: router.IntentNote, Slots: router.Slots{Text: "кофе закончился"}}) if got != "записала, кофе закончился" { t.Errorf("got %q, want %q", got, "записала, кофе закончился") } @@ -42,7 +42,7 @@ func TestLLMReplierFallsBackToStubOnEmpty(t *testing.T) { // the clarify deck rather than the stub's single sentence. func TestLLMReplierClarifyReadsTheDeck(t *testing.T) { r := newLLMReplier(stubCompleter{out: "я всё поняла"}, nil) - got := r.Reply(router.Decision{Clarify: true, Utterance: "мгм"}) + got := r.Reply(context.Background(), router.Decision{Clarify: true, Utterance: "мгм"}) if got == "я всё поняла" { t.Fatal("a clarify must not be phrased by the model") } @@ -50,7 +50,7 @@ func TestLLMReplierClarifyReadsTheDeck(t *testing.T) { t.Errorf("on clarify: got %q, want %q", got, want) } // Two different misses do not sound identical. - if same := r.Reply(router.Decision{Clarify: true, Utterance: "а"}); same == got { + if same := r.Reply(context.Background(), router.Decision{Clarify: true, Utterance: "а"}); same == got { t.Log("two utterances hashed to the same line, which is allowed but should be rare") } } @@ -60,14 +60,14 @@ func TestLLMReplierClarifyReadsTheDeck(t *testing.T) { // produce, which is the same claim without pinning one wording. func assertAck(t *testing.T, r *llmReplier, d router.Decision, key, what string) { t.Helper() - if got := r.Reply(d); !phraser.IsAck(key, nil, got) { + if got := r.Reply(context.Background(), d); !phraser.IsAck(key, nil, got) { t.Errorf("on %s: got %q, want a %q line", what, got, key) } } func assertStub(t *testing.T, r *llmReplier, d router.Decision, what string) { t.Helper() - got, want := r.Reply(d), voice.NewStubReplier().Reply(d) + got, want := r.Reply(context.Background(), d), voice.NewStubReplier().Reply(context.Background(), d) if got != want { t.Errorf("on %s: got %q, want stub %q", what, got, want) } diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index 4e12bce..d148ac9 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -458,7 +458,7 @@ func (h *reactiveHandler) runTurn(ctx context.Context, text string, src turnSour // 9. replier — phrase the reply across the router decision. if replyText == "" { - replyText = h.replier.Reply(dec) + replyText = h.replier.Reply(ctx, dec) } return withNotice(expiredNotice, replyText) } diff --git a/internal/voice/replier.go b/internal/voice/replier.go index ef05de0..0d52515 100644 --- a/internal/voice/replier.go +++ b/internal/voice/replier.go @@ -26,6 +26,8 @@ package voice import ( + "context" + "github.com/kami/maven/internal/phraser" "github.com/kami/maven/internal/router" ) @@ -38,8 +40,10 @@ import ( // decision's Intent + Slots + Clarify. The Intent largely names the reply // shape (act/reminder/fact/note/query/clarify); the Slots carry the // specifics that personalise it ("got it: water at 14:00"). +// The context is the turn's, and it is the only bound an LLM-backed impl has +// besides the phraser timeout (V-638). A floor impl ignores it. type Replier interface { - Reply(d router.Decision) string + Reply(ctx context.Context, d router.Decision) string } // StubReplier — the deterministic, no-model floor. Canned per intent; @@ -54,7 +58,8 @@ func NewStubReplier() *StubReplier { return &StubReplier{} } // Reply dispatches on Intent + Clarify. Each branch is short; the LLM impl // will replace this with prompted text and the same dispatch shape. -func (s *StubReplier) Reply(d router.Decision) string { +// It makes no model call, so the context is unused. +func (s *StubReplier) Reply(_ context.Context, d router.Decision) string { if d.Clarify { return "не совсем поняла — можешь переформулировать?" } -- 2.52.0 From 123b9aa961e97ac0aae9139a9c6fda06f0e54c4e Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 22:54:19 +0400 Subject: [PATCH 2/7] an ipc call can be cancelled and always has a deadline (V-638) roundtrip set no connection deadline and call checked the context once, before sending, so a daemon that read the frame and stopped answering parked the caller for as long as the socket stayed open. The deadline comes from the caller's context, falling back to 120s, the convention internal/voice/client.go already had. A watchdog closes the conn on ctx.Done(); it closes rather than calling drop, because drop wants c.mu and call is holding it. The retry split is unchanged and now also declines a retry the caller has stopped waiting for. A cancelled mutation stays ErrAmbiguousOutcome, because it may have committed. A cancelled read reports the cancellation. Three tests against a server that accepts and never answers. There was no test for this, which is why it went unnoticed. --- internal/ipc/cancel_test.go | 119 ++++++++++++++++++++++++++++++++++++ internal/ipc/client.go | 49 +++++++++++++-- 2 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 internal/ipc/cancel_test.go diff --git a/internal/ipc/cancel_test.go b/internal/ipc/cancel_test.go new file mode 100644 index 0000000..bd843ff --- /dev/null +++ b/internal/ipc/cancel_test.go @@ -0,0 +1,119 @@ +package ipc + +import ( + "context" + "errors" + "net" + "path/filepath" + "testing" + "time" +) + +// A cancelled context has to abort a call that is already in flight. It did not +// until V-638: call checked ctx once before sending and then blocked in +// roundtrip with no connection deadline, so a daemon that read the frame and +// never answered parked the caller for as long as the socket stayed open. +// +// The server here is that daemon: it accepts, reads nothing, replies nothing. + +func deafServer(t *testing.T) string { + t.Helper() + sock := filepath.Join(t.TempDir(), "deaf.sock") + ln, err := net.Listen("unix", sock) + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { _ = ln.Close() }) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + // Hold it open and say nothing. Closed by the listener cleanup. + t.Cleanup(func() { _ = conn.Close() }) + } + }() + return sock +} + +func TestClientCancelAbortsAReadInFlight(t *testing.T) { + c, err := Dial(deafServer(t)) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer c.Close() + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(50 * time.Millisecond) + cancel() + }() + + done := make(chan error, 1) + go func() { + _, err := c.Ping(ctx) + done <- err + }() + + select { + case err := <-done: + // Ping is read-only, so the cancellation is reported as itself rather + // than as an ambiguous mutation. + if !errors.Is(err, context.Canceled) { + t.Errorf("got %v, want context.Canceled", err) + } + case <-time.After(5 * time.Second): + t.Fatal("a cancelled Ping did not return") + } +} + +// A mutation cancelled while awaiting the reply may already have committed, so +// it is ErrAmbiguousOutcome and never a retry. That split is the invariant +// internal/ipc/maperr_test.go's neighbours rest on. +func TestClientCancelLeavesAMutationAmbiguous(t *testing.T) { + c, err := Dial(deafServer(t)) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer c.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + + done := make(chan error, 1) + go func() { + _, err := c.WriteFact(ctx, WriteFactReq{Key: "water", Value: "drank"}) + done <- err + }() + + select { + case err := <-done: + if !errors.Is(err, ErrAmbiguousOutcome) { + t.Errorf("got %v, want ErrAmbiguousOutcome", err) + } + case <-time.After(5 * time.Second): + t.Fatal("a cancelled WriteFact did not return") + } +} + +// The deadline itself, with no cancellation: a call on a context with no +// deadline used to have no bound at all. This one has one and must respect it. +func TestClientDeadlineBoundsACall(t *testing.T) { + c, err := Dial(deafServer(t)) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer c.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + start := time.Now() + if _, err := c.Ping(ctx); err == nil { + t.Fatal("a deaf server answered a Ping") + } + if elapsed := time.Since(start); elapsed > 3*time.Second { + t.Errorf("Ping took %v, want the context deadline to bound it", elapsed) + } +} diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 5cf1fc9..e2b8c40 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -29,6 +29,12 @@ type Client struct { mu sync.Mutex } +// defaultCallTimeout bounds a call whose context carries no deadline. It is +// the same 120s internal/voice/client.go settles on: long enough for a model +// call on a cold resident model, short enough that a daemon which stopped +// answering does not park the caller forever. +const defaultCallTimeout = 120 * time.Second + // errWriteLost marks a conn drop while sending the request frame: the request // never reached the server (or the server never saw a complete frame), so // retrying is always safe regardless of method — nothing was applied to @@ -163,16 +169,26 @@ func (c *Client) call(ctx context.Context, m Method, params, result any) error { } var resp Response - err := c.roundtrip(m, raw, &resp) + err := c.roundtrip(ctx, m, raw, &resp) switch { case errors.Is(err, errWriteLost): // The request never left; a duplicate send can't double-apply. // Redial (roundtrip re-dials on a nil conn) and retry exactly once. - err = c.roundtrip(m, raw, &resp) + // Not when the caller has given up — a retry would only be a second + // frame nobody is waiting for. + if ctx.Err() == nil { + err = c.roundtrip(ctx, m, raw, &resp) + } case errors.Is(err, errReadLost): if readOnlyMethods[m] { + if ctx.Err() != nil { + // The caller cancelled the read it was waiting for. Nothing + // was applied, so this is the cancellation and not an + // ambiguity. + return ctx.Err() + } // A duplicate read can't double-apply either — safe to replay. - err = c.roundtrip(m, raw, &resp) + err = c.roundtrip(ctx, m, raw, &resp) } else { // The mutation may have already committed server-side. Do not // retry: report the ambiguity instead of guessing. @@ -199,7 +215,14 @@ func (c *Client) call(ctx context.Context, m Method, params, result any) error { // failure is wrapped in errReadLost (ambiguous — call() only retries it for // read-only methods). Either way a failed conn is dropped so the next call // re-dials clean. Caller holds c.mu. -func (c *Client) roundtrip(m Method, raw json.RawMessage, resp *Response) error { +// +// The connection carries a deadline derived from ctx, falling back to +// defaultCallTimeout, and a watchdog closes it if ctx is cancelled mid-call +// (V-638). Before that a daemon which stopped answering parked the caller for +// as long as the socket stayed open. The watchdog closes the conn rather than +// calling drop, because drop wants c.mu and the caller is holding it — the +// closed socket fails the read, and roundtrip drops it on the way out. +func (c *Client) roundtrip(ctx context.Context, m Method, raw json.RawMessage, resp *Response) error { if c.conn == nil { conn, err := netaddr.Dial(c.addr) if err != nil { @@ -207,6 +230,24 @@ func (c *Client) roundtrip(m Method, raw json.RawMessage, resp *Response) error } c.conn = conn } + conn := c.conn + if dl, ok := ctx.Deadline(); ok { + _ = conn.SetDeadline(dl) + } else { + _ = conn.SetDeadline(time.Now().Add(defaultCallTimeout)) + } + defer conn.SetDeadline(time.Time{}) + + done := make(chan struct{}) + defer close(done) + go func() { + select { + case <-ctx.Done(): + _ = conn.Close() + case <-done: + } + }() + if err := writeFrame(c.conn, Request{Method: m, Params: raw}); err != nil { c.drop() return fmt.Errorf("%w: %v", errWriteLost, err) -- 2.52.0 From cbd8077d2cd6d84983d9e356e256a77ffb895e0d Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 22:59:02 +0400 Subject: [PATCH 3/7] a request context on the ipc server, cancelled by Close (V-638) serveConn dispatched under context.Background(), so a dispatch in flight during shutdown could not be told to stop and closeGrace could only abandon it. The server now carries a context, Close cancels it, and each conn derives its own so nothing outlives the connection. A zero-value Server built outside Listen falls back to Background; two wiring tests do that. Client.Close read c.conn with no lock while roundtrip re-dialed and dropped it, which -race caught on the new test. The conn field now has its own mutex, held only across a read or an assignment, so Close and the watchdog reach the connection without queueing behind the call they are interrupting. --- internal/ipc/cancel_test.go | 51 +++++++++++++++++++++++++++++++++++++ internal/ipc/client.go | 43 +++++++++++++++++++++++++------ internal/ipc/server.go | 36 ++++++++++++++++++++++---- 3 files changed, 117 insertions(+), 13 deletions(-) diff --git a/internal/ipc/cancel_test.go b/internal/ipc/cancel_test.go index bd843ff..88db143 100644 --- a/internal/ipc/cancel_test.go +++ b/internal/ipc/cancel_test.go @@ -117,3 +117,54 @@ func TestClientDeadlineBoundsACall(t *testing.T) { t.Errorf("Ping took %v, want the context deadline to bound it", elapsed) } } + +// blockingAPI parks Presence until its context is cancelled and records what +// cancelled it. Every other method is the unimplemented floor. +type blockingAPI struct { + UnimplementedCoreAPI + entered chan struct{} + err chan error +} + +func (b *blockingAPI) Presence(ctx context.Context) (Presence, error) { + close(b.entered) + <-ctx.Done() + b.err <- ctx.Err() + return Presence{}, ctx.Err() +} + +// serveConn dispatched under context.Background() until V-638, so Close could +// only abandon a dispatch in flight and never tell it to stop. +func TestServerCloseCancelsADispatchInFlight(t *testing.T) { + api := &blockingAPI{entered: make(chan struct{}), err: make(chan error, 1)} + srv, err := Listen(filepath.Join(t.TempDir(), "core.sock"), api) + if err != nil { + t.Fatalf("listen: %v", err) + } + served := make(chan struct{}) + go func() { _ = srv.Serve(); close(served) }() + + cli, err := Dial(srv.Path()) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer cli.Close() + go func() { _, _ = cli.Presence(context.Background()) }() + + select { + case <-api.entered: + case <-time.After(5 * time.Second): + t.Fatal("the handler was never dispatched") + } + + _ = srv.Close() + <-served + select { + case got := <-api.err: + if !errors.Is(got, context.Canceled) { + t.Errorf("handler saw %v, want context.Canceled", got) + } + case <-time.After(5 * time.Second): + t.Fatal("Close did not cancel the dispatch") + } +} diff --git a/internal/ipc/client.go b/internal/ipc/client.go index e2b8c40..7deb6ed 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -26,7 +26,12 @@ type Client struct { conn net.Conn path string // the address as configured, kept for errors and logs addr netaddr.Addr // parsed, so a dropped conn can be re-dialed (core restart) - mu sync.Mutex + mu sync.Mutex // one request at a time, so a frame and its reply pair up + + // connMu guards the conn field alone, and is held only across an assignment + // or a read. It exists so Close and the cancellation watchdog can reach the + // connection without waiting for the call that is holding c.mu (V-638). + connMu sync.Mutex } // defaultCallTimeout bounds a call whose context carries no deadline. It is @@ -109,11 +114,18 @@ func Dial(path string) (*Client, error) { return &Client{conn: c, path: path, addr: addr}, nil } +// Close closes the connection out from under a call in flight, on purpose: a +// shutdown must not wait out a parked read. It takes connMu and never c.mu, so +// it cannot block behind the call it is interrupting. func (c *Client) Close() error { + c.connMu.Lock() + defer c.connMu.Unlock() if c.conn == nil { return nil } - return c.conn.Close() + err := c.conn.Close() + c.conn = nil + return err } // DialWait is Dial with patience: it retries with capped backoff until the @@ -223,14 +235,15 @@ func (c *Client) call(ctx context.Context, m Method, params, result any) error { // calling drop, because drop wants c.mu and the caller is holding it — the // closed socket fails the read, and roundtrip drops it on the way out. func (c *Client) roundtrip(ctx context.Context, m Method, raw json.RawMessage, resp *Response) error { - if c.conn == nil { - conn, err := netaddr.Dial(c.addr) + conn := c.currentConn() + if conn == nil { + dialed, err := netaddr.Dial(c.addr) if err != nil { return fmt.Errorf("%w: dial %s: %v", errWriteLost, c.addr, err) } - c.conn = conn + c.setConn(dialed) + conn = dialed } - conn := c.conn if dl, ok := ctx.Deadline(); ok { _ = conn.SetDeadline(dl) } else { @@ -248,11 +261,11 @@ func (c *Client) roundtrip(ctx context.Context, m Method, raw json.RawMessage, r } }() - if err := writeFrame(c.conn, Request{Method: m, Params: raw}); err != nil { + if err := writeFrame(conn, Request{Method: m, Params: raw}); err != nil { c.drop() return fmt.Errorf("%w: %v", errWriteLost, err) } - if err := readFrame(c.conn, resp); err != nil { + if err := readFrame(conn, resp); err != nil { c.drop() return fmt.Errorf("%w: %v", errReadLost, err) } @@ -261,12 +274,26 @@ func (c *Client) roundtrip(ctx context.Context, m Method, raw json.RawMessage, r // drop closes and forgets the current conn so the next call re-dials. func (c *Client) drop() { + c.connMu.Lock() + defer c.connMu.Unlock() if c.conn != nil { _ = c.conn.Close() c.conn = nil } } +func (c *Client) currentConn() net.Conn { + c.connMu.Lock() + defer c.connMu.Unlock() + return c.conn +} + +func (c *Client) setConn(conn net.Conn) { + c.connMu.Lock() + defer c.connMu.Unlock() + c.conn = conn +} + // hydrate rehydrates a wire RpcError into the matching package sentinel. The // code↔sentinel table is the only place the wire "knows" about errors; keep it // in sync with codeOf in wire.go. diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 97ee006..1f1a608 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -30,6 +30,14 @@ type Server struct { done chan struct{} accept sync.Mutex // guards wg.Add vs Close's wg.Wait sequence + // ctx — server-scoped, cancelled by Close, and the parent of every request + // context. serveConn dispatched under context.Background() until V-638, so + // a dispatch in flight during shutdown could not be told to stop and the + // closeGrace below could only abandon it. Cancelling gives a handler that + // respects its context the chance to return instead. + ctx context.Context + cancel context.CancelFunc + // conns — every accepted connection still being served. Close needs these // because closing the listener does nothing to a connection already // accepted: serveConn is parked in readFrame waiting for a peer that may @@ -208,11 +216,14 @@ func Listen(path string, api CoreAPI) (*Server, error) { if err != nil { return nil, err } + ctx, cancel := context.WithCancel(context.Background()) s := &Server{ - path: path, - addr: addr, - ln: ln, - done: make(chan struct{}), + path: path, + addr: addr, + ln: ln, + done: make(chan struct{}), + ctx: ctx, + cancel: cancel, } s.api.Store(api) return s, nil @@ -250,7 +261,10 @@ func (s *Server) Serve() error { func (s *Server) serveConn(c net.Conn) { caller, callerOK := peerCaller(c) - ctx := context.Background() + // Derived from the server's, so Close cancels a dispatch in flight, and + // cancelled when this conn ends so nothing a handler spawned outlives it. + ctx, cancel := context.WithCancel(s.serverContext()) + defer cancel() if callerOK { ctx = WithCaller(ctx, caller) } @@ -274,6 +288,15 @@ func (s *Server) serveConn(c net.Conn) { } } +// serverContext is s.ctx, or Background for a Server built as a zero value +// rather than by Listen (the wiring tests do that). +func (s *Server) serverContext() context.Context { + if s.ctx == nil { + return context.Background() + } + return s.ctx +} + func (s *Server) safeDispatch(ctx context.Context, req Request) (result json.RawMessage, err error) { defer func() { if r := recover(); r != nil { @@ -720,6 +743,9 @@ func (s *Server) Close() error { default: close(s.done) } + if s.cancel != nil { + s.cancel() + } err := s.ln.Close() // Closing the listener stops new connections; it does nothing to the ones // already accepted. Close those too, or every serveConn parked in readFrame -- 2.52.0 From 60e64dd83c4f4a3b6d7afeb0a94eebbd14a77401 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 22:59:53 +0400 Subject: [PATCH 4/7] chat gets its own core connection, so a turn stops blocking every page (V-638) ipc.Client serialises every call on one mutex, and mavweb routed 28 handlers plus /api/chat through the shared client. A turn is bounded only by phraser.timeout, 60s in deploy, so a page load behind one could wait that long. /models already had its own connection for this reason. Chat gets the third one. A failed dial logs and falls back to the shared client, which is how it behaved before. /api/ptt needs nothing here: it proxies to the voice port and never touches this client. A connection pool inside ipc.Client is the general form and stays unbuilt until a second module is measured queueing. --- cmd/mavweb/main.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index 22578cd..a42d959 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -58,6 +58,12 @@ func main() { // mutex, so sharing the connection would freeze every other page for the // length of the load. See handleModels. var swapConn modelController + // turnConn — a third connection, for POST /api/chat and nothing else, for + // the same reason /models has one (V-638). A chat turn routes, phrases and + // may act, bounded only by phraser.timeout at 60s, and every other handler + // on this server queues behind it on the shared client's one mutex. Nil ⇒ + // chat shares the main connection, which is how it behaved before. + var turnConn ipc.CoreAPI if *coreSock != "" { c, err := ipc.DialWait(*coreSock, 60*time.Second) if err != nil { @@ -71,6 +77,12 @@ func main() { defer sc.Close() swapConn = sc } + if tc, err := ipc.Dial(*coreSock); err != nil { + log.Printf("chat: third core connection failed (%v) — /api/chat will share the main one and a turn will block the other pages", err) + } else { + defer tc.Close() + turnConn = tc + } } // stepUpSession stays nil unless the passkey endpoints are wired below — it @@ -208,7 +220,13 @@ func main() { // decides how every utterance is routed and how every reply is worded. mux.HandleFunc("/tools", gatedPage(handleTools)) mux.HandleFunc("/routines", gatedPage(handleRoutines)) - mux.HandleFunc("/api/chat", gatedPage(handleChatAPI)) + mux.HandleFunc("/api/chat", func(w http.ResponseWriter, r *http.Request) { + c := turnConn + if c == nil { + c = core + } + handleChatAPI(w, r, c, stepUpSession, *requireStepUp) + }) mux.HandleFunc("/api/revert", gatedPage(handleRevert)) mux.HandleFunc("/api/correct", gatedPage(handleCorrectAPI)) mux.HandleFunc("/models", func(w http.ResponseWriter, r *http.Request) { -- 2.52.0 From 2e6b274bff0ab38cf75205f779db72696946e60f Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 23:00:14 +0400 Subject: [PATCH 5/7] the plan doc records what landed and what came out differently (V-638) --- docs/plans/24-no-deadline-on-the-turn-path.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/plans/24-no-deadline-on-the-turn-path.md b/docs/plans/24-no-deadline-on-the-turn-path.md index 331223f..f8adc58 100644 --- a/docs/plans/24-no-deadline-on-the-turn-path.md +++ b/docs/plans/24-no-deadline-on-the-turn-path.md @@ -1,6 +1,14 @@ # No deadline on the turn path -Last verified: 06-08-2026 @ 06c1cf2 +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`. -- 2.52.0 From bd5337261617d26b34798505753385a918bc8c9a Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 23:05:53 +0400 Subject: [PATCH 6/7] Close takes connMu by hand, not by defer (V-638) Review on PR #188. The deferred unlock made it unclear where the lock was released, and it held connMu across conn.Close(), which contradicts the invariant stated one line above it: the field accesses only. A close on a tcp conn can block, and connMu is on the path of every call. --- internal/ipc/client.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 7deb6ed..ba983b0 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -117,15 +117,19 @@ func Dial(path string) (*Client, error) { // Close closes the connection out from under a call in flight, on purpose: a // shutdown must not wait out a parked read. It takes connMu and never c.mu, so // it cannot block behind the call it is interrupting. +// +// The lock is taken and released by hand, around the two field accesses and +// nothing else. The socket close happens outside it, because a close on a tcp +// conn can block and connMu is on the path of every call. func (c *Client) Close() error { c.connMu.Lock() - defer c.connMu.Unlock() - if c.conn == nil { + conn := c.conn + c.conn = nil + c.connMu.Unlock() + if conn == nil { return nil } - err := c.conn.Close() - c.conn = nil - return err + return conn.Close() } // DialWait is Dial with patience: it retries with capped backoff until the -- 2.52.0 From 9cdec11346123f4018f3aa612b89ea683a90d333 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 23:09:05 +0400 Subject: [PATCH 7/7] a cancelled call does not leave its conn for the next one (V-638) The watchdog and the end of the call race by construction: a cancellation landing as the reply arrives closes a conn the call had already finished with, and c.conn still pointed at the closed socket. It was survivable before this. A write to a closed socket is errWriteLost, which re-dials and retries, and that retry is safe because nothing was sent. So this buys one round trip, not a correctness fix, and the new test says so rather than pretending to catch a break. --- internal/ipc/cancel_test.go | 22 ++++++++++++++++++++++ internal/ipc/client.go | 12 +++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/internal/ipc/cancel_test.go b/internal/ipc/cancel_test.go index 88db143..084d7b7 100644 --- a/internal/ipc/cancel_test.go +++ b/internal/ipc/cancel_test.go @@ -168,3 +168,25 @@ func TestServerCloseCancelsADispatchInFlight(t *testing.T) { t.Fatal("Close did not cancel the dispatch") } } + +// The watchdog closes the conn, and it races the end of the call: a +// cancellation landing as the reply arrives can close a conn the call was +// already done with. That is survivable either way, because a write to a closed +// socket is errWriteLost and errWriteLost re-dials and retries, so this test +// passes with or without the drop in roundtrip's defer. What it pins is that +// the recovery is real and costs one round trip at most, never an error the +// caller sees. +func TestClientSurvivesACancelledCall(t *testing.T) { + _, _, cli, _ := newServerWithStore(t) + + for i := 0; i < 20; i++ { + ctx, cancel := context.WithCancel(context.Background()) + go cancel() // races the reply on purpose + _, _ = cli.Ping(ctx) + cancel() + + if _, err := cli.Ping(context.Background()); err != nil { + t.Fatalf("call %d after a cancelled one: %v", i, err) + } + } +} diff --git a/internal/ipc/client.go b/internal/ipc/client.go index ba983b0..08c885c 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -255,8 +255,18 @@ func (c *Client) roundtrip(ctx context.Context, m Method, raw json.RawMessage, r } defer conn.SetDeadline(time.Time{}) + // The watchdog and the end of the call race by construction: a cancellation + // landing just as the reply arrives can close a conn this call is already + // done with, and c.conn would still point at the closed socket. So a call + // whose context ended does not leave the conn behind for the next one, + // whichever of the two got there first. done := make(chan struct{}) - defer close(done) + defer func() { + close(done) + if ctx.Err() != nil { + c.drop() + } + }() go func() { select { case <-ctx.Done(): -- 2.52.0