diff options
| -rw-r--r-- | .agent/critiques.md | 78 | ||||
| -rw-r--r-- | internal/api/interfaces.go | 1 | ||||
| -rw-r--r-- | internal/api/plantoeat.go | 5 | ||||
| -rw-r--r-- | internal/handlers/agent.go | 10 | ||||
| -rw-r--r-- | internal/handlers/agent_test.go | 41 | ||||
| -rw-r--r-- | internal/handlers/handlers.go | 34 | ||||
| -rw-r--r-- | internal/handlers/handlers_test.go | 80 |
7 files changed, 215 insertions, 34 deletions
diff --git a/.agent/critiques.md b/.agent/critiques.md new file mode 100644 index 0000000..a507cbd --- /dev/null +++ b/.agent/critiques.md @@ -0,0 +1,78 @@ +# Critiques (running list) + +Working notes on structural/design weaknesses, gathered as they come up in +conversation rather than invented in the abstract. Not a plan — a list to +react to and prioritize before anything here gets acted on. + +## Data model + +1. **`native_tasks` is one flat table doing ~4 jobs.** A task, a + chain-membership, a bucket-membership, and a recurrence-series are all + the same row, distinguished only by which subset of bolt-on columns + (`chain_id/position/unlocked`, `bucket_id/state/last_active_at`, + `recurrence_freq/interval/weekdays/series_id/override`) are non-empty. + Every new feature adds more nullable-in-practice columns to the same + row; every query has to reason about which columns are "live" for a + given row's shape. Cost grows linearly with feature count. + +2. **No real task-dependency graph.** "Blocking" / "depends on" only exist + as (a) a chain's implicit position ordering, or (b) free-text notes a + human has to read and self-enforce. There's no way to express "task B + can't start until task A" when A isn't B's chain predecessor, without + falling back to prose. This is the literal gap hit building the Pelagic + Marine Electronics project (5 real dependency edges, none enforceable). + +3. **Chains and projects are welded 1:1.** `CreateChain` always creates its + own backing project — no chain-within-an-existing-project, no + multiple-chains-sharing-a-project. This is why one business plan with 7 + phases produced 7 separate entries in the project list. Looks like a + modeling shortcut, not a deliberate choice. + +4. **Three incompatible "shapes" of repeat/pooled work.** Chains (WIP-1, + ordered, position-based), buckets (pick-N pool, cycle-based, + staleness-ordered), and recurrence (single-task series, date-anchored) + solve overlapping problems with three separate non-composable + mechanisms and three separate column sets on the same table. A task + can't be "a bucket item that's also gated behind another task" — none + of the three concepts compose. + +5. **Labels have no referential integrity.** `labels(name, color)` is + color metadata only; actual label assignment is a raw JSON array on + `native_tasks` with no FK to `labels.name`. A typo in a label string + silently creates an new, uncolored "label" rather than erroring or + reusing the existing one. + +## Process / instruction-file cruft (doot-scoped) + +6. **`.agent/config.md`'s "ULTRA-STRICT ROOT SAFETY PROTOCOL"** (wait for + explicit "GO" before any system-changing call) contradicts how this + project actually runs — confirmed stale 2026-08-14. Pending: fold into + whatever the workspace-wide instruction-file cleanup lands on, rather + than patch in isolation. + +7. **`.agent/worklog.md`** references issues (#66-73) that don't match any + real recent work — leftover scaffolding, not maintained. Confirmed + stale 2026-08-14. Same pending treatment as #6. + +## Integrations + +Reviewed 2026-08-14. Five immediate papercuts found and fixed same day: +`getAtomDetails`'s gtasks case was a stub (logged every completed Google +Task as literally "Google Task" with no due date); gtasks completion via +the web Tasks/Timeline tab and the Agent API skipped cache invalidation +(trello had it, gtasks didn't, in two call sites); `HandleTaskDetailPage` +hand-duplicated `loadTaskDetailData`'s lookup instead of calling it, so it +never got the earlier gtasks-detail fix; `PlanToEatAPI.GetRecipes` was +dead code (own comment: "for Phase 2," never called) — deleted. + +Long-term questions surfaced, not decided: +- Should `GoogleTasksAPI` grow real create/due-date methods (the REST API + supports both; the interface just never exposed them)? +- Should the ~13 copy-pasted `switch source` dispatch blocks collapse into + one shared resolver? Not urgent at 3 actionable sources. +- The ad hoc `findGoogleTask`/`findCard` linear-scan-by-ID lookups are + copy-pasted inline in ~5 places instead of reused — style issue at + current (personal-scale) data volumes, not a performance one. +- Read-path caching is already well-unified (generic `CacheFetcher[T]`) — + don't over-correct that side, the inconsistency was only ever on + write/invalidation. diff --git a/internal/api/interfaces.go b/internal/api/interfaces.go index e764130..02e0cb5 100644 --- a/internal/api/interfaces.go +++ b/internal/api/interfaces.go @@ -21,7 +21,6 @@ type TrelloAPI interface { type PlanToEatAPI interface { GetUpcomingMeals(ctx context.Context, days int) ([]models.Meal, error) GetShoppingList(ctx context.Context) ([]models.ShoppingItem, error) - GetRecipes(ctx context.Context) error } // GoogleCalendarAPI defines the interface for Google Calendar operations diff --git a/internal/api/plantoeat.go b/internal/api/plantoeat.go index 770987a..edd494a 100644 --- a/internal/api/plantoeat.go +++ b/internal/api/plantoeat.go @@ -184,11 +184,6 @@ func normalizeMealType(mealType string) string { } } -// GetRecipes fetches recipes (for Phase 2) -func (c *PlanToEatClient) GetRecipes(ctx context.Context) error { - return fmt.Errorf("not implemented yet") -} - // GetShoppingList fetches the shopping list by scraping the web interface // Requires a valid session cookie set via SetSessionCookie func (c *PlanToEatClient) GetShoppingList(ctx context.Context) ([]models.ShoppingItem, error) { diff --git a/internal/handlers/agent.go b/internal/handlers/agent.go index 151520b..577ff87 100644 --- a/internal/handlers/agent.go +++ b/internal/handlers/agent.go @@ -450,12 +450,18 @@ func (h *Handler) handleAgentTaskToggle(w http.ResponseWriter, r *http.Request, if complete { title, dueDate := h.getAtomDetails(id, source) _ = h.store.SaveCompletedTask(source, id, title, dueDate) - if source == "trello" { + switch source { + case "trello": _ = h.store.DeleteCard(id) + case "gtasks": + _ = h.store.InvalidateCache(store.CacheKeyGoogleTasks) } } else { - if source == "trello" { + switch source { + case "trello": _ = h.store.InvalidateCache(store.CacheKeyTrelloBoards) + case "gtasks": + _ = h.store.InvalidateCache(store.CacheKeyGoogleTasks) } } diff --git a/internal/handlers/agent_test.go b/internal/handlers/agent_test.go index 2893a13..ea661b1 100644 --- a/internal/handlers/agent_test.go +++ b/internal/handlers/agent_test.go @@ -13,6 +13,7 @@ import ( "task-dashboard/internal/config" "task-dashboard/internal/models" + "task-dashboard/internal/store" ) func TestHandleAgentAuthRequest(t *testing.T) { @@ -833,6 +834,46 @@ func TestHandleAgentTaskWriteOperations(t *testing.T) { }) } +func TestHandleAgentTaskComplete_Gtasks_InvalidatesCache(t *testing.T) { + db, cleanup := setupTestDB(t) + defer cleanup() + + if err := db.SaveGoogleTasks([]models.GoogleTask{ + {ID: "g1", Title: "Renew passport", ListID: "list-a", UpdatedAt: time.Now()}, + }); err != nil { + t.Fatal(err) + } + if err := db.UpdateCacheMetadata(store.CacheKeyGoogleTasks, 60); err != nil { + t.Fatal(err) + } + + h := &Handler{store: db, googleTasksClient: &mockGoogleTasksClient{}, config: &config.Config{}} + + session := &models.AgentSession{RequestToken: "rt", AgentName: "A", AgentID: "a-uuid", ExpiresAt: time.Now().Add(5 * time.Minute)} + db.CreateAgentSession(session) + db.ApproveAgentSession("rt", "st", time.Now().Add(time.Hour)) + + req := httptest.NewRequest(http.MethodPost, "/agent/tasks/g1/complete?source=gtasks&listId=list-a", nil) + req = req.WithContext(context.WithValue(req.Context(), agentSessionContextKey, session)) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("id", "g1") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + w := httptest.NewRecorder() + + h.HandleAgentTaskComplete(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String()) + } + meta, err := db.GetCacheMetadata(store.CacheKeyGoogleTasks) + if err != nil { + t.Fatal(err) + } + if meta != nil { + t.Error("expected gtasks cache metadata to be invalidated after agent-api completion, but it still exists") + } +} + func TestHandleAgentCreateOperations(t *testing.T) { db, cleanup := setupTestDB(t) defer cleanup() diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 7220474..aedbd11 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -667,6 +667,8 @@ func (h *Handler) handleAtomToggle(w http.ResponseWriter, r *http.Request, compl switch source { case "trello": _ = h.store.DeleteCard(id) + case "gtasks": + _ = h.store.InvalidateCache(store.CacheKeyGoogleTasks) case "doot": // native tasks stay in DB marked completed; no cache to invalidate } @@ -712,7 +714,9 @@ func (h *Handler) getAtomDetails(id, source string) (string, *time.Time) { } } case "gtasks": - return "Google Task", nil + if t, ok := h.findGoogleTask(id); ok { + return t.Title, t.DueDate + } } return "Task", nil } @@ -906,30 +910,8 @@ func (h *Handler) HandleTaskDetailPage(w http.ResponseWriter, r *http.Request) { return } - var title, description string - switch source { - case "doot": - if tasks, err := h.store.GetNativeTasks(); err == nil { - for _, t := range tasks { - if t.ID == id { - title, description = t.Content, t.Description - break - } - } - } - case "trello": - if boards, err := h.store.GetBoards(); err == nil { - for _, b := range boards { - for _, c := range b.Cards { - if c.ID == id { - title, description = c.Name, c.Description - break - } - } - } - } - } - + detail := h.loadTaskDetailData(id, source) + title := detail.Title if title == "" { title = "Task" } @@ -941,7 +923,7 @@ func (h *Handler) HandleTaskDetailPage(w http.ResponseWriter, r *http.Request) { Description string CSRFToken string Saved bool - }{title, id, source, description, auth.GetCSRFTokenFromContext(r.Context()), r.URL.Query().Get("saved") == "1"} + }{title, id, source, detail.Description, auth.GetCSRFTokenFromContext(r.Context()), r.URL.Query().Get("saved") == "1"} if err := h.renderer.Render(w, "task-detail-page.html", data); err != nil { http.Error(w, "Failed to render template", http.StatusInternalServerError) diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 8900f66..7b18cf1 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -2475,6 +2475,86 @@ func TestHandleCompleteAtom_DootShowsTitle(t *testing.T) { } } +func TestHandleCompleteAtom_Gtasks_LogsRealTitleAndInvalidatesCache(t *testing.T) { + h, cleanup := setupTestHandler(t) + defer cleanup() + + due := time.Now().Add(24 * time.Hour) + if err := h.store.SaveGoogleTasks([]models.GoogleTask{ + {ID: "g1", Title: "Renew passport", ListID: "list-a", DueDate: &due, UpdatedAt: time.Now()}, + }); err != nil { + t.Fatal(err) + } + if err := h.store.UpdateCacheMetadata(store.CacheKeyGoogleTasks, 60); err != nil { + t.Fatal(err) + } + h.googleTasksClient = &mockGoogleTasksClient{} + + req := httptest.NewRequest("POST", "/complete-atom", nil) + req.Form = map[string][]string{"id": {"g1"}, "source": {"gtasks"}, "listId": {"list-a"}} + w := httptest.NewRecorder() + h.HandleCompleteAtom(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String()) + } + + completed, err := h.store.GetCompletedTasks(10) + if err != nil { + t.Fatal(err) + } + if len(completed) != 1 || completed[0].Title != "Renew passport" { + t.Errorf("completed log = %+v, want title %q (was hardcoded \"Google Task\" before the getAtomDetails fix)", completed, "Renew passport") + } + + meta, err := h.store.GetCacheMetadata(store.CacheKeyGoogleTasks) + if err != nil { + t.Fatal(err) + } + if meta != nil { + t.Error("expected gtasks cache metadata to be invalidated after completion, but it still exists") + } +} + +func TestHandleTaskDetailPage_GtasksSource_LoadsRealTaskFields(t *testing.T) { + h, cleanup := setupTestHandler(t) + defer cleanup() + + if err := h.store.SaveGoogleTasks([]models.GoogleTask{ + {ID: "g1", Title: "Renew passport", Notes: "bring photo", ListID: "list-a", UpdatedAt: time.Now()}, + }); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest("GET", "/task?id=g1&source=gtasks", nil) + w := httptest.NewRecorder() + h.HandleTaskDetailPage(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String()) + } + + mock := h.renderer.(*MockRenderer) + if len(mock.Calls) != 1 { + t.Fatalf("expected 1 render call, got %d", len(mock.Calls)) + } + type pageData struct { + Title string + Description string + } + jsonBytes, _ := json.Marshal(mock.Calls[0].Data) + var got pageData + if err := json.Unmarshal(jsonBytes, &got); err != nil { + t.Fatalf("failed to unmarshal render data: %v", err) + } + if got.Title != "Renew passport" { + t.Errorf("Title = %q, want %q", got.Title, "Renew passport") + } + if got.Description != "bring photo" { + t.Errorf("Description = %q, want %q", got.Description, "bring photo") + } +} + func TestHandleTimeline_IncludesBudgetStatusWhenTrackedTaskExists(t *testing.T) { h, cleanup := setupTestHandler(t) defer cleanup() |