From bd0d0f42fc5f58798f85a0259aa3686002f1a9a5 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sun, 15 Mar 2026 16:47:59 -0700 Subject: [PATCH] fix: use UTC_TIMESTAMP() for defer_until comparisons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go stores defer_until as UTC via time.Time, but Dolt's NOW() returns the server's local time. On non-UTC machines (e.g., PDT = UTC-7), the comparison 'defer_until <= NOW()' produces wrong results — a past-deferred issue may not appear in bd ready because NOW() returns a local time hours behind the UTC-stored defer_until. Fix: Replace NOW() with UTC_TIMESTAMP() in all 4 defer_until SQL comparisons (queries.go lines 169/1005, schema.go ready_issues view). Bump schema version to 8 so the view gets recreated. Adds two unit tests: - TestGetReadyWork_PastDeferredIssueIsReady - TestGetReadyWork_FutureDeferredIssueExcluded --- internal/storage/dolt/queries.go | 4 +- internal/storage/dolt/queries_test.go | 103 ++++++++++++++++++++++++++ internal/storage/dolt/schema.go | 8 +- 3 files changed, 109 insertions(+), 6 deletions(-) diff --git a/internal/storage/dolt/queries.go b/internal/storage/dolt/queries.go index 95636d87a8..c7e73c64dd 100644 --- a/internal/storage/dolt/queries.go +++ b/internal/storage/dolt/queries.go @@ -93,7 +93,7 @@ func (s *DoltStore) GetReadyWork(ctx context.Context, filter types.WorkFilter) ( } // Exclude future-deferred issues unless IncludeDeferred is set if !filter.IncludeDeferred { - whereClauses = append(whereClauses, "(defer_until IS NULL OR defer_until <= NOW())") + whereClauses = append(whereClauses, "(defer_until IS NULL OR defer_until <= UTC_TIMESTAMP())") } // Exclude children of future-deferred parents (GH#1190) // Pre-compute excluded IDs using separate single-table queries to avoid @@ -773,7 +773,7 @@ func (s *DoltStore) getChildrenOfDeferredParents(ctx context.Context) ([]string, // Step 1: Get IDs of issues with future defer_until deferredRows, err := s.queryContext(ctx, ` SELECT id FROM issues - WHERE defer_until IS NOT NULL AND defer_until > NOW() + WHERE defer_until IS NOT NULL AND defer_until > UTC_TIMESTAMP() `) if err != nil { return nil, wrapQueryError("deferred parents: get deferred issues", err) diff --git a/internal/storage/dolt/queries_test.go b/internal/storage/dolt/queries_test.go index 35e0ea83fa..7ea10cd2d2 100644 --- a/internal/storage/dolt/queries_test.go +++ b/internal/storage/dolt/queries_test.go @@ -411,6 +411,109 @@ func TestGetReadyWork_CustomStatusBlockerStillBlocks(t *testing.T) { } } +// TestGetReadyWork_PastDeferredIssueIsReady verifies that an issue whose +// defer_until is in the past appears in ready work. Regression test for a +// timezone bug: Go stores defer_until as UTC, but Dolt's NOW() returns local +// time. On non-UTC machines, the comparison defer_until <= NOW() would +// incorrectly exclude past-deferred issues. The fix uses UTC_TIMESTAMP(). +func TestGetReadyWork_PastDeferredIssueIsReady(t *testing.T) { + store, cleanup := setupTestStore(t) + defer cleanup() + + ctx, cancel := testContext(t) + defer cancel() + + // Create an issue and set defer_until to 1 hour in the past (UTC). + pastDeferred := &types.Issue{ + ID: "rw-past-deferred", + Title: "Past Deferred Task", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, pastDeferred, "tester"); err != nil { + t.Fatalf("failed to create issue: %v", err) + } + pastTime := time.Now().UTC().Add(-1 * time.Hour) + if err := store.UpdateIssue(ctx, pastDeferred.ID, map[string]interface{}{ + "defer_until": pastTime, + }, "tester"); err != nil { + t.Fatalf("failed to set defer_until: %v", err) + } + + // Create a normal issue (no defer) as a control. + normal := &types.Issue{ + ID: "rw-normal", + Title: "Normal Task", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, normal, "tester"); err != nil { + t.Fatalf("failed to create issue: %v", err) + } + + work, err := store.GetReadyWork(ctx, types.WorkFilter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + foundPastDeferred := false + foundNormal := false + for _, w := range work { + if w.ID == pastDeferred.ID { + foundPastDeferred = true + } + if w.ID == normal.ID { + foundNormal = true + } + } + if !foundNormal { + t.Error("normal issue should appear in ready work") + } + if !foundPastDeferred { + t.Error("past-deferred issue (defer_until in the past) should appear in ready work") + } +} + +// TestGetReadyWork_FutureDeferredIssueExcluded verifies that an issue whose +// defer_until is in the future does NOT appear in ready work. +func TestGetReadyWork_FutureDeferredIssueExcluded(t *testing.T) { + store, cleanup := setupTestStore(t) + defer cleanup() + + ctx, cancel := testContext(t) + defer cancel() + + futureDeferred := &types.Issue{ + ID: "rw-future-deferred", + Title: "Future Deferred Task", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, futureDeferred, "tester"); err != nil { + t.Fatalf("failed to create issue: %v", err) + } + futureTime := time.Now().UTC().Add(24 * time.Hour) + if err := store.UpdateIssue(ctx, futureDeferred.ID, map[string]interface{}{ + "defer_until": futureTime, + }, "tester"); err != nil { + t.Fatalf("failed to set defer_until: %v", err) + } + + work, err := store.GetReadyWork(ctx, types.WorkFilter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + for _, w := range work { + if w.ID == futureDeferred.ID { + t.Error("future-deferred issue should NOT appear in ready work") + } + } +} + // ============================================================================= // GetBlockedIssues tests // ============================================================================= diff --git a/internal/storage/dolt/schema.go b/internal/storage/dolt/schema.go index 24b472952b..eda3db1681 100644 --- a/internal/storage/dolt/schema.go +++ b/internal/storage/dolt/schema.go @@ -294,14 +294,14 @@ LEFT JOIN blocked_transitively bt ON bt.issue_id = i.id WHERE i.status = 'open' AND (i.ephemeral = 0 OR i.ephemeral IS NULL) AND bt.issue_id IS NULL - AND (i.defer_until IS NULL OR i.defer_until <= NOW()) + AND (i.defer_until IS NULL OR i.defer_until <= UTC_TIMESTAMP()) AND NOT EXISTS ( SELECT 1 FROM dependencies d_parent JOIN issues parent ON parent.id = d_parent.depends_on_id WHERE d_parent.issue_id = i.id AND d_parent.type = 'parent-child' AND parent.defer_until IS NOT NULL - AND parent.defer_until > NOW() + AND parent.defer_until > UTC_TIMESTAMP() ); ` @@ -378,14 +378,14 @@ LEFT JOIN blocked_transitively bt ON bt.issue_id = i.id WHERE i.status IN (` + statusList + `) AND (i.ephemeral = 0 OR i.ephemeral IS NULL) AND bt.issue_id IS NULL - AND (i.defer_until IS NULL OR i.defer_until <= NOW()) + AND (i.defer_until IS NULL OR i.defer_until <= UTC_TIMESTAMP()) AND NOT EXISTS ( SELECT 1 FROM dependencies d_parent JOIN issues parent ON parent.id = d_parent.depends_on_id WHERE d_parent.issue_id = i.id AND d_parent.type = 'parent-child' AND parent.defer_until IS NOT NULL - AND parent.defer_until > NOW() + AND parent.defer_until > UTC_TIMESTAMP() ); ` }