Try every event database file on delete and say which was left (closes #275)

DeleteDB stopped at the first file it could not remove, and the
webhook delete handler logged one message for every failure. Once
the database file itself is gone, its events are lost, and telling
the operator only that a file was left behind suggests otherwise.

DeleteDB now tries the database file and both sidecars, and its
error wraps ErrEventDBNotRemoved when the database file is still on
disk, or ErrSidecarNotRemoved when only a -wal or -shm sidecar is
left. The handler logs a different message for each.

Model: opus-5-5

internal/database/webhook_db_manager.go

@@ -33,6 +33,19 @@ var errInvalidCachedDBType = errors.New(
 	"invalid cached database type",
 )
 
+// ErrEventDBNotRemoved is in DeleteDB's error when the event
+// database file itself could not be removed: it is still on disk.
+var ErrEventDBNotRemoved = errors.New(
+	"event database file not removed",
+)
+
+// ErrSidecarNotRemoved is in DeleteDB's error when the event
+// database file was removed, so its events are gone, but its -wal
+// or -shm sidecar could not be.
+var ErrSidecarNotRemoved = errors.New(
+	"event database file removed, but a -wal or -shm sidecar was not",
+)
+
 // WebhookDBManager manages per-webhook SQLite database files
 // for event storage. Each webhook gets its own dedicated
 // database containing Events, Deliveries, DeliveryResults and the
@@ -151,7 +164,10 @@ func (m *WebhookDBManager) DBExists(
 }
 
 // DeleteDB closes the connection and deletes the database file
-// for a webhook. The file is permanently removed.
+// for a webhook, with its -wal and -shm sidecars. The files are
+// permanently removed. Each file is tried even when another could
+// not be removed, and the error wraps ErrEventDBNotRemoved or
+// ErrSidecarNotRemoved to say which was left, naming each file.
 func (m *WebhookDBManager) DeleteDB(
 	webhookID string,
 ) error {
@@ -170,16 +186,23 @@ func (m *WebhookDBManager) DeleteDB(
 		}
 	}
 
-	// Delete the main DB file and WAL/SHM files
 	path := m.dbPath(webhookID)
-	for _, suffix := range []string{"", "-wal", "-shm"} {
-		err := os.Remove(path + suffix)
-		if err != nil && !os.IsNotExist(err) {
-			return fmt.Errorf(
-				"deleting webhook database file %s%s: %w",
-				path, suffix, err,
-			)
-		}
+
+	dbErr := removeFile(path)
+	sidecarErr := errors.Join(
+		removeFile(path+"-wal"),
+		removeFile(path+"-shm"),
+	)
+
+	if dbErr != nil {
+		return fmt.Errorf(
+			"%w: %w",
+			ErrEventDBNotRemoved, errors.Join(dbErr, sidecarErr),
+		)
+	}
+
+	if sidecarErr != nil {
+		return fmt.Errorf("%w: %w", ErrSidecarNotRemoved, sidecarErr)
 	}
 
 	m.log.Info(
@@ -190,6 +213,17 @@ func (m *WebhookDBManager) DeleteDB(
 	return nil
 }
 
+// removeFile removes path. A file that is already gone counts as
+// removed; the error from any other failure names the file.
+func removeFile(path string) error {
+	err := os.Remove(path)
+	if errors.Is(err, os.ErrNotExist) {
+		return nil
+	}
+
+	return err
+}
+
 // CloseAll closes all open per-webhook database connections.
 // Called during application shutdown.
 func (m *WebhookDBManager) CloseAll() error {

internal/database/webhook_db_manager_test.go

@@ -182,17 +182,91 @@ func TestWebhookDBManager_DeleteDB(t *testing.T) {
 	}
 	require.NoError(t, db.Create(event).Error)
 
+	// Under WAL, an open database that has been written to has both
+	// sidecars beside it.
+	dbPath := mgr.DBPath(webhookID)
+	require.FileExists(t, dbPath+"-wal")
+	require.FileExists(t, dbPath+"-shm")
+
 	// Delete the DB
 	require.NoError(t, mgr.DeleteDB(webhookID))
 
 	// File should no longer exist
 	assert.False(t, mgr.DBExists(webhookID))
 
-	// Verify the file is actually gone from disk
+	// Verify the files are actually gone from disk
+	assert.NoFileExists(t, dbPath)
+	assert.NoFileExists(t, dbPath+"-wal")
+	assert.NoFileExists(t, dbPath+"-shm")
+}
+
+// blockRemoval puts a non-empty directory at path, which os.Remove
+// cannot remove whoever runs the test, root included.
+func blockRemoval(t *testing.T, path string) {
+	t.Helper()
+
+	require.NoError(t, os.MkdirAll(filepath.Join(path, "keep"), 0o700))
+}
+
+// TestWebhookDBManager_DeleteDBKeepsDatabaseFile proves that when the
+// event database file cannot be removed, the error says so, and both
+// sidecars are still removed.
+func TestWebhookDBManager_DeleteDBKeepsDatabaseFile(t *testing.T) {
+	t.Parallel()
+
+	mgr, lc := setupTestWebhookDBManager(t)
+	ctx := context.Background()
+	require.NoError(t, lc.Start(ctx))
+
+	defer func() { require.NoError(t, lc.Stop(ctx)) }()
+
+	webhookID := uuid.New().String()
 	dbPath := mgr.DBPath(webhookID)
 
-	_, err = os.Stat(dbPath)
-	assert.True(t, os.IsNotExist(err))
+	blockRemoval(t, dbPath)
+	require.NoError(t, os.WriteFile(dbPath+"-wal", nil, 0o600))
+	require.NoError(t, os.WriteFile(dbPath+"-shm", nil, 0o600))
+
+	err := mgr.DeleteDB(webhookID)
+
+	require.ErrorIs(t, err, database.ErrEventDBNotRemoved)
+	require.NotErrorIs(t, err, database.ErrSidecarNotRemoved)
+	assert.Contains(t, err.Error(), dbPath)
+	assert.NoFileExists(t, dbPath+"-wal")
+	assert.NoFileExists(t, dbPath+"-shm")
+}
+
+// TestWebhookDBManager_DeleteDBKeepsSidecar proves that when the
+// event database file is removed but a sidecar is not, the error
+// says the database file is gone, and the other sidecar is still
+// removed.
+func TestWebhookDBManager_DeleteDBKeepsSidecar(t *testing.T) {
+	t.Parallel()
+
+	mgr, lc := setupTestWebhookDBManager(t)
+	ctx := context.Background()
+	require.NoError(t, lc.Start(ctx))
+
+	defer func() { require.NoError(t, lc.Stop(ctx)) }()
+
+	webhookID := uuid.New().String()
+	dbPath := mgr.DBPath(webhookID)
+
+	require.NoError(t, mgr.CreateDB(webhookID))
+	// Closing removes the sidecars, so the ones below are the only
+	// ones there.
+	require.NoError(t, mgr.CloseAll())
+
+	blockRemoval(t, dbPath+"-wal")
+	require.NoError(t, os.WriteFile(dbPath+"-shm", nil, 0o600))
+
+	err := mgr.DeleteDB(webhookID)
+
+	require.ErrorIs(t, err, database.ErrSidecarNotRemoved)
+	require.NotErrorIs(t, err, database.ErrEventDBNotRemoved)
+	assert.Contains(t, err.Error(), dbPath+"-wal")
+	assert.NoFileExists(t, dbPath)
+	assert.NoFileExists(t, dbPath+"-shm")
 }
 
 func TestWebhookDBManager_LazyCreation(t *testing.T) {