diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index 591ff9a..1604d9d 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -106,11 +106,11 @@ func trimClarifyExpired(s string) string { // out, and "" when nothing was parked. Call it right after // resolveClarifyAnswer: a live question is answered there, an expired one is // only reported here — the words themselves still go on to be routed fresh. -func (h *reactiveHandler) clarifyExpiredNotice() string { +func (h *reactiveHandler) clarifyExpiredNotice(ctx context.Context) string { if h.clarifyStore == nil { return "" } - if !h.clarifyStore.TakeExpired(voiceDialogueID, h.now()) { + if !h.clarifyStore.TakeExpired(dialogueIDOf(ctx), h.now()) { return "" } log.Printf("voice: clarify — parked question expired, telling him and routing the words fresh") @@ -157,7 +157,7 @@ func clarifyQuestion(dec router.Decision) (dialogue.Slot, string, bool) { // askClarify parks the request and returns the question to ask instead of the // canned "не поняла". Returns ("", false) when there is nothing to ask about, so // the caller falls back to the canned reply. -func (h *reactiveHandler) askClarify(dec router.Decision) (string, bool) { +func (h *reactiveHandler) askClarify(ctx context.Context, dec router.Decision) (string, bool) { if h.clarifyStore == nil { return "", false } @@ -165,7 +165,7 @@ func (h *reactiveHandler) askClarify(dec router.Decision) (string, bool) { if !ok { return "", false } - h.clarifyStore.Put(voiceDialogueID, &dialogue.PendingQuestion{ + h.clarifyStore.Put(dialogueIDOf(ctx), &dialogue.PendingQuestion{ Intent: dialogue.Intent(dec.Intent), Slots: toDialogueSlots(dec.Slots), Missing: []dialogue.Slot{slot}, @@ -192,7 +192,7 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) if h.clarifyStore == nil { return "", false } - q := h.clarifyStore.Get(voiceDialogueID, h.now()) + q := h.clarifyStore.Get(dialogueIDOf(ctx), h.now()) if q == nil { return "", false } @@ -206,9 +206,9 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) // would fire at 11:00 saying "напомни" and nothing else. q.Utterance = foldAnswerIntoUtterance(q.Utterance, merged.Text) if len(dialogue.StillMissing(q.Missing, merged)) > 0 { - return h.reaskOrGiveUp(q, merged, text), true + return h.reaskOrGiveUp(ctx, q, merged, text), true } - h.clarifyStore.Delete(voiceDialogueID) + h.clarifyStore.Delete(dialogueIDOf(ctx)) // One gap filled is not the same as a complete request. askClarify parks // only the first gap, because one question per turn is the rule, but a @@ -217,7 +217,7 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) // a reminder with no time, which answered "не получилось разобрать время // напоминания." — an error for a request she never finished asking about. // Re-enter the loop instead, one question at a time as before. - if reply, asked := h.askRemainingGap(q, intent, merged); asked { + if reply, asked := h.askRemainingGap(ctx, q, intent, merged); asked { return reply, true } @@ -261,7 +261,7 @@ func foldAnswerIntoUtterance(utterance, subject string) string { // The attempt budget is shared with the re-ask path on purpose. A second gap // costs a question exactly like a second try at the first one does, so the cap // still bounds how many times she can speak before acting or letting go. -func (h *reactiveHandler) askRemainingGap(q *dialogue.PendingQuestion, intent router.Intent, merged dialogue.Slots) (string, bool) { +func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.PendingQuestion, intent router.Intent, merged dialogue.Slots) (string, bool) { remaining := dialogue.StillMissing(wantedSlots[intent], merged) if len(remaining) == 0 { return "", false @@ -270,7 +270,7 @@ func (h *reactiveHandler) askRemainingGap(q *dialogue.PendingQuestion, intent ro if !ok || !q.CanAsk() { return "", false } - h.clarifyStore.Put(voiceDialogueID, &dialogue.PendingQuestion{ + h.clarifyStore.Put(dialogueIDOf(ctx), &dialogue.PendingQuestion{ Intent: q.Intent, Slots: merged, Missing: []dialogue.Slot{remaining[0]}, @@ -287,13 +287,13 @@ func (h *reactiveHandler) askRemainingGap(q *dialogue.PendingQuestion, intent ro // reaskOrGiveUp handles an answer that left the gap open: ask the same question // again while she has attempts left, otherwise say she did not understand and // let the request go. Never returns "" — a mute give-up reads as "done". -func (h *reactiveHandler) reaskOrGiveUp(q *dialogue.PendingQuestion, merged dialogue.Slots, text string) string { +func (h *reactiveHandler) reaskOrGiveUp(ctx context.Context, q *dialogue.PendingQuestion, merged dialogue.Slots, text string) string { question := "" if len(q.Missing) > 0 { question = clarifyQuestions[q.Missing[0]] } if question == "" || !q.CanAsk() { - h.clarifyStore.Delete(voiceDialogueID) + h.clarifyStore.Delete(dialogueIDOf(ctx)) log.Printf("voice: clarify — gave up on %v after %d question(s), answer was %q", q.Missing, q.Attempts, text) return clarifyGaveUp } @@ -302,7 +302,7 @@ func (h *reactiveHandler) reaskOrGiveUp(q *dialogue.PendingQuestion, merged dial q.Slots = merged q.Attempts++ q.Asked = h.now() - h.clarifyStore.Put(voiceDialogueID, q) + h.clarifyStore.Put(dialogueIDOf(ctx), q) log.Printf("voice: clarify — answer %q did not fill %v, asking again (attempt %d)", text, q.Missing, q.Attempts) return question } diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index 8b377e1..cd879f8 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -81,7 +81,7 @@ func TestClarifyReminderCompletesOnAnswer(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - question, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")) + question, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")) if !asked || question != "Когда?" { t.Fatalf("expected the time question, got %q asked=%v", question, asked) } @@ -112,7 +112,7 @@ func TestClarifyFactCompletesOnAnswer(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentFact, router.Slots{Text: "запиши"}, "запиши")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentFact, router.Slots{Text: "запиши"}, "запиши")); !asked { t.Fatal("a fact with no key should be asked about") } if reply, handled := h.resolveClarifyAnswer(ctx, "пил воду"); !handled || reply == clarifyGaveUp { @@ -128,7 +128,7 @@ func TestClarifyAnswerAfterTTLIsANewRequest(t *testing.T) { ctx := context.Background() h, st, now := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } *now = now.Add(clarifyTTL + time.Second) @@ -147,7 +147,7 @@ func TestClarifyAsksThreeTimesThenSaysSo(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a first question") } // Two more unclear answers ⇒ two more questions (3 asks in total). @@ -185,7 +185,7 @@ func TestClarifyMaxAttemptsIsConfigurable(t *testing.T) { h, _, _ := newClarifyHandler(t) h.clarifyMaxAttempts = 1 - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } if reply, handled := h.resolveClarifyAnswer(ctx, "ну не знаю"); !handled || reply != clarifyGaveUp { @@ -199,7 +199,7 @@ func TestClarifyRestatedAnswerWins(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")); !asked { t.Fatal("expected a question") } // First answer parses, but re-park it by hand as if she had asked again: @@ -232,7 +232,7 @@ func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) { h, st, _ := newClarifyHandler(t) marker := filepath.Join(t.TempDir(), "not-allowed-ran") - if _, asked := h.askClarify(clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { t.Fatal("an act with no fn should be asked about") } reply, handled := h.resolveClarifyAnswer(ctx, "rm "+marker) @@ -260,7 +260,7 @@ func TestClarifiedDestructiveActStillNeedsConfirm(t *testing.T) { t.Fatal(err) } - if _, asked := h.askClarify(clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { t.Fatal("expected a question") } reply, handled := h.resolveClarifyAnswer(ctx, "delete_backups") @@ -284,7 +284,7 @@ func TestNoQuestionWhenNothingIsMissing(t *testing.T) { clarifyDec(router.IntentQuery, router.Slots{Text: "ммм"}, "ммм"), clarifyDec(router.IntentNote, router.Slots{Text: "..."}, "..."), } { - if question, asked := h.askClarify(dec); asked { + if question, asked := h.askClarify(context.Background(), dec); asked { t.Fatalf("intent %s should keep the canned reply, got %q", dec.Intent, question) } } @@ -296,29 +296,29 @@ func TestNoQuestionWhenNothingIsMissing(t *testing.T) { // TestClarifyExpiryIsAnnouncedAndWordsStillRoute — his answer lands after the // TTL: she must say the old request is gone AND still answer the new words. func TestClarifyExpiryIsAnnouncedAndWordsStillRoute(t *testing.T) { - ctx := context.Background() + ctx := withDialogueID(context.Background(), dialogueIDFor(sourceText, "")) h, _, now := newClarifyHandler(t) emb := router.NewHashEmbedder(1024) h.embedder = emb h.router = buildRouter(emb, h.matcher, 0.55, nil) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } *now = now.Add(clarifyTTL + time.Second) - reply := h.handleText(ctx, "как дела") + reply := h.handleText(ctx, "", "как дела") if !isClarifyExpired(reply) { t.Fatalf("expired question must be announced first, got %q", reply) } if trimClarifyExpired(reply) == "" { t.Fatalf("the new words must still be answered, got only the notice: %q", reply) } - if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + if h.clarifyStore.Get(textDialogueID, h.now()) != nil { t.Fatal("the expired question must be gone") } // The notice is said once, not on every later utterance. - if reply := h.handleText(ctx, "как дела"); isClarifyExpired(reply) { + if reply := h.handleText(ctx, "", "как дела"); isClarifyExpired(reply) { t.Fatalf("notice repeated on a later turn: %q", reply) } } @@ -340,7 +340,7 @@ func TestClarifyAsksAboutTheSecondGapToo(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - question, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{}, "напомни")) + question, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{}, "напомни")) if !asked || question != "О чём напомнить?" { t.Fatalf("expected the subject question, got %q asked=%v", question, asked) } @@ -380,7 +380,7 @@ func TestClarifySecondGapRespectsTheAttemptCap(t *testing.T) { h, _, _ := newClarifyHandler(t) h.clarifyMaxAttempts = 1 - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked { t.Fatal("expected the subject question") } reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме") @@ -440,10 +440,10 @@ func TestClarifyProseHoldsThePersona(t *testing.T) { // The confirm turn used to return before the notice was even computed, so he // answered the confirm and never heard that the older request was let go. func TestExpiryNoticeSurvivesAConfirmTurn(t *testing.T) { - ctx := context.Background() + ctx := withDialogueID(context.Background(), dialogueIDFor(sourceText, "")) h, _, now := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } // A confirm parked with a longer life than the question, so only the @@ -451,7 +451,7 @@ func TestExpiryNoticeSurvivesAConfirmTurn(t *testing.T) { h.pending = &pendingAct{fn: "delete_backups", phrase: "удалить бэкапы", expiry: now.Add(time.Hour)} *now = now.Add(clarifyTTL + time.Second) - reply := h.handleText(ctx, "нет") + reply := h.handleText(ctx, "", "нет") if !isClarifyExpired(reply) { t.Fatalf("the expired question must be announced on a confirm turn too, got %q", reply) } @@ -461,7 +461,7 @@ func TestExpiryNoticeSurvivesAConfirmTurn(t *testing.T) { if h.pending != nil { t.Fatal("the confirm must still have been consumed") } - if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + if h.clarifyStore.Get(textDialogueID, h.now()) != nil { t.Fatal("the expired question must be gone") } } @@ -476,7 +476,7 @@ func TestClarifySubjectAnswerFillsRatherThanClobbers(t *testing.T) { h, st, _ := newClarifyHandler(t) at := h.now().Add(2 * time.Hour) - question, asked := h.askClarify(clarifyDec(router.IntentReminder, + question, asked := h.askClarify(ctx, 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) @@ -501,3 +501,32 @@ func TestClarifySubjectAnswerFillsRatherThanClobbers(t *testing.T) { t.Fatalf("the answer clobbered the original request: %q", reminders[0].Payload) } } + +// TestClarifyIsPerConversation — the parked question belongs to the reach that +// was asked. Before this the clarify store had one global key, so a question +// asked in the web chat and never answered captured the next utterance from +// telegram, or from the mic, and answered it against a request the speaker had +// never made (Vikunja #466). +func TestClarifyIsPerConversation(t *testing.T) { + h, _, _ := newClarifyHandler(t) + web := withDialogueID(context.Background(), dialogueIDFor(sourceText, "web")) + telegram := withDialogueID(context.Background(), dialogueIDFor(sourceText, "telegram:42")) + + if _, asked := h.askClarify(web, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + t.Fatal("expected a question on the web conversation") + } + if _, handled := h.resolveClarifyAnswer(telegram, "в 11:00"); handled { + t.Fatal("a question asked on the web must not eat a telegram utterance") + } + if _, handled := h.resolveClarifyAnswer(voiceCtx(), "в 11:00"); handled { + t.Fatal("a question asked on the web must not eat what he says at the mic") + } + if reply, handled := h.resolveClarifyAnswer(web, "в 11:00"); !handled || reply == clarifyGaveUp { + t.Fatalf("the asker's own answer must land, handled=%v reply=%q", handled, reply) + } +} + +// voiceCtx — the mic's conversation, which carries no id of its own. +func voiceCtx() context.Context { + return withDialogueID(context.Background(), dialogueIDFor(sourceVoice, "")) +} diff --git a/cmd/mavend/followup.go b/cmd/mavend/followup.go index a8b0d75..2ec0f49 100644 --- a/cmd/mavend/followup.go +++ b/cmd/mavend/followup.go @@ -1,17 +1,64 @@ package main import ( + "context" "time" "github.com/kami/maven/internal/dialogue" "github.com/kami/maven/internal/router" ) -// voiceDialogueID — the single dialogue-session key. This is a single-user box -// (ponytail), so one slot suffices; a second speaker would need per-speaker ids, -// which waits on voice-print attribution (see PROGRESS multi-user deferral). +// voiceDialogueID — the dialogue-session key for the microphone, and the +// clarify key for it too. This is a single-user box (ponytail), so one slot +// suffices; a second speaker would need per-speaker ids, which waits on +// voice-print attribution (see PROGRESS multi-user deferral). const voiceDialogueID = "voice" +// textDialogueID — the clarify key for a text turn that named no conversation. +// Separate from the mic: an old client that sends no id still must not answer +// a question she asked out loud. +const textDialogueID = "text" + +// dialogueKey — the context key carrying the id of the conversation this turn +// belongs to. It rides the context rather than a parameter for the same reason +// the correlation id does: every step of the turn needs it, most of them only +// to hand to the next one, and threading it by hand would put it in six +// clarify signatures that have nothing else to say about it. +type dialogueKey struct{} + +// dialogueIDFor builds the id a turn is held under: the conversation the reach +// named, qualified by the tap it arrived on, or the tap's own fallback when it +// named none. +// +// A parked clarifying question used to be held under voiceDialogueID no matter +// where the turn came from, so one unanswerable question captured the next +// three utterances from anywhere. Three independent curl sessions fed a +// capture attempt that had already failed, and a reminder among them was lost +// (Vikunja #466). +func dialogueIDFor(src turnSource, conversation string) string { + if conversation != "" { + return string(src) + ":" + conversation + } + if src == sourceVoice { + return voiceDialogueID + } + return textDialogueID +} + +// withDialogueID tags a turn with that id. +func withDialogueID(ctx context.Context, id string) context.Context { + return context.WithValue(ctx, dialogueKey{}, id) +} + +// dialogueIDOf reads it back. Falls back to the microphone's slot, which is +// what an unthreaded caller — a test, an internal replay — gets. +func dialogueIDOf(ctx context.Context) string { + if id, ok := ctx.Value(dialogueKey{}).(string); ok && id != "" { + return id + } + return voiceDialogueID +} + // toDialogueSlots and applyDialogueSlots are the only bridge between // router.Slots and dialogue.Slots. dialogue must not import router (import // cycle), so the two structs are hand-kept copies and every field has to be diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index 8926a47..75d5600 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -986,7 +986,7 @@ type daemonAPI struct { getTrace func() *loop.TickTrace getMorningStatus func(ctx context.Context) []ipc.MorningRoutineStatus getDayPlan func(ctx context.Context) ipc.DayPlan - chatFn func(ctx context.Context, text string) string + chatFn func(ctx context.Context, conversation, text string) string getMCPServers func() []ipc.MCPServerStatus getEvents func(n int) []ipc.IntakeEvent } @@ -1002,11 +1002,11 @@ func (d *daemonAPI) RecentEvents(ctx context.Context, n int) ([]ipc.IntakeEvent, return d.getEvents(n), nil } -func (d *daemonAPI) Chat(ctx context.Context, text string) (string, error) { +func (d *daemonAPI) Chat(ctx context.Context, conversation, text string) (string, error) { if d.chatFn == nil { return "", errors.New("mavend: chat not available") } - return d.chatFn(ctx, text), nil + return d.chatFn(ctx, conversation, text), nil } // MCPServers — the configured MCP servers and their health (Vikunja #251). diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index c86a09a..eb8be4c 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -220,9 +220,9 @@ func (h *reactiveHandler) upgradeAPI(api ipc.CoreAPI) { // handleText — the core reactive path without stt/tts. Used by the IPC Chat // endpoint (and eventually by telegram). Splits out the audio bookends from // HandlePushToTalk so text channels share the same routing logic. -func (h *reactiveHandler) handleText(ctx context.Context, text string) string { +func (h *reactiveHandler) handleText(ctx context.Context, conversation, text string) string { log.Printf("voice: handleText: %q", text) - return h.runTurn(ctx, text, sourceText) + return h.runTurn(withDialogueID(ctx, dialogueIDFor(sourceText, conversation)), text, sourceText) } // turnSource — which channel this utterance arrived on, in the same provenance @@ -255,7 +255,7 @@ func (h *reactiveHandler) runTurn(ctx context.Context, text string, src turnSour // early. He can be asked a question, walk off, come back and say "да" to a // confirm that is still parked; computing the notice after that return meant // he answered the confirm and never heard that the older request was let go. - expiredNotice := h.clarifyExpiredNotice() + expiredNotice := h.clarifyExpiredNotice(ctx) // 2. confirm turn — if a destructive act is parked, this utterance is its // y/n answer, not a fresh command. Handled before routing so "да" doesn't @@ -351,7 +351,7 @@ func (h *reactiveHandler) runTurn(ctx context.Context, text string, src turnSour // and park the request (clarify.go); otherwise the replier's canned reply // stands. if dec.Clarify { - if question, asked := h.askClarify(dec); asked { + if question, asked := h.askClarify(ctx, dec); asked { return withNotice(expiredNotice, question) } } diff --git a/cmd/mavweb/handlers_test.go b/cmd/mavweb/handlers_test.go index 6b56c51..53aa286 100644 --- a/cmd/mavweb/handlers_test.go +++ b/cmd/mavweb/handlers_test.go @@ -77,7 +77,7 @@ func (f *fakeCore) MCPServers(context.Context) ([]ipc.MCPServerStatus, error) { return f.mcpServers, f.mcpErr } -func (f *fakeCore) Chat(_ context.Context, text string) (string, error) { +func (f *fakeCore) Chat(_ context.Context, _, text string) (string, error) { f.chatText = text if f.chatErr != nil { return "", f.chatErr diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index be7e5eb..a80d951 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -1678,7 +1678,13 @@ func handleChatAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, ses http.Redirect(w, r, "/chat", http.StatusSeeOther) return } - reply, err := core.Chat(r.Context(), text) + // One conversation id for the whole web chat, and a different one from + // telegram or the mic. A parked question belongs to the reach that was + // asked; before this, a clarify nobody answered on the web ate the next + // utterance spoken at the mic (Vikunja #466). This server has no + // per-browser session, so every browser tab is the same conversation — + // which is right for a single-owner box. + reply, err := core.Chat(r.Context(), "web", text) if err != nil { log.Printf("chat api: %v", err) http.Redirect(w, r, "/chat", http.StatusSeeOther) diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 8de46ac..f84b564 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -311,7 +311,7 @@ func TestGate_IpcServer_CheckWiredThroughSocket(t *testing.T) { if fake.writes != 0 { t.Errorf("auth refused but CoreAPI was called %d time(s); refused calls must not reach CoreAPI", fake.writes) } - _, err = cli.Chat(context.Background(), "привет") + _, err = cli.Chat(context.Background(), "web", "привет") if !errors.Is(err, ipc.ErrForbidden) { t.Errorf("wire: chat from unenrolled uid = %v; want ipc.ErrForbidden", err) } @@ -344,7 +344,7 @@ func TestGate_IpcServer_ChatAllowedForEnrolledCaller(t *testing.T) { t.Fatalf("dial: %v", err) } t.Cleanup(func() { _ = cli.Close() }) - reply, err := cli.Chat(context.Background(), "привет") + reply, err := cli.Chat(context.Background(), "web", "привет") if err != nil { t.Fatalf("Chat: %v", err) } @@ -373,7 +373,7 @@ func (r *recordingAPI) WriteFact(_ context.Context, _ ipc.WriteFactReq) (int64, return int64(r.writes), nil } -func (r *recordingAPI) Chat(_ context.Context, text string) (string, error) { +func (r *recordingAPI) Chat(_ context.Context, _, text string) (string, error) { r.chats++ return "echo: " + text, nil } diff --git a/internal/ipc/api.go b/internal/ipc/api.go index 1584055..eb3def0 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -593,8 +593,14 @@ type MCPServerStatus struct { } // chatReq / chatResp — text chat round-trip for the IPC Chat method. +// +// Conversation names the thread this utterance belongs to: a mavweb session, a +// telegram chat. It is opaque to the daemon and only has to be stable for one +// conversation and distinct across them. Empty is allowed and means "the +// unattributed text tap", which is what an old client sends. type chatReq struct { - Text string `json:"text"` + Text string `json:"text"` + Conversation string `json:"conversation,omitempty"` } type chatResp struct { Reply string `json:"reply"` @@ -759,7 +765,12 @@ type CoreAPI interface { // Chat routes a text utterance through the reactive handler's core path // (router → dialogue → action → replier) and returns the reply text. // No audio or stt/tts — for text channels (mavweb, telegram). - Chat(ctx context.Context, text string) (string, error) + // + // conversation names the thread. A parked clarifying question is held per + // conversation, so an unanswered question on one reach cannot eat the next + // utterance from another (Vikunja #466). Empty means the unattributed text + // tap and is still one conversation of its own, separate from the mic. + Chat(ctx context.Context, conversation, text string) (string, error) // RecentEvents returns the daemon's unified intake journal, newest first // (Vikunja #283) — one envelope per thing that arrived, whatever direction diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 5d3bd38..4cba25c 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -623,9 +623,9 @@ func (c *Client) AcceptProposedRoutine(ctx context.Context, id int64) error { return c.call(ctx, MethodAcceptProposedRoutine, acceptProposedRoutineReq{ID: id}, nil) } -func (c *Client) Chat(ctx context.Context, text string) (string, error) { +func (c *Client) Chat(ctx context.Context, conversation, text string) (string, error) { var r chatResp - if err := c.call(ctx, MethodChat, chatReq{Text: text}, &r); err != nil { + if err := c.call(ctx, MethodChat, chatReq{Text: text, Conversation: conversation}, &r); err != nil { return "", err } return r.Reply, nil diff --git a/internal/ipc/ipc_test.go b/internal/ipc/ipc_test.go index 5ba6654..5b8ca2b 100644 --- a/internal/ipc/ipc_test.go +++ b/internal/ipc/ipc_test.go @@ -401,7 +401,7 @@ func TestChatViaClient(t *testing.T) { } t.Cleanup(func() { _ = cli.Close() }) - reply, err := cli.Chat(context.Background(), "привет") + reply, err := cli.Chat(context.Background(), "web", "привет") if err != nil { t.Fatalf("Chat: %v", err) } @@ -417,7 +417,7 @@ type chatTestAPI struct { UnimplementedCoreAPI } -func (a *chatTestAPI) Chat(ctx context.Context, text string) (string, error) { +func (a *chatTestAPI) Chat(ctx context.Context, _, text string) (string, error) { if text == "привет" { return "и тебе привет!", nil } diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 8e991e4..f9b2692 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -240,7 +240,7 @@ func (a *storeAPI) RevertFact(ctx context.Context, key string) (int64, error) { return newID, mapErr(err) } -func (a *storeAPI) Chat(ctx context.Context, text string) (string, error) { +func (a *storeAPI) Chat(ctx context.Context, conversation, text string) (string, error) { return "", errors.New("store: chat not available via direct store API") } @@ -974,7 +974,7 @@ var methodTable = map[Method]handlerFunc{ return map[string]int64{"new_id": newID}, nil }), MethodChat: withParams(func(ctx context.Context, api CoreAPI, p chatReq) (chatResp, error) { - reply, err := api.Chat(ctx, p.Text) + reply, err := api.Chat(ctx, p.Conversation, p.Text) return chatResp{Reply: reply}, err }), MethodTickTrace: withoutParams(func(ctx context.Context, api CoreAPI) (TickTrace, error) { diff --git a/internal/ipc/unimplemented.go b/internal/ipc/unimplemented.go index 4aea172..45575f3 100644 --- a/internal/ipc/unimplemented.go +++ b/internal/ipc/unimplemented.go @@ -141,6 +141,6 @@ func (UnimplementedCoreAPI) MCPServers(ctx context.Context) ([]MCPServerStatus, func (UnimplementedCoreAPI) DayPlan(ctx context.Context) (DayPlan, error) { return DayPlan{}, ErrNotImplemented } -func (UnimplementedCoreAPI) Chat(ctx context.Context, text string) (string, error) { +func (UnimplementedCoreAPI) Chat(ctx context.Context, conversation, text string) (string, error) { return "", ErrNotImplemented }