From a6b17ada8b0be864df1d03385343f158912c7f2c Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 5 Aug 2026 20:39:11 +0400 Subject: [PATCH 1/2] a live task can be edited, a resolved one cannot (V-509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SetTaskStatus was the only mutation on a task row, so a typo in a dictated task was permanent and a deadline could not move. EditTask rewrites the three fields capture set — text, due date and weight — and nothing else. Status stays the one-way ladder SetTaskStatus owns. Two things the task asked to settle. A text edit re-normalises the dedupe key and can collide with another live row. That is ErrTaskDuplicate, a refusal rather than a merge: two live rows carry two provenances, two capture times and possibly two external identities, and merging picks a winner for all three with nobody asked. The surface names the row that holds the text. A resolved task is refused outright (ErrTaskResolved). Its text is the record of what was finished, and rewriting it rewrites history. due nil clears the date, because clearing has to be sayable — an absent date and "remove the date" cannot be one argument. --- internal/ipc/api.go | 17 ++++++++ internal/ipc/client.go | 4 ++ internal/ipc/coreapi.go | 5 +++ internal/ipc/maperr_test.go | 2 + internal/ipc/server.go | 3 ++ internal/ipc/storeapi.go | 8 ++++ internal/ipc/unimplemented.go | 3 ++ internal/ipc/wire.go | 1 + internal/store/tasks.go | 54 ++++++++++++++++++++++++ internal/store/tasks_test.go | 77 +++++++++++++++++++++++++++++++++++ 10 files changed, 174 insertions(+) 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) + } +} From b95a0278a4a456a4d8c944cb47d3eeaad7544657 Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 5 Aug 2026 20:39:22 +0400 Subject: [PATCH 2/2] /tasks edits a task in place (V-509) The open list carries the text, the date and the importance as an inline form with a save button. The status is not in it: that ladder is one-way and has its own two buttons. The step-up gate was re-argued rather than inherited, which is what the task asked for, and edit stays ungated. It rewrites a line on a list he reads himself, the same blast radius drop already has here, and the store refuses the two edits that would cost something. A collision is named ("another open task already says this"), not merged. A weight outside the three rungs keeps its own option in the select, or saving an unrelated edit would silently reset it to normal. --- cmd/mavweb/main.go | 115 ++++++++++++++++++++++++++++++++++-------- cmd/mavweb/tasks.html | 21 ++++++-- 2 files changed, 112 insertions(+), 24 deletions(-) diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index dfc94bf..e36ed3d 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -883,14 +883,19 @@ type taskRow struct { Created string Resolved string ResolvedBy string + // DueValue and Weight are the raw values the edit form posts back + // (Vikunja #509). Due above is for reading and says "—" for no date; a + // date input needs "2026-08-07" or the empty string. + DueValue string + Weight int // Why — the ranker's reason for this row's position (Vikunja #129), in // Russian, empty when nothing distinguished the task. Blank is the honest // rendering: he never said this one mattered more. Why string } -// handleTasks serves the task review surface (GET) and the four writes it -// offers (POST): add, confirm, done, drop. +// handleTasks serves the task review surface (GET) and the five writes it +// offers (POST): add, edit, confirm, done, drop. // // Not step-up gated, unlike /tools and /routines, and the difference is the // point: enabling a tool defines argv Maven will execute, and accepting a @@ -900,6 +905,13 @@ type taskRow struct { // still sits behind whatever transport auth fronts mavweb, like every other // page. // +// "edit" was re-argued on the same terms rather than inheriting the exemption +// (Vikunja #509), and it stays ungated. It rewrites a line on a list he reads +// himself, the same blast radius "drop" already has on this page, and the store +// refuses the two edits that would cost something: a resolved task keeps the +// text it was finished under, and a text collision with another live row is +// named instead of merged. +// // "confirm" is the only interesting move: it promotes a candidate Maven derived // from something she read into work he owns. That review step is why derived // tasks are captured as candidates in the first place. @@ -965,6 +977,7 @@ func handleTasks(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { ID: t.ID, Text: t.Text, Source: t.Source, Evidence: t.Evidence, Status: t.Status, Created: fmtTaskTime(&t.CreatedTs), Due: fmtTaskDate(t.Due), Resolved: fmtTaskTime(t.Resolved), + DueValue: fmtTaskDateValue(t.Due), Weight: t.Weight, Why: r.Reason, } if t.Status == "candidate" { @@ -1000,27 +1013,16 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri return "", errors.New("empty task text") } req := ipc.CaptureTaskReq{Text: text, Source: "tap:web", Status: "open", Ts: now()} - // Importance is his, stated on the form. Out-of-range values are - // clamped rather than rejected — a bad select is not worth a 400. - if v := r.FormValue("weight"); v != "" { - // strconv, not Sscanf: Sscanf("3junk", "%d") succeeds with 3, and a - // form value is not a place to accept trailing garbage. - wgt, err := strconv.Atoi(v) - if err != nil || wgt < 0 { - return "", fmt.Errorf("bad weight %q", v) - } - if wgt > tasks.MaxWeight { - wgt = tasks.MaxWeight - } - req.Weight = wgt + wgt, err := formWeight(r) + if err != nil { + return "", err } - if d := r.FormValue("due"); d != "" { - due, err := time.ParseInLocation("2006-01-02", d, now().Location()) - if err != nil { - return "", fmt.Errorf("bad due date %q", d) - } - req.Due = &due + req.Weight = wgt + due, err := formDue(r, now()) + if err != nil { + return "", err } + req.Due = due resp, err := core.CaptureTask(ctx, req) if err != nil { return "", err @@ -1038,6 +1040,36 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri if err != nil { return "", errors.New("invalid id") } + + if action == "edit" { + // The three fields capture set, and only those (Vikunja #509). Status + // is not editable here: that ladder is one-way and has its own buttons. + text := strings.TrimSpace(r.FormValue("text")) + if text == "" { + return "", errors.New("empty task text") + } + wgt, err := formWeight(r) + if err != nil { + return "", err + } + due, err := formDue(r, now()) + if err != nil { + return "", err + } + switch err := core.EditTask(ctx, id, text, due, wgt); { + case err == nil: + return "saved task", nil + case errors.Is(err, ipc.ErrTaskDuplicate): + // Naming the collision instead of merging: two live rows carry two + // provenances, and picking one is not the page's call. + return "", errors.New("another open task already says this — drop one of the two") + case errors.Is(err, ipc.ErrTaskResolved): + return "", errors.New("a resolved task keeps the text it was finished under") + default: + return "", err + } + } + var status, msg string switch action { case "confirm": @@ -1061,6 +1093,47 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri return msg, nil } +// fmtTaskDateValue renders a due date the way requires, or +// "" for no date. Separate from fmtTaskDate, which renders it for reading. +// formWeight reads the importance select. Out-of-range clamps rather than +// rejects — a bad select is not worth a 400 — but trailing garbage is refused, +// because strconv is not Sscanf and "3junk" is not a 3. +func formWeight(r *http.Request) (int, error) { + v := r.FormValue("weight") + if v == "" { + return 0, nil + } + wgt, err := strconv.Atoi(v) + if err != nil || wgt < 0 { + return 0, fmt.Errorf("bad weight %q", v) + } + if wgt > tasks.MaxWeight { + wgt = tasks.MaxWeight + } + return wgt, nil +} + +// formDue reads the date input. An empty field is nil, which on an edit means +// "clear the date" — the form has no other way to say it. +func formDue(r *http.Request, now time.Time) (*time.Time, error) { + d := r.FormValue("due") + if d == "" { + return nil, nil + } + due, err := time.ParseInLocation("2006-01-02", d, now.Location()) + if err != nil { + return nil, fmt.Errorf("bad due date %q", d) + } + return &due, nil +} + +func fmtTaskDateValue(t *time.Time) string { + if t == nil || t.IsZero() { + return "" + } + return t.Local().Format("2006-01-02") +} + func fmtTaskTime(t *time.Time) string { if t == nil || t.IsZero() { return "—" diff --git a/cmd/mavweb/tasks.html b/cmd/mavweb/tasks.html index e6daba3..583a486 100644 --- a/cmd/mavweb/tasks.html +++ b/cmd/mavweb/tasks.html @@ -60,12 +60,27 @@

open {{len .Open}}

most pressing first — by the deadlines and the urgency you gave. nothing about a task is guessed; the only signal that is not yours is age, which lifts anything sitting here for weeks.
{{if .Open}}
- + {{range .Open}} - + + -
taskwhyfromduecaptured
taskwhyfromcaptured
{{.Text}}
+ + + + + +
 {{.Why}} {{.Source}}{{.Due}} {{.Created}}