diff --git a/cmd/mavend/actions_act.go b/cmd/mavend/actions_act.go index 374fd4f..16211f3 100644 --- a/cmd/mavend/actions_act.go +++ b/cmd/mavend/actions_act.go @@ -52,6 +52,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 phraser.A(phraser.ActConfirm, map[string]string{"name": 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/cmd/mavend/actions_act_risk_test.go b/cmd/mavend/actions_act_risk_test.go new file mode 100644 index 0000000..31447f1 --- /dev/null +++ b/cmd/mavend/actions_act_risk_test.go @@ -0,0 +1,75 @@ +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) + } + // PR 112's review cut «скажи «да» или «нет».» — he knows how to answer a + // yes/no question — so the confirm turn is recognised by the question. + 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/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_list_test.go b/cmd/mavend/actions_list_test.go new file mode 100644 index 0000000..6e8c288 --- /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 askList(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 := askList(t, h, u); !ok { + t.Fatalf("%q was not claimed (reply %q)", u, reply) + } + } + if reply, ok := askList(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 := askList(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 := askList(t, h, u); !ok { + t.Fatalf("%q was not claimed", u) + } + } + reply, ok := askList(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 := askList(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 := askList(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) + } +} diff --git a/cmd/mavend/actions_note.go b/cmd/mavend/actions_note.go index f8b79b5..5d90b12 100644 --- a/cmd/mavend/actions_note.go +++ b/cmd/mavend/actions_note.go @@ -18,6 +18,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 dd4ef47..eb54c00 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 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/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 4f5dbaa..ab5987b 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -177,6 +177,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/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index 8bf2361..363a17e 100644 --- a/cmd/mavend/voicewire.go +++ b/cmd/mavend/voicewire.go @@ -382,6 +382,7 @@ func buildRouter(emb router.Embedder, acts router.ActMatcher, threshold float64, // After the agenda rules: "расскажи, что у меня сегодня" is an agenda // question first and a narrative request second (Vikunja #498). grammars = append(grammars, router.NarrativeQueryGrammars()...) + grammars = append(grammars, router.ListGrammars()...) grammars = append(grammars, router.ReminderGrammar()) return router.New(router.Config{ Grammars: grammars, 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 diff --git a/docs/design.md b/docs/design.md index a5db622..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) @@ -630,6 +664,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 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/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, ""} diff --git a/internal/router/agenda_test.go b/internal/router/agenda_test.go index 7ee8420..19ad16f 100644 --- a/internal/router/agenda_test.go +++ b/internal/router/agenda_test.go @@ -99,6 +99,31 @@ func TestNarrativeGrammarsRouteToQuery(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) + } + } +} + // A narrative verb next to a capture verb is him asking for a note. Stage 0 // declines and the extractor gets its turn. func TestNarrativeGrammarLeavesCapturesAlone(t *testing.T) { @@ -112,3 +137,22 @@ func TestNarrativeGrammarLeavesCapturesAlone(t *testing.T) { t.Errorf("stage 0 claimed a capture: %+v", d) } } + +// 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 2de1e25..68d5b1d 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/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 + }, + }, + } +} 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") + } +} 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 } diff --git a/internal/router/stage0.go b/internal/router/stage0.go index b02a3be..fac9fb6 100644 --- a/internal/router/stage0.go +++ b/internal/router/stage0.go @@ -182,6 +182,29 @@ 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, + }, } } @@ -252,6 +275,12 @@ func narrativeQueryBuild(m []string) (Decision, bool) { return agendaQueryBuild(m) } +// 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. 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/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) + } +} diff --git a/internal/store/migrations.go b/internal/store/migrations.go index a266efe..8a34ed2 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -208,6 +208,38 @@ 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. + `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 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) + } +} 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/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) + } +} 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