From 0e57ea874a96aec46b796fb0ef7782ccfd337856 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 07:32:49 +0000 Subject: [PATCH] Report a prune count that could not be read as unknown, not 0 (closes #96) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PruneDatabase read seven table counts with the error discarded, so a query that could not run silently became 0 and the before/after delta computed from it looked like real work. Each read now goes through a helper that logs at warn on failure and returns nil; nil renders as "unknown", never "0", so an empty table is distinguishable from one that could not be queried. Deltas built from an unknown count are themselves unknown. The failure is surfaced, not propagated, so the command's failure conditions are unchanged. These counts have no --json output — under --json the summary is suppressed entirely — so nothing there can show a false 0. Model: opus-4-8 --- TODO.md | 9 +++ internal/vaultik/prune_count_test.go | 79 ++++++++++++++++++++ internal/vaultik/snapshot.go | 105 ++++++++++++++++++++------- 3 files changed, 167 insertions(+), 26 deletions(-) create mode 100644 internal/vaultik/prune_count_test.go diff --git a/TODO.md b/TODO.md index b3011b5..54ab41e 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ release" is exactly the contradiction # Completed Steps +- 2026-09-21: Stopped `prune` from reporting a failed row count as 0 + ([issue #96](https://git.eeqj.de/sneak/vaultik/issues/96)). The seven + `getTableCount` reads in `PruneDatabase` discarded their error, so a + query that could not run became a plausible `0` and the before/after + delta computed from it looked like real work. Each read now logs at + warn on failure and renders as `unknown`, never `0`, so an empty table + is distinguishable from one that could not be queried. The counts have + no `--json` representation — under `--json` the summary is suppressed + entirely — so nothing there can show a false `0`. - 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as corrupt. Its final blob-integrity check hashed the encrypted downloaded bytes with a single SHA256 and compared that to the blob diff --git a/internal/vaultik/prune_count_test.go b/internal/vaultik/prune_count_test.go new file mode 100644 index 0000000..fbc3e5d --- /dev/null +++ b/internal/vaultik/prune_count_test.go @@ -0,0 +1,79 @@ +package vaultik //nolint:testpackage // exercises unexported count helpers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/log" +) + +// TestTableCountForReportSurfacesReadFailure is the regression guard for +// the discarded-error bug: getTableCount for a table its query cannot +// resolve must not silently become 0. A count that could not be read is +// reported as unknown, which a reader can tell apart from an empty table. +// +//nolint:paralleltest // installs the global logger via log.Initialize +func TestTableCountForReportSurfacesReadFailure(t *testing.T) { + log.Initialize(log.Config{}) + + ctx := context.Background() + + db, err := database.New(ctx, ":memory:") + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + v := &Vaultik{DB: db} + v.SetContext(ctx) + + // A table present in the schema reads as a real count. + blobs := v.tableCountForReport("blobs") + require.NotNil(t, blobs, "an existing table must read as a real count") + assert.Equal(t, int64(0), *blobs) + + // A syntactically valid name the sanitizer accepts but whose table + // the query cannot resolve is the exact shape #96 describes: a + // would-be loud failure that used to be discarded into a 0. + _, err = v.getTableCount("snapshots_missing") + require.Error(t, err, "a query against a nonexistent table must fail") + + missing := v.tableCountForReport("snapshots_missing") + assert.Nil(t, missing, "a failed read is unknown, not a count") + + // The rendered count for a failed read must say unknown, never 0. + assert.Equal(t, countUnknown, countText(missing)) + assert.NotEqual(t, "0", countText(missing)) +} + +// TestCountTextDistinguishesEmptyFromUnknown pins the distinction the +// output has to preserve: 0 means the table was empty, "unknown" means +// the count could not be read. +func TestCountTextDistinguishesEmptyFromUnknown(t *testing.T) { + t.Parallel() + + zero := int64(0) + seven := int64(7) + + assert.Equal(t, "0", countText(&zero)) + assert.Equal(t, "7", countText(&seven)) + assert.Equal(t, countUnknown, countText(nil)) +} + +// TestCountDiffUnknownWhenEitherSideUnknown checks that a delta computed +// from an unreadable count is itself unknown rather than a plausible +// number. +func TestCountDiffUnknownWhenEitherSideUnknown(t *testing.T) { + t.Parallel() + + before := int64(10) + after := int64(3) + + require.NotNil(t, countDiff(&before, &after)) + assert.Equal(t, int64(7), *countDiff(&before, &after)) + + assert.Nil(t, countDiff(nil, &after), "unknown before yields unknown delta") + assert.Nil(t, countDiff(&before, nil), "unknown after yields unknown delta") + assert.Nil(t, countDiff(nil, nil)) +} diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 16bba39..8c12ec7 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -8,6 +8,7 @@ import ( "path/filepath" "regexp" "sort" + "strconv" "strings" "time" @@ -1540,12 +1541,17 @@ func (v *Vaultik) outputRemoveJSON(result *RemoveResult) error { return encoder.Encode(result) } -// PruneResult contains statistics about the prune operation +// PruneResult contains statistics about the prune operation. +// SnapshotsDeleted counts snapshots actually deleted. FilesDeleted, +// ChunksDeleted, and BlobsDeleted are derived from before/after row +// counts of the local index; each is nil when a count could not be read, +// so an unreadable count is reported as unknown rather than silently +// as 0. type PruneResult struct { SnapshotsDeleted int64 - FilesDeleted int64 - ChunksDeleted int64 - BlobsDeleted int64 + FilesDeleted *int64 + ChunksDeleted *int64 + BlobsDeleted *int64 } // PruneDatabase removes incomplete snapshots and orphaned files, chunks, @@ -1560,7 +1566,7 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) { result := &PruneResult{} // Snapshot counts before deletion of incompletes. - snapshotCountBefore, _ := v.getTableCount("snapshots") + snapshotCountBefore := v.tableCountForReport("snapshots") // First, delete any incomplete snapshots incompleteSnapshots, err := v.Repositories.Snapshots.GetIncompleteSnapshots(v.ctx) @@ -1575,9 +1581,9 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) { } // Get counts before cleanup for reporting - fileCountBefore, _ := v.getTableCount("files") - chunkCountBefore, _ := v.getTableCount("chunks") - blobCountBefore, _ := v.getTableCount("blobs") + fileCountBefore := v.tableCountForReport("files") + chunkCountBefore := v.tableCountForReport("chunks") + blobCountBefore := v.tableCountForReport("blobs") // Run the cleanup err = v.SnapshotManager.CleanupOrphanedData(v.ctx) @@ -1586,36 +1592,83 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) { } // Get counts after cleanup - fileCountAfter, _ := v.getTableCount("files") - chunkCountAfter, _ := v.getTableCount("chunks") - blobCountAfter, _ := v.getTableCount("blobs") + fileCountAfter := v.tableCountForReport("files") + chunkCountAfter := v.tableCountForReport("chunks") + blobCountAfter := v.tableCountForReport("blobs") - result.FilesDeleted = fileCountBefore - fileCountAfter - result.ChunksDeleted = chunkCountBefore - chunkCountAfter - result.BlobsDeleted = blobCountBefore - blobCountAfter + result.FilesDeleted = countDiff(fileCountBefore, fileCountAfter) + result.ChunksDeleted = countDiff(chunkCountBefore, chunkCountAfter) + result.BlobsDeleted = countDiff(blobCountBefore, blobCountAfter) log.Info("Local database prune complete", "incomplete_snapshots", result.SnapshotsDeleted, - "orphaned_files", result.FilesDeleted, - "orphaned_chunks", result.ChunksDeleted, - "orphaned_blobs", result.BlobsDeleted, + "orphaned_files", countText(result.FilesDeleted), + "orphaned_chunks", countText(result.ChunksDeleted), + "orphaned_blobs", countText(result.BlobsDeleted), ) - snapshotCountAfter := snapshotCountBefore - result.SnapshotsDeleted + // Snapshots remaining after removing the incomplete ones; unknown if + // the pre-prune snapshot count could not be read. + snapshotsRemain := countDiff(snapshotCountBefore, &result.SnapshotsDeleted) v.UI.Completef("Pruned local index database.") - v.UI.Detailf("Incomplete snapshots: %d removed (%d remain).", - result.SnapshotsDeleted, snapshotCountAfter) - v.UI.Detailf("Orphaned files: %d removed (%d remain).", - result.FilesDeleted, fileCountAfter) - v.UI.Detailf("Orphaned chunks: %d removed (%d remain).", - result.ChunksDeleted, chunkCountAfter) - v.UI.Detailf("Orphaned blobs: %d removed (%d remain).", - result.BlobsDeleted, blobCountAfter) + v.UI.Detailf("Incomplete snapshots: %s removed (%s remain).", + countText(&result.SnapshotsDeleted), countText(snapshotsRemain)) + v.UI.Detailf("Orphaned files: %s removed (%s remain).", + countText(result.FilesDeleted), countText(fileCountAfter)) + v.UI.Detailf("Orphaned chunks: %s removed (%s remain).", + countText(result.ChunksDeleted), countText(chunkCountAfter)) + v.UI.Detailf("Orphaned blobs: %s removed (%s remain).", + countText(result.BlobsDeleted), countText(blobCountAfter)) return result, nil } +// countUnknown is what a count reads as when its query could not be run, +// distinct from "0", which means the table really was empty. +const countUnknown = "unknown" + +// tableCountForReport returns the row count of a table for the prune +// summary, or nil if the count could not be read. A read failure is +// logged at warn — visible even under --json, which routes warnings to +// stderr — and then rendered as unknown rather than silently becoming 0, +// so a broken query is a visible failure instead of a plausible wrong +// number. +func (v *Vaultik) tableCountForReport(tableName string) *int64 { + count, err := v.getTableCount(tableName) + if err != nil { + log.Warn("could not read table row count for prune summary", + "table", tableName, "error", err) + + return nil + } + + return &count +} + +// countDiff returns before-after, or nil if either count is unknown so +// that an unreadable count does not collapse into a plausible delta. +func countDiff(before, after *int64) *int64 { + if before == nil || after == nil { + return nil + } + + diff := *before - *after + + return &diff +} + +// countText renders a count that may be unknown: nil (the read failed) +// becomes "unknown", never "0", so a reader can tell an empty table from +// one that could not be queried. +func countText(count *int64) string { + if count == nil { + return countUnknown + } + + return strconv.FormatInt(*count, 10) +} + // validTableNameRe matches table names containing only lowercase // alphanumeric characters and underscores. var validTableNameRe = regexp.MustCompile(`^[a-z0-9_]+$`)