diff options
| -rw-r--r-- | internal/handlers/widget.go | 14 | ||||
| -rw-r--r-- | internal/handlers/widget_test.go | 20 | ||||
| -rw-r--r-- | internal/store/native_tasks.go | 60 | ||||
| -rw-r--r-- | internal/store/native_tasks_test.go | 96 |
4 files changed, 180 insertions, 10 deletions
diff --git a/internal/handlers/widget.go b/internal/handlers/widget.go index 876aaad..6c3b455 100644 --- a/internal/handlers/widget.go +++ b/internal/handlers/widget.go @@ -2,12 +2,14 @@ package handlers import ( "encoding/json" + "errors" "net/http" "strings" "time" "task-dashboard/internal/config" "task-dashboard/internal/models" + "task-dashboard/internal/store" ) // WidgetAuthMiddleware validates the static bearer token for widget API endpoints. @@ -126,6 +128,10 @@ func (h *Handler) HandleWidgetReschedule(w http.ResponseWriter, r *http.Request) tz := config.GetDisplayTimezone() dueDate := time.Date(parsed.Year(), parsed.Month(), parsed.Day(), 0, 0, 0, 0, tz) if err := h.store.RescheduleNativeTask(req.ID, dueDate); err != nil { + if errors.Is(err, store.ErrNativeTaskNotFound) { + http.Error(w, "task not found", http.StatusNotFound) + return + } http.Error(w, "failed to reschedule", http.StatusInternalServerError) return } @@ -143,6 +149,14 @@ func (h *Handler) HandleWidgetComplete(w http.ResponseWriter, r *http.Request) { switch req.Source { case "doot": if err := h.store.CompleteNativeTask(req.ID); err != nil { + if errors.Is(err, store.ErrNativeTaskNotFound) { + // Surfaced as 404 (not a silent 200) so the caller -- the + // Android widget -- can tell "nothing changed" apart from + // "it worked", instead of the previous behavior where a + // stale/wrong id looked identical to a real completion. + http.Error(w, "task not found", http.StatusNotFound) + return + } http.Error(w, "failed to complete task", http.StatusInternalServerError) return } diff --git a/internal/handlers/widget_test.go b/internal/handlers/widget_test.go index f56fe07..850dc07 100644 --- a/internal/handlers/widget_test.go +++ b/internal/handlers/widget_test.go @@ -121,6 +121,26 @@ func TestHandleWidgetComplete_NonCompletable(t *testing.T) { } } +// TestHandleWidgetComplete_UnknownID_Returns404 proves the 2026-07-12 fix at +// the handler layer: a "doot" completion for an id that doesn't exist must +// surface as 404, not the previous silent 200 (see +// store.ErrNativeTaskNotFound's doc comment for the underlying bug this +// closes -- a real production incident where the widget's completeTask tap +// intermittently looked like it worked but changed nothing). +func TestHandleWidgetComplete_UnknownID_Returns404(t *testing.T) { + s, cleanup := setupTestDB(t) + defer cleanup() + h := &Handler{store: s} + body := `{"id":"does-not-exist","source":"doot"}` + req := httptest.NewRequest("POST", "/api/widget/complete", strings.NewReader(body)) + w := httptest.NewRecorder() + http.HandlerFunc(h.HandleWidgetComplete).ServeHTTP(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d", w.Code) + } +} + func TestWidgetAuthMiddleware_EmptyToken(t *testing.T) { inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) h := WidgetAuthMiddleware("", inner) diff --git a/internal/store/native_tasks.go b/internal/store/native_tasks.go index 5758407..6f8f999 100644 --- a/internal/store/native_tasks.go +++ b/internal/store/native_tasks.go @@ -1,12 +1,21 @@ package store import ( + "database/sql" "encoding/json" + "errors" "time" "task-dashboard/internal/models" ) +// ErrNativeTaskNotFound is returned by CompleteNativeTask, UncompleteNativeTask, +// and RescheduleNativeTask when no row matches the given id -- previously these +// three silently reported success on a 0-row UPDATE (Exec's err is nil even when +// no rows match), so a stale or wrong id from a caller looked identical to a real +// completion: the HTTP response was 200, but nothing in the database changed. +var ErrNativeTaskNotFound = errors.New("native task not found") + // GetNativeTasks returns all non-completed native tasks. func (s *Store) GetNativeTasks() ([]models.Task, error) { rows, err := s.db.Query(` @@ -71,31 +80,62 @@ func (s *Store) UpdateNativeTask(id, content, description string) error { return err } -// CompleteNativeTask marks a task as completed. +// CompleteNativeTask marks a task as completed. Returns ErrNativeTaskNotFound +// if id doesn't match any row. func (s *Store) CompleteNativeTask(id string) error { - _, err := s.db.Exec(` + result, err := s.db.Exec(` UPDATE native_tasks SET completed = 1, updated_at = CURRENT_TIMESTAMP WHERE id = ? `, id) - return err + if err != nil { + return err + } + return checkRowsAffected(result) } -// RescheduleNativeTask sets a new due date on a task. +// RescheduleNativeTask sets a new due date on a task. Returns +// ErrNativeTaskNotFound if id doesn't match any row. func (s *Store) RescheduleNativeTask(id string, dueDate time.Time) error { - _, err := s.db.Exec(` + result, err := s.db.Exec(` UPDATE native_tasks SET due_date = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ? `, dueDate, id) - return err + if err != nil { + return err + } + return checkRowsAffected(result) } -// UncompleteNativeTask marks a task as not completed. +// UncompleteNativeTask marks a task as not completed. Returns +// ErrNativeTaskNotFound if id doesn't match any row. func (s *Store) UncompleteNativeTask(id string) error { - _, err := s.db.Exec(` + result, err := s.db.Exec(` UPDATE native_tasks SET completed = 0, updated_at = CURRENT_TIMESTAMP WHERE id = ? `, id) - return err + if err != nil { + return err + } + return checkRowsAffected(result) +} + +// checkRowsAffected returns ErrNativeTaskNotFound if the update matched no +// rows -- mirrors the RowsAffected() check already used in sqlite.go's +// ApproveAgentSession/DenyAgentSession for the same "silent 0-row update" +// class of bug. +func checkRowsAffected(result sql.Result) error { + affected, err := result.RowsAffected() + if err != nil { + return err + } + if affected == 0 { + return ErrNativeTaskNotFound + } + return nil } -func scanNativeTasks(rows interface{ Next() bool; Scan(...interface{}) error; Err() error }) ([]models.Task, error) { +func scanNativeTasks(rows interface { + Next() bool + Scan(...interface{}) error + Err() error +}) ([]models.Task, error) { var tasks []models.Task for rows.Next() { var t models.Task diff --git a/internal/store/native_tasks_test.go b/internal/store/native_tasks_test.go new file mode 100644 index 0000000..5c11d3a --- /dev/null +++ b/internal/store/native_tasks_test.go @@ -0,0 +1,96 @@ +package store + +import ( + "database/sql" + "errors" + "path/filepath" + "testing" + "time" + + _ "github.com/mattn/go-sqlite3" +) + +// newNativeTasksTestStore creates a Store backed by a fresh temp sqlite DB +// with just the native_tasks table -- enough to exercise +// CompleteNativeTask/UncompleteNativeTask/RescheduleNativeTask without +// running the full migration set. +func newNativeTasksTestStore(t *testing.T) *Store { + t.Helper() + dbPath := filepath.Join(t.TempDir(), "test.db") + db, err := sql.Open("sqlite3", dbPath) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Close() }) + if _, err := db.Exec(` + CREATE TABLE native_tasks ( + id TEXT PRIMARY KEY, + content TEXT NOT NULL, + description TEXT DEFAULT '', + project_name TEXT DEFAULT '', + due_date DATETIME, + priority INTEGER DEFAULT 1, + completed BOOLEAN DEFAULT 0, + labels TEXT DEFAULT '[]', + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME DEFAULT CURRENT_TIMESTAMP + ) + `); err != nil { + t.Fatal(err) + } + if _, err := db.Exec(`INSERT INTO native_tasks (id, content) VALUES ('real-1', 'Real task')`); err != nil { + t.Fatal(err) + } + return &Store{db: db} +} + +// TestCompleteNativeTask_UnknownID_ReturnsErrNotFound proves the 2026-07-12 +// fix: a plain UPDATE ... WHERE id = ? silently "succeeds" with a nil error +// when 0 rows match (this is how database/sql's Exec behaves for an UPDATE +// that matches nothing -- no error, just RowsAffected() == 0). Before this +// fix, CompleteNativeTask returned that nil error straight through, so a +// stale/wrong id from a caller (the Android widget, in the real incident +// this was found from) looked identical to a real completion: HTTP 200, +// nothing changed in the database. +func TestCompleteNativeTask_UnknownID_ReturnsErrNotFound(t *testing.T) { + s := newNativeTasksTestStore(t) + + err := s.CompleteNativeTask("does-not-exist") + if !errors.Is(err, ErrNativeTaskNotFound) { + t.Fatalf("expected ErrNativeTaskNotFound, got %v", err) + } +} + +func TestCompleteNativeTask_RealID_Succeeds(t *testing.T) { + s := newNativeTasksTestStore(t) + + if err := s.CompleteNativeTask("real-1"); err != nil { + t.Fatalf("CompleteNativeTask: %v", err) + } + + var completed bool + if err := s.db.QueryRow(`SELECT completed FROM native_tasks WHERE id = 'real-1'`).Scan(&completed); err != nil { + t.Fatal(err) + } + if !completed { + t.Error("expected task to be marked completed") + } +} + +func TestUncompleteNativeTask_UnknownID_ReturnsErrNotFound(t *testing.T) { + s := newNativeTasksTestStore(t) + + err := s.UncompleteNativeTask("does-not-exist") + if !errors.Is(err, ErrNativeTaskNotFound) { + t.Fatalf("expected ErrNativeTaskNotFound, got %v", err) + } +} + +func TestRescheduleNativeTask_UnknownID_ReturnsErrNotFound(t *testing.T) { + s := newNativeTasksTestStore(t) + + err := s.RescheduleNativeTask("does-not-exist", time.Now()) + if !errors.Is(err, ErrNativeTaskNotFound) { + t.Fatalf("expected ErrNativeTaskNotFound, got %v", err) + } +} |
