diff --git a/internal/ipc/api.go b/internal/ipc/api.go index d2e597e..4cfa642 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -549,6 +549,16 @@ type setTaskStatusReq struct { By string `json:"by,omitempty"` } +// editTaskReq — the rewrite of the three fields capture set (Vikunja #509). +// Due nil clears the date, so "no date given" and "remove the date" cannot be +// the same request. +type editTaskReq struct { + ID int64 `json:"id"` + Text string `json:"text"` + Due *time.Time `json:"due,omitempty"` + Weight int `json:"weight,omitempty"` +} + // setTaskFieldsReq — the write for the two board columns. Both are sent every // time and both may be empty: clearing a blocker is as ordinary as setting one, // so an omitted field cannot mean "leave it alone" without a second way to say @@ -872,6 +882,13 @@ var ErrToolNotFound = errors.New("ipc: tool not found") // form can say which refusal it hit rather than "не найдено". var ErrTaskNoDoneWhen = errors.New("ipc: task has no definition of done") +// ErrTaskDuplicate — an edit would collide with another live task's normalised +// text (Vikunja #509). The surface says which row holds it rather than merging. +var ErrTaskDuplicate = errors.New("ipc: another live task already has this text") + +// ErrTaskResolved — a resolved task is not editable. +var ErrTaskResolved = errors.New("ipc: task is resolved") + // callerKey — context key for the authenticated caller. Server sets it from // SO_PEERCRED before dispatch; in-process callers omit it (the adapter treats // a missing Caller as "trusted same-process", the equivalent of the socket's diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 591a382..f917d01 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -515,6 +515,10 @@ func (c *Client) SetTaskStatus(ctx context.Context, id int64, status string, ts return c.call(ctx, MethodSetTaskStatus, setTaskStatusReq{ID: id, Status: status, Ts: ts, By: by}, nil) } +func (c *Client) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + return c.call(ctx, MethodEditTask, editTaskReq{ID: id, Text: text, Due: due, Weight: weight}, nil) +} + func (c *Client) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error { return c.call(ctx, MethodSetTaskFields, setTaskFieldsReq{ID: id, DoneWhen: doneWhen, BlockedOn: blockedOn}, nil) } diff --git a/internal/ipc/coreapi.go b/internal/ipc/coreapi.go index b4aa538..bec310a 100644 --- a/internal/ipc/coreapi.go +++ b/internal/ipc/coreapi.go @@ -142,6 +142,11 @@ type TaskAPI interface { // SetTaskStatus moves a task forward once: candidate→open|dropped, // open→done|dropped. Any other move is refused. SetTaskStatus(ctx context.Context, id int64, status string, ts time.Time, by string) error + // EditTask rewrites the three fields capture set: text, due date and + // weight. Status is not among them — that ladder is one-way and belongs to + // SetTaskStatus. A resolved task is refused, and a text edit that would + // duplicate another live task is refused rather than merged. + EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error // SetTaskFields writes the definition of done and the blocker. Not a // status move, so it is not one-way: he may sharpen a criterion, and a // blocker clears when the person answers. blockedOn is a canonical Nexus diff --git a/internal/ipc/maperr_test.go b/internal/ipc/maperr_test.go index 4857368..7bdc4b3 100644 --- a/internal/ipc/maperr_test.go +++ b/internal/ipc/maperr_test.go @@ -32,6 +32,8 @@ var mapErrPairs = []struct { {"ErrReminderState", store.ErrReminderState, ErrReminderState}, {"ErrToolNotFound", store.ErrToolNotFound, ErrToolNotFound}, {"ErrTaskNoDoneWhen", store.ErrTaskNoDoneWhen, ErrTaskNoDoneWhen}, + {"ErrTaskDuplicate", store.ErrTaskDuplicate, ErrTaskDuplicate}, + {"ErrTaskResolved", store.ErrTaskResolved, ErrTaskResolved}, } // unmappedStoreErrors — store sentinels that deliberately have no wire twin, diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 1e5b8fc..f68edb0 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -540,6 +540,9 @@ var methodTable = map[Method]handlerFunc{ MethodSetTaskStatus: withParamsVoid(func(ctx context.Context, api CoreAPI, p setTaskStatusReq) error { return api.SetTaskStatus(ctx, p.ID, p.Status, p.Ts, p.By) }), + MethodEditTask: withParamsVoid(func(ctx context.Context, api CoreAPI, p editTaskReq) error { + return api.EditTask(ctx, p.ID, p.Text, p.Due, p.Weight) + }), MethodSetTaskFields: withParamsVoid(func(ctx context.Context, api CoreAPI, p setTaskFieldsReq) error { return api.SetTaskFields(ctx, p.ID, p.DoneWhen, p.BlockedOn) }), diff --git a/internal/ipc/storeapi.go b/internal/ipc/storeapi.go index 656efb3..3fb2a30 100644 --- a/internal/ipc/storeapi.go +++ b/internal/ipc/storeapi.go @@ -356,6 +356,10 @@ func (a *storeAPI) SetTaskStatus(ctx context.Context, id int64, status string, t return mapErr(a.s.SetTaskStatus(ctx, id, status, ts, by)) } +func (a *storeAPI) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + return mapErr(a.s.EditTask(ctx, id, text, due, weight)) +} + func (a *storeAPI) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error { return mapErr(a.s.SetTaskFields(ctx, id, doneWhen, blockedOn)) } @@ -468,6 +472,10 @@ func mapErr(err error) error { return ErrToolNotFound case errors.Is(err, store.ErrTaskNoDoneWhen): return ErrTaskNoDoneWhen + case errors.Is(err, store.ErrTaskDuplicate): + return ErrTaskDuplicate + case errors.Is(err, store.ErrTaskResolved): + return ErrTaskResolved } return err } diff --git a/internal/ipc/unimplemented.go b/internal/ipc/unimplemented.go index 86b83a2..63ce042 100644 --- a/internal/ipc/unimplemented.go +++ b/internal/ipc/unimplemented.go @@ -114,6 +114,9 @@ func (UnimplementedCoreAPI) ListTasks(ctx context.Context, status string) ([]Tas func (UnimplementedCoreAPI) SetTaskStatus(ctx context.Context, id int64, status string, ts time.Time, by string) error { return ErrNotImplemented } +func (UnimplementedCoreAPI) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + return ErrNotImplemented +} func (UnimplementedCoreAPI) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error { return ErrNotImplemented } diff --git a/internal/ipc/wire.go b/internal/ipc/wire.go index 68ee161..d19888b 100644 --- a/internal/ipc/wire.go +++ b/internal/ipc/wire.go @@ -56,6 +56,7 @@ const ( MethodListTasks Method = "list_tasks" MethodSetTaskStatus Method = "set_task_status" MethodSetTaskFields Method = "set_task_fields" + MethodEditTask Method = "edit_task" MethodIngestMail Method = "ingest_mail" MethodSwapModel Method = "swap_model" MethodModelStatus Method = "model_status" diff --git a/internal/store/tasks.go b/internal/store/tasks.go index 16f4f9d..8e37258 100644 --- a/internal/store/tasks.go +++ b/internal/store/tasks.go @@ -98,6 +98,16 @@ var ( // finish line nobody wrote is how a board fills with rows that can never // leave it. Dropping such a candidate stays legal. ErrTaskNoDoneWhen = errors.New("store: task has no definition of done") + // ErrTaskDuplicate — an edit would give this task the normalised text of + // another live row (Vikunja #509). A refusal, not a merge: two live rows + // carry two provenances, two capture times and possibly two external + // identities, and merging picks a winner for all three silently. The + // surface tells the owner which row already holds the text and lets him + // drop one. + ErrTaskDuplicate = errors.New("store: another live task already has this text") + // ErrTaskResolved — a resolved task is not editable. Its text is the + // record of what was finished, and rewriting it rewrites history. + ErrTaskResolved = errors.New("store: task is resolved") ) // liveTaskStatuses — the two statuses that count as outstanding work. @@ -381,6 +391,50 @@ func (s *Store) setTaskStatus(ctx context.Context, id int64, status string, ts t return nil } +// EditTask rewrites the three fields capture set and nothing else: text, due +// date and weight (Vikunja #509). Status stays the one-way ladder SetTaskStatus +// owns, and a resolved task is refused outright — its text is the record of +// what was finished. +// +// Editing text re-normalises the dedupe key, which can collide with another +// live row. That is ErrTaskDuplicate and it is a refusal: merging would pick +// one row's provenance, capture time and external identity over the other's +// with nobody asked. +// +// due nil clears the date. Clearing has to be sayable, so an absent date and +// "remove the date" cannot be the same argument. +func (s *Store) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + text = strings.TrimSpace(text) + if text == "" { + return ErrTaskEmpty + } + cur, err := s.LookupTask(ctx, id) + if err != nil { + return err + } + if cur.Status != TaskCandidate && cur.Status != TaskOpen { + return fmt.Errorf("%w: id=%d is %s", ErrTaskResolved, id, cur.Status) + } + norm := NormalizeTaskText(text) + if norm != NormalizeTaskText(cur.Text) { + if other, err := s.lookupLiveTaskByNorm(ctx, norm); err == nil && other.ID != id { + return fmt.Errorf("%w: id=%d holds it", ErrTaskDuplicate, other.ID) + } else if err != nil && !errors.Is(err, ErrTaskNotFound) { + return err + } + } + var dueVal sql.NullInt64 + if due != nil { + dueVal = sql.NullInt64{Int64: due.UnixMilli(), Valid: true} + } + if _, err := s.db.ExecContext(ctx, + `UPDATE tasks SET text = ?, norm = ?, due_ts = ?, weight = ? WHERE id = ?`, + text, norm, dueVal, weight, id); err != nil { + return fmt.Errorf("edit task: %w", err) + } + return nil +} + // SetTaskFields writes the two board columns. Separate from SetTaskStatus // because a status move is one-way and these are not: he may sharpen a // definition of done, and a blocker clears when the person answers. diff --git a/internal/store/tasks_test.go b/internal/store/tasks_test.go index e525b01..1128924 100644 --- a/internal/store/tasks_test.go +++ b/internal/store/tasks_test.go @@ -465,3 +465,80 @@ func TestPromotingACandidateNeedsADefinitionOfDone(t *testing.T) { t.Errorf("status = %q, want open", got.Status) } } + +func TestEditTaskRewritesTheCaptureFields(t *testing.T) { + ctx := context.Background() + st := newTestStore(t) + now := time.Date(2026, 8, 5, 9, 0, 0, 0, time.UTC) + due := now.Add(48 * time.Hour) + + res, err := st.CaptureTask(ctx, Task{Text: "купить малако", Source: "tap:voice", CreatedTs: now, Weight: 1}) + if err != nil { + t.Fatal(err) + } + if err := st.EditTask(ctx, res.ID, " купить молоко ", &due, 3); err != nil { + t.Fatal(err) + } + got, err := st.LookupTask(ctx, res.ID) + if err != nil { + t.Fatal(err) + } + if got.Text != "купить молоко" || got.Weight != 3 || got.Due == nil || !got.Due.Equal(due) { + t.Fatalf("task = %+v, want the dictation typo fixed with the date and weight", got) + } + // Clearing the date has to be sayable, or an absent date and "remove the + // date" would be one argument. + if err := st.EditTask(ctx, res.ID, "купить молоко", nil, 3); err != nil { + t.Fatal(err) + } + if got, err = st.LookupTask(ctx, res.ID); err != nil { + t.Fatal(err) + } else if got.Due != nil { + t.Errorf("due = %v, want it cleared", got.Due) + } + // The dedupe key moved with the text: capturing the old wording is new work. + again, err := st.CaptureTask(ctx, Task{Text: "купить малако", Source: "tap:voice", CreatedTs: now}) + if err != nil { + t.Fatal(err) + } + if !again.Created { + t.Error("the old normalised text must be free after the edit") + } + if err := st.EditTask(ctx, res.ID, "", nil, 0); !errors.Is(err, ErrTaskEmpty) { + t.Errorf("empty text = %v, want ErrTaskEmpty", err) + } +} + +func TestEditTaskRefusesACollisionAndAResolvedRow(t *testing.T) { + ctx := context.Background() + st := newTestStore(t) + now := time.Date(2026, 8, 5, 9, 0, 0, 0, time.UTC) + + first, err := st.CaptureTask(ctx, Task{Text: "купить молоко", Source: "tap:voice", CreatedTs: now}) + if err != nil { + t.Fatal(err) + } + second, err := st.CaptureTask(ctx, Task{Text: "оплатить интернет", Source: "tap:voice", CreatedTs: now}) + if err != nil { + t.Fatal(err) + } + // Two live rows carry two provenances and two capture times, so a merge + // would pick a winner for both with nobody asked. + if err := st.EditTask(ctx, second.ID, "Купить молоко!", nil, 0); !errors.Is(err, ErrTaskDuplicate) { + t.Errorf("collision = %v, want ErrTaskDuplicate", err) + } + // Editing a row to the text it already has is not a collision with itself. + if err := st.EditTask(ctx, first.ID, "купить молоко", nil, 2); err != nil { + t.Errorf("re-saving the same text: %v", err) + } + if err := st.SetTaskStatus(ctx, first.ID, TaskDone, now.Add(time.Hour), "tap:web"); err != nil { + t.Fatal(err) + } + // A resolved task's text is the record of what was finished. + if err := st.EditTask(ctx, first.ID, "купить кефир", nil, 0); !errors.Is(err, ErrTaskResolved) { + t.Errorf("editing a resolved task = %v, want ErrTaskResolved", err) + } + if err := st.EditTask(ctx, 9999, "что-то", nil, 0); !errors.Is(err, ErrTaskNotFound) { + t.Errorf("editing a missing row = %v, want ErrTaskNotFound", err) + } +}