From f1a809121b4c8d7832ff21d19d1de2f7a1059236 Mon Sep 17 00:00:00 2001 From: kami Date: Sat, 1 Aug 2026 20:05:52 +0400 Subject: [PATCH] shutdown: close the sockets, or the database never gets sealed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mavend seals its encrypted database in `defer st.Close()` when run() returns. It had not returned since 2026-07-21. Every restart since then decrypted the same eleven-day-old ciphertext and rolled back everything written in between: the Telegram nudge that kept firing was a fact being un-written on each boot. The goroutine dump named it. main → srv.Close() → ipc.(*Server).Close → wg.Wait(), waiting on per-connection goroutines parked in readFrame. Close shut the listener and nothing else, so the idle persistent sockets held by mavweb, mavpoll, mavcaldav and mavmaild blocked shutdown forever. `docker compose stop -t 60` spent the whole sixty seconds and then took a SIGKILL. So: track the accepted conns and close them, in ipc and in voice, which had the identical defect. Bound all three waits — the two per-server ones and the worker wait in main — because the seal matters more than any single in-flight call. A dropped RPC costs one reply; a missed seal costs a session. The regression test leaves a client connected and idle, which is the case the old tests avoided by closing the client first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TrVSBKe3RFDF4fGYKWYQnX --- HANDOFF.md | 120 +++++++++++++++++++++++++ QA-PLAN.md | 174 ++++++++++++++++++++++++++++++++++++ cmd/mavend/actions_query.go | 1 - cmd/mavend/main.go | 30 ++++++- cmd/mavseal/main.go | 119 ++++++++++++++++++++++++ internal/ipc/ipc_test.go | 27 ++++++ internal/ipc/server.go | 88 +++++++++++++++++- internal/store/crypt.go | 31 +++++++ internal/voice/server.go | 71 ++++++++++++++- 9 files changed, 657 insertions(+), 4 deletions(-) create mode 100644 HANDOFF.md create mode 100644 QA-PLAN.md create mode 100644 cmd/mavseal/main.go diff --git a/HANDOFF.md b/HANDOFF.md new file mode 100644 index 0000000..b2ad4d6 --- /dev/null +++ b/HANDOFF.md @@ -0,0 +1,120 @@ +# Handoff + +Uncommitted scratch file. Delete it once the work below is finished. + +Written 2026-08-01. Kami is away for about an hour and will start a new session. + +## Where things stand + +The 35-PR stack (50 to 84 on gitea, one linear chain) has been reviewed and the +findings have been fixed. Two things are done and one is not. + +**Done: reviews.** One review posted on every PR from 50 to 84, as the `claude` +login. Review ids 58 to 92. + +**Done: fixes.** Eleven agents fixed the findings in parallel git worktrees. +All eleven branches are merged onto **`fix/integrated`**, which is 68 commits +ahead of the tip. `make test` and `make build` both pass on it. + +**Not done: landing them.** `overnight/eco-versioned-traces` (the stack tip) has +NOT been moved. Kami chose to land the fixes as commits on that tip. The final +step is a fast-forward, once he has answered the open questions below. + + git checkout overnight/eco-versioned-traces + git merge --ff-only fix/integrated + +Do not push. He merges the stack himself. + +## Ask him these first + +He asked for the list and then left, so none of it is answered. Nothing below +is a blocker for the fast-forward, but all of it is easier to change before the +tip moves than after. + +1. **Hexis refuses to wire when a token is configured.** It used to log once and + then call unauthenticated forever. Fail closed, or warn and continue? +2. **`MethodIngestMail` stayed at `AuthRead`** while `SetTaskStatus` moved to + `AuthWrite`. Those two now disagree about the same kind of question. +3. **"Прости" in `clarifyGaveUp` and one expiry variant.** `CheckCringe` bans + apologies. The fix scoped the ban to nudges and kept his wording, with the + reasoning in a test skip. Absolute ban is the other reading. +4. **Memory evaluation timeout is 60s.** Two agents disagreed here. The merge + kept the short budget plus the gate that yields the slot to voice turns. Five + minutes is safe again now that the gate exists. +5. **Cold-start v1 recovery is narrow.** It fires only when an assertion carries + a PRF secret and that unlock fails. A pre-existing box whose only + authenticator lacks PRF still needs `MAVEN_DB_KEY`. + +## Two things to know before deploy + +**The update block.** The self-update fix refuses at startup any +build-from-source deployment that cannot say how to undo the source. +`deploy/mavend.json` has no `update` block today, so nothing breaks. Adding one +for the docker layout without `source_rollback` will stop the daemon booting. +That is intended. It should not be a surprise. + +**Three migrations became four numbers.** Three agents each wrote a migration +15. Tasks kept 15, ecosystem traces became 16, MCP tool fingerprints became 17. +Any box already carrying an unreleased 15 from a worktree build needs its +schema version checked by hand. + +## Merge decisions already made + +Two agents independently added `llm.Gate`. One is priority between a voice turn +and background work. The other is admission control while the resident model is +swapped. Both were kept. The swap one is now `SwapGate` with `SetSwapGate`. +`Complete` takes priority first and the drain second, so a background request +waiting on priority cannot stall a swap. + +## Still open in the code, on purpose + +- Ambient calendar events never reach the `calendar_busy` gate. `calendar_busy` + is a level, so an ambient writer needs an expiry. Recorded as a comment at the + top of `cmd/mavweb/ambient.go`. +- Hands-free has no session-scoped voice assertion. Noted in the mavweb route + table. +- The Hexis capabilities call sends no correlation id. The vendored client has + no header hook, so closing it means re-vendoring. +- Re-running a stored audio blob by id does not exist. The comments that claimed + the wire offered it were corrected. +- Simulator persona checks read a Go constant on one step, because chat there + goes through `phraser.NewStub()` rather than the replier. +- The PRF value is client-supplied and unbound to the assertion signature. This + is inherent to PRF key wrapping, which needs a fixed salt. Documented in + `cmd/mavweb/webauthn.go`. Do not try to bind it. +- `safeKey` drops Cyrillic, so Russian calendar events on one day collide. Filed + as Vikunja #443 with three fix options. It is a migration, not a patch. + +## The one task left + +Audit what the agents filed in Vikunja. They created QA tasks on their own +initiative, which nobody asked them to do. Kami saw them and said some are new +bugs worth a later sweep. Dedupe against existing tasks, and separate genuine +new bugs from restatements of findings the commits already fixed. That produces +the second sweep list without re-reading 35 PRs. + +## Recipes + +Gitea is `https://gitea.kvmx.ru`, repo `kami/Maven`. Tokens are in +`~/.config/tea/config.yml`: the `homesrv` login reads, the `claude` login posts. +`tea` cannot list review comments, so use the REST API. + +Read the reviews for one PR: + + curl -s -H "Authorization: token " \ + "https://gitea.kvmx.ru/api/v1/repos/kami/Maven/pulls//reviews" + +Do not fetch `/issues/comments` without a `since` filter. It returns 50 stale +comments from a superseded stack. + +The scratchpad from this session holds `RECIPE.md` (how the reviews were done), +`FIXBRIEF.md` (how the fixes were done) and `postreview.sh`. Worktrees are under +`scratchpad/wt/g01` to `g11` and can be removed with `git worktree remove` once +the tip has moved. + +## Constraints that do not change + +Read `CLAUDE.md`. Maven is feminine and calls him "ты". The eval enforces both. +Not a nag, not autonomous. No telemetry, no cloud model, no third-party account. +His notes and facts are never search input. Use `make`, never bare `go build` on +a CGO daemon. diff --git a/QA-PLAN.md b/QA-PLAN.md new file mode 100644 index 0000000..cea8969 --- /dev/null +++ b/QA-PLAN.md @@ -0,0 +1,174 @@ +# QA plan: checking Maven properly + +Written 2026-08-01, after the 35-PR stack landed and the box came back up. + +44 of the 50 open Vikunja tasks are `QA:` tasks. They are verification work, not +build work. Most sat unverifiable while Maven was down for 11 days. That +blocker is gone. + +This plan orders them by what unblocks what. Do sessions 1 and 2 first. Almost everything +downstream assumes the voice loop works, and nobody has confirmed that since +the redeploy. + +--- + +## Before you start + +Two things bite anyone running these checks on homesrv. + +**curl needs `--noproxy '*'`.** The shell exports `http_proxy=http://127.0.0.1:18080`. +Without the flag, every local check returns 503 from the proxy and looks like a +dead service. This cost me a false regression report today. + +**The database is not readable with sqlite3.** Four older QA steps say +`docker compose exec mavend sqlite3 /data/maven.db "select ..."`. That cannot +work: the container has no `sqlite3` binary, and the store is AES-256-GCM at +rest with a tmpfs working copy. Read state through mavweb instead, at +`/history`, `/trace`, `/routines` and `/dash`. + +--- + +## Session 1: the voice loop (half a day) + +Nothing here has been confirmed since the redeploy, and everything else assumes +it works. Do this first. + +Closes or advances: **44** (conversation), **45** (text chat), **287** (voice +session quality), **321** steps 3-5 (quiet mode), **288** (STT fixtures). + +1. Open `http://127.0.0.1:9201/chat` and hold a short conversation in Russian. + Watch for three things: she answers in feminine forms (`рада`, `поняла`), she + says `ты` and never `вы`, and no pet names appear. +2. Press push-to-talk on `/dash`. Say `привет`. Confirm a spoken reply comes + back. This is the only check that covers mic to STT to core to TTS to + speaker as one path. It is also the path the eleven-day outage most likely + broke. +3. Say `тихий режим`. Expect `тихий режим включён. буду реже напоминать.` +4. Say `выключи тихий режим`. Expect `тихий режим выключен.` Negation must win. +5. Say `в комнате тихо`. Quiet mode must NOT flip. Confirm on `/history` that no + `quiet_hours` fact was written. +6. Note anything where she is slow, cuts off, or talks over herself. That is + 287's whole content and it has no written acceptance criteria yet. + +**Expect one known failure.** Single-word Russian utterances get turned into +`не совсем поняла — можешь переформулировать?` even when routed correctly. I saw +it today: `привет` routes as `intent=chat` and the gate clarifies it anyway. +That is the single-token rule in `gateLLMDecision`, an English intuition that +does not survive contact with Russian. Tracked in **319**. Do not chase it +during the smoke test. + +--- + +## Session 2: measurement (half a day, mostly waiting) + +Closes or advances: **320** items 2-4, **278** (make the eval lab routine), +**319** (gate recalibration). + +The resident llama-server cannot be reached by the eval harness. It binds +`--host 127.0.0.1 --port 0` inside the container, so the port is kernel-assigned +and never published. Start a second one on a fixed port instead: + +```sh +llama-server -m /mnt/hdd1/llms/qwen3/Qwen3-1.7B-UD-Q4_K_XL.gguf \ + --host 127.0.0.1 --port 18100 -c 4096 -ngl 99 --no-webui +``` + +`-c 4096` matters. The recorded numbers were measured at that context size, and +a mismatch invalidates the comparison. + +Then: + +```sh +make eval-models MAVEN_LLM_URL=http://127.0.0.1:18100 # want ~72.7% cascade +make eval-router # classifier baseline +MAVEN_LLM_URL=http://127.0.0.1:18100 make eval-phrasing # persona checks, slow +make eval-recall +``` + +A large miss against 72.7% means the deploy differs from the bench harness. + +Two things to decide while the numbers are in front of you: + +- **319's gate recalibration.** The single-token rule needs narrowing or + dropping. This needs your judgement, not a threshold sweep. The fixture and the + daemon disagree about what is correct on two of the three false clarifies. +- **278's real ask** is making the eval lab routine rather than building it. It + is built. Decide whether it runs on a timer, on every merge, or on demand, and + the task can close. + +Item 4 of **320** needs a permission I do not have. Kill the `llama-server` +pid under `maven-mavend-1`, post a turn, and confirm it still completes +through the classifier. Either grant it or run it yourself. It is the only +check that the failure floor catches a mid-session model death. + +--- + +## Session 3: the interaction batch (a day, or three sittings) + +These need real use rather than a command, grouped by what one sitting covers. + +**Morning and delivery** (**280**, **281**, **128**, **282**): open `/morning`, +walk the seven required behaviours, then check the four interruption outcomes +and the digest gap. **282** needs the `desk_active` script enabled on the desk +PC first, which is **15** and needs you at that machine. + +**Tasks and calendar** (**129**, **130**, **127**, **126**, **246**): capture a +task by voice, confirm it lands, check prioritisation ordering is not nonsense. +**246** (mail reader) also exercises the `IngestMail` rung that moved to +`AuthWrite` this morning. + +**Routines and patterns** (**43**, **46**, **247**, **254**): these need history +to detect against. If the database is thin after the outage, they may have +nothing to propose, which is not a failure. Check `/routines` before +concluding anything. + +**Ecosystem** (**272**, **273**, **276**): nexus, hexis and praxis are wired and +logged clean at boot. **276** is the degraded-mode suite, which means taking +siblings down on purpose. Worth doing while you are already in there. + +--- + +## Housekeeping (one sitting, no box needed) + +Four QA tasks will not close no matter how long they sit, because they are +gated on something that does not exist: + +- **125** zenmoney: needs a token you have not minted. +- **256** Home Assistant: needs HA configured. +- **257** Bluetooth: BLOCKED, no bluez on the box. Says so in the title. +- **288** STT golden audio: needs fixtures generated. + +Relabel these so they stop reading as backlog. They are not verification work +that is pending, they are work that has not started. + +Same treatment for the five plan-only tasks (**251** MCP, **252** vision, +**253** hearing, **255** speaker recognition, **259** crawler). A `QA:` prefix on +a plan is misleading. + +--- + +## Needs you specifically + +Not QA. These are blocked on a decision or a credential only you have. + +| # | what | +|---|---| +| 16 | Create the Kuma API key. `-kuma-key uk5_mavpoll-key` in `docker-compose.yml` is still the placeholder. | +| 15 | Deploy `desk_active` on the desk PC. Blocks **282**. | +| 122 | Finish the CPT run for Qwen3-1.7B. The persona fix depends on it. | +| 355 | Deploy the Hexis auth change. Was blocked on Maven being under construction, which it no longer is. The client half is vendored and wired. | +| 357 | Decide whether entity-existence validation is the permanent target guard or whether blessing lands in Nexus. | +| 275 | Hexis native API and MCP parity. | +| — | Decide on `-require-stepup`. Making it the default needs WebAuthn configured first, or it locks you out of your own admin surfaces. See **317**. | +| — | Three nginx sites bind wildcard `:80` (`acme.conf`, `matrix`, `panel`), so the ecosystem's bind-level protection is not in effect and `allow`/`deny` is carrying it alone. See **354**. | + +--- + +## Suggested order + +1. Session 1. If the voice loop is broken, nothing else matters. +2. The `-require-stepup` and Kuma decisions. Five minutes, unblocks **317** fully + and **16**. +3. Session 2. The numbers tell you whether the router is worth its 90x latency. +4. Housekeeping. Cheap, and it makes the remaining backlog honest. +5. Session 3, split whichever way suits you. diff --git a/cmd/mavend/actions_query.go b/cmd/mavend/actions_query.go index 77d7609..8da7e97 100644 --- a/cmd/mavend/actions_query.go +++ b/cmd/mavend/actions_query.go @@ -508,7 +508,6 @@ func (h *reactiveHandler) queryWeb(ctx context.Context, t *queryTurn) (string, b // model's own answer than by more waiting. const kiwixTimeout = 20 * time.Second - // queryKiwix — the offline encyclopedia. The last local source: everything of // his has already had its turn and found nothing, so the question is a world // question, and reading beats recalling for a 1.7B. diff --git a/cmd/mavend/main.go b/cmd/mavend/main.go index b74b080..ddebb49 100644 --- a/cmd/mavend/main.go +++ b/cmd/mavend/main.go @@ -751,7 +751,15 @@ func run(args []string) error { if voiceW != nil { voiceW.close() } - wg.Wait() + // Bounded. Every worker below watches ctx, but one parked in a model call + // or an HTTP fetch can outlast the supervisor's patience, and run() has to + // return for `defer st.Close()` to seal the database. A worker abandoned + // mid-tick loses one tick; a shutdown that never returns loses every write + // since the last clean stop — which is how the deployed ciphertext went + // eleven days stale in July 2026. + if !waitWorkers(&wg, workerGrace) { + log.Printf("mavend: workers still running after %s, sealing anyway", workerGrace) + } log.Printf("mavend: bye") return nil } @@ -782,3 +790,23 @@ func contextBlockFn(cfg *config.Config, now func() time.Time) func() string { f := personaFacts(cfg) return func() string { return f.Block(now()) } } + +// workerGrace — how long shutdown waits for the background workers before it +// goes ahead and seals without them. Comfortably inside docker's ten-second +// default so the seal still lands before SIGKILL. +const workerGrace = 4 * time.Second + +// waitWorkers waits on wg for at most d. Reports whether they all finished. +func waitWorkers(wg *sync.WaitGroup, d time.Duration) bool { + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + return true + case <-time.After(d): + return false + } +} diff --git a/cmd/mavseal/main.go b/cmd/mavseal/main.go new file mode 100644 index 0000000..7dbeca2 --- /dev/null +++ b/cmd/mavseal/main.go @@ -0,0 +1,119 @@ +// Command mavseal encrypts a live tmpfs working copy back to the ciphertext +// file, for the case mavend could not do it itself. +// +// mavend seals its database in `defer st.Close()` when run() returns. A daemon +// that is killed rather than shut down never gets there, and because the +// working copy lives in the container's /dev/shm it dies with the container: +// everything written since the last clean shutdown is lost, and the next boot +// silently rolls back to the stale ciphertext. That is not hypothetical — on +// 2026-08-01 the deployed ciphertext was eleven days old. +// +// This is a recovery tool, not part of the daemon. It is safe to run against a +// live database: it takes a consistent snapshot with VACUUM INTO rather than +// mutating the working copy the daemon owns. +// +// Usage: +// +// mavseal -plain /dev/shm/maven-plain.db -cipher /var/lib/maven/maven.db.enc +// +// The key is read from MAVEN_DB_KEY (base64, 32 bytes decoded), the same +// variable the daemon uses. It is never taken as an argument: an argument ends +// up in the shell history and in ps. +package main + +import ( + "context" + "database/sql" + "encoding/base64" + "flag" + "fmt" + "log" + "os" + + "github.com/kami/maven/internal/store" + _ "modernc.org/sqlite" +) + +func main() { + log.SetFlags(0) + if err := run(); err != nil { + log.Fatalf("mavseal: %v", err) + } +} + +func run() error { + plain := flag.String("plain", "", "path to the plaintext working copy (required)") + cipher := flag.String("cipher", "", "path to write the ciphertext to (required)") + keep := flag.Bool("keep-snapshot", false, "leave the intermediate snapshot on disk for inspection") + flag.Parse() + + if *plain == "" || *cipher == "" { + flag.Usage() + return fmt.Errorf("both -plain and -cipher are required") + } + key, err := readKey() + if err != nil { + return err + } + + // VACUUM INTO rather than a WAL checkpoint on the file itself. The daemon + // is usually still running and still writing when this is needed, and + // checkpointing its working copy mutates a database it owns. VACUUM INTO + // reads a consistent snapshot into a new file and touches nothing else, so + // the worst case is a snapshot a few seconds stale instead of a torn one. + snap := *plain + ".mavseal-snapshot" + os.Remove(snap) + if err := snapshot(*plain, snap); err != nil { + return err + } + if !*keep { + defer os.Remove(snap) + } + + before := fileSize(*cipher) + if err := store.SealPlaintext(snap, *cipher, key); err != nil { + return err + } + log.Printf("sealed %s → %s (%d bytes, was %d)", *plain, *cipher, fileSize(*cipher), before) + return nil +} + +// readKey pulls the same base64 key the daemon reads. Fails closed: a short or +// unparseable key must not silently produce a file nothing can open. +func readKey() ([]byte, error) { + raw := os.Getenv("MAVEN_DB_KEY") + if raw == "" { + return nil, fmt.Errorf("MAVEN_DB_KEY is not set") + } + key, err := base64.StdEncoding.DecodeString(raw) + if err != nil { + return nil, fmt.Errorf("MAVEN_DB_KEY is not valid base64: %w", err) + } + if len(key) != 32 { + return nil, fmt.Errorf("MAVEN_DB_KEY decodes to %d bytes, want 32", len(key)) + } + return key, nil +} + +// snapshot writes a consistent copy of src to dst with VACUUM INTO. The copy +// includes everything committed to the write-ahead log, which is most of what +// is worth saving on a daemon that has been up for hours. +func snapshot(src, dst string) error { + db, err := sql.Open("sqlite", src) + if err != nil { + return fmt.Errorf("open working copy: %w", err) + } + defer db.Close() + if _, err := db.ExecContext(context.Background(), "VACUUM INTO ?", dst); err != nil { + return fmt.Errorf("snapshot: %w", err) + } + return nil +} + +func fileSize(path string) int64 { + fi, err := os.Stat(path) + if err != nil { + return 0 + } + return fi.Size() +} diff --git a/internal/ipc/ipc_test.go b/internal/ipc/ipc_test.go index 885d7bd..5ba6654 100644 --- a/internal/ipc/ipc_test.go +++ b/internal/ipc/ipc_test.go @@ -640,3 +640,30 @@ func TestSwapModel_Hook(t *testing.T) { t.Fatalf("swap to a non-allowlisted path = %v; want ErrForbidden", err) } } + +// The eleven-day bug: Close shut the listener but not the accepted conns, so +// an idle client left serveConn parked in readFrame and wg.Wait never +// returned. mavend deadlocked before `defer st.Close()` could re-encrypt the +// database, and every write since the last clean stop was rolled back on the +// next boot. The client here stays connected and idle on purpose. +func TestCloseReturnsWithAnIdleClientConnected(t *testing.T) { + _, srv, cli, _ := newServerWithStore(t) + // Prove the conn is live and then leave it alone — no cli.Close(). + if _, err := cli.RecentFacts(context.Background(), 1); err != nil { + t.Fatalf("warm-up call: %v", err) + } + + done := make(chan error, 1) + go func() { done <- srv.Close() }() + select { + case err := <-done: + if err != nil { + t.Fatalf("close: %v", err) + } + // Deliberately shorter than closeGrace: the grace timer is the backstop, + // not the mechanism. Closing the conns is what makes readFrame return, and + // if that regresses this waits out the full grace and fails here. + case <-time.After(closeGrace / 2): + t.Fatal("Close blocked on an idle connection — the shutdown deadlock is back") + } +} diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 179f697..a7ed978 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "log" "net" "os" "sync" @@ -451,6 +452,19 @@ type Server struct { done chan struct{} accept sync.Mutex // guards wg.Add vs Close's wg.Wait sequence + // 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 + // never say anything again, and the wg.Wait below would block forever. + // + // This was not theoretical. mavweb, mavpoll, mavcaldav and mavmaild all + // hold a long-lived connection open, so on 2026-08-01 mavend deadlocked on + // every single shutdown, never returned from run(), and never reached the + // `defer st.Close()` that seals the database. The deployed ciphertext was + // eleven days stale before anyone noticed. + connMu sync.Mutex + conns map[net.Conn]struct{} + // Check — optional authorization hook. dispatch runs it BEFORE method // dispatch, with the raw params, so the auth layer can make verdicts // that depend on the call's shape (e.g. WriteFact's source). A non-nil @@ -648,8 +662,10 @@ func (s *Server) Serve() error { s.accept.Lock() s.wg.Add(1) s.accept.Unlock() + s.trackConn(c) go func(c net.Conn) { defer s.wg.Done() + defer s.untrackConn(c) defer c.Close() s.serveConn(c) }(c) @@ -1231,18 +1247,88 @@ func (s *Server) Close() error { close(s.done) } 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 + // waits on a peer that has no reason to hang up and the Wait below never + // returns. See the comment on Server.conns. + s.closeConns() // Under accept lock: after the listener closes, no new Accept can complete, // so no new wg.Add will be called. The Wait is safe to observe the wg // counter because any in-flight Accept that already got a conn either // already called wg.Add (before releasing the lock) or will see the closed // listener error and not call wg.Add at all. s.accept.Lock() - s.wg.Wait() + waited := waitTimeout(&s.wg, closeGrace) s.accept.Unlock() + if !waited { + // Bounded on purpose. A dispatch can be mid-call into the resident + // model, which has its own timeout measured in tens of seconds, and the + // caller of Close is on its way to sealing the database with whatever + // grace the supervisor allows. Abandoning one in-flight RPC is cheap; + // missing the seal costs every write since the last clean shutdown. + log.Printf("ipc: %d connection(s) still busy after %s, closing anyway", s.liveConns(), closeGrace) + } _ = os.Remove(s.path) return err } +// closeGrace — how long Close waits for in-flight dispatches to finish before +// giving up on them. Well inside the ten seconds docker allows by default, so +// the caller still has time to seal. +const closeGrace = 3 * time.Second + +func (s *Server) trackConn(c net.Conn) { + s.connMu.Lock() + defer s.connMu.Unlock() + if s.conns == nil { + s.conns = make(map[net.Conn]struct{}) + } + s.conns[c] = struct{}{} +} + +func (s *Server) untrackConn(c net.Conn) { + s.connMu.Lock() + defer s.connMu.Unlock() + delete(s.conns, c) +} + +func (s *Server) liveConns() int { + s.connMu.Lock() + defer s.connMu.Unlock() + return len(s.conns) +} + +// closeConns closes every live connection, which is what unblocks the reads. +// The serveConn goroutines see the resulting error and return. +func (s *Server) closeConns() { + s.connMu.Lock() + live := make([]net.Conn, 0, len(s.conns)) + for c := range s.conns { + live = append(live, c) + } + s.connMu.Unlock() + for _, c := range live { + _ = c.Close() + } +} + +// waitTimeout waits on wg for at most d, reporting whether it finished. The +// abandoned goroutines are still holding a wg count, so nothing may reuse the +// WaitGroup afterwards — Close is terminal, which is what makes this safe. +func waitTimeout(wg *sync.WaitGroup, d time.Duration) bool { + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + return true + case <-time.After(d): + return false + } +} + // Path returns the filesystem path of the listening socket. func (s *Server) Path() string { return s.path } diff --git a/internal/store/crypt.go b/internal/store/crypt.go index 9cb96f0..fceca0f 100644 --- a/internal/store/crypt.go +++ b/internal/store/crypt.go @@ -268,3 +268,34 @@ func zero(b []byte) { b[i] = 0 } } + +// SealPlaintext encrypts an existing plaintext sqlite file at plainPath and +// writes the ciphertext to cipherPath, atomically. key must be 32 bytes. The +// plaintext file is left alone: this is a recovery path, and deleting the only +// good copy of the data on the strength of a write that just succeeded is not +// a trade worth making here. +// +// It exists for the case closeAndSeal cannot cover: a daemon that was killed +// rather than shut down, leaving a live working copy in tmpfs and a stale +// ciphertext on disk. mavseal folds the WAL in first, so what arrives here is +// a single complete database. +// +// Nothing else should call this. The normal path is Close, which seals and +// then wipes the plaintext and the key. +func SealPlaintext(plainPath, cipherPath string, key []byte) error { + if len(key) != keyLen { + return ErrKeyLen + } + plain, err := os.ReadFile(plainPath) + if err != nil { + return fmt.Errorf("read working copy: %w", err) + } + blob, err := encrypt(key, plain) + if err != nil { + return fmt.Errorf("encrypt: %w", err) + } + if err := atomicWrite(cipherPath, blob); err != nil { + return fmt.Errorf("seal ciphertext: %w", err) + } + return nil +} diff --git a/internal/voice/server.go b/internal/voice/server.go index 8efd937..27c7790 100644 --- a/internal/voice/server.go +++ b/internal/voice/server.go @@ -57,8 +57,20 @@ type Server struct { ln net.Listener wg sync.WaitGroup done chan struct{} + + // Accepted conns, tracked so Close can shut them. Same defect as + // ipc/server.go had: closing only the listener leaves every idle client + // parked in readFrame, wg.Wait never returns, and the daemon dies to + // SIGKILL without sealing the database. + connMu sync.Mutex + conns map[net.Conn]struct{} } +// closeGrace — how long Close waits for in-flight dispatches before dropping +// them. A push-to-talk turn can be mid-inference; abandoning one costs a reply, +// hanging costs every write since the last clean shutdown. +const closeGrace = 3 * time.Second + // NewServer builds a Server bound to addr (e.g. "127.0.0.1:9100" for a // local-only smoke; production: a wg-tunnel address). handler is the // reactive handler; sessions is shared with the voicesink (the daemon @@ -111,8 +123,10 @@ func (s *Server) Serve() error { } } s.wg.Add(1) + s.trackConn(c) go func(c net.Conn) { defer s.wg.Done() + defer s.untrackConn(c) s.serveConn(c) }(c) } @@ -212,10 +226,65 @@ func (s *Server) Close() error { if s.ln != nil { err = s.ln.Close() } - s.wg.Wait() + // Close the accepted conns too, or a client that is merely idle keeps + // serveConn blocked in readFrame forever. + s.closeConns() + if !waitTimeout(&s.wg, closeGrace) { + log.Printf("voice: %d connection(s) still busy after %s, closing anyway", s.liveConns(), closeGrace) + } return err } +func (s *Server) trackConn(c net.Conn) { + s.connMu.Lock() + defer s.connMu.Unlock() + if s.conns == nil { + s.conns = make(map[net.Conn]struct{}) + } + s.conns[c] = struct{}{} +} + +func (s *Server) untrackConn(c net.Conn) { + s.connMu.Lock() + defer s.connMu.Unlock() + delete(s.conns, c) +} + +func (s *Server) liveConns() int { + s.connMu.Lock() + defer s.connMu.Unlock() + return len(s.conns) +} + +// closeConns unblocks every parked reader. serveConn's own defer closes the +// conn again; a second Close on a net.Conn is a harmless error. +func (s *Server) closeConns() { + s.connMu.Lock() + conns := make([]net.Conn, 0, len(s.conns)) + for c := range s.conns { + conns = append(conns, c) + } + s.connMu.Unlock() + for _, c := range conns { + _ = c.Close() + } +} + +// waitTimeout waits on wg, but not forever. Reports whether it finished. +func waitTimeout(wg *sync.WaitGroup, d time.Duration) bool { + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + return true + case <-time.After(d): + return false + } +} + func unmarshalParams(raw json.RawMessage, v any) error { if len(raw) == 0 { raw = []byte("null")