From dc5fa21bb891b58e77f7edec78a588a21e6732cb Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 15 Oct 2026 08:10:38 +0000 Subject: [PATCH] Store delivery and archive times without the monotonic reading (closes #529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit finished_at, archived_at and the updated_at stamp a re-dispatch writes came from a bare time.Now(). The SQLite driver stores a time as time.Time.String(), which appends the monotonic clock reading (" m=+…") to a time read from time.Now(). All three now come from GORM's own clock, NowFunc(), which stamps created_at, so they are stored in the same form. The tests for each write read the stored text back. Model: opus-5-5 --- internal/delivery/delivery_totals_test.go | 15 +++++++++++++-- internal/delivery/engine.go | 10 +++++++--- internal/delivery/engine_test.go | 12 ++++++++++++ internal/delivery/inflight_test.go | 12 +++++++++++- internal/delivery/target_database_archive.go | 6 +++++- internal/delivery/target_database_test.go | 7 +++++++ 6 files changed, 55 insertions(+), 7 deletions(-) diff --git a/internal/delivery/delivery_totals_test.go b/internal/delivery/delivery_totals_test.go index 617396d..ffa98ef 100644 --- a/internal/delivery/delivery_totals_test.go +++ b/internal/delivery/delivery_totals_test.go @@ -32,8 +32,9 @@ func targetTotals( // TestUpdateDeliveryStatus_FinishTimeAndTargetTotals pins what a status // write records for the webhook page's statistics: the time a delivery -// finished, set only when it becomes delivered or failed, and one more -// on its target's delivered or failed total. +// finished, set only when it becomes delivered or failed and stored in +// the same form as created_at, and one more on its target's delivered +// or failed total. func TestUpdateDeliveryStatus_FinishTimeAndTargetTotals(t *testing.T) { t.Parallel() @@ -75,6 +76,16 @@ func TestUpdateDeliveryStatus_FinishTimeAndTargetTotals(t *testing.T) { if tt.finished { require.NotNil(t, stored.FinishedAt) assert.False(t, stored.FinishedAt.Before(before)) + + var createdAt, finishedAt string + + require.NoError(t, db.Raw( + `SELECT CAST(created_at AS TEXT), + CAST(finished_at AS TEXT) + FROM deliveries WHERE id = ?`, d.ID, + ).Row().Scan(&createdAt, &finishedAt)) + assertStoredTime(t, createdAt) + assertStoredTime(t, finishedAt) } else { assert.Nil(t, stored.FinishedAt) } diff --git a/internal/delivery/engine.go b/internal/delivery/engine.go index fcf2bc9..be78e66 100644 --- a/internal/delivery/engine.go +++ b/internal/delivery/engine.go @@ -1701,8 +1701,11 @@ func writeDeliveryStatus( database.DeliveryStatusFailed, }). Updates(map[string]any{ - "status": status, - "finished_at": time.Now(), + "status": status, + // GORM's clock, the one that stamps created_at. A bare + // time.Now() carries a monotonic clock reading, which the + // driver would store as an " m=+…" suffix. + "finished_at": tx.NowFunc(), }) if res.Error != nil || res.RowsAffected == 0 { return res.Error @@ -1906,12 +1909,13 @@ func (e *Engine) takeForRedispatch( return false } + // GORM's clock, as writeDeliveryStatus explains. res := webhookDB. Model(&database.Delivery{}). Where( "id = ? AND status = ?", deliveryID, eligible, ). - UpdateColumn("updated_at", time.Now()) + UpdateColumn("updated_at", webhookDB.NowFunc()) if res.Error != nil { e.log.Error( diff --git a/internal/delivery/engine_test.go b/internal/delivery/engine_test.go index eca808d..7ebb016 100644 --- a/internal/delivery/engine_test.go +++ b/internal/delivery/engine_test.go @@ -117,6 +117,18 @@ func seedDelivery( return d } +// assertStoredTime asserts that text, a timestamp as SQLite holds it, +// is in the form GORM stores created_at in: the layout of +// time.Time.String, with no " m=+…" monotonic clock reading after it. +func assertStoredTime(t *testing.T, text string) { + t.Helper() + + _, err := time.Parse( + "2006-01-02 15:04:05.999999999 -0700 MST", text, + ) + assert.NoError(t, err, "stored as %q", text) +} + // httpDeliveryFixture holds the task and delivery needed // for HTTP delivery tests. type httpDeliveryFixture struct { diff --git a/internal/delivery/inflight_test.go b/internal/delivery/inflight_test.go index 81ad665..059aa5a 100644 --- a/internal/delivery/inflight_test.go +++ b/internal/delivery/inflight_test.go @@ -462,7 +462,8 @@ func TestScheduledRetryIsNotSweptDuringBackoff(t *testing.T) { // TestRedispatchStampsTheRow pins the cadence control: a stranded // delivery that has just been handed out is not selected again by the -// next tick a minute later. +// next tick a minute later. The stamp is stored in the same form as +// created_at. func TestRedispatchStampsTheRow(t *testing.T) { t.Parallel() @@ -493,4 +494,13 @@ func TestRedispatchStampsTheRow(t *testing.T) { "a re-dispatched delivery must be stamped so the "+ "next tick does not select it again", ) + + var createdAt, updatedAt string + + require.NoError(t, s.WebhookDB.Raw( + `SELECT CAST(created_at AS TEXT), CAST(updated_at AS TEXT) + FROM deliveries WHERE id = ?`, d.ID, + ).Row().Scan(&createdAt, &updatedAt)) + assertStoredTime(t, createdAt) + assertStoredTime(t, updatedAt) } diff --git a/internal/delivery/target_database_archive.go b/internal/delivery/target_database_archive.go index 4dcff5c..519af99 100644 --- a/internal/delivery/target_database_archive.go +++ b/internal/delivery/target_database_archive.go @@ -277,7 +277,11 @@ func (w *archiveWriter) write( } } - row.ArchivedAt = time.Now() + // GORM's clock, the one that stamps created_at in the other + // databases, so this is stored in the same form. A bare time.Now() + // carries a monotonic clock reading, which the driver would store + // as an " m=+…" suffix. + row.ArchivedAt = w.db.NowFunc() err := w.db.Create(&row).Error if err != nil { diff --git a/internal/delivery/target_database_test.go b/internal/delivery/target_database_test.go index 5f93849..7c96842 100644 --- a/internal/delivery/target_database_test.go +++ b/internal/delivery/target_database_test.go @@ -150,6 +150,13 @@ func TestArchiveWriter_WritesRow(t *testing.T) { assert.Equal(t, row.ContentType, got[0].ContentType) assert.JSONEq(t, `{"hello":"world"}`, got[0].Body) assert.False(t, got[0].ArchivedAt.IsZero()) + + var archivedAt string + + require.NoError(t, w.DB().Raw( + "SELECT CAST(archived_at AS TEXT) FROM archived_events", + ).Row().Scan(&archivedAt)) + assertStoredTime(t, archivedAt) } func TestArchiveWriter_RecreatesAfterRemoval(