Merge the definition of done and the blocker (#185)

cmd/mavweb/main.go

@@ -1050,6 +1050,12 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri
 		return "", fmt.Errorf("unknown action %q", action)
 	}
 	if err := core.SetTaskStatus(ctx, id, status, now(), "tap:web"); err != nil {
+		if errors.Is(err, ipc.ErrTaskNoDoneWhen) {
+			// The refusal has to name what is missing, or the button looks
+			// broken. The field it asks for arrives with the intake form
+			// (Vikunja #511).
+			return "", errors.New("write a definition of done before confirming this candidate")
+		}
 		return "", err
 	}
 	return msg, nil

internal/ipc/api.go

@@ -142,6 +142,11 @@ type Task struct {
 	Weight     int        `json:"weight,omitempty"`
 	Resolved   *time.Time `json:"resolved,omitempty"`
 	ResolvedBy string     `json:"resolved_by,omitempty"`
+	// DoneWhen — the acceptance criterion. Empty until he writes one, and a
+	// candidate with no criterion cannot be promoted to open (Vikunja #510).
+	DoneWhen string `json:"done_when,omitempty"`
+	// BlockedOn — a canonical Nexus entity id, never a name.
+	BlockedOn string `json:"blocked_on,omitempty"`
 }
 
 // CaptureTaskReq — THE INTAKE SEAM. Everything that captures a task goes
@@ -170,6 +175,11 @@ type CaptureTaskReq struct {
 	Due        *time.Time `json:"due,omitempty"`
 	Weight     int        `json:"weight,omitempty"`
 	Ts         time.Time  `json:"ts"`
+	// DoneWhen and BlockedOn are optional at intake. A derived source leaves
+	// both empty: mail says what to do, not what finishing means, and guessing
+	// a criterion would put Maven's reading in the field he is meant to write.
+	DoneWhen  string `json:"done_when,omitempty"`
+	BlockedOn string `json:"blocked_on,omitempty"`
 }
 
 // CaptureTaskResp — Created is false when the same live task already existed,
@@ -539,6 +549,16 @@ type setTaskStatusReq struct {
 	By string `json:"by,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
+// "make it empty".
+type setTaskFieldsReq struct {
+	ID        int64  `json:"id"`
+	DoneWhen  string `json:"done_when,omitempty"`
+	BlockedOn string `json:"blocked_on,omitempty"`
+}
+
 // idReq — methods keyed by a single id.
 type idReq struct {
 	ID int64 `json:"id"`
@@ -847,6 +867,11 @@ type unlockReq struct {
 // wire round-tripping via errors.Is).
 var ErrToolNotFound = errors.New("ipc: tool not found")
 
+// ErrTaskNoDoneWhen — a candidate cannot be promoted to open with no
+// definition of done (Vikunja #510). Carried across the wire so the /tasks
+// form can say which refusal it hit rather than "не найдено".
+var ErrTaskNoDoneWhen = errors.New("ipc: task has no definition of done")
+
 // 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

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) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error {
+	return c.call(ctx, MethodSetTaskFields, setTaskFieldsReq{ID: id, DoneWhen: doneWhen, BlockedOn: blockedOn}, nil)
+}
+
 // IngestMail hands one fetched message to core for extraction. ErrUnknownMethod
 // means core has no email block configured — the caller should stop asking, not
 // retry.

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
+	// 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
+	// entity id or empty, never a name the caller had lying around.
+	SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error
 }
 
 // SystemAPI — what the daemon knows about itself, plus the one method that

internal/ipc/maperr_test.go

@@ -31,6 +31,7 @@ var mapErrPairs = []struct {
 	{"ErrReminderNotFound", store.ErrReminderNotFound, ErrReminderNotFound},
 	{"ErrReminderState", store.ErrReminderState, ErrReminderState},
 	{"ErrToolNotFound", store.ErrToolNotFound, ErrToolNotFound},
+	{"ErrTaskNoDoneWhen", store.ErrTaskNoDoneWhen, ErrTaskNoDoneWhen},
 }
 
 // unmappedStoreErrors — store sentinels that deliberately have no wire twin,

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)
 	}),
+	MethodSetTaskFields: withParamsVoid(func(ctx context.Context, api CoreAPI, p setTaskFieldsReq) error {
+		return api.SetTaskFields(ctx, p.ID, p.DoneWhen, p.BlockedOn)
+	}),
 	MethodListProposedRoutines: withoutParams(func(ctx context.Context, api CoreAPI) (listProposedRoutinesResp, error) {
 		out, err := api.ListProposedRoutines(ctx)
 		if err != nil {

internal/ipc/storeapi.go

@@ -317,6 +317,8 @@ func (a *storeAPI) CaptureTask(ctx context.Context, req CaptureTaskReq) (Capture
 		Status:     req.Status,
 		Due:        req.Due,
 		Weight:     req.Weight,
+		DoneWhen:   req.DoneWhen,
+		BlockedOn:  req.BlockedOn,
 	})
 	if err != nil {
 		return CaptureTaskResp{}, mapErr(err)
@@ -343,6 +345,8 @@ func (a *storeAPI) ListTasks(ctx context.Context, status string) ([]Task, error)
 			Weight:     t.Weight,
 			Resolved:   t.ResolvedTs,
 			ResolvedBy: t.ResolvedBy,
+			DoneWhen:   t.DoneWhen,
+			BlockedOn:  t.BlockedOn,
 		}
 	}
 	return out, nil
@@ -352,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) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error {
+	return mapErr(a.s.SetTaskFields(ctx, id, doneWhen, blockedOn))
+}
+
 func (a *storeAPI) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, error) {
 	rs, err := a.s.ListProposedRoutines(ctx)
 	if err != nil {
@@ -458,6 +466,8 @@ func mapErr(err error) error {
 		return ErrReminderState
 	case errors.Is(err, store.ErrToolNotFound):
 		return ErrToolNotFound
+	case errors.Is(err, store.ErrTaskNoDoneWhen):
+		return ErrTaskNoDoneWhen
 	}
 	return err
 }

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) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error {
+	return ErrNotImplemented
+}
 func (UnimplementedCoreAPI) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, error) {
 	return nil, ErrNotImplemented
 }

internal/ipc/wire.go

@@ -55,6 +55,7 @@ const (
 	MethodCaptureTask            Method = "capture_task"
 	MethodListTasks              Method = "list_tasks"
 	MethodSetTaskStatus          Method = "set_task_status"
+	MethodSetTaskFields          Method = "set_task_fields"
 	MethodIngestMail             Method = "ingest_mail"
 	MethodSwapModel              Method = "swap_model"
 	MethodModelStatus            Method = "model_status"

internal/store/migrations.go

@@ -290,6 +290,16 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2
 	 ALTER TABLE nudges_new RENAME TO nudges;
 	 CREATE INDEX IF NOT EXISTS idx_nudges_rule_ts ON nudges (rule, ts DESC);
 	 CREATE INDEX IF NOT EXISTS idx_nudges_outcome ON nudges (outcome);`,
+
+	// #22 — the two columns that make tasks a work board rather than a to-do
+	// list (Vikunja #510). done_when is the acceptance criterion, and blocked_on
+	// is a canonical Nexus entity id: it names a person, identity lives in
+	// Nexus, and a local free-text name would be a second answer to a question
+	// Nexus already owns. Both default to empty rather than NULL, because "he
+	// has not written one" and "there is nothing to write" are the same state
+	// here and no caller has to tell them apart.
+	`ALTER TABLE tasks ADD COLUMN done_when TEXT NOT NULL DEFAULT '';
+	 ALTER TABLE tasks ADD COLUMN blocked_on TEXT NOT NULL DEFAULT '';`,
 }
 
 // migrate applies every migration with a number greater than the DB's current

internal/store/tasks.go

@@ -66,6 +66,16 @@ type Task struct {
 	Weight     int
 	ResolvedTs *time.Time
 	ResolvedBy string
+
+	// DoneWhen — the acceptance criterion, in his words. It must be able to
+	// close on either outcome: "it already works" counts as complete, and a
+	// criterion only one result satisfies is a wish rather than a definition
+	// (Vikunja #510). Empty until he writes one.
+	DoneWhen string
+	// BlockedOn — a canonical Nexus entity id, never a name. Identity lives in
+	// Nexus, so storing "Саша" here would be a second answer to a question
+	// Nexus already owns. Empty when nothing blocks the task.
+	BlockedOn string
 }
 
 // CaptureResult — what CaptureTask did. Created is a new row. Promoted is an
@@ -82,6 +92,12 @@ var (
 	ErrTaskNotFound = errors.New("store: task not found")
 	ErrTaskEmpty    = errors.New("store: task text is empty")
 	ErrTaskStatus   = errors.New("store: invalid task status")
+	// ErrTaskNoDoneWhen — a candidate cannot be promoted to open without a
+	// definition of done (Vikunja #510). Same refusal ParseTaskCapture makes
+	// for a capture marker with nothing after it: confirming work whose
+	// 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")
 )
 
 // liveTaskStatuses — the two statuses that count as outstanding work.
@@ -160,10 +176,10 @@ func (s *Store) CaptureTask(ctx context.Context, t Task) (CaptureResult, error)
 	// Untargeted DO NOTHING: either unique index may be the one that fires, and
 	// the lookup below sorts out which.
 	res, err := s.db.ExecContext(ctx,
-		`INSERT INTO tasks (created_ts, text, norm, source, evidence, ext_id, status, due_ts, weight)
+		`INSERT INTO tasks (created_ts, text, norm, source, evidence, ext_id, status, due_ts, weight, done_when, blocked_on)
-		 VALUES (?,?,?,?,?,?,?,?,?)
+		 VALUES (?,?,?,?,?,?,?,?,?,?,?)
 		 ON CONFLICT DO NOTHING`,
-		created2.UnixMilli(), text, norm, t.Source, t.Evidence, ext, status, due, t.Weight)
+		created2.UnixMilli(), text, norm, t.Source, t.Evidence, ext, status, due, t.Weight, t.DoneWhen, t.BlockedOn)
 	if err != nil {
 		return CaptureResult{}, fmt.Errorf("capture task: %w", err)
 	}
@@ -194,7 +210,7 @@ func (s *Store) CaptureTask(ctx context.Context, t Task) (CaptureResult, error)
 		}
 	}
 	if status == TaskOpen && existing.Status == TaskCandidate {
-		if err := s.SetTaskStatus(ctx, existing.ID, TaskOpen, created2, t.Source); err != nil {
+		if err := s.setTaskStatus(ctx, existing.ID, TaskOpen, created2, t.Source, false); err != nil {
 			return CaptureResult{}, fmt.Errorf("capture task: promote candidate: %w", err)
 		}
 		return CaptureResult{ID: existing.ID, Promoted: true}, nil
@@ -231,7 +247,7 @@ func (s *Store) lookupLiveTaskByNorm(ctx context.Context, norm string) (Task, er
 	return t, nil
 }
 
-const taskSelect = `SELECT id, created_ts, text, source, evidence, COALESCE(ext_id,''), status, due_ts, weight, resolved_ts, resolved_by FROM tasks`
+const taskSelect = `SELECT id, created_ts, text, source, evidence, COALESCE(ext_id,''), status, due_ts, weight, resolved_ts, resolved_by, done_when, blocked_on FROM tasks`
 
 // LookupTask returns one task by id.
 func (s *Store) LookupTask(ctx context.Context, id int64) (Task, error) {
@@ -304,6 +320,16 @@ func (s *Store) ListTasks(ctx context.Context, status string) ([]Task, error) {
 // "tap:voice"). It is recorded on the row, so a task that turns up resolved
 // says what resolved it.
 func (s *Store) SetTaskStatus(ctx context.Context, id int64, status string, ts time.Time, by string) error {
+	return s.setTaskStatus(ctx, id, status, ts, by, true)
+}
+
+// setTaskStatus — the move, with the promotion gate optional.
+//
+// It is optional for exactly one caller: CaptureTask promoting a candidate he
+// stated out loud (Vikunja #510). Refusing there would deny intake rather than
+// ask for a criterion, and a direct open capture never had one either — the gate
+// belongs to the deliberate promotion on /tasks, where there is a form to fill.
+func (s *Store) setTaskStatus(ctx context.Context, id int64, status string, ts time.Time, by string, gateDoneWhen bool) error {
 	var from []string
 	switch status {
 	case TaskOpen:
@@ -329,6 +355,13 @@ func (s *Store) SetTaskStatus(ctx context.Context, id int64, status string, ts t
 	for _, f := range from {
 		args = append(args, f)
 	}
+	if status == TaskOpen && gateDoneWhen {
+		// Promotion needs an acceptance criterion. Checked in the same
+		// statement rather than read-then-write, so two callers confirming one
+		// candidate cannot race past it; the row is read afterwards only to say
+		// WHICH refusal this was.
+		q += ` AND done_when <> ''`
+	}
 	res, err := s.db.ExecContext(ctx, q, args...)
 	if err != nil {
 		return fmt.Errorf("set task status: %w", err)
@@ -338,11 +371,40 @@ func (s *Store) SetTaskStatus(ctx context.Context, id int64, status string, ts t
 		return fmt.Errorf("set task status: rows affected: %w", err)
 	}
 	if n == 0 {
+		if status == TaskOpen && gateDoneWhen {
+			if t, lookErr := s.LookupTask(ctx, id); lookErr == nil && t.Status == TaskCandidate && t.DoneWhen == "" {
+				return fmt.Errorf("%w: id=%d", ErrTaskNoDoneWhen, id)
+			}
+		}
 		return fmt.Errorf("%w: id=%d not in %v", ErrTaskNotFound, id, from)
 	}
 	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.
+//
+// blockedOn is a canonical Nexus entity id or empty. Free text does not belong
+// here — identity lives in Nexus, and a local name would be a second answer to
+// a question Nexus already owns. The caller resolves before it writes.
+func (s *Store) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error {
+	res, err := s.db.ExecContext(ctx,
+		`UPDATE tasks SET done_when = ?, blocked_on = ? WHERE id = ?`,
+		strings.TrimSpace(doneWhen), strings.TrimSpace(blockedOn), id)
+	if err != nil {
+		return fmt.Errorf("set task fields: %w", err)
+	}
+	n, err := res.RowsAffected()
+	if err != nil {
+		return fmt.Errorf("set task fields: rows affected: %w", err)
+	}
+	if n == 0 {
+		return fmt.Errorf("%w: id=%d", ErrTaskNotFound, id)
+	}
+	return nil
+}
+
 // NormalizeTaskText is the dedupe key: lowercased, punctuation dropped,
 // whitespace collapsed. Exported because the intake seam (and its tests) needs
 // to reason about what will and will not be treated as the same task.
@@ -372,7 +434,7 @@ func scanTask(sc scanner) (Task, error) {
 	var t Task
 	var created int64
 	var due, resolved sql.NullInt64
-	if err := sc.Scan(&t.ID, &created, &t.Text, &t.Source, &t.Evidence, &t.ExternalID, &t.Status, &due, &t.Weight, &resolved, &t.ResolvedBy); err != nil {
+	if err := sc.Scan(&t.ID, &created, &t.Text, &t.Source, &t.Evidence, &t.ExternalID, &t.Status, &due, &t.Weight, &resolved, &t.ResolvedBy, &t.DoneWhen, &t.BlockedOn); err != nil {
 		return Task{}, err
 	}
 	t.CreatedTs = time.UnixMilli(created).UTC()

internal/store/tasks_test.go

@@ -126,6 +126,11 @@ func TestSetTaskStatusMovesOnceForwardOnly(t *testing.T) {
 	if err := st.SetTaskStatus(ctx, cand, TaskDone, now, "tap:web"); !errors.Is(err, ErrTaskNotFound) {
 		t.Errorf("candidate→done err = %v, want ErrTaskNotFound", err)
 	}
+	// Promotion needs a definition of done; see
+	// TestPromotingACandidateNeedsADefinitionOfDone for that refusal.
+	if err := st.SetTaskFields(ctx, cand, "запись есть", ""); err != nil {
+		t.Fatal(err)
+	}
 	if err := st.SetTaskStatus(ctx, cand, TaskOpen, now, "tap:web"); err != nil {
 		t.Fatal(err)
 	}
@@ -245,7 +250,10 @@ func TestCaptureTaskExternalIDSurvivesResolution(t *testing.T) {
 	if !first.Created {
 		t.Fatal("first capture must create a row")
 	}
-	// He confirms it and does it.
+	// He confirms it and does it. Confirming needs a criterion (Vikunja #510).
+	if err := st.SetTaskFields(ctx, first.ID, "страховка продлена", ""); err != nil {
+		t.Fatal(err)
+	}
 	if err := st.SetTaskStatus(ctx, first.ID, TaskOpen, now.Add(time.Hour), "tap:web"); err != nil {
 		t.Fatal(err)
 	}
@@ -374,3 +382,86 @@ func TestListTasksIsBounded(t *testing.T) {
 		t.Errorf("first = %q, want the newest", all[0].Text)
 	}
 }
+
+func TestTaskBoardColumnsRoundTrip(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	now := time.Date(2026, 8, 5, 9, 0, 0, 0, time.UTC)
+
+	res, err := st.CaptureTask(ctx, Task{
+		Text: "оплатить интернет", Source: "tap:voice", CreatedTs: now,
+		DoneWhen: "квитанция оплачена", BlockedOn: "ent_kate",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	got, err := st.LookupTask(ctx, res.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.DoneWhen != "квитанция оплачена" || got.BlockedOn != "ent_kate" {
+		t.Fatalf("task = %+v, want both board columns back", got)
+	}
+
+	// Not one-way, unlike a status move: he sharpens the criterion, and the
+	// blocker clears when the person answers.
+	if err := st.SetTaskFields(ctx, res.ID, "  пришло подтверждение  ", ""); err != nil {
+		t.Fatal(err)
+	}
+	got, err = st.LookupTask(ctx, res.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.DoneWhen != "пришло подтверждение" {
+		t.Errorf("done_when = %q, want the trimmed rewrite", got.DoneWhen)
+	}
+	if got.BlockedOn != "" {
+		t.Errorf("blocked_on = %q, want it cleared", got.BlockedOn)
+	}
+	if err := st.SetTaskFields(ctx, 9999, "x", ""); !errors.Is(err, ErrTaskNotFound) {
+		t.Errorf("SetTaskFields on a missing row = %v, want ErrTaskNotFound", err)
+	}
+}
+
+func TestPromotingACandidateNeedsADefinitionOfDone(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	now := time.Date(2026, 8, 5, 9, 0, 0, 0, time.UTC)
+
+	res, err := st.CaptureTask(ctx, Task{
+		Text: "продлить домен", Source: "email:main", Status: TaskCandidate,
+		ExternalID: "msg-1:0", Evidence: "Domain expiring", CreatedTs: now,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// A board row whose finish line nobody wrote can never leave the board.
+	if err := st.SetTaskStatus(ctx, res.ID, TaskOpen, now, "tap:web"); !errors.Is(err, ErrTaskNoDoneWhen) {
+		t.Fatalf("promotion with no criterion = %v, want ErrTaskNoDoneWhen", err)
+	}
+	// Dropping it stays legal — declining work does not need one.
+	dropped, err := st.CaptureTask(ctx, Task{
+		Text: "перезвонить в банк", Source: "email:main", Status: TaskCandidate,
+		ExternalID: "msg-2:0", CreatedTs: now,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := st.SetTaskStatus(ctx, dropped.ID, TaskDropped, now, "tap:web"); err != nil {
+		t.Fatalf("dropping a candidate with no criterion: %v", err)
+	}
+
+	if err := st.SetTaskFields(ctx, res.ID, "домен продлён до 2027", ""); err != nil {
+		t.Fatal(err)
+	}
+	if err := st.SetTaskStatus(ctx, res.ID, TaskOpen, now, "tap:web"); err != nil {
+		t.Fatalf("promotion after writing a criterion: %v", err)
+	}
+	got, err := st.LookupTask(ctx, res.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.Status != TaskOpen {
+		t.Errorf("status = %q, want open", got.Status)
+	}
+}