From ad34d0d1e063ee47f0339cd7331436abe5c7118f Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 15 Sep 2026 17:31:55 +0800 Subject: [PATCH] fix(task): make review finding status updates idempotent on resolved_at (#3670) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio --- .../task/repository/sqlite/task_review.go | 35 ++++-- .../task_review_status_concurrency_test.go | 76 ++++++++++++ .../task_review_status_postgres_test.go | 81 ++++++++++++ .../repository/sqlite/task_review_test.go | 17 +-- .../internal/task/service/review_service.go | 16 +-- .../service/review_service_status_test.go | 116 ++++++++++++++++++ .../task/service/review_service_test.go | 37 ++++++ .../plan.md | 28 +++++ ...task-01-review-finding-status-atomicity.md | 36 ++++++ .../native-code-review-status-atomicity.md | 19 +++ 10 files changed, 427 insertions(+), 34 deletions(-) create mode 100644 apps/backend/internal/task/repository/sqlite/task_review_status_concurrency_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/task_review_status_postgres_test.go create mode 100644 apps/backend/internal/task/service/review_service_status_test.go create mode 100644 docs/plans/native-code-review-status-atomicity/plan.md create mode 100644 docs/plans/native-code-review-status-atomicity/task-01-review-finding-status-atomicity.md create mode 100644 docs/specs/agents/system-design/native-code-review-status-atomicity.md diff --git a/apps/backend/internal/task/repository/sqlite/task_review.go b/apps/backend/internal/task/repository/sqlite/task_review.go index ca5d7b4264..4164e22f2d 100644 --- a/apps/backend/internal/task/repository/sqlite/task_review.go +++ b/apps/backend/internal/task/repository/sqlite/task_review.go @@ -298,19 +298,32 @@ func (r *Repository) GetTaskReviewFinding(ctx context.Context, findingID string) return f, err } -// UpdateTaskReviewFindingStatus sets a finding's status and resolved_at. -func (r *Repository) UpdateTaskReviewFindingStatus(ctx context.Context, findingID string, status models.ReviewFindingStatus, resolvedAt *time.Time) error { - result, err := r.db.ExecContext(ctx, r.db.Rebind(` - UPDATE task_review_findings SET status = ?, resolved_at = ?, updated_at = ? WHERE id = ? - `), string(status), resolvedAt, time.Now().UTC(), findingID) +// TransitionTaskReviewFindingStatus updates a finding and returns the stored +// row. The CASE expression reads the old status inside the same database write, +// so concurrent callers cannot apply a timestamp decision from stale state. +func (r *Repository) TransitionTaskReviewFindingStatus(ctx context.Context, findingID string, status models.ReviewFindingStatus) (*models.TaskReviewFinding, error) { + now := time.Now().UTC() + row := r.db.QueryRowxContext(ctx, r.db.Rebind(` + UPDATE task_review_findings + SET status = ?, + resolved_at = CASE + WHEN status = ? THEN resolved_at + WHEN ? IN ('resolved', 'dismissed') THEN ? + ELSE NULL + END, + updated_at = ? + WHERE id = ? + RETURNING `+reviewFindingColumns), + string(status), string(status), string(status), now, + now, findingID) + finding, err := scanReviewFinding(row) + if errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("%w: %s", models.ErrTaskReviewFindingNotFound, findingID) + } if err != nil { - return fmt.Errorf("failed to update task review finding status: %w", err) + return nil, fmt.Errorf("failed to transition task review finding status: %w", err) } - rows, _ := result.RowsAffected() - if rows == 0 { - return fmt.Errorf("%w: %s", models.ErrTaskReviewFindingNotFound, findingID) - } - return nil + return finding, nil } // DeleteSupersededTaskReviewFindings removes still-open findings from earlier diff --git a/apps/backend/internal/task/repository/sqlite/task_review_status_concurrency_test.go b/apps/backend/internal/task/repository/sqlite/task_review_status_concurrency_test.go new file mode 100644 index 0000000000..4c83419479 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/task_review_status_concurrency_test.go @@ -0,0 +1,76 @@ +package sqlite + +import ( + "context" + "sync" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +// TestTaskReviewFinding_ConcurrentTransitionsKeepResolvedTimestamp proves that +// concurrent duplicate resolutions return the same persisted timestamp and do +// not overwrite each other's resolved_at value. +func TestTaskReviewFinding_ConcurrentTransitionsKeepResolvedTimestamp(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedReviewTask(t, ctx, repo, "task-status-concurrent") + run := newReviewRun(t, ctx, repo, "task-status-concurrent") + f := finding(run.ID, "task-status-concurrent", "a.go", "issue", 4) + if err := repo.CreateTaskReviewFindings(ctx, []*models.TaskReviewFinding{f}); err != nil { + t.Fatalf("CreateTaskReviewFindings: %v", err) + } + + start := make(chan struct{}) + results := make(chan struct { + finding *models.TaskReviewFinding + err error + }, 2) + var wg sync.WaitGroup + for range 2 { + wg.Add(1) + go func() { + defer wg.Done() + <-start + got, err := repo.TransitionTaskReviewFindingStatus(ctx, f.ID, models.ReviewFindingResolved) + results <- struct { + finding *models.TaskReviewFinding + err error + }{finding: got, err: err} + }() + } + close(start) + wg.Wait() + close(results) + + var resolvedAt *models.TaskReviewFinding + for result := range results { + if result.err != nil { + t.Fatalf("concurrent transition: %v", result.err) + } + if result.finding == nil || result.finding.Status != models.ReviewFindingResolved || result.finding.ResolvedAt == nil { + t.Fatalf("concurrent transition returned invalid finding: %+v", result.finding) + } + if resolvedAt == nil { + resolvedAt = result.finding + continue + } + if !result.finding.ResolvedAt.Equal(*resolvedAt.ResolvedAt) { + t.Fatalf("concurrent transitions returned different resolved_at values: %v and %v", resolvedAt.ResolvedAt, result.finding.ResolvedAt) + } + } + + final, err := repo.GetTaskReviewFinding(ctx, f.ID) + if err != nil { + t.Fatalf("GetTaskReviewFinding: %v", err) + } + if final.Status != models.ReviewFindingResolved || final.ResolvedAt == nil { + t.Fatalf("final finding is not resolved: %+v", final) + } + if resolvedAt == nil { + t.Fatal("concurrent transitions returned no findings") + } + if !final.ResolvedAt.Equal(*resolvedAt.ResolvedAt) { + t.Fatalf("final resolved_at=%v, want %v", final.ResolvedAt, resolvedAt.ResolvedAt) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/task_review_status_postgres_test.go b/apps/backend/internal/task/repository/sqlite/task_review_status_postgres_test.go new file mode 100644 index 0000000000..cb9dc72168 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/task_review_status_postgres_test.go @@ -0,0 +1,81 @@ +package sqlite + +import ( + "context" + "sync" + "testing" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/testutil" +) + +// TestPostgresTaskReviewFinding_ConcurrentTransitionsKeepResolvedTimestamp +// exercises the same transition through two real PostgreSQL connections. +// It skips unless KANDEV_TEST_POSTGRES_DSN is set. +func TestPostgresTaskReviewFinding_ConcurrentTransitionsKeepResolvedTimestamp(t *testing.T) { + db := openIsolatedPostgresMultiConn(t, testutil.PostgresDSNFromEnv(t), 2) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + ctx := context.Background() + seedReviewTask(t, ctx, repo, "task-status-postgres-concurrent") + run := newReviewRun(t, ctx, repo, "task-status-postgres-concurrent") + f := finding(run.ID, "task-status-postgres-concurrent", "a.go", "issue", 4) + if err := repo.CreateTaskReviewFindings(ctx, []*models.TaskReviewFinding{f}); err != nil { + t.Fatalf("CreateTaskReviewFindings: %v", err) + } + + start := make(chan struct{}) + results := make(chan struct { + finding *models.TaskReviewFinding + err error + }, 2) + var wg sync.WaitGroup + for range 2 { + wg.Add(1) + go func() { + defer wg.Done() + <-start + got, err := repo.TransitionTaskReviewFindingStatus(ctx, f.ID, models.ReviewFindingResolved) + results <- struct { + finding *models.TaskReviewFinding + err error + }{finding: got, err: err} + }() + } + close(start) + wg.Wait() + close(results) + + var resolvedAt *models.TaskReviewFinding + for result := range results { + if result.err != nil { + t.Fatalf("concurrent transition: %v", result.err) + } + if result.finding == nil || result.finding.Status != models.ReviewFindingResolved || result.finding.ResolvedAt == nil { + t.Fatalf("concurrent transition returned invalid finding: %+v", result.finding) + } + if resolvedAt == nil { + resolvedAt = result.finding + continue + } + if !result.finding.ResolvedAt.Equal(*resolvedAt.ResolvedAt) { + t.Fatalf("concurrent transitions returned different resolved_at values: %v and %v", resolvedAt.ResolvedAt, result.finding.ResolvedAt) + } + } + + final, err := repo.GetTaskReviewFinding(ctx, f.ID) + if err != nil { + t.Fatalf("GetTaskReviewFinding: %v", err) + } + if final.Status != models.ReviewFindingResolved || final.ResolvedAt == nil { + t.Fatalf("final finding is not resolved: %+v", final) + } + if resolvedAt == nil { + t.Fatal("concurrent transitions returned no findings") + } + if !final.ResolvedAt.Equal(*resolvedAt.ResolvedAt) { + t.Fatalf("final resolved_at=%v, want %v", final.ResolvedAt, resolvedAt.ResolvedAt) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/task_review_test.go b/apps/backend/internal/task/repository/sqlite/task_review_test.go index 13b55db3b1..bc5038042d 100644 --- a/apps/backend/internal/task/repository/sqlite/task_review_test.go +++ b/apps/backend/internal/task/repository/sqlite/task_review_test.go @@ -199,30 +199,23 @@ func TestTaskReviewFinding_UpdateStatus(t *testing.T) { t.Fatalf("CreateTaskReviewFindings: %v", err) } - now := f.CreatedAt - if err := repo.UpdateTaskReviewFindingStatus(ctx, f.ID, models.ReviewFindingResolved, &now); err != nil { - t.Fatalf("UpdateTaskReviewFindingStatus: %v", err) - } - got, err := repo.GetTaskReviewFinding(ctx, f.ID) + got, err := repo.TransitionTaskReviewFindingStatus(ctx, f.ID, models.ReviewFindingResolved) if err != nil { - t.Fatalf("GetTaskReviewFinding: %v", err) + t.Fatalf("TransitionTaskReviewFindingStatus: %v", err) } if got.Status != models.ReviewFindingResolved || got.ResolvedAt == nil { t.Fatalf("expected resolved with resolved_at, got %+v", got) } - if err := repo.UpdateTaskReviewFindingStatus(ctx, f.ID, models.ReviewFindingOpen, nil); err != nil { - t.Fatalf("reopen: %v", err) - } - got, err = repo.GetTaskReviewFinding(ctx, f.ID) + got, err = repo.TransitionTaskReviewFindingStatus(ctx, f.ID, models.ReviewFindingOpen) if err != nil { - t.Fatalf("GetTaskReviewFinding after reopen: %v", err) + t.Fatalf("reopen: %v", err) } if got.Status != models.ReviewFindingOpen || got.ResolvedAt != nil { t.Fatalf("expected reopen to clear resolved_at, got %+v", got) } - err = repo.UpdateTaskReviewFindingStatus(ctx, "missing", models.ReviewFindingOpen, nil) + _, err = repo.TransitionTaskReviewFindingStatus(ctx, "missing", models.ReviewFindingOpen) if !errors.Is(err, models.ErrTaskReviewFindingNotFound) { t.Fatalf("expected ErrTaskReviewFindingNotFound, got %v", err) } diff --git a/apps/backend/internal/task/service/review_service.go b/apps/backend/internal/task/service/review_service.go index 829f50e74f..0d15195c01 100644 --- a/apps/backend/internal/task/service/review_service.go +++ b/apps/backend/internal/task/service/review_service.go @@ -50,7 +50,7 @@ type reviewRepo interface { CreateTaskReviewFindings(ctx context.Context, findings []*models.TaskReviewFinding) error ListTaskReviewFindings(ctx context.Context, taskID string) ([]*models.TaskReviewFinding, error) GetTaskReviewFinding(ctx context.Context, findingID string) (*models.TaskReviewFinding, error) - UpdateTaskReviewFindingStatus(ctx context.Context, findingID string, status models.ReviewFindingStatus, resolvedAt *time.Time) error + TransitionTaskReviewFindingStatus(ctx context.Context, findingID string, status models.ReviewFindingStatus) (*models.TaskReviewFinding, error) DeleteSupersededTaskReviewFindings(ctx context.Context, taskID, runID string, keys []models.ReviewFindingKey) ([]string, error) DeleteTaskReviewByTask(ctx context.Context, taskID string) error } @@ -468,7 +468,9 @@ func buildReviewFinding(taskID, runID string, in ReviewFindingInput) (*models.Ta } // UpdateFindingStatus records the human's disposition of a finding. Moving to -// resolved stamps resolved_at; returning to open clears it. +// resolved or dismissed stamps resolved_at; returning to open clears it. +// Resubmitting the finding's current status is idempotent and leaves +// resolved_at unchanged rather than re-stamping it to now. func (s *ReviewService) UpdateFindingStatus(ctx context.Context, findingID string, status models.ReviewFindingStatus) (*models.TaskReviewFinding, error) { if findingID == "" { return nil, fmt.Errorf("%w: finding id is required", ErrReviewFindingNotFound) @@ -476,15 +478,7 @@ func (s *ReviewService) UpdateFindingStatus(ctx context.Context, findingID strin if !models.ValidReviewFindingStatus(status) { return nil, fmt.Errorf("%w: unknown status %q", ErrInvalidReviewFinding, status) } - var resolvedAt *time.Time - if status == models.ReviewFindingResolved || status == models.ReviewFindingDismissed { - now := time.Now().UTC() - resolvedAt = &now - } - if err := s.repo.UpdateTaskReviewFindingStatus(ctx, findingID, status, resolvedAt); err != nil { - return nil, err - } - finding, err := s.repo.GetTaskReviewFinding(ctx, findingID) + finding, err := s.repo.TransitionTaskReviewFindingStatus(ctx, findingID, status) if err != nil { return nil, err } diff --git a/apps/backend/internal/task/service/review_service_status_test.go b/apps/backend/internal/task/service/review_service_status_test.go new file mode 100644 index 0000000000..2cfb6d7e09 --- /dev/null +++ b/apps/backend/internal/task/service/review_service_status_test.go @@ -0,0 +1,116 @@ +package service + +import ( + "context" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" +) + +// orderedFindingRepo forces two status requests to reach the repository before +// it applies the reopen-then-resolve ordering that exposes stale state. +type orderedFindingRepo struct { + reviewRepo + transitionsReady chan struct{} + releaseTransitions chan struct{} + transitionCalls int32 + openApplied chan struct{} + openOnce sync.Once +} + +func (r *orderedFindingRepo) awaitTransitionStart() { + n := atomic.AddInt32(&r.transitionCalls, 1) + if n > 2 { + return + } + if n == 2 { + close(r.transitionsReady) + } + <-r.releaseTransitions +} + +func (r *orderedFindingRepo) TransitionTaskReviewFindingStatus( + ctx context.Context, + findingID string, + status models.ReviewFindingStatus, +) (*models.TaskReviewFinding, error) { + r.awaitTransitionStart() + transitioner, ok := r.reviewRepo.(interface { + TransitionTaskReviewFindingStatus(context.Context, string, models.ReviewFindingStatus) (*models.TaskReviewFinding, error) + }) + if !ok { + return nil, nil + } + if status == models.ReviewFindingResolved { + <-r.openApplied + } + finding, err := transitioner.TransitionTaskReviewFindingStatus(ctx, findingID, status) + if status == models.ReviewFindingOpen { + r.openOnce.Do(func() { close(r.openApplied) }) + } + return finding, err +} + +func TestReviewService_UpdateFindingStatusDoesNotRestoreStaleResolvedAt(t *testing.T) { + base, eventBus, repo := createTestReviewService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-status-race") + _, findings, err := base.PublishFindings(ctx, PublishFindingsRequest{ + TaskID: "task-status-race", + Findings: []ReviewFindingInput{{ + FilePath: "a.go", StartLine: 1, EndLine: 1, Severity: "major", + Category: "correctness", Title: "stale", Body: "stale state", + }}, + }) + if err != nil { + t.Fatalf("PublishFindings: %v", err) + } + initial, err := base.UpdateFindingStatus(ctx, findings[0].ID, models.ReviewFindingResolved) + if err != nil || initial.ResolvedAt == nil { + t.Fatalf("seed resolved finding: finding=%+v err=%v", initial, err) + } + initialResolvedAt := *initial.ResolvedAt + + ordered := &orderedFindingRepo{ + reviewRepo: repo, + transitionsReady: make(chan struct{}), + releaseTransitions: make(chan struct{}), + openApplied: make(chan struct{}), + } + svc := NewReviewService(ordered, eventBus, base.logger) + results := make(chan error, 2) + go func() { + _, updateErr := svc.UpdateFindingStatus(ctx, findings[0].ID, models.ReviewFindingOpen) + results <- updateErr + }() + go func() { + _, updateErr := svc.UpdateFindingStatus(ctx, findings[0].ID, models.ReviewFindingResolved) + results <- updateErr + }() + + select { + case <-ordered.transitionsReady: + case <-time.After(5 * time.Second): + t.Fatal("status requests did not reach the transition barrier") + } + close(ordered.releaseTransitions) + for range 2 { + if updateErr := <-results; updateErr != nil { + t.Fatalf("UpdateFindingStatus: %v", updateErr) + } + } + + final, err := repo.GetTaskReviewFinding(ctx, findings[0].ID) + if err != nil { + t.Fatalf("GetTaskReviewFinding: %v", err) + } + if final.Status != models.ReviewFindingResolved || final.ResolvedAt == nil { + t.Fatalf("expected the final status to be resolved with a timestamp, got %+v", final) + } + if final.ResolvedAt.Equal(initialResolvedAt) { + t.Fatalf("stale resolve restored the old timestamp %v", final.ResolvedAt) + } +} diff --git a/apps/backend/internal/task/service/review_service_test.go b/apps/backend/internal/task/service/review_service_test.go index e2fa228162..4962bac28d 100644 --- a/apps/backend/internal/task/service/review_service_test.go +++ b/apps/backend/internal/task/service/review_service_test.go @@ -5,6 +5,7 @@ import ( "errors" "strings" "testing" + "time" "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/events" @@ -443,6 +444,42 @@ func TestReviewService_UpdateFindingStatusStampsAndClearsResolvedAt(t *testing.T } } +func TestReviewService_UpdateFindingStatusResubmittingCurrentStatusIsIdempotent(t *testing.T) { + svc, eventBus, repo := createTestReviewService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-12b") + _, findings, err := svc.PublishFindings(ctx, PublishFindingsRequest{ + TaskID: "task-12b", + Findings: []ReviewFindingInput{validFindingInput()}, + }) + if err != nil { + t.Fatalf("PublishFindings: %v", err) + } + + resolved, err := svc.UpdateFindingStatus(ctx, findings[0].ID, models.ReviewFindingResolved) + if err != nil { + t.Fatalf("resolve: %v", err) + } + firstResolvedAt := resolved.ResolvedAt + if firstResolvedAt == nil { + t.Fatal("expected resolved_at to be stamped") + } + + time.Sleep(2 * time.Millisecond) + eventBus.ClearEvents() + + resubmitted, err := svc.UpdateFindingStatus(ctx, findings[0].ID, models.ReviewFindingResolved) + if err != nil { + t.Fatalf("resubmit: %v", err) + } + if resubmitted.ResolvedAt == nil || !resubmitted.ResolvedAt.Equal(*firstResolvedAt) { + t.Fatalf("resubmitting the current status must leave resolved_at unchanged, got %v want %v", resubmitted.ResolvedAt, firstResolvedAt) + } + if countEvents(eventBus.GetPublishedEvents(), events.TaskReviewFindingUpdated) != 1 { + t.Fatalf("resubmitting the current status should still publish an update event, got %v", eventTypes(eventBus.GetPublishedEvents())) + } +} + func TestReviewService_UpdateFindingStatusRejectsUnknownStatus(t *testing.T) { svc, _, repo := createTestReviewService(t) ctx := context.Background() diff --git a/docs/plans/native-code-review-status-atomicity/plan.md b/docs/plans/native-code-review-status-atomicity/plan.md new file mode 100644 index 0000000000..38632e025d --- /dev/null +++ b/docs/plans/native-code-review-status-atomicity/plan.md @@ -0,0 +1,28 @@ +--- +created: 2026-09-14 +status: completed +requirements: + - REQ-AGENTS-NATIVE-CODE-REVIEW-001 +system_design: + - ../../specs/agents/system-design/native-code-review-status-atomicity.md +legacy_specs: [] +--- + +# Implementation plan: Native review finding status atomicity + +Review finding status changes must be safe when more than one client updates the +same finding at the same time. The repository owns one atomic transition that +reads the stored status, derives `resolved_at`, and returns the stored finding. +The service keeps validation and publishes the existing update event after the +transition succeeds. + +## Work order + +- [x] [Make review finding status transitions atomic](task-01-review-finding-status-atomicity.md) + +## Scope + +- Keep the existing review finding and WebSocket contracts. +- Use one repository transition for status and timestamp updates. +- Cover concurrent SQLite writers and an environment-gated PostgreSQL + multi-connection path. diff --git a/docs/plans/native-code-review-status-atomicity/task-01-review-finding-status-atomicity.md b/docs/plans/native-code-review-status-atomicity/task-01-review-finding-status-atomicity.md new file mode 100644 index 0000000000..131499c3d6 --- /dev/null +++ b/docs/plans/native-code-review-status-atomicity/task-01-review-finding-status-atomicity.md @@ -0,0 +1,36 @@ +--- +id: review-finding-status-atomicity +title: Make review finding status transitions atomic +status: completed +wave: 1 +depends_on: [] +plan: plan.md +requirements: + - REQ-AGENTS-NATIVE-CODE-REVIEW-001 +acceptance_criteria: + - AC-AGENTS-NATIVE-CODE-REVIEW-001.2 + - AC-AGENTS-NATIVE-CODE-REVIEW-001.4 +system_design: + - ../../specs/agents/system-design/native-code-review-status-atomicity.md +--- + +# Make review finding status transitions atomic + +## Scope + +Replace the review service read/update/read sequence with one repository +transition. The SQL update derives `resolved_at` from the status stored by that +same statement and returns the final finding row. Keep status validation and +`TaskReviewFindingUpdated` publication in the service. + +## Verification + +- Review service concurrent update coverage passes. +- SQLite repository concurrent transition coverage passes. +- The real PostgreSQL two-connection test runs when its environment flag is + enabled and remains skipped by default. +- The public WebSocket payload remains unchanged. + +## Result + +The implementation is complete at commit `674dd00f9dae51d1982870ae88583390e1e3aa68`. diff --git a/docs/specs/agents/system-design/native-code-review-status-atomicity.md b/docs/specs/agents/system-design/native-code-review-status-atomicity.md new file mode 100644 index 0000000000..bcac076077 --- /dev/null +++ b/docs/specs/agents/system-design/native-code-review-status-atomicity.md @@ -0,0 +1,19 @@ +--- +status: current +system: agents +requirements: + - REQ-AGENTS-NATIVE-CODE-REVIEW-001 +--- + +# Native review finding status atomicity + +The task review repository is the owner of persisted finding state. A status +transition updates `status` and `resolved_at` in one database statement and +returns the row produced by that statement. The service validates the requested +status before it calls the repository, then publishes the existing finding +updated event with the returned row. + +Concurrent writers can therefore observe different final rows without creating +a timestamp that describes an earlier read. SQLite tests provide deterministic +coverage. A PostgreSQL test with two database connections checks the same +transaction boundary when a PostgreSQL environment is available.