tasks: key derived captures by external id and record who resolved

A task extracted from mail deduped on the live-norm index only, so once he finished it the row left the live set and the next poll of the same immutable message re-extracted it as a fresh candidate. mavmaild is a read-only reader and marks nothing read, so that repeats forever. Derived rows now carry an ext_id built from the message uid and the extracted span, unique across every status, while voice keeps live-only norm dedupe because saying an errand again is the recurrence signal. A derived source can no longer capture straight to open, and saying a task out loud that Maven had only proposed promotes the candidate instead of answering that it is already in the list. SetTaskStatus was classified AuthRead. Resolving a task is not additive, it erases work off his list, so it is a write, and the row now records the caller that moved it. ListTasks was unbounded. The list-query matcher claimed any utterance with "что мне делать", including "с чем мне помочь", and the urgency stripper matched inside words. Found in review of #60.
2026-08-01 14:16:39 +04:00
parent 7f42cc73be
commit 708a69375f
14 changed files with 571 additions and 132 deletions
@@ -3,6 +3,7 @@ package store
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 	"time"
 )
@@ -12,24 +13,24 @@ func TestCaptureTaskDedupesLiveWork(t *testing.T) {
 	st := newTestStore(t)
 	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)

-	id, created, err := st.CaptureTask(ctx, Task{Text: "купить молоко", Source: "tap:voice", CreatedTs: now})
+	first, err := st.CaptureTask(ctx, Task{Text: "купить молоко", Source: "tap:voice", CreatedTs: now})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !created {
+	if !first.Created {
 		t.Fatal("first capture must create a row")
 	}

 	// Same work, different casing and punctuation — one task, not two.
-	again, created, err := st.CaptureTask(ctx, Task{Text: "Купить молоко!", Source: "email:kami", CreatedTs: now})
+	again, err := st.CaptureTask(ctx, Task{Text: "Купить молоко!", Source: "tap:web", CreatedTs: now})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if created {
+	if again.Created {
 		t.Error("second capture of the same live work must not create a row")
 	}
-	if again != id {
-		t.Errorf("dedupe returned id %d, want the existing %d", again, id)
+	if again.ID != first.ID {
+		t.Errorf("dedupe returned id %d, want the existing %d", again.ID, first.ID)
 	}

 	live, err := st.ListTasks(ctx, "live")
@@ -46,20 +47,22 @@ func TestCaptureTaskAfterDoneIsANewTask(t *testing.T) {
 	st := newTestStore(t)
 	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)

-	id, _, err := st.CaptureTask(ctx, Task{Text: "полить цветы", Source: "tap:voice", CreatedTs: now})
+	first, err := st.CaptureTask(ctx, Task{Text: "полить цветы", Source: "tap:voice", CreatedTs: now})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := st.SetTaskStatus(ctx, id, TaskDone, now.Add(time.Hour)); err != nil {
+	id := first.ID
+	if err := st.SetTaskStatus(ctx, id, TaskDone, now.Add(time.Hour), "tap:web"); err != nil {
 		t.Fatal(err)
 	}
 	// The dedupe key is free again: a recurring errand must be capturable.
-	id2, created, err := st.CaptureTask(ctx, Task{Text: "полить цветы", Source: "tap:voice", CreatedTs: now.AddDate(0, 0, 7)})
+	second, err := st.CaptureTask(ctx, Task{Text: "полить цветы", Source: "tap:voice", CreatedTs: now.AddDate(0, 0, 7)})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !created || id2 == id {
-		t.Fatalf("re-capture after done: created=%v id=%d (previous %d)", created, id2, id)
+	id2 := second.ID
+	if !second.Created || id2 == id {
+		t.Fatalf("re-capture after done: created=%v id=%d (previous %d)", second.Created, id2, id)
 	}
 	live, err := st.ListTasks(ctx, "live")
 	if err != nil {
@@ -76,7 +79,7 @@ func TestCaptureTaskCandidateKeepsEvidence(t *testing.T) {
 	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)
 	due := now.Add(48 * time.Hour)

-	id, _, err := st.CaptureTask(ctx, Task{
+	res, err := st.CaptureTask(ctx, Task{
 		Text:      "продлить страховку",
 		Source:    "email:kami",
 		Evidence:  "Re: страховой полис истекает",
@@ -88,7 +91,7 @@ func TestCaptureTaskCandidateKeepsEvidence(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	got, err := st.LookupTask(ctx, id)
+	got, err := st.LookupTask(ctx, res.ID)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -114,22 +117,23 @@ func TestSetTaskStatusMovesOnceForwardOnly(t *testing.T) {
 	st := newTestStore(t)
 	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)

-	cand, _, err := st.CaptureTask(ctx, Task{Text: "записаться к врачу", Source: "email:kami", Status: TaskCandidate, CreatedTs: now})
+	res, err := st.CaptureTask(ctx, Task{Text: "записаться к врачу", Source: "email:kami", Status: TaskCandidate, CreatedTs: now})
 	if err != nil {
 		t.Fatal(err)
 	}
+	cand := res.ID
 	// candidate → done is not a legal move: he has to confirm it first.
-	if err := st.SetTaskStatus(ctx, cand, TaskDone, now); !errors.Is(err, ErrTaskNotFound) {
+	if err := st.SetTaskStatus(ctx, cand, TaskDone, now, "tap:web"); !errors.Is(err, ErrTaskNotFound) {
 		t.Errorf("candidate→done err = %v, want ErrTaskNotFound", err)
 	}
-	if err := st.SetTaskStatus(ctx, cand, TaskOpen, now); err != nil {
+	if err := st.SetTaskStatus(ctx, cand, TaskOpen, now, "tap:web"); err != nil {
 		t.Fatal(err)
 	}
-	if err := st.SetTaskStatus(ctx, cand, TaskDone, now.Add(time.Hour)); err != nil {
+	if err := st.SetTaskStatus(ctx, cand, TaskDone, now.Add(time.Hour), "tap:web"); err != nil {
 		t.Fatal(err)
 	}
 	// Already resolved — a second resolve must not move it again.
-	if err := st.SetTaskStatus(ctx, cand, TaskDropped, now.Add(2*time.Hour)); !errors.Is(err, ErrTaskNotFound) {
+	if err := st.SetTaskStatus(ctx, cand, TaskDropped, now.Add(2*time.Hour), "tap:web"); !errors.Is(err, ErrTaskNotFound) {
 		t.Errorf("second resolve err = %v, want ErrTaskNotFound", err)
 	}
 	got, err := st.LookupTask(ctx, cand)
@@ -147,14 +151,15 @@ func TestSetTaskStatusMovesOnceForwardOnly(t *testing.T) {
 func TestSetTaskStatusRejectsUnknownStatus(t *testing.T) {
 	ctx := context.Background()
 	st := newTestStore(t)
-	id, _, err := st.CaptureTask(ctx, Task{Text: "что-то", Source: "tap:web"})
+	res, err := st.CaptureTask(ctx, Task{Text: "что-то", Source: "tap:web"})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := st.SetTaskStatus(ctx, id, "candidate", time.Now()); !errors.Is(err, ErrTaskStatus) {
+	id := res.ID
+	if err := st.SetTaskStatus(ctx, id, "candidate", time.Now(), "tap:web"); !errors.Is(err, ErrTaskStatus) {
 		t.Errorf("→candidate err = %v, want ErrTaskStatus", err)
 	}
-	if err := st.SetTaskStatus(ctx, id, "urgent", time.Now()); !errors.Is(err, ErrTaskStatus) {
+	if err := st.SetTaskStatus(ctx, id, "urgent", time.Now(), "tap:web"); !errors.Is(err, ErrTaskStatus) {
 		t.Errorf("→urgent err = %v, want ErrTaskStatus", err)
 	}
 }
@@ -162,7 +167,7 @@ func TestSetTaskStatusRejectsUnknownStatus(t *testing.T) {
 func TestCaptureTaskRejectsEmptyText(t *testing.T) {
 	ctx := context.Background()
 	st := newTestStore(t)
-	if _, _, err := st.CaptureTask(ctx, Task{Text: "   ", Source: "tap:voice"}); !errors.Is(err, ErrTaskEmpty) {
+	if _, err := st.CaptureTask(ctx, Task{Text: "   ", Source: "tap:voice"}); !errors.Is(err, ErrTaskEmpty) {
 		t.Errorf("err = %v, want ErrTaskEmpty", err)
 	}
 }
@@ -172,10 +177,10 @@ func TestListTasksFiltersByStatus(t *testing.T) {
 	st := newTestStore(t)
 	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)

-	open1, _, _ := st.CaptureTask(ctx, Task{Text: "первая", Source: "tap:voice", CreatedTs: now})
-	_, _, _ = st.CaptureTask(ctx, Task{Text: "вторая", Source: "email:kami", Status: TaskCandidate, CreatedTs: now.Add(time.Minute)})
-	done, _, _ := st.CaptureTask(ctx, Task{Text: "третья", Source: "tap:voice", CreatedTs: now.Add(2 * time.Minute)})
-	if err := st.SetTaskStatus(ctx, done, TaskDone, now.Add(time.Hour)); err != nil {
+	open1, _ := st.CaptureTask(ctx, Task{Text: "первая", Source: "tap:voice", CreatedTs: now})
+	_, _ = st.CaptureTask(ctx, Task{Text: "вторая", Source: "email:kami", Status: TaskCandidate, CreatedTs: now.Add(time.Minute)})
+	done, _ := st.CaptureTask(ctx, Task{Text: "третья", Source: "tap:voice", CreatedTs: now.Add(2 * time.Minute)})
+	if err := st.SetTaskStatus(ctx, done.ID, TaskDone, now.Add(time.Hour), "tap:web"); err != nil {
 		t.Fatal(err)
 	}

@@ -190,7 +195,7 @@ func TestListTasksFiltersByStatus(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(opens) != 1 || opens[0].ID != open1 {
+	if len(opens) != 1 || opens[0].ID != open1.ID {
 		t.Fatalf("open = %+v", opens)
 	}
 	all, err := st.ListTasks(ctx, "")
@@ -219,3 +224,153 @@ func TestNormalizeTaskText(t *testing.T) {
 		}
 	}
 }
+
+// A mail the reader keeps seeing must not resurrect work he already finished.
+// The norm key alone frees on resolve, which is right for voice and wrong for a
+// mailbox: mavmaild never marks anything read, so the same message is extracted
+// again on every poll, forever.
+func TestCaptureTaskExternalIDSurvivesResolution(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)
+	mail := Task{
+		Text: "продлить страховку", Source: "email:kami", Status: TaskCandidate,
+		ExternalID: "email:kami#412:продлить страховку", CreatedTs: now,
+	}
+
+	first, err := st.CaptureTask(ctx, mail)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !first.Created {
+		t.Fatal("first capture must create a row")
+	}
+	// He confirms it and does it.
+	if err := st.SetTaskStatus(ctx, first.ID, TaskOpen, now.Add(time.Hour), "tap:web"); err != nil {
+		t.Fatal(err)
+	}
+	if err := st.SetTaskStatus(ctx, first.ID, TaskDone, now.Add(2*time.Hour), "tap:web"); err != nil {
+		t.Fatal(err)
+	}
+
+	// The next poll reads the same message again.
+	mail.CreatedTs = now.AddDate(0, 0, 1)
+	again, err := st.CaptureTask(ctx, mail)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if again.Created {
+		t.Error("re-reading the same mail created a second task after the first was done")
+	}
+	if again.ID != first.ID {
+		t.Errorf("id = %d, want the resolved row %d", again.ID, first.ID)
+	}
+	live, err := st.ListTasks(ctx, "live")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(live) != 0 {
+		t.Fatalf("live = %+v, want nothing: he already did this", live)
+	}
+}
+
+// Stating out loud a task Maven only proposed is a confirmation. Leaving it a
+// candidate had her read it straight back as something he had not confirmed.
+func TestCaptureTaskPromotesCandidate(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)
+
+	cand, err := st.CaptureTask(ctx, Task{
+		Text: "продлить страховку", Source: "email:kami", Status: TaskCandidate,
+		ExternalID: "email:kami#7:продлить страховку", CreatedTs: now,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	spoken, err := st.CaptureTask(ctx, Task{
+		Text: "продлить страховку", Source: "tap:voice", Status: TaskOpen,
+		CreatedTs: now.Add(time.Minute),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if spoken.Created {
+		t.Error("capture over a live candidate must not create a second row")
+	}
+	if !spoken.Promoted {
+		t.Error("capture with status open over a candidate must promote it")
+	}
+	got, err := st.LookupTask(ctx, cand.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.Status != TaskOpen {
+		t.Errorf("status = %q, want open", got.Status)
+	}
+	if got.ResolvedTs != nil {
+		t.Errorf("resolved_ts = %v, want nil: the task is still live", got.ResolvedTs)
+	}
+}
+
+// A derived source may only ever file a candidate. The intake seam documented
+// this and nothing enforced it, so a caller could skip review entirely.
+func TestCaptureTaskRefusesOpenFromDerivedSource(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	_, err := st.CaptureTask(ctx, Task{Text: "оплатить счёт", Source: "email:kami", Status: TaskOpen})
+	if !errors.Is(err, ErrTaskStatus) {
+		t.Errorf("err = %v, want ErrTaskStatus", err)
+	}
+}
+
+// resolved_ts said when a task was resolved and never by what.
+func TestSetTaskStatusRecordsWho(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)
+	res, err := st.CaptureTask(ctx, Task{Text: "выкинуть мусор", Source: "tap:voice", CreatedTs: now})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := st.SetTaskStatus(ctx, res.ID, TaskDone, now.Add(time.Hour), "tap:voice"); err != nil {
+		t.Fatal(err)
+	}
+	got, err := st.LookupTask(ctx, res.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.ResolvedBy != "tap:voice" {
+		t.Errorf("resolved_by = %q, want tap:voice", got.ResolvedBy)
+	}
+}
+
+// The resolved history only grows; an unbounded read of it is a page that gets
+// slower every month.
+func TestListTasksIsBounded(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	now := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)
+	for i := 0; i < MaxTaskRows+5; i++ {
+		res, err := st.CaptureTask(ctx, Task{
+			Text: fmt.Sprintf("задача %d", i), Source: "tap:voice",
+			CreatedTs: now.Add(time.Duration(i) * time.Minute),
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := st.SetTaskStatus(ctx, res.ID, TaskDone, now.Add(time.Hour), "tap:web"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	all, err := st.ListTasks(ctx, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(all) != MaxTaskRows {
+		t.Fatalf("all = %d rows, want the %d-row bound", len(all), MaxTaskRows)
+	}
+	if all[0].Text != fmt.Sprintf("задача %d", MaxTaskRows+4) {
+		t.Errorf("first = %q, want the newest", all[0].Text)
+	}
+}