Pin the close before the reopen in the archive sweep (closes #103)

The archive sweep closes a target's archive connection before it reopens the file, and no test noticed if that close was removed, which would leak one SQLite connection per target per sweep. A test now keeps the connection from before a sweep that reopens the archive and requires it closed afterwards. The sweeper's query for database targets takes the sweep's context; a sweep whose context is done returns without an error line, so stopping is not logged as a failure, and a test pins that. The comment on the sweeper's cancel function gives the true reason it needs no lock: fx calls the stop hook only after the start hook has returned.

Model: opus-5-5

internal/delivery/archive_sweeper.go

@@ -45,8 +45,13 @@ type ArchiveSweeper struct {
 	eng      *Engine
 	log      *slog.Logger
 	interval time.Duration
-	cancel   context.CancelFunc
-	wg       sync.WaitGroup
+
+	// cancel needs no lock: fx calls the stop hook only after the
+	// start hook has returned, so stop never reads it while start
+	// is still setting it.
+	cancel context.CancelFunc
+
+	wg sync.WaitGroup
 }
 
 // NewArchiveSweeper creates the archive sweeper and registers
@@ -163,10 +168,18 @@ func (s *ArchiveSweeper) sweep(ctx context.Context) {
 	var targets []database.Target
 
 	err := s.db.DB().
+		WithContext(ctx).
 		Model(&database.Target{}).
 		Where("type = ?", database.TargetTypeDatabase).
 		Find(&targets).Error
 	if err != nil {
+		// The app stopping as a sweep starts cancels the listing.
+		// Stopping is not a failure, so it must not produce an
+		// error line.
+		if ctx.Err() != nil {
+			return
+		}
+
 		s.log.Error(
 			"archive sweep: failed to list database targets",
 			"error", err,

internal/delivery/archive_sweeper_test.go

@@ -1,9 +1,11 @@
 package delivery_test
 
 import (
+	"bytes"
 	"context"
 	"database/sql"
 	"fmt"
+	"log/slog"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -681,6 +683,64 @@ func TestArchiveSweep_ClosesHandleOfRegisteredWriter(
 	)
 }
 
+// TestArchiveSweep_ClosesHandleBeforeReopening proves the sweep
+// closes the handle it finds open before it reopens the file.
+// TestArchiveSweep_LeavesArchiveClosed cannot see this: without the
+// close, the reopen replaces the handle without closing it, the
+// sweep then closes only the new one, and one connection leaks per
+// archive per sweep.
+func TestArchiveSweep_ClosesHandleBeforeReopening(t *testing.T) {
+	t.Parallel()
+
+	path := filepath.Join(t.TempDir(), "archive.db")
+
+	w := delivery.NewExportArchiveWriter(
+		path, archiveTestLogger(), 0,
+	)
+
+	require.NoError(t, w.Open(time.Hour))
+
+	before, err := w.DB().DB()
+	require.NoError(t, err)
+
+	require.NoError(t, w.SweepExpired(time.Hour))
+
+	assert.Error(
+		t, before.PingContext(t.Context()),
+		"the handle open before the sweep must be closed by it",
+	)
+}
+
+// TestArchiveSweep_CancelledSweepLogsNoError proves a sweep whose
+// context is already cancelled, as when the app stops just as a
+// sweep starts, returns without an error line: stopping is not a
+// failure.
+func TestArchiveSweep_CancelledSweepLogsNoError(t *testing.T) {
+	t.Parallel()
+
+	env := setupArchiveTest(t)
+
+	var errorLines bytes.Buffer
+
+	sweeper := delivery.NewTestArchiveSweeper(
+		env.mainDB, env.eng,
+		slog.New(slog.NewTextHandler(
+			&errorLines,
+			&slog.HandlerOptions{Level: slog.LevelError},
+		)),
+	)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	sweeper.ExportSweep(ctx)
+
+	assert.Empty(
+		t, errorLines.String(),
+		"a cancelled sweep must not log at error level",
+	)
+}
+
 // TestArchiveSweep_NeverExpiryUntouched proves the sweep is a
 // no-op for the default retention policy, so archives with no
 // expiry (or the literal "never") behave exactly as before.