Merge the routine accept path and store gaps

internal/auth/auth_test.go

@@ -446,6 +446,9 @@ func (r *recordingAPI) RevertFact(_ context.Context, _ string) (int64, error) {
 func (r *recordingAPI) ListProposedRoutines(_ context.Context) ([]ipc.ProposedRoutine, error) {
 	return nil, nil
 }
+func (r *recordingAPI) AcceptProposedRoutine(_ context.Context, _, _ int64) error {
+	return nil
+}
 func (r *recordingAPI) DismissProposedRoutine(_ context.Context, _ int64) error {
 	return nil
 }

internal/ipc/api.go

@@ -237,6 +237,11 @@ type dismissProposedRoutineReq struct {
 	ID int64 `json:"id"`
 }
 
+type acceptProposedRoutineReq struct {
+	ID         int64 `json:"id"`
+	ReminderID int64 `json:"reminder_id"`
+}
+
 // CoreAPI — what core exposes to modules. One Go interface, satisfied by:
 //   - the in-process store adapter (server.go storeAPI) — used by the daemon
 //     for modules that live in-process for now (router, delivery) and by tests,
@@ -286,6 +291,9 @@ type CoreAPI interface {
 	ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, error)
 	// DismissProposedRoutine flips a proposed routine to 'dismissed'.
 	DismissProposedRoutine(ctx context.Context, id int64) error
+	// AcceptProposedRoutine flips a proposed routine to 'accepted' and links
+	// the reminder that will fire it. The caller creates the reminder first.
+	AcceptProposedRoutine(ctx context.Context, id, reminderID int64) error
 
 	// TickTrace returns the most recent tick's rule trace. The daemon caches
 	// this after every tick; the store adapter returns an error (trace is not

internal/ipc/client.go

@@ -430,6 +430,10 @@ func (c *Client) DismissProposedRoutine(ctx context.Context, id int64) error {
 	return c.call(ctx, MethodDismissProposedRoutine, dismissProposedRoutineReq{ID: id}, nil)
 }
 
+func (c *Client) AcceptProposedRoutine(ctx context.Context, id, reminderID int64) error {
+	return c.call(ctx, MethodAcceptProposedRoutine, acceptProposedRoutineReq{ID: id, ReminderID: reminderID}, nil)
+}
+
 func (c *Client) Chat(ctx context.Context, text string) (string, error) {
 	var r chatResp
 	if err := c.call(ctx, MethodChat, chatReq{Text: text}, &r); err != nil {

internal/ipc/ipc_test.go

@@ -478,6 +478,9 @@ func (a *chatTestAPI) DeleteTool(ctx context.Context, name string) error {
 func (a *chatTestAPI) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, error) {
 	return nil, ErrUnknownMethod
 }
+func (a *chatTestAPI) AcceptProposedRoutine(ctx context.Context, id, remID int64) error {
+	return nil
+}
 func (a *chatTestAPI) DismissProposedRoutine(ctx context.Context, id int64) error {
 	return ErrUnknownMethod
 }

internal/ipc/server.go

@@ -253,6 +253,10 @@ func (a *storeAPI) DismissProposedRoutine(ctx context.Context, id int64) error {
 	return mapErr(a.s.DismissProposedRoutine(ctx, id))
 }
 
+func (a *storeAPI) AcceptProposedRoutine(ctx context.Context, id, reminderID int64) error {
+	return mapErr(a.s.AcceptProposedRoutine(ctx, id, reminderID))
+}
+
 func toTool(t store.Tool) Tool {
 	return Tool{
 		Name: t.Name, Scope: t.Scope, Cmd: t.Cmd, Destructive: t.Destructive,
@@ -774,6 +778,13 @@ func (s *Server) dispatch(ctx context.Context, req Request) (json.RawMessage, er
 		}
 		return marshalResult(nil), api.DismissProposedRoutine(ctx, p.ID)
 
+	case MethodAcceptProposedRoutine:
+		var p acceptProposedRoutineReq
+		if err := unmarshalParams(req.Params, &p); err != nil {
+			return nil, err
+		}
+		return marshalResult(nil), api.AcceptProposedRoutine(ctx, p.ID, p.ReminderID)
+
 	case MethodRevertFact:
 		var p struct {
 			Key string `json:"key"`

internal/ipc/wire.go

@@ -41,6 +41,7 @@ const (
 	MethodDeleteTool             Method = "delete_tool"
 	MethodListProposedRoutines   Method = "list_proposed_routines"
 	MethodDismissProposedRoutine Method = "dismiss_proposed_routine"
+	MethodAcceptProposedRoutine  Method = "accept_proposed_routine"
 	MethodRevertFact             Method = "revert_fact"
 	MethodTickTrace          Method = "tick_trace"
 	MethodMorningStatus      Method = "morning_status"

internal/store/proposed_routines.go

@@ -8,6 +8,14 @@ import (
 	"time"
 )
 
+// The three states a proposal can be in. A proposal starts 'proposed' and
+// moves once, either way, and never moves again.
+const (
+	RoutineProposed  = "proposed"
+	RoutineAccepted  = "accepted"
+	RoutineDismissed = "dismissed"
+)
+
 // ProposedRoutine — a detected pattern the system wants to turn into a
 // recurring reminder. Status 'proposed' means awaiting human confirmation;
 // 'accepted' means the human confirmed and a reminder was created (reminder_id
@@ -30,6 +38,15 @@ var (
 // CreateProposedRoutine inserts a new proposed routine. Returns
 // ErrProposedRoutineExists if one already exists for this action+object (any
 // status) — the pattern detector should only propose once per pair.
+//
+// action+object is the "same routine" key. It is UNIQUE in the table, so a
+// routine the human already dismissed can never come back: the detector will
+// keep finding the pattern, and every re-propose is refused here. Maven is not
+// a nag.
+//
+// TODO(vikunja#46): the detector currently only writes here from the voice
+// path. Once digestion runs the detector on its own tick, that tick should
+// call this too, so a pattern gets noticed even with nobody at the mic.
 func (s *Store) CreateProposedRoutine(ctx context.Context, action, object string, intervalDays float64, ts time.Time) (int64, error) {
 	res, err := s.db.ExecContext(ctx,
 		`INSERT INTO proposed_routines (action, object, interval_days, status, created_ts)
@@ -70,14 +87,29 @@ func (s *Store) LookupProposedRoutine(ctx context.Context, action, object string
 	return &r, nil
 }
 
-// ListProposedRoutines returns all proposed routines with status='proposed',
-// newest first.
+// ListProposedRoutines returns the routines still waiting for an answer,
+// newest first. This is what the /routines page shows.
 func (s *Store) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, error) {
-	rows, err := s.db.QueryContext(ctx, `
-		SELECT id, action, object, interval_days, status, created_ts, reminder_id
-		  FROM proposed_routines
-		 WHERE status = 'proposed'
-		 ORDER BY created_ts DESC, id DESC`)
+	return s.ListProposedRoutinesByStatus(ctx, RoutineProposed)
+}
+
+// ListProposedRoutinesByStatus returns routines in one status, newest first.
+// An empty status returns every row.
+//
+// TODO(vikunja#46): the tick loop should read the accepted ones from here so a
+// routine the human said yes to has a home the loop can see, instead of only
+// the reminder row that accepting happened to create.
+func (s *Store) ListProposedRoutinesByStatus(ctx context.Context, status string) ([]ProposedRoutine, error) {
+	q := `SELECT id, action, object, interval_days, status, created_ts, reminder_id
+	        FROM proposed_routines`
+	var args []any
+	if status != "" {
+		q += ` WHERE status = ?`
+		args = append(args, status)
+	}
+	q += ` ORDER BY created_ts DESC, id DESC`
+
+	rows, err := s.db.QueryContext(ctx, q, args...)
 	if err != nil {
 		return nil, fmt.Errorf("list proposed routines: %w", err)
 	}
@@ -93,8 +125,19 @@ func (s *Store) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, er
 	return out, rows.Err()
 }
 
+// Status changes below are an in-place UPDATE, on purpose. Facts are
+// append-only (a correction writes a new row and sets voids_id) because a fact
+// is a claim about the world and the old claim is still history worth keeping.
+// A proposal is not a claim, it is a question with one answer, and the same
+// shape already exists for tools (tools.status flips in place). The guard
+// `AND status = 'proposed'` makes the move one-way: an answered proposal can
+// never be answered again.
+//
 // AcceptProposedRoutine flips status to 'accepted', links a reminder_id.
 // Returns error if not in 'proposed' status.
+//
+// TODO(vikunja#46): the /routines page calls this through ipc to flip status
+// from the authed surface.
 func (s *Store) AcceptProposedRoutine(ctx context.Context, id, reminderID int64) error {
 	res, err := s.db.ExecContext(ctx,
 		`UPDATE proposed_routines SET status = 'accepted', reminder_id = ? WHERE id = ? AND status = 'proposed'`,

internal/store/proposed_routines_test.go

@@ -131,6 +131,96 @@ func TestListProposedRoutines(t *testing.T) {
 	}
 }
 
+// A dismissed routine must never be proposed again. The detector will keep
+// finding the same pattern; the store is what stops maven nagging about it.
+func TestDismissedProposedRoutineStaysDismissed(t *testing.T) {
+	s := newTestStore(t)
+	ctx := context.Background()
+	now := time.Now().UTC()
+
+	id, err := s.CreateProposedRoutine(ctx, "clean", "litter_box", 3.0, now)
+	if err != nil {
+		t.Fatalf("CreateProposedRoutine: %v", err)
+	}
+	if err := s.DismissProposedRoutine(ctx, id); err != nil {
+		t.Fatalf("DismissProposedRoutine: %v", err)
+	}
+
+	// The detector re-proposes the same pattern.
+	_, err = s.CreateProposedRoutine(ctx, "clean", "litter_box", 3.0, now.Add(24*time.Hour))
+	if !errors.Is(err, ErrProposedRoutineExists) {
+		t.Fatalf("want ErrProposedRoutineExists on re-propose, got %v", err)
+	}
+
+	// And it must not reappear on the review page.
+	list, err := s.ListProposedRoutines(ctx)
+	if err != nil {
+		t.Fatalf("ListProposedRoutines: %v", err)
+	}
+	if len(list) != 0 {
+		t.Fatalf("want 0 proposed, got %d", len(list))
+	}
+
+	// Dismissing again is a no-op, and accepting is refused.
+	if err := s.DismissProposedRoutine(ctx, id); err != nil {
+		t.Fatalf("second DismissProposedRoutine: %v", err)
+	}
+	if err := s.AcceptProposedRoutine(ctx, id, 1); !errors.Is(err, ErrProposedRoutineNotFound) {
+		t.Fatalf("want ErrProposedRoutineNotFound accepting a dismissed routine, got %v", err)
+	}
+	r, err := s.LookupProposedRoutine(ctx, "clean", "litter_box")
+	if err != nil {
+		t.Fatalf("LookupProposedRoutine: %v", err)
+	}
+	if r.Status != RoutineDismissed {
+		t.Fatalf("want status=dismissed, got %s", r.Status)
+	}
+}
+
+func TestListProposedRoutinesByStatus(t *testing.T) {
+	s := newTestStore(t)
+	ctx := context.Background()
+	now := time.Now().UTC()
+
+	keep, err := s.CreateProposedRoutine(ctx, "water", "plants", 4.0, now)
+	if err != nil {
+		t.Fatalf("CreateProposedRoutine: %v", err)
+	}
+	drop, err := s.CreateProposedRoutine(ctx, "walk", "dog", 1.0, now.Add(time.Hour))
+	if err != nil {
+		t.Fatalf("CreateProposedRoutine: %v", err)
+	}
+	remID, err := s.CreateReminder(ctx, now.Add(4*24*time.Hour), `{"text":"water plants"}`, "")
+	if err != nil {
+		t.Fatalf("CreateReminder: %v", err)
+	}
+	if err := s.AcceptProposedRoutine(ctx, keep, remID); err != nil {
+		t.Fatalf("AcceptProposedRoutine: %v", err)
+	}
+	if err := s.DismissProposedRoutine(ctx, drop); err != nil {
+		t.Fatalf("DismissProposedRoutine: %v", err)
+	}
+
+	cases := []struct {
+		status string
+		want   int
+	}{
+		{RoutineProposed, 0},
+		{RoutineAccepted, 1},
+		{RoutineDismissed, 1},
+		{"", 2}, // empty status ⇒ every row
+	}
+	for _, c := range cases {
+		list, err := s.ListProposedRoutinesByStatus(ctx, c.status)
+		if err != nil {
+			t.Fatalf("ListProposedRoutinesByStatus(%q): %v", c.status, err)
+		}
+		if len(list) != c.want {
+			t.Fatalf("status %q: want %d, got %d", c.status, c.want, len(list))
+		}
+	}
+}
+
 func TestLookupMissingProposedRoutine(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()