From eda1112f3ba777a5ed7ca394553bf909861e95eb Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 02:01:17 +0400 Subject: [PATCH 01/14] mavgpud: a yield stops writing a core and reads as a yield (V-491) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit llama-server aborts inside its own static teardown on SIGTERM — the handler calls exit(), stream_session_manager's destructor throws, and the process dies "signal: aborted (core dumped)". mavgpud sends that signal on every eviction, so a routine yield wrote a multi-gigabyte core into systemd-coredump and logged the same line a real crash would. LimitCORE=0 in the unit stops the disk cost. A yielding flag, set by stop and cleared by start, makes the log distinguish the two: only an exit we did not ask for is still reported as an exit. Not filed upstream. Searched ggml-org/llama.cpp for "ggml_uncaught_exception" with SIGTERM and for stream_session_manager and found nothing matching, so the issue still wants writing — by someone with an account on that tracker, which is why it is not in this commit. --- cmd/mavgpud/runner.go | 20 +++++++++++-- cmd/mavgpud/runner_test.go | 59 ++++++++++++++++++++++++++++++++++++++ deploy/mavgpud.service | 4 +++ 3 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 cmd/mavgpud/runner_test.go diff --git a/cmd/mavgpud/runner.go b/cmd/mavgpud/runner.go index 39ce341..83a68b5 100644 --- a/cmd/mavgpud/runner.go +++ b/cmd/mavgpud/runner.go @@ -24,7 +24,12 @@ type runner struct { mu sync.Mutex cmd *exec.Cmd ready bool - http *http.Client + // yielding — stop() has sent the signal and the exit that follows is ours. + // llama-server aborts on SIGTERM (its static teardown throws, upstream + // ggml-org/llama.cpp), so a routine yield and a real crash produce the same + // "signal: aborted" and used to log identically (Vikunja #491). + yielding bool + http *http.Client } func newRunner(bin string, args []string, readyURL string) *runner { @@ -70,13 +75,18 @@ func (r *runner) start() error { if err := cmd.Start(); err != nil { return err } - r.cmd, r.ready = cmd, false + r.cmd, r.ready, r.yielding = cmd, false, false log.Printf("mavgpud: started llama-server pid=%d", cmd.Process.Pid) go func() { err := cmd.Wait() r.mu.Lock() - r.cmd, r.ready = nil, false + yielded := r.yielding + r.cmd, r.ready, r.yielding = nil, false, false r.mu.Unlock() + if yielded { + log.Printf("mavgpud: llama-server stopped, card yielded (%v)", err) + return + } log.Printf("mavgpud: llama-server exited: %v", err) }() return nil @@ -90,6 +100,10 @@ func (r *runner) stop(grace time.Duration) { r.mu.Lock() cmd := r.cmd r.ready = false + if cmd != nil && cmd.Process != nil { + // The exit that follows is ours, not a crash. + r.yielding = true + } r.mu.Unlock() if cmd == nil || cmd.Process == nil { return diff --git a/cmd/mavgpud/runner_test.go b/cmd/mavgpud/runner_test.go new file mode 100644 index 0000000..e9cae0d --- /dev/null +++ b/cmd/mavgpud/runner_test.go @@ -0,0 +1,59 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// fakeServer writes an executable standing in for llama-server: it ignores +// SIGTERM the way the real one effectively does — by dying messily rather than +// cleanly — and reports a non-zero status. +func fakeServer(t *testing.T, body string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "fake-llama-server") + if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body+"\n"), 0o755); err != nil { + t.Fatal(err) + } + return path +} + +// A deliberate stop is a yield, and the log has to say so. +// +// llama-server aborts inside its own static teardown on SIGTERM, so the exit +// status of a routine yield is identical to that of a real crash. Reading the +// mavgpud log, the two were indistinguishable (Vikunja #491). +func TestStopMarksTheExitAsAYield(t *testing.T) { + r := newRunner(fakeServer(t, "while : ; do sleep 1 ; done"), nil, "") + if err := r.start(); err != nil { + t.Fatalf("start: %v", err) + } + r.mu.Lock() + if r.yielding { + t.Error("a freshly started server is already marked as yielding") + } + r.mu.Unlock() + + r.stop(2 * time.Second) + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if !r.running() { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("the child outlived stop") +} + +// Stopping when nothing is running must not arm the flag for the next child. +// The next exit after that would be a real crash logged as a yield. +func TestStopWithNoChildDoesNotArmTheFlag(t *testing.T) { + r := newRunner("/nonexistent", nil, "") + r.stop(10 * time.Millisecond) + r.mu.Lock() + defer r.mu.Unlock() + if r.yielding { + t.Error("stop armed the yield flag with no child running") + } +} diff --git a/deploy/mavgpud.service b/deploy/mavgpud.service index b200baa..dc0d6f2 100644 --- a/deploy/mavgpud.service +++ b/deploy/mavgpud.service @@ -19,6 +19,10 @@ RestartSec=5 # llama-server on SIGTERM, so give it longer than stop_grace to do that. KillSignal=SIGTERM TimeoutStopSec=60 +# llama-server aborts inside its own static teardown on SIGTERM, so every +# routine yield used to write a multi-gigabyte core into systemd-coredump +# (Vikunja #491). Yielding is meant to happen several times a day. +LimitCORE=0 [Install] WantedBy=default.target From 6d3f5b5b01dc1b3cbda4dc749f1555acafd37963 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 02:49:02 +0400 Subject: [PATCH 02/14] router: a reminder with no subject asks instead of guessing (V-383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slots.Text was the raw utterance for every intent, so a reminder could not have an empty subject. StillMissing never reported SlotText, the question "О чём напомнить?" was unaskable, and the branch in PendingQuestion.Answer that fills a text slot could only overwrite the whole request. The LLM path now keeps the model's own text, empty included, and the gate turns a subjectless reminder into a question. The classifier path is unchanged: it has no subject parser, so the utterance is the only signal it has. --- cmd/mavend/clarify_test.go | 36 +++++++++++++++++++++++++++++++ internal/router/llmrouter.go | 6 +++++- internal/router/llmrouter_test.go | 32 +++++++++++++++++++++++++++ internal/router/router.go | 16 +++++++++++++- 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index e07298a..8b377e1 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -465,3 +465,39 @@ func TestExpiryNoticeSurvivesAConfirmTurn(t *testing.T) { t.Fatal("the expired question must be gone") } } + +// The other half of the subject question: his answer must fill the empty slot, +// not replace the request. Slots.Text used to be the whole raw utterance for +// every intent, so the branch that fills a text slot could only ever overwrite +// (Vikunja #383). Here the parked request holds the hour and the answer holds +// what to say at it, and the reminder that lands has both. +func TestClarifySubjectAnswerFillsRatherThanClobbers(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + at := h.now().Add(2 * time.Hour) + + question, asked := h.askClarify(clarifyDec(router.IntentReminder, + router.Slots{Time: at, HasTime: true}, "напомни в 11")) + if !asked || question != "О чём напомнить?" { + t.Fatalf("expected the subject question, got %q asked=%v", question, asked) + } + + reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме") + if !handled { + t.Fatal("the answer to an open question must be consumed as an answer") + } + if reply == clarifyGaveUp { + t.Fatalf("a good answer must not drop the request: %q", reply) + } + + reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)) + if err != nil || len(reminders) != 1 { + t.Fatalf("clarified reminder was not created: reminders=%v err=%v", reminders, err) + } + if !strings.Contains(reminders[0].Payload, "маме") { + t.Fatalf("the answer never reached the reminder: %q", reminders[0].Payload) + } + if !strings.Contains(reminders[0].Payload, "11") { + t.Fatalf("the answer clobbered the original request: %q", reminders[0].Payload) + } +} diff --git a/internal/router/llmrouter.go b/internal/router/llmrouter.go index 5b88593..341193b 100644 --- a/internal/router/llmrouter.go +++ b/internal/router/llmrouter.go @@ -210,7 +210,11 @@ func (lr *LLMRouter) Route(ctx context.Context, utterance string, now time.Time) d.Slots.HasKey = a.Key != "" case IntentReminder: d.Intent = IntentReminder - d.Slots.Text = firstNonEmpty(a.Text, utterance) + // No utterance fallback here, unlike every other intent below. The + // model returning no text for a reminder means it found no subject, + // and "напомни в 11" is not a subject. Leaving Text empty is what + // lets the gate turn that into a question (Vikunja #383). + d.Slots.Text = a.Text case IntentNote: d.Intent = IntentNote d.Slots.Text = firstNonEmpty(a.Text, utterance) diff --git a/internal/router/llmrouter_test.go b/internal/router/llmrouter_test.go index 49279c1..f82e016 100644 --- a/internal/router/llmrouter_test.go +++ b/internal/router/llmrouter_test.go @@ -356,3 +356,35 @@ func TestRouterLLMFactWithResolvedKeyStaysConfident(t *testing.T) { t.Fatalf("a fact the parser could key must not clarify: %+v", d) } } + +// A reminder with a time and no subject must come back empty and gated, not +// backfilled with the raw words. "напомни в 11" carries an hour and nothing to +// say at that hour; parking the utterance in Text made the request look +// complete, so the daemon set a reminder that fires saying "напомни в 11" +// (Vikunja #383). +func TestLLMReminderWithoutSubjectAsksInsteadOfGuessing(t *testing.T) { + r := newLLMTestRouter(t, `{"intent":"reminder"}`) + d, err := r.Route(context.Background(), "напомни в 11", refNow()) + if err != nil { + t.Fatalf("route: %v", err) + } + if d.Slots.Text != "" { + t.Fatalf("subject backfilled from the utterance: %q", d.Slots.Text) + } + if !d.Clarify { + t.Fatalf("a subjectless reminder was accepted, confidence %v", d.Confidence) + } +} + +// The gate is about the subject, not about reminders in general: one that has +// both halves still runs without a question. +func TestLLMReminderWithSubjectIsNotGated(t *testing.T) { + r := newLLMTestRouter(t, `{"intent":"reminder","text":"позвонить маме"}`) + d, err := r.Route(context.Background(), "напомни в 11 позвонить маме", refNow()) + if err != nil { + t.Fatalf("route: %v", err) + } + if d.Clarify { + t.Fatalf("a complete reminder was sent back as a question: %+v", d.Slots) + } +} diff --git a/internal/router/router.go b/internal/router/router.go index bf9b430..1df278c 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -147,7 +147,15 @@ func (r *Router) fillSlots(ctx context.Context, d *Decision, now time.Time) { d.Slots.Fn, d.Slots.Args, d.Slots.HasFn = fn, args, true } } - if d.Slots.Text == "" { + // The extractor's Text is the raw utterance, which is the payload for a + // note, a query or a chat turn but not for a reminder — there Text is the + // subject, what she says at the hour. Backfilling it made Text impossible + // to be empty, so StillMissing never reported SlotText and "О чём + // напомнить?" was unaskable; the answer to a question she did manage to + // ask then overwrote the whole request instead of filling one gap + // (Vikunja #383). A reminder with no subject stays empty and is gated + // below into a question. + if d.Slots.Text == "" && d.Intent != IntentReminder { d.Slots.Text = ex.Text } // Stage stays 1: it says who decided the route, and that was the LLM. @@ -177,6 +185,12 @@ func (r *Router) gateLLMDecision(d *Decision) { if d.Intent == IntentAct && !d.Slots.HasFn && d.Confidence > llmThinConfidence { d.Confidence = llmThinConfidence } + // A reminder with no subject: she knows when but not what to say then. + // Setting it anyway fires an empty reminder at the hour, which reads as a + // bug to him and cannot be repaired after the fact. Ask (Vikunja #383). + if d.Intent == IntentReminder && d.Slots.Text == "" && d.Confidence > llmThinConfidence { + d.Confidence = llmThinConfidence + } if d.Confidence < r.threshold { d.Clarify = true } From 43f2c37538f5c8002d43f8df8d8bbb12bbbd8558 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 02:52:51 +0400 Subject: [PATCH 03/14] router: stage 0 claims the other days and the named event (V-471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "какие планы на сегодня" worked and "какие планы на завтра" answered "пока не умею": the agenda rule needs "у меня" or a calendar noun, and that phrasing carries neither. "когда планёрка?" had the same shape. Two rules. One takes a plan noun aimed at a named day, one takes a closed list of event nouns after "когда"/"во сколько". Both route intent only, so the query chain still decides which source answers. classifier+onnx over the fixture: 55/79, 69.6% full, with the two new cases passing and no case moving the other way. --- internal/router/agenda_test.go | 44 +++++++++++++++++++++++++ internal/router/eval/ru_routing_v1.json | 2 ++ internal/router/stage0.go | 29 ++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/internal/router/agenda_test.go b/internal/router/agenda_test.go index 4ef3407..9789c90 100644 --- a/internal/router/agenda_test.go +++ b/internal/router/agenda_test.go @@ -75,3 +75,47 @@ func TestAgendaGrammarSparesStatements(t *testing.T) { } } } + +// The tomorrow form and the bare event noun. Both were measured answering +// "пока не умею" on the deployed daemon, 02-08-2026, while the same question +// about today worked — the first rule set needed "у меня" or a calendar noun +// and these phrasings carry neither (Vikunja #471). +func TestAgendaCoversOtherDaysAndNamedEvents(t *testing.T) { + r := agendaRouter(t) + for _, u := range []string{ + "какие планы на завтра?", + "какие планы на послезавтра", + "что по делам в среду", + "какие планы на выходные", + "когда планёрка?", + "во сколько созвон", + "когда будет совещание", + } { + d, err := r.Route(context.Background(), u, refNow()) + if err != nil { + t.Fatalf("route(%q): %v", u, err) + } + if d.Intent != IntentQuery { + t.Errorf("route(%q) = %s, want query", u, d.Intent) + } + } +} + +// The two new rules are narrow on purpose. A world question that opens with +// "когда" is not an agenda question, and telling her about a plan is not +// asking about one. +func TestAgendaGrammarsLeaveTheWorldAlone(t *testing.T) { + r := agendaRouter(t) + for _, u := range []string{ + "когда была битва при ватерлоо", + "когда изобрели телефон", + } { + d, err := r.Route(context.Background(), u, refNow()) + if err != nil { + t.Fatalf("route(%q): %v", u, err) + } + if d.Stage == 0 { + t.Errorf("route(%q) was claimed at stage 0 as %s", u, d.Intent) + } + } +} diff --git a/internal/router/eval/ru_routing_v1.json b/internal/router/eval/ru_routing_v1.json index 6c765f0..2f6004e 100644 --- a/internal/router/eval/ru_routing_v1.json +++ b/internal/router/eval/ru_routing_v1.json @@ -23,6 +23,8 @@ { "id": "ru-query-012", "utterance": "какие заметки я оставил про полив", "lang": "ru", "intent": "query", "tags": ["recall"] }, { "id": "ru-query-013", "utterance": "во сколько у меня встреча", "lang": "ru", "intent": "query", "tags": ["calendar"] }, { "id": "ru-query-019", "utterance": "что у меня стоит в календаре на послезавтра", "lang": "ru", "intent": "query", "tags": ["calendar", "hard"], "note": "agenda, not the clock: the daemon answers this from CalendarEvents inside the query branch, so the clock/date system rule must not swallow it" }, + { "id": "ru-query-022", "utterance": "какие планы на завтра?", "lang": "ru", "intent": "query", "tags": ["calendar"], "note": "the same agenda question as ru-query-019 aimed at another day; it answered \u043f\u043e\u043a\u0430 \u043d\u0435 \u0443\u043c\u0435\u044e on the deployed daemon while the today form worked (Vikunja #471)" }, + { "id": "ru-query-023", "utterance": "\u043a\u043e\u0433\u0434\u0430 \u043f\u043b\u0430\u043d\u0451\u0440\u043a\u0430?", "lang": "ru", "intent": "query", "tags": ["calendar", "hard"], "note": "a named event with no calendar word — the noun is the only signal that this is a question about his day" }, { "id": "ru-query-014", "utterance": "я успеваю до дедлайна", "lang": "ru", "intent": "query", "tags": ["hard", "no-question-word"] }, { "id": "ru-query-015", "utterance": "сколько я прошёл шагов", "lang": "ru", "intent": "query", "tags": ["aggregate"] }, { "id": "ru-query-016", "utterance": "покажи давление за неделю", "lang": "ru", "intent": "query", "tags": ["hard", "imperative"], "note": "imperative form but a read — must not route to act" }, diff --git a/internal/router/stage0.go b/internal/router/stage0.go index 615a4fc..eafc89a 100644 --- a/internal/router/stage0.go +++ b/internal/router/stage0.go @@ -182,9 +182,38 @@ func AgendaQueryGrammars() []Grammar { Pattern: regexp.MustCompile(`(?i)^\s*(что|чего|какие|сколько|во\s+сколько|когда)\s+у\s+меня(\s|[?!.]|$)`), Build: agendaQueryBuild, }, + { + // A plan noun aimed at a named day, with no possessive to anchor + // on: "какие планы на завтра", "что по делам в среду". The rule + // above wants "у меня" and this phrasing never has it, so + // "какие планы на завтра" answered "пока не умею" while "какие + // планы на сегодня" worked (Vikunja #471). The day word is what + // makes it an agenda question rather than a topic. + Name: "plan-day-query", + // Only "план" and "дел". A verb stem like "встреч" would take + // "встречаемся в среду", which is him telling her something, not + // asking. + Pattern: regexp.MustCompile(`(?i)(^|\s)(план|дел)[а-я]*\s+(на|в|во|по)\s+` + dayWordPattern + `(\s|[?!.]|$)`), + Build: agendaQueryBuild, + }, + { + // A named event with no calendar word at all: "когда планёрка?", + // "во сколько созвон". He is asking when something on his calendar + // happens, and the noun is the only signal. Closed list, so "когда + // битва при Ватерлоо" is still a world question. + Name: "event-time-query", + Pattern: regexp.MustCompile(`(?i)^\s*(когда|во\s+сколько|в\s+котором\s+часу)\s+(будет\s+|у\s+нас\s+)?(планёрк|планерк|встреч|созвон|митинг|совещани|звонок|созвон|приём|прием|интервью|собеседовани|тренировк|урок|занятие|пара)[а-я]*(\s|[?!.]|$)`), + Build: agendaQueryBuild, + }, } } +// dayWordPattern — the day words an agenda question can name. Weekdays appear +// in the accusative and prepositional forms the questions actually use ("в +// среду", "на среде"), which is why the stems carry an inflection tail rather +// than a fixed ending. +const dayWordPattern = `(сегодня|завтра|послезавтра|выходн[а-я]+|недел[а-я]+|понедельник[а-я]*|вторник[а-я]*|сред[ауые][а-я]*|четверг[а-я]*|пятниц[ауые][а-я]*|суббот[ауые][а-я]*|воскресень[ея][а-я]*)` + // agendaQueryBuild — shared Build for the agenda grammars. Confidence 1.0 on // the intent only: the utterance travels intact and the query chain's own // matchers decide the rest. From 908d92a7e80c921d0eb8368577fc33387042ba21 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 02:56:09 +0400 Subject: [PATCH 04/14] calendar: a Russian summary keeps its letters in the fact key (V-443) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit safeKey kept ASCII only, so "Встреча с Аней" and "Обед с мамой" both reduced to "--" and shared one key on one day. The second event of the day overwrote the first, silently, and his calendar is Russian. Letters and digits in any script now pass. Migration #18 deletes the rows written under the old rule instead of rewriting them: a calendar fact is derived, the next poll writes the day again, and a stale row reads as an extra meeting. --- internal/calendar/calendar.go | 15 ++++++++++---- internal/calendar/calendar_test.go | 19 +++++++++++++++++ internal/store/migrations.go | 11 ++++++++++ internal/store/migrations_test.go | 33 ++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 4 deletions(-) diff --git a/internal/calendar/calendar.go b/internal/calendar/calendar.go index 829a9e0..cb62305 100644 --- a/internal/calendar/calendar.go +++ b/internal/calendar/calendar.go @@ -18,6 +18,7 @@ import ( "sort" "strings" "time" + "unicode" ) // Fact sources. A calendar event reaches the store as a @@ -153,14 +154,20 @@ func Overlapping(events []Event, from, to time.Time) []Event { return out } -// safeKey makes a summary safe to use inside a fact key (ASCII alphanumerics -// and dashes). Non-Latin summaries collapse to their punctuation, which is why -// the day prefix carries the identity and this only disambiguates within a day. +// safeKey makes a summary safe to use inside a fact key: letters and digits in +// any script, plus dashes, with space and underscore folded to a dash. +// +// It kept ASCII only until 04-08-2026, and dropped everything else. His +// calendar is Russian, so "Встреча с Аней" and "Обед с мамой" both reduced to +// "--" and produced the same key on the same day — the second event of the day +// silently overwrote the first (Vikunja #443). Letting the letters through is +// what makes the key identify the event. Migration #18 drops the keys written +// under the old rule; they are re-derived on the next poll. func safeKey(s string) string { var b strings.Builder for _, r := range s { switch { - case (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-': + case unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-': b.WriteRune(r) case r == ' ' || r == '_': b.WriteRune('-') diff --git a/internal/calendar/calendar_test.go b/internal/calendar/calendar_test.go index a7ffdbb..e701176 100644 --- a/internal/calendar/calendar_test.go +++ b/internal/calendar/calendar_test.go @@ -139,6 +139,9 @@ func TestSafeKey(t *testing.T) { {"Hello_World", "Hello-World"}, {"special@#$chars!!", "specialchars"}, {"ALL_CAPS_123", "ALL-CAPS-123"}, + // His calendar is Russian. These reduced to "--" and "--" (Vikunja #443). + {"Встреча с Аней", "Встреча-с-Аней"}, + {"Обед с мамой", "Обед-с-мамой"}, } for _, tt := range tests { if got := safeKey(tt.in); got != tt.want { @@ -263,3 +266,19 @@ func TestSourceTrust(t *testing.T) { t.Errorf("Sources() = %v", Sources()) } } + +// Two Russian events on one day must not share a key. They did: safeKey kept +// ASCII only, so both summaries collapsed to their spaces and the second event +// overwrote the first in the store (Vikunja #443). +func TestFactKeyDistinguishesRussianEventsOnOneDay(t *testing.T) { + day := time.Date(2026, 8, 4, 0, 0, 0, 0, time.UTC) + a := Event{Summary: "Встреча с Аней", Start: day.Add(10 * time.Hour), End: day.Add(11 * time.Hour)} + b := Event{Summary: "Обед с мамой", Start: day.Add(13 * time.Hour), End: day.Add(14 * time.Hour)} + if FactKeyIn(a, time.UTC) == FactKeyIn(b, time.UTC) { + t.Fatalf("both events keyed as %q", FactKeyIn(a, time.UTC)) + } + // The day prefix still has to survive, because the store range-scans on it. + if !strings.HasPrefix(FactKeyIn(a, time.UTC), KeyPrefixForDay(day)) { + t.Fatalf("key %q lost the day prefix %q", FactKeyIn(a, time.UTC), KeyPrefixForDay(day)) + } +} diff --git a/internal/store/migrations.go b/internal/store/migrations.go index a266efe..8a94f64 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -208,6 +208,17 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2 // list_tasks into something that writes without the row changing by one // byte. The fingerprint is the declared shape at approval time, so a // redefinition is a re-approval instead of a silent upgrade. + `DELETE FROM facts + WHERE key LIKE 'calendar_event_%' + AND replace(substr(key, 25), '-', '') = '';`, + // #18 — drop the calendar keys written while safeKey dropped Cyrillic + // (Vikunja #443). Everything after the date prefix was punctuation, so + // every Russian event on one day shared one key and only the last one + // survived. Deleting rather than rewriting: a calendar fact is derived + // data, the next poll writes the day again under keys that identify the + // event, and the old rows would otherwise be recited as extra meetings. + // The filter is exact — it keeps any key whose summary part still has a + // letter or a digit in it. } // migrate applies every migration with a number greater than the DB's current diff --git a/internal/store/migrations_test.go b/internal/store/migrations_test.go index e561413..e0b2663 100644 --- a/internal/store/migrations_test.go +++ b/internal/store/migrations_test.go @@ -47,3 +47,36 @@ func TestMigrateAppliesOnceAndIsIdempotent(t *testing.T) { t.Fatalf("after re-migrate user_version = %d, want %d", v, want) } } + +// Migration #18 clears the calendar keys written while safeKey dropped +// Cyrillic. Those rows are indistinguishable from real events on read, so +// leaving them would recite one meeting as several (Vikunja #443). +func TestCollapsedCalendarKeysAreDropped(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + rows := []string{ + "calendar_event_20260804_--", // "Встреча с Аней" under the old rule + "calendar_event_20260804_", // a one-word Russian summary + "calendar_event_20260804_Встреча-с-Аней", // the new format + "calendar_event_20260804_Standup", // an ASCII summary, always fine + } + for _, key := range rows { + if _, err := s.db.ExecContext(ctx, + `INSERT INTO facts (ts, kind, key, value, source, confidence) VALUES (0, 'env', ?, 'x', 'poll:caldav', 1.0)`, + key); err != nil { + t.Fatalf("seed %q: %v", key, err) + } + } + if _, err := s.db.ExecContext(ctx, migrations[17]); err != nil { + t.Fatalf("migration 18: %v", err) + } + + var got int + if err := s.db.QueryRowContext(ctx, `SELECT count(*) FROM facts WHERE key LIKE 'calendar_event_%'`).Scan(&got); err != nil { + t.Fatal(err) + } + if got != 2 { + t.Fatalf("%d calendar rows left, want the 2 that identify their event", got) + } +} From afac8fb670cca26e89923d105919b1f1c4188a42 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:35:42 +0400 Subject: [PATCH 05/14] mavend: run the persona checks before she speaks (V-399) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checks stay in the eval package and the daemon calls three of them: feminine, address, and a new leaked-reasoning test. No retry — it doubles the latency on the turn that is already going badly, and on the nudge path the moment has passed. A failure falls back to the deterministic floor and is logged with the whole rejected text and counted by check name. hisgender is deliberately not run: the simulator showed it rejecting "записала, что ты выпил воды", which is her own correct self-reference. --- cmd/mavend/personaguard.go | 134 ++++++++++++++++++++++++++++++++ cmd/mavend/personaguard_test.go | 75 ++++++++++++++++++ cmd/mavend/replier_llm.go | 5 ++ cmd/mavend/tick.go | 3 + internal/phraser/eval/checks.go | 13 ++++ 5 files changed, 230 insertions(+) create mode 100644 cmd/mavend/personaguard.go create mode 100644 cmd/mavend/personaguard_test.go diff --git a/cmd/mavend/personaguard.go b/cmd/mavend/personaguard.go new file mode 100644 index 0000000..eaa5d52 --- /dev/null +++ b/cmd/mavend/personaguard.go @@ -0,0 +1,134 @@ +package main + +import ( + "context" + "log" + "regexp" + "strings" + "sync" + + "github.com/kami/maven/internal/delivery" + "github.com/kami/maven/internal/loop" + "github.com/kami/maven/internal/phraser" + "github.com/kami/maven/internal/phraser/eval" +) + +// The persona checks, run before she speaks (Vikunja #399). +// +// RunChecks and RunTalkChecks only ever ran from the eval package, so +// everything the fixtures measured was offline knowledge: we could say "about +// one reply in three is broken" and still ship every one of them. This runs the +// cheap half of that on the live path, and replaces a failing message with the +// deterministic floor. +// +// Which checks: the unambiguous string tests only — feminine self-reference, +// how she addresses him, and a leaked-reasoning test. Not length, which is +// path-specific, and not ontopic, which compares against fragments the fixture +// supplies and runtime does not have. Not hisgender either — see guardSpoken. +// +// No retry. A retry doubles the latency on the exact turn that is already going +// badly, and on the nudge path the moment has passed. +// +// The known cost, written down because it is real: a wrongly flagged good reply +// is replaced by a flatter stub one. That is the right trade — a stub sentence +// is dull, a leaked reasoning trace is broken — but it means these checks can +// no longer be tuned for sensitivity alone. + +// checkLeak — the name reported when the model's scaffolding reaches the text. +const checkLeak = "leak" + +// leakPatterns — reasoning and protocol that belongs to the model, not to him. +// The resident model is a Thinking variant, so an unclosed reasoning block is +// the failure mode, not a hypothetical (Vikunja #398). +var leakPatterns = []*regexp.Regexp{ + regexp.MustCompile(`(?i)<\s*/?\s*think`), + regexp.MustCompile(`(?i)thinking\s*(process|:)`), + regexp.MustCompile(`(?i)^\s*(assistant|user|system)\s*:`), + // Raw contract JSON: the parser already unwraps a good one, so a body that + // still carries the keys is one it could not read. + regexp.MustCompile(`"(response|mood|body|summary)"\s*:`), + // The persona block quoted back at him. + regexp.MustCompile(`(?i)(ты\s+—?\s*мэйвен|системный промпт|system prompt)`), +} + +// checkPersonaLeak reports whether the model's own scaffolding is in the text. +func checkPersonaLeak(body string) (string, bool) { + for _, re := range leakPatterns { + if m := re.FindString(body); m != "" { + return "leaked " + strings.TrimSpace(m), false + } + } + return "", true +} + +// personaRejects counts what the guard caught, by check name, so the real +// production rate is knowable rather than inferred from the fixture. +var personaRejects = struct { + mu sync.Mutex + by map[string]int +}{by: map[string]int{}} + +func personaRejectCounts() map[string]int { + personaRejects.mu.Lock() + defer personaRejects.mu.Unlock() + out := make(map[string]int, len(personaRejects.by)) + for k, v := range personaRejects.by { + out[k] = v + } + return out +} + +// guardSpoken checks a phrased message. It returns the failed check and false +// when the message must not be said; path names the caller, for the log. +// +// An empty message passes: the caller already treats that as a failure and +// falls back on its own, and reporting it as a persona breach would put a +// misleading line in the count. +func guardSpoken(path, body string) (string, bool) { + if strings.TrimSpace(body) == "" { + return "", true + } + if detail, ok := checkPersonaLeak(body); !ok { + return rejectSpoken(path, checkLeak, detail, body), false + } + // Feminine and address only. HisGender is not run here: it reads a + // sentence-initial feminine verb with no pronoun — "записала, что ты выпил + // воды" — as a woman being addressed, when it is her own correct + // self-reference. Offline that is a point of score; on this path it would + // replace a good reply with a stub one on every fact she confirms. + for _, r := range []eval.Result{eval.Feminine(body), eval.Address(body)} { + if !r.Pass { + return rejectSpoken(path, r.Name, r.Detail, body), false + } + } + return "", true +} + +// rejectSpoken logs what she nearly said and counts it. The whole text, not a +// prefix: the point of the log line is that the failure can be read back later +// and argued with. +func rejectSpoken(path, check, detail, body string) string { + personaRejects.mu.Lock() + personaRejects.by[check]++ + personaRejects.mu.Unlock() + log.Printf("persona: %s rejected on %s (%s): %q", path, check, detail, body) + return check +} + +// guardNudge checks a phrased nudge and falls back to the deterministic floor +// when it fails. The nudge path, unlike the reply path, cannot ask again: the +// tick has already decided she speaks, so the choice is the floor's wording or +// a broken sentence. +func guardNudge(pn delivery.PhrasedNudge, cand loop.Candidate) delivery.PhrasedNudge { + if _, ok := guardSpoken("nudge", pn.Body); ok { + return pn + } + stub, err := phraser.NewStub().PhraseNudge(context.Background(), cand) + if err != nil { + // The Stub is templates over the candidate and does not fail. If it + // somehow does, the model's text is still what the rule decided to + // say, and saying nothing is the worse outcome. + return pn + } + return stub +} diff --git a/cmd/mavend/personaguard_test.go b/cmd/mavend/personaguard_test.go new file mode 100644 index 0000000..26c599a --- /dev/null +++ b/cmd/mavend/personaguard_test.go @@ -0,0 +1,75 @@ +package main + +import ( + "strings" + "testing" + + "github.com/kami/maven/internal/delivery" + "github.com/kami/maven/internal/loop" +) + +func TestGuardPassesWhatSheShouldSay(t *testing.T) { + good := []string{ + "записала: купить хлеб.", + "поняла, напомню в 11:00.", + "ты не пил воду с утра.", + "я рада, что получилось.", + "", + } + for _, body := range good { + if check, ok := guardSpoken("test", body); !ok { + t.Errorf("guardSpoken(%q) rejected on %s", body, check) + } + } +} + +func TestGuardStopsWhatSheShouldNot(t *testing.T) { + bad := []struct { + body string + want string + }{ + {"он просил воду попей воды.", checkLeak}, + {"Thinking Process: он давно не пил.", checkLeak}, + {`{"response": "попей воды", "mood": "neutral"}`, checkLeak}, + {"я напомнил тебе про воду.", "feminine"}, + {"вы давно не пили воду.", "address"}, + } + for _, c := range bad { + check, ok := guardSpoken("test", c.body) + if ok { + t.Errorf("guardSpoken(%q) let it through", c.body) + continue + } + if check != c.want { + t.Errorf("guardSpoken(%q) failed on %s; want %s", c.body, check, c.want) + } + } +} + +func TestGuardCountsWhatItCaught(t *testing.T) { + before := personaRejectCounts()[checkLeak] + if _, ok := guardSpoken("test", "…"); ok { + t.Fatal("a leaked reasoning block was let through") + } + if after := personaRejectCounts()[checkLeak]; after != before+1 { + t.Errorf("leak count %d; want %d", after, before+1) + } +} + +// TestGuardNudgeFallsBackToTheFloor — a broken nudge is replaced by the +// deterministic wording, not dropped and not retried. +func TestGuardNudgeFallsBackToTheFloor(t *testing.T) { + cand := loop.Candidate{Rule: loop.Rule{Name: "water"}} + bad := delivery.PhrasedNudge{Candidate: cand, Body: "Thinking Process: он не пил.", Mood: "neutral"} + got := guardNudge(bad, cand) + if got.Body == bad.Body { + t.Fatal("the broken nudge was delivered unchanged") + } + if strings.TrimSpace(got.Body) == "" { + t.Fatal("the nudge was dropped rather than re-worded") + } + good := delivery.PhrasedNudge{Candidate: cand, Body: "попей воды.", Mood: "neutral"} + if guardNudge(good, cand).Body != good.Body { + t.Error("a good nudge was replaced") + } +} diff --git a/cmd/mavend/replier_llm.go b/cmd/mavend/replier_llm.go index 20967dd..496afdb 100644 --- a/cmd/mavend/replier_llm.go +++ b/cmd/mavend/replier_llm.go @@ -30,5 +30,10 @@ func (r *llmReplier) Reply(d router.Decision) string { if err != nil || out == "" { return r.stub.Reply(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 out } diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index 8926a47..04ff8af 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -176,6 +176,9 @@ func (t *tickLoop) tick(ctx context.Context, now time.Time) { t.queueNudge(ctx, cand, state, now) } else { pn, err := t.phraser.PhraseNudge(ctx, *cand) + if err == nil { + pn = guardNudge(pn, *cand) + } if err != nil { log.Printf("tick: phrase nudge %s: %v", cand.Rule.Name, err) } else { diff --git a/internal/phraser/eval/checks.go b/internal/phraser/eval/checks.go index 4fffc46..fdbbe48 100644 --- a/internal/phraser/eval/checks.go +++ b/internal/phraser/eval/checks.go @@ -67,6 +67,19 @@ func RunChecks(c Case, body, mood string) []Result { } } +// Feminine, HisGender and Address expose three checks one at a time, so the +// daemon can run them on a phrased message before he hears it (Vikunja #399). +// Only these three: they are unambiguous string tests with nothing to compare +// against, while length is path-specific and ontopic needs the fixture's +// expected fragments, which do not exist at runtime. +func Feminine(body string) Result { return checkFeminine(body) } + +// HisGender — see checkHisGender. +func HisGender(body string) Result { return checkHisGender(body) } + +// Address — see checkAddress. +func Address(body string) Result { return checkAddress(body) } + func checkMood(mood string) Result { if Moods[mood] { return Result{CheckMood, true, ""} From 5bd303788b8cb18f6afdbd4401b786ffb56b4f4b Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:39:24 +0400 Subject: [PATCH 06/14] store: add list_items, the fourth append-only shape (V-453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A list is a standing set of short strings under a tag. Not a task, because milk is not work and the prioritiser must not count it as an errand; not a fact, because it claims nothing. Nothing predicates over it, so two people adding to the same list at once costs nothing. Migration #19, plus AddListItem, ListItems, SetListItemStatus and ClearList. The live-only unique index is the tasks one, per list: молоко twice before the shop is one row, молоко again after it was crossed off is a new one. --- internal/store/listitems.go | 194 +++++++++++++++++++++++++++++++++++ internal/store/migrations.go | 21 ++++ 2 files changed, 215 insertions(+) create mode 100644 internal/store/listitems.go diff --git a/internal/store/listitems.go b/internal/store/listitems.go new file mode 100644 index 0000000..1f8cfd2 --- /dev/null +++ b/internal/store/listitems.go @@ -0,0 +1,194 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + "time" +) + +// List items — the fourth append-only shape (Vikunja #453). +// +// A list is a standing set of short strings under a tag: покупки, аптека, +// хозяйство. It is not work and it is not a claim about the world, which is +// why it is neither a task nor a fact. Nothing here is prioritised, nothing +// nudges about it, and the digestion worker does not read it. The only two +// things a list does are grow and shrink. +// +// The consequence that made it worth a table: because no predicate touches a +// list item, several people adding to the same list at once cost nothing. There +// is no ranking to disagree about and no lifecycle beyond crossed-off. +const ( + // ListItemOpen — on the list. + ListItemOpen = "open" + // ListItemDone — bought, taken, crossed off. + ListItemDone = "done" + // ListItemDropped — removed without being got. + ListItemDropped = "dropped" +) + +// DefaultList — the list a capture lands on when he names none. Almost every +// spoken list item is groceries, and asking "в какой список?" for the common +// case would be a nag. +const DefaultList = "покупки" + +// ListItem — one line on one list. +type ListItem struct { + ID int64 + CreatedTs time.Time + List string + Item string + Source string + Status string + ResolvedTs *time.Time +} + +var ( + ErrListItemNotFound = errors.New("store: list item not found") + ErrListItemEmpty = errors.New("store: list item is empty") + ErrListItemStatus = errors.New("store: invalid list item status") +) + +// NormalizeListName folds a list tag to its dedupe form. Lists are named out +// loud, so "Покупки" and "покупки " are the same list. +func NormalizeListName(s string) string { + n := NormalizeTaskText(s) + if n == "" { + return DefaultList + } + return n +} + +// AddListItem puts an item on a list, or returns the existing row when the same +// item is already on it. Created says which happened, so the caller can say +// "уже есть" instead of pretending it wrote something. +func (s *Store) AddListItem(ctx context.Context, li ListItem) (CaptureResult, error) { + item := strings.TrimSpace(li.Item) + if item == "" { + return CaptureResult{}, ErrListItemEmpty + } + list := NormalizeListName(li.List) + norm := NormalizeTaskText(item) + created := li.CreatedTs + if created.IsZero() { + created = time.Now() + } + res, err := s.db.ExecContext(ctx, + `INSERT INTO list_items (created_ts, list, item, norm, source, status) + VALUES (?,?,?,?,?,?) + ON CONFLICT DO NOTHING`, + created.UnixMilli(), list, item, norm, li.Source, ListItemOpen) + if err != nil { + return CaptureResult{}, fmt.Errorf("add list item: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return CaptureResult{}, fmt.Errorf("add list item: rows affected: %w", err) + } + if n > 0 { + id, err := res.LastInsertId() + if err != nil { + return CaptureResult{}, fmt.Errorf("add list item: last insert id: %w", err) + } + return CaptureResult{ID: id, Created: true}, nil + } + var id int64 + err = s.db.QueryRowContext(ctx, + `SELECT id FROM list_items WHERE list = ? AND norm = ? AND status = ?`, + list, norm, ListItemOpen).Scan(&id) + if errors.Is(err, sql.ErrNoRows) { + return CaptureResult{}, ErrListItemNotFound + } + if err != nil { + return CaptureResult{}, fmt.Errorf("add list item: lookup: %w", err) + } + return CaptureResult{ID: id}, nil +} + +// ListItems reads one list in the order it was added. An empty status reads the +// open items, which is what reading the list aloud means. +func (s *Store) ListItems(ctx context.Context, list, status string) ([]ListItem, error) { + if status == "" { + status = ListItemOpen + } + rows, err := s.db.QueryContext(ctx, + `SELECT id, created_ts, list, item, source, status, resolved_ts + FROM list_items WHERE list = ? AND status = ? + ORDER BY created_ts, id`, + NormalizeListName(list), status) + if err != nil { + return nil, fmt.Errorf("list items: %w", err) + } + defer rows.Close() + var out []ListItem + for rows.Next() { + var ( + li ListItem + created int64 + resolved sql.NullInt64 + ) + if err := rows.Scan(&li.ID, &created, &li.List, &li.Item, &li.Source, &li.Status, &resolved); err != nil { + return nil, fmt.Errorf("list items: scan: %w", err) + } + li.CreatedTs = time.UnixMilli(created) + if resolved.Valid { + t := time.UnixMilli(resolved.Int64) + li.ResolvedTs = &t + } + out = append(out, li) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("list items: %w", err) + } + return out, nil +} + +// SetListItemStatus crosses an item off, or removes it. Moving an item that is +// already resolved is not an error — crossing off twice is the same list. +func (s *Store) SetListItemStatus(ctx context.Context, id int64, status string, at time.Time) error { + if status != ListItemOpen && status != ListItemDone && status != ListItemDropped { + return fmt.Errorf("%w: %q", ErrListItemStatus, status) + } + var resolved sql.NullInt64 + if status != ListItemOpen { + if at.IsZero() { + at = time.Now() + } + resolved = sql.NullInt64{Int64: at.UnixMilli(), Valid: true} + } + res, err := s.db.ExecContext(ctx, + `UPDATE list_items SET status = ?, resolved_ts = ? WHERE id = ?`, + status, resolved, id) + if err != nil { + return fmt.Errorf("set list item status: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("set list item status: rows affected: %w", err) + } + if n == 0 { + return ErrListItemNotFound + } + return nil +} + +// ClearList crosses off every open item on a list and reports how many. This is +// "всё купил", which is one sentence and must not become one turn per item. +func (s *Store) ClearList(ctx context.Context, list string, at time.Time) (int, error) { + if at.IsZero() { + at = time.Now() + } + res, err := s.db.ExecContext(ctx, + `UPDATE list_items SET status = ?, resolved_ts = ? WHERE list = ? AND status = ?`, + ListItemDone, at.UnixMilli(), NormalizeListName(list), ListItemOpen) + if err != nil { + return 0, fmt.Errorf("clear list: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return 0, fmt.Errorf("clear list: rows affected: %w", err) + } + return int(n), nil +} diff --git a/internal/store/migrations.go b/internal/store/migrations.go index 8a94f64..8a34ed2 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -219,6 +219,27 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2 // event, and the old rows would otherwise be recited as extra meetings. // The filter is exact — it keeps any key whose summary part still has a // letter or a digit in it. + `CREATE TABLE IF NOT EXISTS list_items ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + created_ts INTEGER NOT NULL, + list TEXT NOT NULL, + item TEXT NOT NULL, + norm TEXT NOT NULL, + source TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'open' CHECK (status IN ('open','done','dropped')), + resolved_ts INTEGER + ); + CREATE UNIQUE INDEX IF NOT EXISTS idx_list_items_live ON list_items (list, norm) WHERE status = 'open'; + CREATE INDEX IF NOT EXISTS idx_list_items_list ON list_items (list, status, created_ts);`, + // #19 — standing lists (Vikunja #453). The fourth append-only shape, after + // facts, notes and tasks, and the reason it is its own table rather than a + // tag on tasks: milk on the shopping list is not work. Nothing prioritises + // it, nothing nudges about it, and the prioritiser must not start counting + // groceries as outstanding errands. + // + // The live-only unique index is the tasks one, per list: saying "молоко" + // twice before the shop keeps one row, saying it again next week after the + // last one was crossed off writes a new one. } // migrate applies every migration with a number greater than the DB's current From 0d52344d27d86c8f153eca0b440f05687a3a73e5 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:39:24 +0400 Subject: [PATCH 07/14] store: cover the list_items shape with tests (V-453) --- internal/store/listitems_test.go | 149 +++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 internal/store/listitems_test.go diff --git a/internal/store/listitems_test.go b/internal/store/listitems_test.go new file mode 100644 index 0000000..eb2a464 --- /dev/null +++ b/internal/store/listitems_test.go @@ -0,0 +1,149 @@ +package store + +import ( + "context" + "errors" + "testing" + "time" +) + +var listNow = time.Date(2026, 8, 4, 12, 0, 0, 0, time.UTC) + +func TestAddListItemDedupesTheOpenList(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + first, err := s.AddListItem(ctx, ListItem{Item: "молоко", Source: "tap:voice", CreatedTs: listNow}) + if err != nil { + t.Fatalf("add: %v", err) + } + if !first.Created { + t.Fatal("the first молоко did not create a row") + } + again, err := s.AddListItem(ctx, ListItem{Item: " Молоко ", Source: "tap:voice", CreatedTs: listNow}) + if err != nil { + t.Fatalf("add again: %v", err) + } + if again.Created { + t.Error("молоко was added twice") + } + if again.ID != first.ID { + t.Errorf("second add points at %d; want the existing %d", again.ID, first.ID) + } + if _, err := s.AddListItem(ctx, ListItem{Item: " "}); !errors.Is(err, ErrListItemEmpty) { + t.Errorf("empty item: %v; want ErrListItemEmpty", err) + } +} + +// A crossed-off item does not block the next one: buying milk again next week +// is a new line, the way saying an errand again is a new task. +func TestCrossedOffItemComesBack(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + first, err := s.AddListItem(ctx, ListItem{Item: "молоко", CreatedTs: listNow}) + if err != nil { + t.Fatalf("add: %v", err) + } + if err := s.SetListItemStatus(ctx, first.ID, ListItemDone, listNow); err != nil { + t.Fatalf("cross off: %v", err) + } + next, err := s.AddListItem(ctx, ListItem{Item: "молоко", CreatedTs: listNow.Add(time.Hour)}) + if err != nil { + t.Fatalf("add after: %v", err) + } + if !next.Created || next.ID == first.ID { + t.Errorf("second молоко reused row %d; want a new one", next.ID) + } + open, err := s.ListItems(ctx, "", "") + if err != nil { + t.Fatalf("list: %v", err) + } + if len(open) != 1 || open[0].ID != next.ID { + t.Errorf("open list %+v; want only the new row", open) + } +} + +// Lists are separate stores under one table: the same word on two lists is two +// items, and reading one never reads the other. +func TestListsDoNotSeeEachOther(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + if _, err := s.AddListItem(ctx, ListItem{List: "покупки", Item: "вода", CreatedTs: listNow}); err != nil { + t.Fatalf("add: %v", err) + } + if _, err := s.AddListItem(ctx, ListItem{List: "Аптека", Item: "вода", CreatedTs: listNow}); err != nil { + t.Fatalf("add: %v", err) + } + for _, c := range []struct{ list, want string }{ + {"покупки", "покупки"}, + {"аптека", "аптека"}, + {"", "покупки"}, + } { + got, err := s.ListItems(ctx, c.list, "") + if err != nil { + t.Fatalf("list %q: %v", c.list, err) + } + if len(got) != 1 { + t.Fatalf("list %q has %d items; want 1", c.list, len(got)) + } + if got[0].List != c.want { + t.Errorf("list %q returned tag %q; want %q", c.list, got[0].List, c.want) + } + } +} + +func TestClearListCrossesOffEverythingOpen(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + for _, item := range []string{"молоко", "хлеб", "яйца"} { + if _, err := s.AddListItem(ctx, ListItem{Item: item, CreatedTs: listNow}); err != nil { + t.Fatalf("add %s: %v", item, err) + } + } + if _, err := s.AddListItem(ctx, ListItem{List: "аптека", Item: "бинт", CreatedTs: listNow}); err != nil { + t.Fatalf("add: %v", err) + } + n, err := s.ClearList(ctx, "покупки", listNow) + if err != nil { + t.Fatalf("clear: %v", err) + } + if n != 3 { + t.Errorf("cleared %d; want 3", n) + } + left, err := s.ListItems(ctx, "покупки", "") + if err != nil { + t.Fatalf("list: %v", err) + } + if len(left) != 0 { + t.Errorf("%d items still open; want none", len(left)) + } + done, err := s.ListItems(ctx, "покупки", ListItemDone) + if err != nil { + t.Fatalf("list done: %v", err) + } + if len(done) != 3 || done[0].ResolvedTs == nil { + t.Errorf("done list %+v; want 3 rows carrying a resolved time", done) + } + other, err := s.ListItems(ctx, "аптека", "") + if err != nil { + t.Fatalf("list: %v", err) + } + if len(other) != 1 { + t.Error("clearing покупки touched аптека") + } +} + +func TestSetListItemStatusRejectsWhatIsNotAStatus(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + if err := s.SetListItemStatus(ctx, 1, "куплено", listNow); !errors.Is(err, ErrListItemStatus) { + t.Errorf("bad status: %v; want ErrListItemStatus", err) + } + if err := s.SetListItemStatus(ctx, 999, ListItemDone, listNow); !errors.Is(err, ErrListItemNotFound) { + t.Errorf("missing row: %v; want ErrListItemNotFound", err) + } +} From e023638135b3e83086d8697f06463878e02e23a3 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:45:13 +0400 Subject: [PATCH 08/14] router: parse list capture, read-back and crossing off (V-453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same posture as task capture and for the same reason: the intent enum is a contract shared with the relabelling prompt, so a list is not an eighth intent. It is a note-shaped or query-shaped utterance carrying an explicit marker, and the marker is a lookup. The markers are deliberately explicit — "молоко закончилось" is an observation and stays a note. The list tag is matched by stem, because Russian declines it: "список покупок", "в покупки" and "в покупках" are one list. ListGrammars puts both halves at stage 0, so an add and a read-back never depend on the model having a good turn. --- internal/router/list.go | 247 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 247 insertions(+) create mode 100644 internal/router/list.go diff --git a/internal/router/list.go b/internal/router/list.go new file mode 100644 index 0000000..499775a --- /dev/null +++ b/internal/router/list.go @@ -0,0 +1,247 @@ +package router + +import ( + "regexp" + "strings" +) + +// Standing lists, matched deterministically (Vikunja #453). +// +// Same posture as task capture in task.go and for the same reason: the intent +// enum is a contract shared with the relabelling prompt, so a list is not an +// eighth intent. It is a note-shaped or query-shaped utterance carrying an +// explicit marker, and the marker is a lookup. +// +// The markers are deliberately explicit. "молоко закончилось" is an +// observation about the world and belongs in a note; only an instruction to +// put something on a list puts it there. + +// listStems — the lists he can name, by the stem every case form shares. +// Russian declines the tag ("список покупок", "в покупки", "в покупках"), so +// matching a stem is what makes those the same list. +var listStems = []struct{ stem, list string }{ + {"покуп", "покупки"}, + {"продукт", "покупки"}, + {"магазин", "покупки"}, + {"аптек", "аптека"}, + {"хозяйств", "хозяйство"}, + {"shopping", "покупки"}, + {"groceries", "покупки"}, + {"pharmacy", "аптека"}, +} + +// listCapturePrefixes — an instruction to add to a list. Longest match wins. +var listCapturePrefixes = []string{ + "добавь в список", + "добавь в покупки", + "добавь к покупкам", + "запиши в список", + "внеси в список", + "положи в список", + "в список покупок", + "add to the list", + "add to my list", + "add to the shopping list", + "put on the list", +} + +// listQueryPrefixes — an ask to read a list back. +var listQueryPrefixes = []string{ + "что в списке", + "что в покупках", + "что мне купить", + "что нужно купить", + "что надо купить", + "покажи список", + "прочитай список", + "список покупок", + "мой список", + "what is on the list", + "what's on the list", + "read me the list", + "show me the list", + "shopping list", +} + +// listClearPhrases — the whole list is got. One sentence, one turn. +var listClearPhrases = []string{ + "всё купил", + "все купил", + "всё взял", + "все взял", + "очисти список", + "очисти покупки", + "список пустой", + "got everything", + "clear the list", +} + +// listRemovePrefixes — one item off the list. +var listRemovePrefixes = []string{ + "вычеркни", + "убери из списка", + "убери со списка", + "купил", + "взял", + "cross off", + "remove from the list", +} + +// listTrimCut — punctuation and connectives to strip off a parsed remainder. +const listTrimCut = " .,;:!?—-" + +// ListCapture — a parsed list instruction: which list, and the item. +type ListCapture struct { + List string + Item string +} + +// ParseListCapture reports whether an utterance puts something on a list, and +// returns the list tag and the item. A marker with nothing usable after it is +// not a capture: there is no item in "добавь в список покупок". +func ParseListCapture(text string) (ListCapture, bool) { + rest, ok := afterLongestPrefix(text, listCapturePrefixes) + if !ok { + return ListCapture{}, false + } + list, rest := takeListTag(rest) + rest = strings.Trim(rest, listTrimCut) + if rest == "" { + return ListCapture{}, false + } + return ListCapture{List: list, Item: rest}, true +} + +// ParseListQuery reports whether an utterance asks for a list, and which one. +func ParseListQuery(text string) (string, bool) { + rest, ok := afterLongestPrefix(text, listQueryPrefixes) + if !ok { + return "", false + } + list, _ := takeListTag(rest) + return list, true +} + +// ParseListClear reports whether an utterance crosses off a whole list. +func ParseListClear(text string) (string, bool) { + lower := strings.ToLower(strings.Trim(strings.TrimSpace(text), listTrimCut)) + for _, p := range listClearPhrases { + if lower == p || strings.HasPrefix(lower, p+" ") { + list, _ := takeListTag(strings.TrimSpace(lower[len(p):])) + return list, true + } + } + return "", false +} + +// ParseListRemove reports whether an utterance takes one named item off a +// list, and returns the list and the item. +// +// The item is required. "купил" on its own is him reporting he shopped, which +// ParseListClear reads first, and it must not fall through to here and remove +// nothing while sounding like it did. +func ParseListRemove(text string) (ListCapture, bool) { + rest, ok := afterLongestPrefix(text, listRemovePrefixes) + if !ok { + return ListCapture{}, false + } + list, rest := takeListTag(rest) + rest = strings.Trim(rest, listTrimCut) + for _, lead := range []string{"из списка ", "со списка ", "из ", "from the list "} { + rest = strings.TrimPrefix(rest, lead) + } + rest = strings.Trim(rest, listTrimCut) + if rest == "" { + return ListCapture{}, false + } + return ListCapture{List: list, Item: rest}, true +} + +// afterLongestPrefix matches the longest prefix in the table and returns what +// follows it, trimmed. Lowercasing does not change the byte length of Russian +// or English letters, so the index carries over to the original text. +func afterLongestPrefix(text string, prefixes []string) (string, bool) { + trimmed := strings.TrimSpace(text) + lower := strings.ToLower(trimmed) + best := "" + for _, p := range prefixes { + if strings.HasPrefix(lower, p) && len(p) > len(best) { + best = p + } + } + if best == "" { + return "", false + } + return strings.Trim(trimmed[len(best):], listTrimCut), true +} + +// takeListTag reads a list name off the front of the remainder and returns the +// list plus what is left. A remainder naming no list is the default list, and +// nothing is consumed — "добавь в список молоко" names no list and the item is +// молоко. +func takeListTag(rest string) (string, string) { + fields := strings.Fields(rest) + if len(fields) == 0 { + return "покупки", "" + } + head := strings.ToLower(strings.Trim(fields[0], listTrimCut)) + // "в список покупок" leaves "покупок"; "в списке" leaves nothing. + if head == "список" || head == "списке" || head == "списка" || head == "list" { + fields = fields[1:] + if len(fields) == 0 { + return "покупки", "" + } + head = strings.ToLower(strings.Trim(fields[0], listTrimCut)) + } + for _, s := range listStems { + if strings.HasPrefix(head, s.stem) { + return s.list, strings.Join(fields[1:], " ") + } + } + return "покупки", strings.Join(fields, " ") +} + +// ListGrammars — stage 0 for the list (Vikunja #453). +// +// Both patterns match everything and the Build functions are the real filter, +// the shape the wake-word act grammar already uses: the parsers above are the +// definition of a list utterance and duplicating them as regexps would give +// two answers to one question. +// +// Why stage 0 at all: an add and a read-back are deterministic and cheap, and +// leaving them to the model means "добавь в список покупок молоко" lands as an +// act or a fact on the turns the model has a bad day. The action handlers still +// re-parse, so a list turn that arrives by any other route still works. +func ListGrammars() []Grammar { + anything := regexp.MustCompile(`(?s)^(.*)$`) + return []Grammar{ + { + Name: "list-query", + Pattern: anything, + Build: func(m []string) (Decision, bool) { + if _, ok := ParseListQuery(m[1]); !ok { + return Decision{}, false + } + return Decision{Stage: 0, Intent: IntentQuery, Confidence: 1.0}, true + }, + }, + { + Name: "list-capture", + Pattern: anything, + Build: func(m []string) (Decision, bool) { + text := m[1] + _, add := ParseListCapture(text) + _, clear := ParseListClear(text) + if !add && !clear { + return Decision{}, false + } + return Decision{ + Stage: 0, + Intent: IntentNote, + Confidence: 1.0, + Slots: Slots{Text: strings.TrimSpace(text)}, + }, true + }, + }, + } +} From d41878c2b1c4d0cc4af7e6aa8c0f86dcbcea3716 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:45:13 +0400 Subject: [PATCH 09/14] router: cover the list parsers and wire the grammars (V-453) --- cmd/mavend/voicewire.go | 1 + internal/router/list_test.go | 89 ++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 internal/router/list_test.go diff --git a/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index f4fd334..4b07dd4 100644 --- a/cmd/mavend/voicewire.go +++ b/cmd/mavend/voicewire.go @@ -379,6 +379,7 @@ func buildRouter(emb router.Embedder, acts router.ActMatcher, threshold float64, // question and must keep reaching replySystem, while "что у меня сегодня" // is an agenda question and must not. grammars = append(grammars, router.AgendaQueryGrammars()...) + grammars = append(grammars, router.ListGrammars()...) grammars = append(grammars, router.ReminderGrammar()) return router.New(router.Config{ Grammars: grammars, diff --git a/internal/router/list_test.go b/internal/router/list_test.go new file mode 100644 index 0000000..5e0bb40 --- /dev/null +++ b/internal/router/list_test.go @@ -0,0 +1,89 @@ +package router + +import "testing" + +func TestParseListCaptureReadsListAndItem(t *testing.T) { + cases := []struct { + utterance string + list string + item string + }{ + {"добавь в список покупок молоко", "покупки", "молоко"}, + {"добавь в список молоко", "покупки", "молоко"}, + {"Добавь в покупки хлеб и яйца", "покупки", "хлеб и яйца"}, + {"запиши в список аптеки бинт", "аптека", "бинт"}, + {"добавь в список хозяйства лампочки.", "хозяйство", "лампочки"}, + {"add to the shopping list milk", "покупки", "milk"}, + } + for _, c := range cases { + got, ok := ParseListCapture(c.utterance) + if !ok { + t.Errorf("ParseListCapture(%q) did not claim it", c.utterance) + continue + } + if got.List != c.list || got.Item != c.item { + t.Errorf("ParseListCapture(%q) = %+v; want list %q item %q", c.utterance, got, c.list, c.item) + } + } +} + +// A marker with no item is not a capture, and an utterance that only mentions +// shopping is not one either. +func TestParseListCapturePasses(t *testing.T) { + for _, u := range []string{ + "добавь в список покупок", + "добавь в список", + "молоко закончилось", + "надо бы съездить в магазин", + "добавь в задачи купить молоко", + } { + if got, ok := ParseListCapture(u); ok { + t.Errorf("ParseListCapture(%q) claimed it as %+v", u, got) + } + } +} + +func TestParseListQueryNamesTheList(t *testing.T) { + cases := []struct{ utterance, list string }{ + {"что в списке покупок?", "покупки"}, + {"что в списке", "покупки"}, + {"что мне купить", "покупки"}, + {"покажи список аптеки", "аптека"}, + {"what's on the list", "покупки"}, + } + for _, c := range cases { + list, ok := ParseListQuery(c.utterance) + if !ok { + t.Errorf("ParseListQuery(%q) did not claim it", c.utterance) + continue + } + if list != c.list { + t.Errorf("ParseListQuery(%q) = %q; want %q", c.utterance, list, c.list) + } + } + if _, ok := ParseListQuery("какие у меня задачи"); ok { + t.Error("ParseListQuery claimed a task question") + } +} + +func TestParseListClearAndRemove(t *testing.T) { + if list, ok := ParseListClear("всё купил"); !ok || list != "покупки" { + t.Errorf("ParseListClear = %q, %v; want покупки, true", list, ok) + } + if list, ok := ParseListClear("очисти список аптеки"); !ok || list != "аптека" { + t.Errorf("ParseListClear = %q, %v; want аптека, true", list, ok) + } + if _, ok := ParseListClear("купил молоко"); ok { + t.Error("ParseListClear claimed a single item") + } + got, ok := ParseListRemove("вычеркни молоко") + if !ok || got.Item != "молоко" || got.List != "покупки" { + t.Errorf("ParseListRemove = %+v, %v; want молоко on покупки", got, ok) + } + if got, ok := ParseListRemove("убери из списка аптеки бинт"); !ok || got.Item != "бинт" || got.List != "аптека" { + t.Errorf("ParseListRemove = %+v, %v; want бинт on аптека", got, ok) + } + if _, ok := ParseListRemove("вычеркни"); ok { + t.Error("ParseListRemove claimed a marker with no item") + } +} From 0990f32808e7a3b410980e88fd13ef76d8cc5e31 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:45:21 +0400 Subject: [PATCH 10/14] mavend: the list is reachable from voice (V-453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An add and a crossing-off run at the top of actionNote, next to task capture and before the embedding is paid for; the read-back is a query source sitting beside "tasks", so the recall pass cannot answer "что мне купить?" from an old note about the shop. Crossing off one item claims the turn only when the list actually holds that item, which is what keeps "купил новый ноутбук" a note. These read h.dataStore rather than the CoreAPI: a list is local to the core and nothing outside it writes one. The ipc seam is what it grows through when something outside mavend needs to add to a list. --- cmd/mavend/actions_list.go | 143 ++++++++++++++++++++++++++++++++++++ cmd/mavend/actions_note.go | 6 ++ cmd/mavend/actions_query.go | 5 ++ 3 files changed, 154 insertions(+) create mode 100644 cmd/mavend/actions_list.go diff --git a/cmd/mavend/actions_list.go b/cmd/mavend/actions_list.go new file mode 100644 index 0000000..e7e7d61 --- /dev/null +++ b/cmd/mavend/actions_list.go @@ -0,0 +1,143 @@ +package main + +import ( + "context" + "log" + "strings" + + "github.com/kami/maven/internal/router" + "github.com/kami/maven/internal/store" +) + +// Standing lists on the voice path (Vikunja #453). +// +// Three halves, mirroring what task capture already does: an add that runs at +// the top of actionNote, a read-back query source, and a crossing-off that runs +// on the same note path because "всё купил" is note-shaped. +// +// These read h.dataStore rather than the CoreAPI. A list is local to the core +// and nothing outside it writes one: the web UI has no list page, no reach +// files groceries, and the digestion worker does not read the table. When +// something outside mavend needs to add to a list, the ipc seam is what it +// grows through — the intake rules that CaptureTaskReq documents are about +// shared intake, and there is none here yet. +// +// Nothing here speaks unprompted. A list is answered when asked about. + +// captureListFromNote claims the turn when the utterance adds to, clears, or +// crosses one item off a list. ("", false) hands the turn back to the note path. +func (h *reactiveHandler) captureListFromNote(ctx context.Context, dec router.Decision) (string, bool) { + if h.dataStore == nil { + return "", false + } + // Clearing is read before removing on purpose: "всё купил" and "купил + // молоко" start with the same word, and only the second one names an item. + if list, ok := router.ParseListClear(dec.Utterance); ok { + n, err := h.dataStore.ClearList(ctx, list, h.now()) + if err != nil { + log.Printf("voice: clear list: %v", err) + return "не получилось обновить список.", true + } + if n == 0 { + return "в списке и так ничего не было.", true + } + return "вычеркнула всё, список пустой.", true + } + if cap, ok := router.ParseListRemove(dec.Utterance); ok { + if reply, ok := h.removeListItem(ctx, cap); ok { + return reply, true + } + // Nothing on the list by that name. "купил новый ноутбук" is a note and + // must stay one, so the turn goes back rather than claiming a removal + // that removed nothing. + return "", false + } + cap, ok := router.ParseListCapture(dec.Utterance) + if !ok { + return "", false + } + res, err := h.dataStore.AddListItem(ctx, store.ListItem{ + List: cap.List, + Item: cap.Item, + Source: "tap:voice", + CreatedTs: h.now(), + }) + if err != nil { + log.Printf("voice: add list item: %v", err) + return "не получилось добавить в список.", true + } + if !res.Created { + return cap.Item + " уже в списке.", true + } + return "добавила в список: " + cap.Item + ".", true +} + +// removeListItem crosses one named item off. It reports false when the list +// holds nothing by that name, which is what keeps the marker words from +// swallowing ordinary notes. +func (h *reactiveHandler) removeListItem(ctx context.Context, cap router.ListCapture) (string, bool) { + items, err := h.dataStore.ListItems(ctx, cap.List, "") + if err != nil { + log.Printf("voice: list items: %v", err) + return "", false + } + want := store.NormalizeTaskText(cap.Item) + for _, li := range items { + if store.NormalizeTaskText(li.Item) != want { + continue + } + if err := h.dataStore.SetListItemStatus(ctx, li.ID, store.ListItemDone, h.now()); err != nil { + log.Printf("voice: cross off list item: %v", err) + return "не получилось обновить список.", true + } + return "вычеркнула: " + li.Item + ".", true + } + return "", false +} + +// queryList — "что в списке покупок?", "что мне купить?". +// +// A query source, so it sits in querySources and either claims the turn or +// passes it on. It is before the recall sources for the reason every specific +// source is: the notes pass would otherwise answer a list question with +// whatever note is nearest. +func (h *reactiveHandler) queryList(ctx context.Context, t *queryTurn) (string, bool) { + list, ok := router.ParseListQuery(t.dec.Utterance) + if !ok || h.dataStore == nil { + return "", false + } + items, err := h.dataStore.ListItems(ctx, list, "") + if err != nil { + log.Printf("voice: list items: %v", err) + return "не получилось посмотреть список.", true + } + return formatListRU(list, items), true +} + +// formatListRU reads a list aloud. One sentence, comma-separated, because a +// shopping list is heard in a shop and a numbered recital is unusable there. +func formatListRU(list string, items []store.ListItem) string { + name := "списке " + listGenitive(list) + if len(items) == 0 { + return "в " + name + " пусто." + } + names := make([]string, 0, len(items)) + for _, li := range items { + names = append(names, li.Item) + } + return "в " + name + ": " + strings.Join(names, ", ") + "." +} + +// listGenitive puts a list tag into the case "список <…>" needs. Russian +// declines the noun and she must not say "в списке покупки". +func listGenitive(list string) string { + switch list { + case "покупки": + return "покупок" + case "аптека": + return "аптеки" + case "хозяйство": + return "хозяйства" + } + return list +} diff --git a/cmd/mavend/actions_note.go b/cmd/mavend/actions_note.go index c7c20a5..b552ddd 100644 --- a/cmd/mavend/actions_note.go +++ b/cmd/mavend/actions_note.go @@ -17,6 +17,12 @@ func (h *reactiveHandler) actionNote(ctx context.Context, dec router.Decision) s if reply, ok := h.captureTaskFromNote(ctx, dec); ok { return reply } + // A standing list is neither work nor recall (Vikunja #453). Checked here + // for the same reason and at the same cost: before the embedding is paid + // for, and it passes the turn straight back when no marker matches. + if reply, ok := h.captureListFromNote(ctx, dec); ok { + return reply + } // embed the note text with the same model the classifier uses, persist // via CoreAPI (source=tap:voice). Semantic recall lives in `notes`, not // facts — no predicate reads it (spec's two-memory split). diff --git a/cmd/mavend/actions_query.go b/cmd/mavend/actions_query.go index c349d2d..b0efef5 100644 --- a/cmd/mavend/actions_query.go +++ b/cmd/mavend/actions_query.go @@ -85,6 +85,11 @@ var querySources = []querySource{ // the money facts the poller wrote, and the notes pass would otherwise // answer it from whatever he once said about spending. Its matcher needs a // money noun plus an actual ask, so "я потратил весь день" is untouched. + // Next to "tasks" and for the same reason: "что мне купить?" is a question + // about the shopping list, and the recall pass would otherwise answer it + // from an old note about the shop. Its matcher needs an explicit list + // marker, so "надо бы съездить в магазин" is untouched. + {name: "list", answer: (*reactiveHandler).queryList}, {name: "money", answer: (*reactiveHandler).queryMoney}, // Before the recall sources and before general knowledge: "что нового?" is // a question about the feeds she reads, and general knowledge would answer From 6c67e6196225b378dd14195673d7bed73c7615a6 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:45:21 +0400 Subject: [PATCH 11/14] mavend: cover the spoken list path (V-453) --- cmd/mavend/actions_list_test.go | 184 ++++++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 cmd/mavend/actions_list_test.go diff --git a/cmd/mavend/actions_list_test.go b/cmd/mavend/actions_list_test.go new file mode 100644 index 0000000..31a94cd --- /dev/null +++ b/cmd/mavend/actions_list_test.go @@ -0,0 +1,184 @@ +package main + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/kami/maven/internal/router" + "github.com/kami/maven/internal/store" +) + +func listNow() time.Time { return time.Date(2026, 8, 4, 9, 0, 0, 0, time.UTC) } + +func listHandler(t *testing.T) *reactiveHandler { + t.Helper() + return &reactiveHandler{dataStore: newTestStore(t), now: listNow} +} + +func say(t *testing.T, h *reactiveHandler, utterance string) (string, bool) { + t.Helper() + return h.captureListFromNote(context.Background(), router.Decision{ + Intent: router.IntentNote, Utterance: utterance, + }) +} + +func TestListCaptureAddsAndReadsBack(t *testing.T) { + h := listHandler(t) + for _, u := range []string{"добавь в список покупок молоко", "добавь в список хлеб"} { + if reply, ok := say(t, h, u); !ok { + t.Fatalf("%q was not claimed (reply %q)", u, reply) + } + } + if reply, ok := say(t, h, "добавь в список покупок молоко"); !ok || !strings.Contains(reply, "уже") { + t.Errorf("second молоко replied %q, %v; want an already-there answer", reply, ok) + } + answer, ok := h.queryList(context.Background(), &queryTurn{ + dec: router.Decision{Intent: router.IntentQuery, Utterance: "что в списке покупок?"}, + }) + if !ok { + t.Fatal("the list question was not claimed") + } + if !strings.Contains(answer, "молоко") || !strings.Contains(answer, "хлеб") { + t.Errorf("answer %q; want both items", answer) + } + if strings.Contains(answer, "списке покупки") { + t.Errorf("answer %q declines the list name wrong", answer) + } +} + +// An utterance with no list marker is a note and must stay one, whichever half +// of the parser it brushes against. +func TestListCapturePassesOrdinaryNotes(t *testing.T) { + h := listHandler(t) + for _, u := range []string{ + "молоко закончилось", + "надо бы съездить в магазин", + "купил новый ноутбук", + "добавь в список покупок", + } { + if reply, ok := say(t, h, u); ok { + t.Errorf("%q was claimed as a list turn: %q", u, reply) + } + } +} + +func TestListCrossOffOneItemAndThenAll(t *testing.T) { + h := listHandler(t) + for _, u := range []string{ + "добавь в список покупок молоко", + "добавь в список покупок хлеб", + "добавь в список аптеки бинт", + } { + if _, ok := say(t, h, u); !ok { + t.Fatalf("%q was not claimed", u) + } + } + reply, ok := say(t, h, "вычеркни молоко") + if !ok || !strings.Contains(reply, "молоко") { + t.Fatalf("cross off replied %q, %v", reply, ok) + } + open, err := h.dataStore.ListItems(context.Background(), "покупки", "") + if err != nil { + t.Fatalf("list: %v", err) + } + if len(open) != 1 || open[0].Item != "хлеб" { + t.Fatalf("open list %+v; want only хлеб", open) + } + if reply, ok := say(t, h, "всё купил"); !ok || !strings.Contains(reply, "пустой") { + t.Errorf("clear replied %q, %v", reply, ok) + } + open, err = h.dataStore.ListItems(context.Background(), "покупки", "") + if err != nil { + t.Fatalf("list: %v", err) + } + if len(open) != 0 { + t.Errorf("%d items still open after всё купил", len(open)) + } + // The other list is untouched, and it is read back on its own. + answer, ok := h.queryList(context.Background(), &queryTurn{ + dec: router.Decision{Intent: router.IntentQuery, Utterance: "покажи список аптеки"}, + }) + if !ok || !strings.Contains(answer, "бинт") { + t.Errorf("аптека answer %q, %v; want бинт", answer, ok) + } +} + +func TestQueryListSaysWhenItIsEmpty(t *testing.T) { + h := listHandler(t) + answer, ok := h.queryList(context.Background(), &queryTurn{ + dec: router.Decision{Intent: router.IntentQuery, Utterance: "что мне купить?"}, + }) + if !ok { + t.Fatal("the list question was not claimed") + } + if !strings.Contains(answer, "пусто") { + t.Errorf("empty answer %q; want it to say so", answer) + } + if _, ok := h.queryList(context.Background(), &queryTurn{ + dec: router.Decision{Intent: router.IntentQuery, Utterance: "какие у меня задачи?"}, + }); ok { + t.Error("the list source claimed a task question") + } +} + +// Stage 0 answers a list turn without the model: the grammars route it, and the +// action handlers re-parse what the grammar matched. +func TestListGrammarsRouteWithoutTheModel(t *testing.T) { + cases := []struct { + utterance string + want router.Intent + }{ + {"добавь в список покупок молоко", router.IntentNote}, + {"что в списке покупок?", router.IntentQuery}, + {"всё купил", router.IntentNote}, + } + for _, c := range cases { + var got router.Intent + claimed := false + for _, g := range router.ListGrammars() { + m := g.Pattern.FindStringSubmatch(c.utterance) + if m == nil { + continue + } + if dec, ok := g.Build(m); ok { + got, claimed = dec.Intent, true + break + } + } + if !claimed { + t.Errorf("no list grammar claimed %q", c.utterance) + continue + } + if got != c.want { + t.Errorf("%q routed to %v; want %v", c.utterance, got, c.want) + } + } + for _, g := range router.ListGrammars() { + m := g.Pattern.FindStringSubmatch("напомни купить молоко завтра") + if m == nil { + continue + } + if _, ok := g.Build(m); ok { + t.Errorf("grammar %s claimed a reminder", g.Name) + } + } +} + +func TestListStoreSourceIsVoice(t *testing.T) { + h := listHandler(t) + if _, ok := say(t, h, "добавь в список покупок молоко"); !ok { + t.Fatal("not claimed") + } + items, err := h.dataStore.ListItems(context.Background(), "покупки", "") + if err != nil { + t.Fatalf("list: %v", err) + } + if len(items) != 1 || items[0].Source != "tap:voice" { + t.Errorf("stored %+v; want one row from tap:voice", items) + } + if items[0].Status != store.ListItemOpen { + t.Errorf("status %q; want open", items[0].Status) + } +} From 947506c7b80db5be9bdb1b5a0c9ad0ba2884f349 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:45:41 +0400 Subject: [PATCH 12/14] docs: a list is the fourth append-only shape (V-453) --- docs/design.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/design.md b/docs/design.md index a5db622..4d204ab 100644 --- a/docs/design.md +++ b/docs/design.md @@ -630,6 +630,31 @@ add a new principle; it applied the existing one at smaller and smaller scope. --- +## A list is the fourth shape + +Facts, notes and tasks were the three append-only shapes. `list_items` is the +fourth (Vikunja #453): an item, a status, and a list tag. + +It is not a task. Milk is not work, nothing prioritises it, and the ranker must +not start counting groceries as outstanding errands. It is not a fact either, +because it claims nothing about the world. What it is, is a set that grows and +shrinks. + +The property that makes the separate table worth it: no predicate reads a list. +Nothing ranks it, nothing nudges about it, the digestion worker ignores it. So +two people adding to the same list at once cost nothing — there is no order to +disagree about and no lifecycle past crossed-off. + +The unique index is the tasks one, per list, and live rows only. Saying "молоко" +twice before the shop is one line; saying it again next week, after the last one +was crossed off, is a new line. + +Spoken, it is four turns: add, read back, cross one item off, cross the lot off. +All four are matched deterministically in `internal/router/list.go` and all four +run at stage 0, because an add and a read-back are cheap and should not depend on +the resident model having a good turn. Crossing one item off claims the turn only +when the list holds that item, which is what keeps "купил новый ноутбук" a note. + ## Calendar Integration with **Radicale** (self-hosted CalDAV), not Nextcloud. Scope is From 0987dabfc40f6f3488d5960635df30ebdf72ba24 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:50:56 +0400 Subject: [PATCH 13/14] tool: risk tiers decide the confirm, not one boolean (V-449) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Destructive column was a mechanism with no policy behind it: nothing said which acts are destructive, whether a confirmed act stays confirmed, or what a new tool domain inherits, so each domain answered for itself. Three tiers, derived from the row rather than stored, so the answer can be argued with in one place instead of being whatever the last person to tick the checkbox believed. Safe runs. Destructive costs a confirm turn, every time — a confirmation binds one capability, one target and one argument list, and it dies with the parked turn. Irreversible is refused: a confirm turn there would be theatre, because the STT, the router and the fuzzy allowlist match are all guesses and a spoken "да" checks none of them. She names the gap; the row stays enabled. An unrecognised dispatch shape inherits destructive, not safe. A domain argues its way down to running freely, never up to being gated. --- cmd/mavend/actions_act.go | 6 ++ docs/design.md | 34 +++++++++ internal/tool/risk.go | 145 ++++++++++++++++++++++++++++++++++++++ internal/tool/tool.go | 23 +++++- 4 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 internal/tool/risk.go diff --git a/cmd/mavend/actions_act.go b/cmd/mavend/actions_act.go index 2b96855..f3395dc 100644 --- a/cmd/mavend/actions_act.go +++ b/cmd/mavend/actions_act.go @@ -51,6 +51,12 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st phrase := actPhrase(dec.Slots.Fn, dec.Slots.Args) h.park(dec.Slots.Fn, dec.Slots.Args, phrase) return "выполнить «" + phrase + "»? скажи «да» или «нет»." + case errors.Is(err, tool.ErrNeedsAuthedSurface): + // Irreversible (internal/tool/risk.go). A confirm turn would not + // help: everything that proposed this act — the STT, the router, + // the fuzzy allowlist match — is a guess, and a spoken "да" checks + // none of it. She names the gap instead. + return "это я из голоса не выполню — после него ничего не вернуть. запусти сам, если правда надо." case errors.Is(err, tool.ErrNotEnabled): return h.proposeGap(ctx, dec) case errors.Is(err, tool.ErrNotConnected), errors.Is(err, mcp.ErrNotConnected), errors.Is(err, mcp.ErrNoServer): diff --git a/docs/design.md b/docs/design.md index 4d204ab..f29a2ad 100644 --- a/docs/design.md +++ b/docs/design.md @@ -293,6 +293,40 @@ don't improvise.** Destructive ones still gate behind confirm. Misroute correction is append-only and grows the router's examples with use — same shape as `nudges.outcome` tuning cooldowns, no retrain. +#### Risk tiers, not one boolean + +`Destructive` on a tool row is one bit set by whoever ticked the checkbox on +`/tools`. It is a mechanism, and it never said which acts are destructive, +whether a confirmed act stays confirmed, or what a new tool domain inherits. +`internal/tool/risk.go` is the policy (Vikunja #449). The tier is DERIVED from +the row, not stored, so it can be argued with in one place instead of being +whatever the last person to enable the tool believed. + +| Tier | What it is | What it costs | +|---|---|---| +| `safe` | a read, or a change he can undo by saying the opposite | runs on first hearing | +| `destructive` | it changes something real and undoing it takes work | one confirm turn, every time | +| `irreversible` | the thing does not come back: a wipe, a format, a delete with no bin | voice may not authorise it at all | + +Three rules fall out, and they are the part that was missing: + +- **Which acts are destructive is not only the checkbox.** A house row always + is, because there is no read-only way to turn the heating off. A row whose + argv names one of the irreversible verbs always is, whatever the row says. +- **A confirmed act never stays confirmed.** At any tier. A confirmation binds + one capability, one target and one argument list, and it dies with the parked + turn (90s). "The same act again" is a new act. A sticky confirm is a standing + grant and nothing on the voice path may hold one. +- **A new domain inherits `destructive`, not `safe`.** A dispatch shape the + policy does not recognise gets the confirm turn. A domain argues its way down + to running freely; it never has to argue its way up to being gated. + +The irreversible tier is refused rather than asked about, because a confirm +turn would be theatre: everything that proposed the act — an STT guess, a +router guess, a fuzzy allowlist match — is a guess, and a spoken "да" checks +none of it. She names the gap and he runs it himself. The row stays enabled; +refusing to run it from voice is not the same as taking it off the allowlist. + --- ## Voice pipeline (STT / TTS) diff --git a/internal/tool/risk.go b/internal/tool/risk.go new file mode 100644 index 0000000..cb1ad38 --- /dev/null +++ b/internal/tool/risk.go @@ -0,0 +1,145 @@ +package tool + +import ( + "strings" + + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/mcp" + "github.com/kami/maven/internal/smarthome" +) + +// Risk tiers (Vikunja #449). +// +// What existed before this file was a mechanism and no policy: one +// `Destructive` boolean per row, set by whoever ticked the checkbox on /tools. +// Nothing said which acts are destructive, whether a confirmed act stays +// confirmed, or what a new tool domain inherits — so every domain answered +// those questions for itself, and two of them answered differently. +// +// The tiers below are the policy. They are derived from the row, not stored: +// a derivation can be argued with and corrected in one place, while a column +// is whatever the last person to enable the tool believed. +// +// The three questions, answered once: +// +// - WHICH ACTS ARE DESTRUCTIVE. A house row always is, because there is no +// read-only way to turn the heating off. A row whose argv names one of the +// irreversible verbs always is, whatever the checkbox says. Everything else +// is what the row was enabled as. +// - DOES A CONFIRMED ACT STAY CONFIRMED. No. Never, at any tier. A +// confirmation binds one capability, one target and one argument list, and +// it expires with the parked turn (confirmTTL, 90s). "Same act again" is a +// new act and costs a new turn. A sticky confirm is a standing grant, and +// nothing on the voice path may hold one. +// - WHAT A NEW DOMAIN INHERITS. The default is TierDestructive, not +// TierSafe. A dispatch shape this file does not recognise gets the confirm +// turn — a new domain must argue its way DOWN to running freely, never up +// to needing a confirm. +type Risk string + +const ( + // TierSafe — a read, or a mutation the owner can undo by saying the + // opposite. Runs on first hearing. + TierSafe Risk = "safe" + // TierDestructive — it changes something real and undoing it takes work. + // One confirm turn, every time, never remembered. + TierDestructive Risk = "destructive" + // TierIrreversible — the thing it acts on does not come back: a wipe, a + // format, a delete with no bin behind it. A confirm turn is not enough, + // because the whole chain that proposed it — an STT guess, a router guess, + // a fuzzy allowlist match — has a spoken "да" as its only check. She names + // the gap and he runs it himself. + TierIrreversible Risk = "irreversible" +) + +// Policy — what a tier requires of the act path. +// +// There is deliberately no "sticky for" field. Non-stickiness is the policy, +// and a knob that could turn it off would be the thing to argue with instead +// of the rule. +type Policy struct { + // Confirm — the act does not run on first hearing. + Confirm bool + // VoiceMayRun — a spoken confirmation is enough authority to run it. + VoiceMayRun bool +} + +// PolicyFor returns the requirements of a tier. An unknown tier is treated as +// destructive, for the same reason the default derivation is. +func PolicyFor(r Risk) Policy { + switch r { + case TierSafe: + return Policy{Confirm: false, VoiceMayRun: true} + case TierIrreversible: + return Policy{Confirm: true, VoiceMayRun: false} + default: + return Policy{Confirm: true, VoiceMayRun: true} + } +} + +// irreversibleVerbs — argv heads and subcommands that destroy the thing they +// name. Matched as whole argv elements, never as substrings: "rm" must not +// fire on "/usr/bin/rmdir-report" and "drop" must not fire on "dropbox". +// +// The list is short on purpose. It is not a sandbox and it does not try to be +// one — an enabled row can already run anything the daemon's user can run. +// What it is, is the set of words that mean "and then it is gone", so that the +// one act nobody can walk back is the one act a spoken "да" cannot authorise. +var irreversibleVerbs = map[string]bool{ + "rm": true, "rmdir": true, "shred": true, "srm": true, + "mkfs": true, "fdisk": true, "parted": true, "wipefs": true, + "dd": true, "format": true, + "drop": true, "drop-database": true, "destroy": true, "purge": true, + "prune": true, "truncate": true, +} + +// RiskOf derives the tier of an enabled tool row. +func RiskOf(t ipc.Tool) Risk { + if isIrreversible(t.Cmd) { + return TierIrreversible + } + // A house row is a physical change to the flat, and the confirm turn on it + // is structural rather than a column: /tools writes the checkbox straight + // through on enable, so unticking it once turned an unlock into a row that + // ran on first hearing. Nothing any surface writes removes the second turn + // from a physical device. + if _, _, ok := smarthome.ParseCmd(t.Cmd); ok { + return TierDestructive + } + // An MCP row is a call to somebody else's server. It is enabled with a + // fingerprint of what it declared at approval time (Vikunja #251), and the + // tier tracks the same flag every other row uses — the point of this branch + // is that it is NOT special-cased into running freely. + if _, _, ok := mcp.ParseCmd(t.Cmd); ok { + if t.Destructive { + return TierDestructive + } + return TierSafe + } + if t.Destructive { + return TierDestructive + } + if len(t.Cmd) == 0 { + // Not a shape this file knows how to read. The default is the confirm + // turn: a new domain argues its way down, not up. + return TierDestructive + } + return TierSafe +} + +// isIrreversible reports whether any argv element is one of the verbs that +// destroys what it names. Every element, not just the head: "sudo rm" and +// "docker volume prune" both hide the verb behind a wrapper. +func isIrreversible(cmd []string) bool { + for _, arg := range cmd { + word := strings.ToLower(strings.TrimSpace(arg)) + // Take the last path element, so /bin/rm reads as rm. + if i := strings.LastIndex(word, "/"); i >= 0 { + word = word[i+1:] + } + if irreversibleVerbs[word] { + return true + } + } + return false +} diff --git a/internal/tool/tool.go b/internal/tool/tool.go index d948d58..6b27924 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -67,6 +67,12 @@ var ( // proposal, and drafting a new proposal for a tool that already exists and // is enabled is a lie about what is wrong. ErrNotConnected = errors.New("tool is enabled but its backend is not connected") + // ErrNeedsAuthedSurface — the row is enabled and the act is understood, + // and its tier is one a spoken "да" may not authorise (risk.go, + // TierIrreversible). Held apart from ErrNeedsConfirm because there is no + // confirm turn that would help: asking again would imply the second answer + // changes the outcome. + ErrNeedsAuthedSurface = errors.New("tool is irreversible and voice may not authorise it") ) // MCPCaller is the seam for an act that is an MCP tool call rather than a @@ -121,7 +127,12 @@ func (e *Executor) WithHome(h HomeCaller) *Executor { // Exec looks up name in the store and runs Cmd+args as argv (no shell). // confirmed=true is the second turn of a destructive act (the user said "да"); // it bypasses the ErrNeedsConfirm gate. Non-enabled ⇒ ErrNotEnabled; a -// destructive tool with confirmed=false ⇒ ErrNeedsConfirm. +// destructive tool with confirmed=false ⇒ ErrNeedsConfirm; an irreversible one +// ⇒ ErrNeedsAuthedSurface, confirmed or not. +// +// Exec IS the voice path. Nothing else calls it, which is why the tier check +// needs no surface argument: the authority it can offer a tool is a spoken +// "да", and TierIrreversible says that is not enough. func (e *Executor) Exec(ctx context.Context, name string, args []string, confirmed bool) (string, error) { t, err := e.api.LookupTool(ctx, name) if errors.Is(err, ipc.ErrToolNotFound) { @@ -133,7 +144,15 @@ func (e *Executor) Exec(ctx context.Context, name string, args []string, confirm if t.Status != "enabled" { return "", ErrNotEnabled } - if t.Destructive && !confirmed { + // The tier decides, not the column (Vikunja #449). RiskOf reads the row and + // answers the three questions the boolean never did: which acts are + // destructive, whether a confirm sticks (it never does), and what an + // unrecognised shape inherits (the confirm turn). + policy := PolicyFor(RiskOf(t)) + if !policy.VoiceMayRun { + return "", ErrNeedsAuthedSurface + } + if policy.Confirm && !confirmed { return "", ErrNeedsConfirm } // An MCP row is a call to a configured server, not a process. Everything From 7db139b83ed44890e4f746f2a7f91ab678385593 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 04:50:56 +0400 Subject: [PATCH 14/14] tool, mavend: cover the tiers end to end (V-449) --- cmd/mavend/actions_act_risk_test.go | 73 ++++++++++++++++++++++++++ internal/tool/risk_test.go | 81 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 cmd/mavend/actions_act_risk_test.go create mode 100644 internal/tool/risk_test.go diff --git a/cmd/mavend/actions_act_risk_test.go b/cmd/mavend/actions_act_risk_test.go new file mode 100644 index 0000000..24fd271 --- /dev/null +++ b/cmd/mavend/actions_act_risk_test.go @@ -0,0 +1,73 @@ +package main + +import ( + "context" + "strings" + "testing" + + "github.com/kami/maven/internal/router" +) + +// The act path speaks each tier (Vikunja #449): a safe row runs, a destructive +// one costs a confirm turn, an irreversible one is refused with the reason. +func TestActPathSpeaksTheTiers(t *testing.T) { + h, st, _ := newClarifyHandler(t) + ctx := context.Background() + now := h.now() + for _, tc := range []struct { + name string + cmd []string + destructive bool + }{ + {"status", []string{"true"}, false}, + {"restart", []string{"true"}, true}, + {"wipe", []string{"rm", "-rf"}, true}, + } { + if _, err := st.ProposeTool(ctx, tc.name, "test", "homelab", now); err != nil { + t.Fatalf("propose %s: %v", tc.name, err) + } + if err := st.EnableTool(ctx, tc.name, tc.cmd, tc.destructive, "homelab", now); err != nil { + t.Fatalf("enable %s: %v", tc.name, err) + } + } + + act := func(fn string) string { + return h.actionAct(ctx, router.Decision{ + Intent: router.IntentAct, + Utterance: fn, + Slots: router.Slots{Fn: fn, HasFn: true}, + }) + } + + if reply := act("status"); !strings.HasPrefix(reply, "готово") { + t.Errorf("safe act replied %q; want it to have run", reply) + } + if reply := act("restart"); !strings.Contains(reply, "скажи «да»") { + t.Errorf("destructive act replied %q; want a confirm turn", reply) + } + // Clear the confirm the destructive act parked, so what is pending after + // the irreversible one is only what the irreversible one parked. + h.mu.Lock() + h.pending = nil + h.mu.Unlock() + + reply := act("wipe") + if strings.Contains(reply, "скажи «да»") { + t.Fatalf("irreversible act asked for a confirm: %q", reply) + } + if !strings.Contains(reply, "не вернуть") { + t.Errorf("irreversible act replied %q; want it to name the reason", reply) + } + // Nothing was parked, so a later "да" cannot pick it up. + h.mu.Lock() + pending := h.pending + h.mu.Unlock() + if pending != nil { + t.Errorf("an irreversible act parked %+v", pending) + } + // And it is still an enabled row — refusing to run it from voice is not + // the same as taking it off the allowlist. + if got, err := st.LookupTool(ctx, "wipe"); err != nil || got.Status != "enabled" { + t.Errorf("wipe is %+v, %v; want it still enabled", got, err) + } +} diff --git a/internal/tool/risk_test.go b/internal/tool/risk_test.go new file mode 100644 index 0000000..4662e1f --- /dev/null +++ b/internal/tool/risk_test.go @@ -0,0 +1,81 @@ +package tool + +import ( + "context" + "errors" + "testing" + + "github.com/kami/maven/internal/ipc" +) + +func TestRiskOfReadsTheRow(t *testing.T) { + cases := []struct { + name string + tool ipc.Tool + want Risk + }{ + {"a plain read", ipc.Tool{Cmd: []string{"systemctl", "status"}}, TierSafe}, + {"the checkbox", ipc.Tool{Cmd: []string{"systemctl", "restart"}, Destructive: true}, TierDestructive}, + {"a wipe", ipc.Tool{Cmd: []string{"rm", "-rf"}}, TierIrreversible}, + {"a wipe behind a wrapper", ipc.Tool{Cmd: []string{"sudo", "/bin/rm"}}, TierIrreversible}, + {"a prune behind a subcommand", ipc.Tool{Cmd: []string{"docker", "volume", "prune"}}, TierIrreversible}, + {"the house", ipc.Tool{Cmd: []string{"smarthome", "light.kitchen", "turn_off"}}, TierDestructive}, + {"the house with the box unticked", ipc.Tool{Cmd: []string{"smarthome", "lock.front", "unlock"}}, TierDestructive}, + {"an mcp read", ipc.Tool{Cmd: []string{"mcp", "vikunja", "list_tasks"}}, TierSafe}, + {"an mcp write", ipc.Tool{Cmd: []string{"mcp", "vikunja", "delete_task"}, Destructive: true}, TierDestructive}, + {"a shape nobody wrote yet", ipc.Tool{}, TierDestructive}, + } + for _, c := range cases { + if got := RiskOf(c.tool); got != c.want { + t.Errorf("%s: RiskOf = %q; want %q", c.name, got, c.want) + } + } +} + +// The default is the confirm turn. A tier this file does not know is not a +// tier that runs freely. +func TestPolicyForDefaultsToConfirming(t *testing.T) { + for _, r := range []Risk{TierDestructive, Risk("whatever-lands-here-next")} { + p := PolicyFor(r) + if !p.Confirm || !p.VoiceMayRun { + t.Errorf("PolicyFor(%q) = %+v; want a confirm turn she may run", r, p) + } + } + if p := PolicyFor(TierSafe); p.Confirm || !p.VoiceMayRun { + t.Errorf("PolicyFor(safe) = %+v; want it to run", p) + } + if p := PolicyFor(TierIrreversible); !p.Confirm || p.VoiceMayRun { + t.Errorf("PolicyFor(irreversible) = %+v; want voice refused", p) + } +} + +// An irreversible act is refused whether or not he said "да", because there is +// no second answer that changes what it would do. +func TestExecRefusesIrreversibleEvenConfirmed(t *testing.T) { + api := fakeAPI{tools: map[string]ipc.Tool{ + "wipe": {Name: "wipe", Status: "enabled", Cmd: []string{"rm", "-rf"}, Destructive: true}, + }} + e := NewExecutor(api, 0) + ran := false + e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil } + for _, confirmed := range []bool{false, true} { + if _, err := e.Exec(context.Background(), "wipe", []string{"/data"}, confirmed); !errors.Is(err, ErrNeedsAuthedSurface) { + t.Errorf("confirmed=%v: %v; want ErrNeedsAuthedSurface", confirmed, err) + } + } + if ran { + t.Fatal("an irreversible act ran from the voice path") + } +} + +// A row with no cmd at all is not a shape this file reads, and it must not +// slide through as safe. +func TestExecConfirmsAnUnreadableRow(t *testing.T) { + api := fakeAPI{tools: map[string]ipc.Tool{ + "mystery": {Name: "mystery", Status: "enabled"}, + }} + e := NewExecutor(api, 0) + if _, err := e.Exec(context.Background(), "mystery", nil, false); !errors.Is(err, ErrNeedsConfirm) { + t.Errorf("%v; want ErrNeedsConfirm", err) + } +}