From 4e802e2cf3ab9857cf6ca4080ffd40b20cc0a3a7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 18:23:42 +0000 Subject: [PATCH] Clear completed_at when a todo is reopened update_todo stamped completed_at on terminal status but never cleared it when PATCH moved the row back to pending or in_progress. HTTP reopen then violated the documented completed_at=0-until-terminal contract. Co-authored-by: Tyler Reckart --- CHANGELOG.md | 8 +++++ docs/api/todos/patch.md | 2 +- docs/concepts/todos.md | 2 +- include/tenant_store.h | 5 +-- src/tenant_store.cpp | 9 ++++-- tests/test_todos.cpp | 70 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 89 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dae12b43..b62887f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +### Fixed +- **Reopened todos clear `completed_at`.** `update_todo` (HTTP PATCH and + batch PATCH) now zeros `completed_at` when status moves back to + `pending` or `in_progress`, unless the caller passes an explicit + timestamp. Completing then reopening left the old stamp, so the + documented `completed_at = 0` until terminal contract was violated. + Subject/description edits on a terminal row still leave the stamp. + ## [0.13.4] — 2026-09-10 ### Fixed diff --git a/docs/api/todos/patch.md b/docs/api/todos/patch.md index a6d46624..851547ef 100644 --- a/docs/api/todos/patch.md +++ b/docs/api/todos/patch.md @@ -2,7 +2,7 @@ **Auth:** tenant — _Status:_ stable -Update one or more mutable fields. Any field omitted from the body is left untouched. Transitioning to a terminal `status` (`completed` or `canceled`) auto-stamps `completed_at = now()`. +Update one or more mutable fields. Any field omitted from the body is left untouched. Transitioning to a terminal `status` (`completed` or `canceled`) auto-stamps `completed_at = now()`. Transitioning back to `pending` or `in_progress` clears `completed_at` (back to `0`). ## Request diff --git a/docs/concepts/todos.md b/docs/concepts/todos.md index 83681cd4..e9a8ec20 100644 --- a/docs/concepts/todos.md +++ b/docs/concepts/todos.md @@ -18,7 +18,7 @@ A conversation-scoped read sees both — pinned rows plus the unscoped pool — | `completed` | Done. Stamps `completed_at` and disappears from the active list. | | `canceled` | Won't be done. Same archive treatment as `completed`. | -Status is the only column that auto-stamps anything else: any transition into `completed` or `canceled` sets `completed_at = now()` unless the caller passes a value explicitly. Pending → in_progress → completed is the canonical flow; you can also skip straight from pending to canceled when scope changes mid-task. +Status is the only column that auto-stamps anything else: any transition into `completed` or `canceled` sets `completed_at = now()` unless the caller passes a value explicitly. A later PATCH back to `pending` or `in_progress` clears `completed_at` (back to `0`) so the column stays `0` until terminal. Pending → in_progress → completed is the canonical flow; you can also skip straight from pending to canceled when scope changes mid-task. HTTP PATCH is the reopen path — `/todo start` only resolves against open rows. ## The `/todo` writ diff --git a/include/tenant_store.h b/include/tenant_store.h index 8afa1db1..cedc84af 100644 --- a/include/tenant_store.h +++ b/include/tenant_store.h @@ -802,8 +802,9 @@ class TenantStore { // PATCH-style: any std::nullopt argument leaves the field // untouched. Setting status to a terminal value stamps - // completed_at automatically (caller can override by passing a - // value through completed_at). + // completed_at automatically; setting it to a non-terminal value + // clears completed_at (caller can override by passing a value + // through completed_at). bool update_todo(int64_t tenant_id, int64_t id, const std::optional& subject, const std::optional& description, diff --git a/src/tenant_store.cpp b/src/tenant_store.cpp index b48878fe..0502da10 100644 --- a/src/tenant_store.cpp +++ b/src/tenant_store.cpp @@ -4410,10 +4410,13 @@ bool TenantStore::update_todo(int64_t tenant_id, int64_t id, if (!db_) return false; const int64_t ts = now_epoch(); - // Auto-stamp completed_at when transitioning to a terminal status, - // unless the caller passed completed_at explicitly. + // Keep completed_at aligned with status unless the caller passed an + // explicit stamp. Terminal statuses get now(); a reopen to pending + // or in_progress zeros the column so "0 until terminal" stays true. std::optional ca = completed_at; - if (status && is_terminal_todo_status(*status) && !ca) ca = ts; + if (status && !ca) { + ca = is_terminal_todo_status(*status) ? ts : int64_t{0}; + } std::string sql = "UPDATE todos SET updated_at = ?"; if (subject) sql += ", subject = ?"; diff --git a/tests/test_todos.cpp b/tests/test_todos.cpp index d94c4239..cadd4ce5 100644 --- a/tests/test_todos.cpp +++ b/tests/test_todos.cpp @@ -79,6 +79,76 @@ TEST_CASE("todo round-trip: create / get / update / delete") { CHECK(v->completed_at > 0); // auto-stamped } + SUBCASE("reopen from a terminal status clears completed_at") { + bool ok = s.update_todo(tid, t.id, std::nullopt, std::nullopt, + std::optional("completed"), + std::nullopt, std::nullopt); + CHECK(ok); + auto done = s.get_todo(tid, t.id); + REQUIRE(done); + CHECK(done->status == "completed"); + CHECK(done->completed_at > 0); + + ok = s.update_todo(tid, t.id, std::nullopt, std::nullopt, + std::optional("pending"), + std::nullopt, std::nullopt); + CHECK(ok); + auto reopened = s.get_todo(tid, t.id); + REQUIRE(reopened); + CHECK(reopened->status == "pending"); + CHECK(reopened->completed_at == 0); + + ok = s.update_todo(tid, t.id, std::nullopt, std::nullopt, + std::optional("canceled"), + std::nullopt, std::nullopt); + CHECK(ok); + ok = s.update_todo(tid, t.id, std::nullopt, std::nullopt, + std::optional("in_progress"), + std::nullopt, std::nullopt); + CHECK(ok); + auto restarted = s.get_todo(tid, t.id); + REQUIRE(restarted); + CHECK(restarted->status == "in_progress"); + CHECK(restarted->completed_at == 0); + } + + SUBCASE("explicit completed_at on reopen is honored") { + bool ok = s.update_todo(tid, t.id, std::nullopt, std::nullopt, + std::optional("completed"), + std::nullopt, std::nullopt); + CHECK(ok); + + ok = s.update_todo(tid, t.id, std::nullopt, std::nullopt, + std::optional("pending"), + std::nullopt, std::optional(12345)); + CHECK(ok); + auto u = s.get_todo(tid, t.id); + REQUIRE(u); + CHECK(u->status == "pending"); + CHECK(u->completed_at == 12345); + } + + SUBCASE("subject edit after completed does not clear completed_at") { + bool ok = s.update_todo(tid, t.id, std::nullopt, std::nullopt, + std::optional("completed"), + std::nullopt, std::nullopt); + CHECK(ok); + auto done = s.get_todo(tid, t.id); + REQUIRE(done); + const int64_t stamped = done->completed_at; + CHECK(stamped > 0); + + ok = s.update_todo(tid, t.id, + std::optional("renamed"), + std::nullopt, std::nullopt, std::nullopt, std::nullopt); + CHECK(ok); + auto u = s.get_todo(tid, t.id); + REQUIRE(u); + CHECK(u->status == "completed"); + CHECK(u->subject == "renamed"); + CHECK(u->completed_at == stamped); + } + SUBCASE("update edits subject/description without touching status") { bool ok = s.update_todo(tid, t.id, std::optional("new subject"),