Merge the accepted-routine fix and drop reminder_id from accept
Two merge fixes on top of the branch: - migrations: keep both new steps, snooze stays #8, the routine columns become #9. Both agents had numbered theirs #8. - accepting no longer takes a reminder id, on the web surface too. The web accept path had the same one-shot-reminder bug the voice path did, so both now just flip the status and let the tick loop schedule. The test that asserted "accept creates a reminder and links it" asserted the bug. It now asserts that accepting creates no reminder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
This commit is contained in:
+1
-1
@@ -166,7 +166,7 @@ func (l *lockedAPI) ListProposedRoutines(ctx context.Context) ([]ipc.ProposedRou
|
||||
return nil, errLocked
|
||||
}
|
||||
func (l *lockedAPI) DismissProposedRoutine(ctx context.Context, id int64) error { return errLocked }
|
||||
func (l *lockedAPI) AcceptProposedRoutine(ctx context.Context, id, remID int64) error {
|
||||
func (l *lockedAPI) AcceptProposedRoutine(ctx context.Context, id int64) error {
|
||||
return errLocked
|
||||
}
|
||||
func (l *lockedAPI) LookupTool(ctx context.Context, name string) (ipc.Tool, error) {
|
||||
|
||||
@@ -186,6 +186,10 @@ func (t *tickLoop) tick(ctx context.Context, now time.Time) {
|
||||
// LLM-phrased — so a routine can't hallucinate. severity comes from config.
|
||||
t.fireRoutines(ctx, now, state)
|
||||
|
||||
// accepted routines: patterns the user confirmed. read straight from the
|
||||
// store each tick so the schedule survives a restart.
|
||||
t.fireAcceptedRoutines(ctx, now, state)
|
||||
|
||||
// morning routines: daily checklists (medicine/water/pets/...), nagged at
|
||||
// most once per day per routine, and only for items still unevidenced at
|
||||
// nudge time. See internal/morning for the "why not four timers" rationale.
|
||||
@@ -377,6 +381,64 @@ func (t *tickLoop) fireRoutines(ctx context.Context, now time.Time, state loop.S
|
||||
}
|
||||
}
|
||||
|
||||
// fireAcceptedRoutines nudges about the routines the user accepted, once per
|
||||
// interval (Vikunja #366). Accepting used to create a single reminder, so a
|
||||
// non-weekly routine fired once and went quiet forever; the schedule lives in
|
||||
// the proposed_routines row now and the loop re-reads it every tick.
|
||||
//
|
||||
// A routine is a care-class nudge and goes through the restraint gate like any
|
||||
// other: quiet hours, away presence and snooze all suppress it. Reminders bypass
|
||||
// that gate; routines must not. A suppressed nudge is NOT marked fired, so it
|
||||
// goes out on the next tick that the gate allows — one nudge, held, not dropped
|
||||
// and not repeated.
|
||||
//
|
||||
// The body is literal text built from the detected action and object, not
|
||||
// LLM-phrased, so a routine can't hallucinate. It nudges; it never acts.
|
||||
func (t *tickLoop) fireAcceptedRoutines(ctx context.Context, now time.Time, state loop.State) {
|
||||
rows, err := t.store.ListAcceptedRoutines(ctx)
|
||||
if err != nil {
|
||||
log.Printf("tick: list accepted routines: %v", err)
|
||||
return
|
||||
}
|
||||
accepted := make([]routine.Accepted, 0, len(rows))
|
||||
for _, r := range rows {
|
||||
if r.AcceptedTs == nil {
|
||||
continue // accepted before the schedule column existed — no clock to start from.
|
||||
}
|
||||
accepted = append(accepted, routine.Accepted{
|
||||
ID: r.ID,
|
||||
Name: r.Action + " " + r.Object,
|
||||
IntervalDays: r.IntervalDays,
|
||||
Accepted: *r.AcceptedTs,
|
||||
LastFired: r.LastFiredTs,
|
||||
})
|
||||
}
|
||||
|
||||
for _, a := range routine.DueAccepted(accepted, now) {
|
||||
rule := loop.Rule{Name: "routine:" + a.Name, Severity: loop.Sev1}
|
||||
if !loop.Gate(state, rule) {
|
||||
continue
|
||||
}
|
||||
body := "пора: " + a.Name
|
||||
pn := delivery.PhrasedNudge{
|
||||
Candidate: loop.Candidate{Rule: rule, Severity: rule.Severity, State: state},
|
||||
Body: body,
|
||||
Summary: body,
|
||||
}
|
||||
sent, err := t.dispatcher.DispatchNudge(ctx, pn, now)
|
||||
if err != nil {
|
||||
log.Printf("tick: dispatch accepted routine %d: %v", a.ID, err)
|
||||
continue
|
||||
}
|
||||
if len(sent) == 0 {
|
||||
continue // routing dropped it — leave it due.
|
||||
}
|
||||
if err := t.store.MarkRoutineFired(ctx, a.ID, now); err != nil {
|
||||
log.Printf("tick: mark routine %d fired: %v", a.ID, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// fireMorningRoutines checks each configured checklist against today's facts
|
||||
// and dispatches a nag listing exactly what's still missing, at most once per
|
||||
// routine per calendar day. Fact reads happen here (not in loop.Gatherer)
|
||||
|
||||
@@ -96,6 +96,115 @@ func TestTickFiresRoutineWhenScheduleCrosses(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestTickFiresAcceptedRoutineEveryInterval — Vikunja #366. An accepted routine
|
||||
// with a 3-day interval must nudge every 3 days, not once. It also must not
|
||||
// replay the occurrences it slept through: after a 30-day gap it nudges once.
|
||||
func TestTickFiresAcceptedRoutineEveryInterval(t *testing.T) {
|
||||
st := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
accepted := refNow()
|
||||
|
||||
id, err := st.CreateProposedRoutine(ctx, "полить", "цветы", 3.0, accepted)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateProposedRoutine: %v", err)
|
||||
}
|
||||
if err := st.AcceptProposedRoutine(ctx, id, accepted); err != nil {
|
||||
t.Fatalf("AcceptProposedRoutine: %v", err)
|
||||
}
|
||||
|
||||
sink := &fakeSink{}
|
||||
tl := newTestTickLoop(t, st, sink, nil)
|
||||
const rule = "routine:полить цветы"
|
||||
|
||||
// Same day as the accept: not due yet.
|
||||
markPresent(t, st, ctx, accepted)
|
||||
tl.tick(ctx, accepted.Add(time.Hour))
|
||||
if n := countSends(sink, rule); n != 0 {
|
||||
t.Fatalf("routine fired %d times before its first interval passed, want 0", n)
|
||||
}
|
||||
|
||||
// Three days later: the first nudge.
|
||||
first := accepted.Add(3 * 24 * time.Hour)
|
||||
markPresent(t, st, ctx, first)
|
||||
tl.tick(ctx, first)
|
||||
if n := countSends(sink, rule); n != 1 {
|
||||
t.Fatalf("first interval: sends = %d, want 1", n)
|
||||
}
|
||||
|
||||
// Next day: still inside the interval, silent.
|
||||
sink.sends = nil
|
||||
markPresent(t, st, ctx, first.Add(24*time.Hour))
|
||||
tl.tick(ctx, first.Add(24*time.Hour))
|
||||
if n := countSends(sink, rule); n != 0 {
|
||||
t.Fatalf("mid-interval: sends = %d, want 0", n)
|
||||
}
|
||||
|
||||
// Three days after the first nudge: it fires again. This is the bug —
|
||||
// a one-shot reminder would never come back.
|
||||
second := first.Add(3 * 24 * time.Hour)
|
||||
markPresent(t, st, ctx, second)
|
||||
tl.tick(ctx, second)
|
||||
if n := countSends(sink, rule); n != 1 {
|
||||
t.Fatalf("second interval: sends = %d, want 1 (a routine repeats)", n)
|
||||
}
|
||||
|
||||
// A long silence must not turn into a backlog of missed nudges.
|
||||
sink.sends = nil
|
||||
late := second.Add(30 * 24 * time.Hour)
|
||||
markPresent(t, st, ctx, late)
|
||||
tl.tick(ctx, late)
|
||||
if n := countSends(sink, rule); n != 1 {
|
||||
t.Fatalf("after a 30-day gap: sends = %d, want exactly 1 (no backlog)", n)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTickAcceptedRoutineRespectsQuietHours — routines are not reminders: they
|
||||
// do not inherit the reminder gate bypass. Away presence drops a care-class
|
||||
// nudge, and the routine stays due so it nudges once the user is back.
|
||||
func TestTickAcceptedRoutineRespectsGate(t *testing.T) {
|
||||
st := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
accepted := refNow()
|
||||
|
||||
id, err := st.CreateProposedRoutine(ctx, "полить", "цветы", 3.0, accepted)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateProposedRoutine: %v", err)
|
||||
}
|
||||
if err := st.AcceptProposedRoutine(ctx, id, accepted); err != nil {
|
||||
t.Fatalf("AcceptProposedRoutine: %v", err)
|
||||
}
|
||||
|
||||
sink := &fakeSink{}
|
||||
tl := newTestTickLoop(t, st, sink, nil)
|
||||
const rule = "routine:полить цветы"
|
||||
|
||||
// No presence probes at all ⇒ away ⇒ the care gate blocks the nudge.
|
||||
due := accepted.Add(3 * 24 * time.Hour)
|
||||
tl.tick(ctx, due)
|
||||
if n := countSends(sink, rule); n != 0 {
|
||||
t.Fatalf("away: sends = %d, want 0 (routine must not bypass the gate)", n)
|
||||
}
|
||||
|
||||
// Back at the desk a minute later: the nudge that was held now goes out.
|
||||
back := due.Add(time.Minute)
|
||||
markPresent(t, st, ctx, back)
|
||||
tl.tick(ctx, back)
|
||||
if n := countSends(sink, rule); n != 1 {
|
||||
t.Fatalf("present again: sends = %d, want 1", n)
|
||||
}
|
||||
}
|
||||
|
||||
// countSends counts captured sends for one rule name.
|
||||
func countSends(sink *fakeSink, rule string) int {
|
||||
n := 0
|
||||
for _, s := range sink.sends {
|
||||
if s.RuleName == rule {
|
||||
n++
|
||||
}
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// refNow — fixed tick time so presence decay + since durations are deterministic.
|
||||
func refNow() time.Time { return time.Date(2026, 6, 30, 12, 0, 0, 0, time.UTC) }
|
||||
|
||||
|
||||
+5
-16
@@ -1421,23 +1421,12 @@ func (h *reactiveHandler) resolveConfirm(ctx context.Context, text string) (stri
|
||||
switch classifyConfirm(text) {
|
||||
case confirmYes:
|
||||
h.pendingRoutine = nil
|
||||
// Create a recurring reminder at the detected interval.
|
||||
// Weekly patterns get a cron expression; arbitrary intervals
|
||||
// fire once and the detector re-proposes on the next cycle.
|
||||
intervalDur := time.Duration(pr.interval * 24 * float64(time.Hour))
|
||||
fire := h.now().Add(intervalDur)
|
||||
cron := ""
|
||||
if pr.interval >= 6.5 && pr.interval <= 7.5 {
|
||||
cron = fmt.Sprintf("0 %d * * %d", fire.Hour(), int(fire.Weekday()))
|
||||
}
|
||||
payload := fmt.Sprintf(`{"text":"%s %s"}`, pr.action, pr.object)
|
||||
remID, err := h.api.CreateReminder(ctx, fire, payload, cron)
|
||||
if err != nil {
|
||||
log.Printf("voice: create routine reminder: %v", err)
|
||||
return "не получилось поставить напоминание.", true
|
||||
}
|
||||
if err := h.dataStore.AcceptProposedRoutine(ctx, pr.routineID, remID); err != nil {
|
||||
// Only record the acceptance. The tick loop reads accepted
|
||||
// routines and nudges on their own interval. Building a reminder
|
||||
// here made a routine fire exactly once (Vikunja #366).
|
||||
if err := h.dataStore.AcceptProposedRoutine(ctx, pr.routineID, h.now()); err != nil {
|
||||
log.Printf("voice: accept proposed routine: %v", err)
|
||||
return "не получилось запомнить рутину.", true
|
||||
}
|
||||
return "буду напоминать.", true
|
||||
case confirmNo:
|
||||
|
||||
@@ -928,7 +928,6 @@ type routineCore struct {
|
||||
routines []ipc.ProposedRoutine
|
||||
dismissed int64
|
||||
acceptedID int64
|
||||
acceptedRe int64
|
||||
remCron string
|
||||
}
|
||||
|
||||
@@ -941,8 +940,8 @@ func (c *routineCore) DismissProposedRoutine(_ context.Context, id int64) error
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *routineCore) AcceptProposedRoutine(_ context.Context, id, remID int64) error {
|
||||
c.acceptedID, c.acceptedRe = id, remID
|
||||
func (c *routineCore) AcceptProposedRoutine(_ context.Context, id int64) error {
|
||||
c.acceptedID = id
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -990,18 +989,22 @@ func TestHandleRoutines_Accept_RequiresStepUp(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleRoutines_Accept_CreatesReminderAndLinksIt(t *testing.T) {
|
||||
// Accepting only flips the status. It used to also create a one-shot reminder,
|
||||
// which is why a non-weekly routine fired once and then went quiet forever
|
||||
// (Vikunja #366). The tick loop owns the schedule now, so a reminder here would
|
||||
// be a second, competing schedule.
|
||||
func TestHandleRoutines_Accept_FlipsStatusAndMakesNoReminder(t *testing.T) {
|
||||
core := weeklyRoutineCore()
|
||||
rr := httptest.NewRecorder()
|
||||
handleRoutines(rr, postRoutine("accept", "3"), core, stepUpSession(), false)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if core.acceptedID != 3 || core.acceptedRe != 77 {
|
||||
t.Fatalf("accepted id=%d reminder=%d, want 3 and 77", core.acceptedID, core.acceptedRe)
|
||||
if core.acceptedID != 3 {
|
||||
t.Fatalf("accepted id = %d, want 3", core.acceptedID)
|
||||
}
|
||||
if core.remCron == "" {
|
||||
t.Fatal("a weekly pattern should get a cron expression")
|
||||
if core.remCron != "" {
|
||||
t.Fatalf("accepting must not create a reminder, got cron %q", core.remCron)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+5
-14
@@ -898,20 +898,11 @@ func acceptRoutine(ctx context.Context, core ipc.CoreAPI, id int64) error {
|
||||
return errors.New("no such proposed routine")
|
||||
}
|
||||
|
||||
fire := time.Now().Add(time.Duration(found.IntervalDays * 24 * float64(time.Hour)))
|
||||
cron := ""
|
||||
if found.IntervalDays >= 6.5 && found.IntervalDays <= 7.5 {
|
||||
cron = fmt.Sprintf("0 %d * * %d", fire.Hour(), int(fire.Weekday()))
|
||||
}
|
||||
payload, err := json.Marshal(map[string]string{"text": found.Action + " " + found.Object})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
remID, err := core.CreateReminder(ctx, fire, string(payload), cron)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return core.AcceptProposedRoutine(ctx, id, remID)
|
||||
// No reminder is created here. Accepting only flips the status; the tick
|
||||
// loop reads accepted routines and nudges on the interval (Vikunja #366).
|
||||
// The old code made a one-shot reminder, so a non-weekly routine fired
|
||||
// once and then went quiet forever.
|
||||
return core.AcceptProposedRoutine(ctx, id)
|
||||
}
|
||||
|
||||
func handleTrace(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) {
|
||||
|
||||
Reference in New Issue
Block a user