From b55cfbbd433bed6035dfa228ee700e2cca060ca4 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Fri, 14 Aug 2026 08:12:29 +0000 Subject: Fix integration papercuts found in cross-source review getAtomDetails's gtasks case was a stub returning a hardcoded "Google Task" title, so every gtask completed via the web Tasks/Timeline tab or the Agent API logged into completed-tasks history with no real title or due date -- now calls findGoogleTask like every other gtasks call site already does. Gtasks completion also skipped cache invalidation in two call sites (HandleCompleteAtom, the Agent API's handleAgentTaskToggle) that already had it for trello; both now invalidate CacheKeyGoogleTasks the same way the widget handlers do. HandleTaskDetailPage (the widget deep-link fallback page) hand -duplicated loadTaskDetailData's lookup instead of calling it, so it never got this session's earlier gtasks-detail fix -- now delegates. Also deleted PlanToEatAPI.GetRecipes, dead code since its introduction ("for Phase 2," never called). Adds a running .agent/critiques.md tracking structural/process critiques surfaced in conversation, separate from a concrete plan. --- internal/api/interfaces.go | 1 - internal/api/plantoeat.go | 5 --- internal/handlers/agent.go | 10 ++++- internal/handlers/agent_test.go | 41 +++++++++++++++++++ internal/handlers/handlers.go | 34 ++++------------ internal/handlers/handlers_test.go | 80 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 34 deletions(-) (limited to 'internal') 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() -- cgit v1.2.3