From f08f06bef47aac2c9effb4cec650d99c2deb2dd7 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Sat, 18 Jul 2026 00:14:45 +0000 Subject: Rework Tasks tab: Chains section, checklist modal, project visibility The flat Tasks-tab atom list was silently dumping every chain step (locked and unlocked) and dormant bucket-pool items in as ordinary undated cards, with no chain/project context and no protection against completing a locked step out of order. - CompleteNativeTask now rejects completing a locked chain task (ErrChainTaskLocked), mapped to 400 in both the widget and web complete-atom handlers. - Chain tasks and dormant bucket items are excluded from the flat atom list; a new "Chains" section shows one card per active/paused chain with the current step and N/M progress. - New chain checklist modal (GET /chains/{id}) lists every position in order with pause/resume/abandon -- the web view originally deferred as Android-only. - Fixed a real bug this surfaced: resuming a paused chain only flipped the status flag, never unlocking the deferred successor, so a chain paused right after a completion stayed stuck forever. SetChainStatus now catches up the deferred advancement on resume, idempotently. - Atom cards gained a project-name chip for general visibility. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EZ7ikw2ukGJFTHE3bJS7zL --- internal/store/chains.go | 111 +++++++++++++++++++++++++++++++++-------- internal/store/chains_test.go | 68 ++++++++++++++++++++++++- internal/store/native_tasks.go | 26 +++++++++- 3 files changed, 180 insertions(+), 25 deletions(-) (limited to 'internal/store') diff --git a/internal/store/chains.go b/internal/store/chains.go index f3bd0d7..f854678 100644 --- a/internal/store/chains.go +++ b/internal/store/chains.go @@ -84,6 +84,31 @@ func (s *Store) GetChain(id string) (*models.Chain, error) { return &c, nil } +// GetChains returns every active or paused chain, oldest first. Completed +// and abandoned chains are excluded -- they're done, not part of "current +// work" surfaces like the Tasks tab. +func (s *Store) GetChains() ([]models.Chain, error) { + rows, err := s.db.Query(` + SELECT id, project_id, status, created_at FROM task_chains + WHERE status IN ('active', 'paused') + ORDER BY created_at ASC + `) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + var chains []models.Chain + for rows.Next() { + var c models.Chain + if err := rows.Scan(&c.ID, &c.ProjectID, &c.Status, &c.CreatedAt); err != nil { + return nil, err + } + chains = append(chains, c) + } + return chains, rows.Err() +} + // GetChainTasks returns every task in the chain, locked and unlocked both, // ordered by chain_position -- the full checklist view. func (s *Store) GetChainTasks(chainID string) ([]models.Task, error) { @@ -104,8 +129,34 @@ func (s *Store) GetChainTasks(chainID string) ([]models.Task, error) { } // SetChainStatus updates a chain's status (active/paused/abandoned/completed). -// Returns ErrNativeTaskNotFound if id doesn't match any row. +// Resuming a paused chain (status "active" from a current status of +// "paused") catches up any advancement that was deferred while paused -- +// per the design, "the chain must be explicitly resumed for the next task +// to unlock," i.e. resuming itself performs the unlock, not merely +// re-arming future completions to do so. Without this, a chain paused +// immediately after a completion (before its successor could unlock) would +// stay stuck forever: no task is ever unlocked, so no future completion +// could trigger advanceChain either. Returns ErrNativeTaskNotFound if id +// doesn't match any row. func (s *Store) SetChainStatus(id, status string) error { + if status == "active" { + current, err := s.GetChain(id) + if err != nil { + return err + } + if current.Status == "paused" { + maxCompleted, err := s.maxCompletedChainPosition(id) + if err != nil { + return err + } + if maxCompleted != nil { + if err := s.advanceChain(id, *maxCompleted); err != nil { + return err + } + } + } + } + result, err := s.db.Exec(`UPDATE task_chains SET status = ? WHERE id = ?`, status, id) if err != nil { return err @@ -113,34 +164,50 @@ func (s *Store) SetChainStatus(id, status string) error { return checkRowsAffected(result) } -// advanceChain is called from CompleteNativeTask when the just-completed -// task has a chain_id set. Per the design, a paused chain does not -// auto-advance -- it must be explicitly resumed first. Completing the last -// position marks the chain completed instead of advancing. -func (s *Store) advanceChain(chainID string, completedPosition int) error { - chain, err := s.GetChain(chainID) +// maxCompletedChainPosition returns the highest chain_position among +// completed tasks in the chain, or nil if none are completed yet. +func (s *Store) maxCompletedChainPosition(chainID string) (*int, error) { + var pos sql.NullInt64 + err := s.db.QueryRow(` + SELECT MAX(chain_position) FROM native_tasks WHERE chain_id = ? AND completed = 1 + `, chainID).Scan(&pos) if err != nil { - return err + return nil, err } - if chain.Status == "paused" { - return nil + if !pos.Valid { + return nil, nil } + p := int(pos.Int64) + return &p, nil +} - now := config.Now() - result, err := s.db.Exec(` - UPDATE native_tasks SET chain_unlocked = 1, due_date = ?, updated_at = ? - WHERE chain_id = ? AND chain_position = ? - `, now, now, chainID, completedPosition+1) - if err != nil { - return err +// advanceChain is called from CompleteNativeTask (when the just-completed +// task has a chain_id set and the chain isn't paused) and from +// SetChainStatus's resume catch-up. Idempotent: if the successor position +// is already unlocked, it's left untouched (so a resume catch-up run +// against a chain that was never actually stuck is a no-op, not a +// due_date-resetting re-unlock). Completing/catching-up-to the last +// position marks the chain completed instead of unlocking a successor. +func (s *Store) advanceChain(chainID string, completedPosition int) error { + var successorID string + var successorUnlocked bool + err := s.db.QueryRow(` + SELECT id, chain_unlocked FROM native_tasks WHERE chain_id = ? AND chain_position = ? + `, chainID, completedPosition+1).Scan(&successorID, &successorUnlocked) + if err == sql.ErrNoRows { + // No next position -- the completed task was the last in the chain. + return s.SetChainStatus(chainID, "completed") } - affected, err := result.RowsAffected() if err != nil { return err } - if affected == 0 { - // No next position -- the completed task was the last in the chain. - return s.SetChainStatus(chainID, "completed") + if successorUnlocked { + return nil } - return nil + + now := config.Now() + _, err = s.db.Exec(` + UPDATE native_tasks SET chain_unlocked = 1, due_date = ?, updated_at = ? WHERE id = ? + `, now, now, successorID) + return err } diff --git a/internal/store/chains_test.go b/internal/store/chains_test.go index 1d16508..6359d11 100644 --- a/internal/store/chains_test.go +++ b/internal/store/chains_test.go @@ -132,10 +132,19 @@ func TestCompleteNativeTask_PausedChain_DoesNotAdvance(t *testing.T) { t.Error("position 1 should still be locked while chain is paused") } - // Resuming re-enables advancement on the *next* completion. + // Resuming performs the deferred unlock itself -- position 1 becomes + // completable immediately, not only after some future completion. if err := s.SetChainStatus(chain.ID, "active"); err != nil { t.Fatal(err) } + resumed, err := s.GetChainTasks(chain.ID) + if err != nil { + t.Fatal(err) + } + if !resumed[1].ChainUnlocked { + t.Fatal("expected resume to unlock position 1 immediately (deferred advancement catch-up)") + } + if err := s.CompleteNativeTask(tasks[1].ID); err != nil { t.Fatalf("CompleteNativeTask after resume: %v", err) } @@ -148,6 +157,63 @@ func TestCompleteNativeTask_PausedChain_DoesNotAdvance(t *testing.T) { } } +func TestCompleteNativeTask_LockedChainTask_ReturnsErrChainTaskLocked(t *testing.T) { + s := newNativeTasksTestStore(t) + + chain, err := s.CreateChain("Track", chainTasks("Step 1", "Step 2")) + if err != nil { + t.Fatal(err) + } + tasks, err := s.GetChainTasks(chain.ID) + if err != nil { + t.Fatal(err) + } + + if err := s.CompleteNativeTask(tasks[1].ID); err != ErrChainTaskLocked { + t.Errorf("err = %v, want ErrChainTaskLocked", err) + } + + // Confirm nothing was mutated -- still locked, still not completed. + after, err := s.GetChainTasks(chain.ID) + if err != nil { + t.Fatal(err) + } + if after[1].Completed || after[1].ChainUnlocked { + t.Errorf("locked task should be untouched by the rejected completion attempt: %+v", after[1]) + } +} + +func TestSetChainStatus_ResumeWithNothingStuck_DoesNotResetDueDate(t *testing.T) { + s := newNativeTasksTestStore(t) + + chain, err := s.CreateChain("Track", chainTasks("Step 1", "Step 2")) + if err != nil { + t.Fatal(err) + } + tasks, err := s.GetChainTasks(chain.ID) + if err != nil { + t.Fatal(err) + } + originalDueDate := *tasks[0].DueDate + + // Pause and resume with nothing completed yet -- position 0 is already + // unlocked and should be left untouched by the resume catch-up. + if err := s.SetChainStatus(chain.ID, "paused"); err != nil { + t.Fatal(err) + } + if err := s.SetChainStatus(chain.ID, "active"); err != nil { + t.Fatal(err) + } + + after, err := s.GetChainTasks(chain.ID) + if err != nil { + t.Fatal(err) + } + if !after[0].DueDate.Equal(originalDueDate) { + t.Errorf("DueDate = %v, want unchanged %v (resume catch-up should be a no-op when nothing was stuck)", after[0].DueDate, originalDueDate) + } +} + func TestGetUndatedNativeTasks_ExcludesLockedChainTasks(t *testing.T) { s := newNativeTasksTestStore(t) diff --git a/internal/store/native_tasks.go b/internal/store/native_tasks.go index 6f8ba7a..11a9197 100644 --- a/internal/store/native_tasks.go +++ b/internal/store/native_tasks.go @@ -161,15 +161,27 @@ func (s *Store) UpdateNativeTaskDescription(id, description string) error { return err } +// ErrChainTaskLocked is returned by CompleteNativeTask when the task +// belongs to a chain but isn't the currently-unlocked position -- without +// this guard, completing a locked task directly by id (bypassing the UI, +// which never renders a checkbox for locked chain tasks) would still run +// advanceChain against the wrong position, breaking the chain's WIP-1 +// invariant. +var ErrChainTaskLocked = errors.New("task is locked in its chain") + // CompleteNativeTask marks a task as completed. If it's the latest // occurrence of a recurring series (no newer row exists yet), it also // creates the next iteration. Returns ErrNativeTaskNotFound if id doesn't -// match any row. +// match any row, or ErrChainTaskLocked if it's a locked (not yet +// actionable) chain task. func (s *Store) CompleteNativeTask(id string) error { task, err := s.GetNativeTaskByID(id) if err != nil { return err } + if task.ChainID != "" && !task.ChainUnlocked { + return ErrChainTaskLocked + } result, err := s.db.Exec(` UPDATE native_tasks SET completed = 1, updated_at = CURRENT_TIMESTAMP WHERE id = ? @@ -182,9 +194,19 @@ func (s *Store) CompleteNativeTask(id string) error { } if task.ChainID != "" { - if err := s.advanceChain(task.ChainID, task.ChainPosition); err != nil { + chain, err := s.GetChain(task.ChainID) + if err != nil { return err } + // A paused chain does not auto-advance -- completing its unlocked + // task is still allowed (it's the one actionable step), but the + // successor stays locked until the chain is explicitly resumed + // (see SetChainStatus's resume catch-up). + if chain.Status != "paused" { + if err := s.advanceChain(task.ChainID, task.ChainPosition); err != nil { + return err + } + } } if task.BucketID != "" { -- cgit v1.2.3