phraser: the startup wait is a config field, and it is tested (V-323)

The last untested arm of startLlamaProc was `case <-time.After(60 *
time.Second)`, and it could not be tested as written — a test would have
had to wait a real minute. It is Config.StartupTimeout now, defaulted to
60s in DefaultConfig and surfaced as phraser.startup_timeout, so the box
can raise it: a cold 1.7B loading off a spinning disk can outrun 60s and
that failed the boot with nothing to turn.

The test asserts the child is dead, not just that the error came back.
That arm is the one most likely to leak, because the child is alive and
busy loading a model rather than already gone — so the fake records its
pid before it hangs.

startLlamaProc 90.9% → 95.7%, package 76.9% → 77.3%.

cmd/mavend/main.go

@@ -246,14 +246,15 @@ func run(args []string) error {
 		phr = phraser.NewStub()
 		if cfg.Phraser != nil {
 			pc := phraser.Config{
-				ModelPath:    cfg.Phraser.ModelPath,
-				BinPath:      cfg.Phraser.BinPath,
-				Listen:       cfg.Phraser.Listen,
-				NGpuLayers:   cfg.Phraser.NGpuLayers,
-				NCtx:         cfg.Phraser.NCtx,
-				Timeout:      time.Duration(cfg.Phraser.Timeout),
-				LLMNudges:    cfg.Phraser.LLMNudges,
-				ContextBlock: contextBlockFn(cfg, time.Now),
+				ModelPath:      cfg.Phraser.ModelPath,
+				BinPath:        cfg.Phraser.BinPath,
+				Listen:         cfg.Phraser.Listen,
+				NGpuLayers:     cfg.Phraser.NGpuLayers,
+				NCtx:           cfg.Phraser.NCtx,
+				Timeout:        time.Duration(cfg.Phraser.Timeout),
+				StartupTimeout: time.Duration(cfg.Phraser.StartupTimeout),
+				LLMNudges:      cfg.Phraser.LLMNudges,
+				ContextBlock:   contextBlockFn(cfg, time.Now),
 			}
 			if pc.BinPath == "" {
 				pc.BinPath = "llama-server"

internal/config/config.go

@@ -1233,6 +1233,13 @@ type PhraserConfig struct {
 	NCtx       int      `json:"n_ctx,omitempty"`
 	Timeout    Duration `json:"timeout,omitempty"`
 
+	// StartupTimeout — how long the daemon waits for llama-server to print its
+	// listen line at boot. Zero ⇒ the phraser's 60s default.
+	//
+	// It is here because a cold 1.7B loading off a spinning disk can outrun 60s,
+	// and that failed the boot with no way to raise it (Vikunja #323).
+	StartupTimeout Duration `json:"startup_timeout,omitempty"`
+
 	// LLMNudges — let the model word nudges again. Off by default: nudges are
 	// worded from hand-written Russian templates now (the model broke the
 	// persona and invented units). Chat, query and reminder phrasing always go

internal/phraser/llmphraser.go

@@ -101,8 +101,21 @@ type Config struct {
 	// ever fights the grammar, the fix should be a config flip on the
 	// deploy box, not a code change and a rebuild.
 	NoGrammar bool
+
+	// StartupTimeout — how long to wait for llama-server to print its listen
+	// line before giving up, killing the child and returning an error.
+	//
+	// A field rather than a constant because the box may legitimately want
+	// longer: a cold 1.7B loading off a spinning disk can outrun the 60s
+	// default, and that used to fail the boot with no way to raise it
+	// (Vikunja #323). Zero ⇒ defaultStartupTimeout.
+	StartupTimeout time.Duration
 }
 
+// defaultStartupTimeout — the wait DefaultConfig sets and the zero value falls
+// back to, so a Config built by hand still has a ceiling.
+const defaultStartupTimeout = 60 * time.Second
+
 func DefaultConfig(modelPath string) Config {
 	return Config{
 		ModelPath:  modelPath,
@@ -111,6 +124,8 @@ func DefaultConfig(modelPath string) Config {
 		NGpuLayers: -1,
 		NCtx:       2048,
 		Timeout:    30 * time.Second,
+
+		StartupTimeout: defaultStartupTimeout,
 	}
 }
 
@@ -278,6 +293,10 @@ func startLlamaProc(ctx context.Context, cfg Config) (*llamaProc, error) {
 		}
 	}()
 
+	startupTimeout := cfg.StartupTimeout
+	if startupTimeout <= 0 {
+		startupTimeout = defaultStartupTimeout
+	}
 	select {
 	case addr := <-portCh:
 		p.base = addr
@@ -290,10 +309,12 @@ func startLlamaProc(ctx context.Context, cfg Config) (*llamaProc, error) {
 		_ = cmd.Process.Kill()
 		_ = cmd.Wait()
 		return nil, ctx.Err()
-	case <-time.After(60 * time.Second):
+	case <-time.After(startupTimeout):
+		// The arm most likely to leak: the child is still loading a model, so
+		// it is alive and busy rather than dead. Kill and reap before the error.
 		_ = cmd.Process.Kill()
 		_ = cmd.Wait()
-		return nil, fmt.Errorf("llm: server did not start within 60s")
+		return nil, fmt.Errorf("llm: server did not start within %s", startupTimeout)
 	}
 }
 

internal/phraser/spawn_test.go

@@ -0,0 +1,285 @@
+package phraser
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"regexp"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+)
+
+// The spawn path (NewLLMPhraser, spawnLlamaServer, startLlamaProc, llamaProc.Close)
+// was at 0% coverage: every test built the phraser with NewLLMPhraserAt, which
+// starts no process. These tests drive the real spawn code against a fake
+// llama-server script, so the startup race arms and the reaping are exercised
+// without a model or a GPU.
+
+// fakeLlama writes an executable script standing in for llama-server and returns
+// its path. body runs after the script has recorded its own pid.
+func fakeLlama(t *testing.T, body string) string {
+	t.Helper()
+	dir := t.TempDir()
+	path := filepath.Join(dir, "fake-llama-server")
+	script := "#!/bin/sh\n" + body + "\n"
+	if err := os.WriteFile(path, []byte(script), 0o755); err != nil {
+		t.Fatalf("write fake server: %v", err)
+	}
+	return path
+}
+
+// listensThenSleeps prints the line startLlamaProc scrapes, then stays alive
+// until killed — the shape of a real llama-server that came up.
+const listensThenSleeps = `echo "srv    load_model: listening on http://127.0.0.1:18081" >&2
+while : ; do sleep 1 ; done`
+
+func testCfg(bin string) Config {
+	cfg := DefaultConfig("/nonexistent/model.gguf")
+	cfg.BinPath = bin
+	return cfg
+}
+
+func TestExtractPort(t *testing.T) {
+	for _, tc := range []struct{ in, want string }{
+		{"127.0.0.1:0", "0"},
+		{"127.0.0.1:8080", "8080"},
+		{"127.0.0.1:", "0"},
+		{"", "0"},
+		{"8080", "0"}, // no colon: Cut yields no port, so the caller gets the "any port" default
+	} {
+		if got := extractPort(tc.in); got != tc.want {
+			t.Errorf("extractPort(%q) = %q, want %q", tc.in, got, tc.want)
+		}
+	}
+}
+
+func TestStartLlamaProcScrapesPortAndReaps(t *testing.T) {
+	bin := fakeLlama(t, listensThenSleeps)
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	p, err := startLlamaProc(ctx, testCfg(bin))
+	if err != nil {
+		t.Fatalf("startLlamaProc: %v", err)
+	}
+	if p.BaseURL() != "http://127.0.0.1:18081" {
+		t.Fatalf("BaseURL = %q, want the scraped address", p.BaseURL())
+	}
+	pid := p.cmd.Process.Pid
+
+	p.cancel = cancel
+	if err := p.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+	// Close must Wait, otherwise the child lingers as a zombie.
+	if p.cmd.ProcessState == nil {
+		t.Fatal("Close did not reap the child: ProcessState is nil")
+	}
+	if err := syscall.Kill(pid, 0); err == nil {
+		t.Fatalf("child %d still exists after Close", pid)
+	}
+}
+
+func TestStartLlamaProcFailureArms(t *testing.T) {
+	t.Run("binary missing", func(t *testing.T) {
+		cfg := testCfg(filepath.Join(t.TempDir(), "does-not-exist"))
+		_, err := startLlamaProc(context.Background(), cfg)
+		if err == nil || !strings.Contains(err.Error(), "llm: start") {
+			t.Fatalf("err = %v, want a start failure", err)
+		}
+	})
+
+	t.Run("server exits without listening", func(t *testing.T) {
+		// stderr closes, so the reader goroutine reports EOF on errCh.
+		bin := fakeLlama(t, `echo "ggml_vulkan: no device" >&2
+exit 1`)
+		_, err := startLlamaProc(context.Background(), testCfg(bin))
+		if err == nil || !strings.Contains(err.Error(), "llm: server output") {
+			t.Fatalf("err = %v, want the server-output arm", err)
+		}
+	})
+
+	t.Run("startup timeout", func(t *testing.T) {
+		// The 60s wait was a literal in the select, so this arm could only be
+		// tested by waiting a real minute (Vikunja #323). It is Config now.
+		//
+		// The child records its pid, because startLlamaProc returns nil on
+		// every failure arm and this is the arm where the child is alive and
+		// busy loading a model rather than already dead.
+		pidFile := filepath.Join(t.TempDir(), "pid")
+		bin := fakeLlama(t, `echo $$ > `+pidFile+`
+while : ; do sleep 1 ; done`)
+		cfg := testCfg(bin)
+		cfg.StartupTimeout = 50 * time.Millisecond
+		_, err := startLlamaProc(context.Background(), cfg)
+		if err == nil || !strings.Contains(err.Error(), "did not start within") {
+			t.Fatalf("err = %v, want the startup-timeout arm", err)
+		}
+		raw, readErr := os.ReadFile(pidFile)
+		if readErr != nil {
+			t.Fatalf("the child never ran: %v", readErr)
+		}
+		pid, convErr := strconv.Atoi(strings.TrimSpace(string(raw)))
+		if convErr != nil {
+			t.Fatalf("pid file %q: %v", raw, convErr)
+		}
+		if err := syscall.Kill(pid, 0); err == nil {
+			t.Errorf("child %d survived the startup timeout", pid)
+		}
+	})
+
+	t.Run("context cancelled during startup", func(t *testing.T) {
+		// Never prints the listen line and never exits: only ctx can end this.
+		bin := fakeLlama(t, `while : ; do sleep 1 ; done`)
+		ctx, cancel := context.WithCancel(context.Background())
+		go func() {
+			time.Sleep(150 * time.Millisecond)
+			cancel()
+		}()
+		defer cancel()
+		_, err := startLlamaProc(ctx, testCfg(bin))
+		if !errors.Is(err, context.Canceled) {
+			t.Fatalf("err = %v, want context.Canceled", err)
+		}
+	})
+}
+
+func TestNewLLMPhraserSpawns(t *testing.T) {
+	bin := fakeLlama(t, listensThenSleeps)
+	p, err := NewLLMPhraser(context.Background(), testCfg(bin))
+	if err != nil {
+		t.Fatalf("NewLLMPhraser: %v", err)
+	}
+	if p.BaseURL() != "http://127.0.0.1:18081" {
+		t.Fatalf("BaseURL = %q", p.BaseURL())
+	}
+	pid := p.be.(*llamaProc).cmd.Process.Pid
+	if err := p.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+	if p.BaseURL() != "" {
+		t.Fatalf("BaseURL after Close = %q, want empty", p.BaseURL())
+	}
+	if err := syscall.Kill(pid, 0); err == nil {
+		t.Fatalf("llama-server %d survived Close", pid)
+	}
+}
+
+func TestNewLLMPhraserSpawnFailure(t *testing.T) {
+	cfg := testCfg(filepath.Join(t.TempDir(), "does-not-exist"))
+	p, err := NewLLMPhraser(context.Background(), cfg)
+	if err == nil {
+		p.Close()
+		t.Fatal("want an error when the server cannot start")
+	}
+	if p != nil {
+		t.Fatalf("want a nil phraser on failure, got %#v", p)
+	}
+}
+
+// TestPdeathsigKillsOrphan is the orphan test the task asked for. A SIGKILLed
+// mavend never runs Close, so nothing but the kernel's Pdeathsig can stop its
+// llama-server. Re-exec this test binary as the "daemon", let it spawn the fake
+// server, SIGKILL the daemon, and assert the grandchild died with it.
+func TestPdeathsigKillsOrphan(t *testing.T) {
+	bin := fakeLlama(t, listensThenSleeps)
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestSpawnHelperProcess", "-test.v=false")
+	cmd.Env = append(os.Environ(), "MAVEN_SPAWN_HELPER=1", "MAVEN_FAKE_LLAMA="+bin)
+	out, err := cmd.StdoutPipe()
+	if err != nil {
+		t.Fatalf("stdout pipe: %v", err)
+	}
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start helper: %v", err)
+	}
+	defer func() { _ = cmd.Process.Kill(); _ = cmd.Wait() }()
+
+	buf := make([]byte, 256)
+	n, err := out.Read(buf)
+	if err != nil {
+		t.Fatalf("read child pid: %v", err)
+	}
+	childPID, err := strconv.Atoi(strings.TrimSpace(string(buf[:n])))
+	if err != nil {
+		t.Fatalf("helper printed %q, want a pid: %v", string(buf[:n]), err)
+	}
+	if err := syscall.Kill(childPID, 0); err != nil {
+		t.Fatalf("llama-server %d not running before the kill: %v", childPID, err)
+	}
+
+	// SIGKILL: the helper gets no chance to clean up, exactly like an OOM kill.
+	if err := cmd.Process.Signal(syscall.SIGKILL); err != nil {
+		t.Fatalf("kill helper: %v", err)
+	}
+	_, _ = cmd.Process.Wait()
+
+	deadline := time.Now().Add(5 * time.Second)
+	for time.Now().Before(deadline) {
+		if err := syscall.Kill(childPID, 0); err != nil {
+			return // gone: Pdeathsig did its job
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+	_ = syscall.Kill(childPID, syscall.SIGKILL)
+	t.Fatalf("llama-server %d outlived the SIGKILLed parent", childPID)
+}
+
+// TestSpawnHelperProcess is not a test. It is the child half of
+// TestPdeathsigKillsOrphan: spawn a llama-server, print its pid, then block.
+func TestSpawnHelperProcess(t *testing.T) {
+	if os.Getenv("MAVEN_SPAWN_HELPER") != "1" {
+		t.Skip("helper for TestPdeathsigKillsOrphan")
+	}
+	cfg := testCfg(os.Getenv("MAVEN_FAKE_LLAMA"))
+	p, err := startLlamaProc(context.Background(), cfg)
+	if err != nil {
+		fmt.Println("spawn failed:", err)
+		os.Exit(1)
+	}
+	fmt.Println(p.cmd.Process.Pid)
+	os.Stdout.Sync()
+	select {} // wait to be killed
+}
+
+// TestKillMavenScriptMatchesRealCommandLine pins kill-maven.sh's fallback
+// pattern to the command line startLlamaProc actually builds. The script leaked
+// orphans twice already, both times because the pattern stopped matching: first
+// `llama-server.*maven`, then a hardcoded model name after the model was swapped.
+func TestKillMavenScriptMatchesRealCommandLine(t *testing.T) {
+	src, err := os.ReadFile("../../kill-maven.sh")
+	if err != nil {
+		t.Fatalf("read kill-maven.sh: %v", err)
+	}
+	m := regexp.MustCompile(`(?m)^\s*LLM='([^']+)'`).FindSubmatch(src)
+	if m == nil {
+		t.Fatal("no default LLM='...' pattern in kill-maven.sh")
+	}
+	pat, err := regexp.Compile(string(m[1]))
+	if err != nil {
+		t.Fatalf("LLM pattern %q does not compile: %v", m[1], err)
+	}
+
+	// Rebuild the command line from the production arg list, so a change to
+	// startLlamaProc that breaks the sweep fails here instead of on the box.
+	cfg := DefaultConfig("/opt/maven/models/llm/Qwen3-1.7B-UD-Q4_K_XL.gguf")
+	cfg.NCtx, cfg.NGpuLayers = 4096, 99
+	cmdline := strings.Join([]string{
+		cfg.BinPath,
+		"-m", cfg.ModelPath,
+		"--host", "127.0.0.1",
+		"--port", extractPort(cfg.Listen),
+		"-c", fmt.Sprintf("%d", cfg.NCtx),
+		"-ngl", fmt.Sprintf("%d", cfg.NGpuLayers),
+		"--no-webui",
+	}, " ")
+	if !pat.MatchString(cmdline) {
+		t.Fatalf("kill-maven.sh pattern %q does not match %q — orphans would leak", m[1], cmdline)
+	}
+}