summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-07-12 05:36:32 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-07-12 05:36:32 +0000
commit9a87f9db5af943ea253b814d4196020341e3c2ba (patch)
treeacaeb91e0fad131b15d4d6d41c5d0ea0254c2966
parent73fbd05a1230552bcef9834d3ee999ef19957693 (diff)
fix(widget): surface unmatched task IDs instead of silently no-opping
CompleteNativeTask/UncompleteNativeTask/RescheduleNativeTask ran a plain UPDATE ... WHERE id = ? and returned whatever error Exec gave back -- which is nil even when 0 rows match, since that's not a SQL error. A stale or wrong id from the widget looked identical to a real completion: HTTP 200, nothing changed in the database. Real incident: 1 of 3 widget completeTask taps silently no-opped this way. Now checks RowsAffected() and returns ErrNativeTaskNotFound (mirrors the existing pattern in sqlite.go's ApproveAgentSession/DenyAgentSession). HandleWidgetComplete and HandleWidgetReschedule surface this as 404 instead of a fake 200, so the widget can tell 'nothing changed' apart from 'it worked.'
-rw-r--r--internal/handlers/widget.go14
-rw-r--r--internal/handlers/widget_test.go20
-rw-r--r--internal/store/native_tasks.go60
-rw-r--r--internal/store/native_tasks_test.go96
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)
+ }
+}