store: edge-path tests for migrations and Open

Three gaps from the coverage plan, none of which were covered before.

internal/store/migrations_test.go:

  TestRunMigrationsIgnoresUnknownAppliedIDs — simulates a DB
  written by a newer banger opened by an older one: schema_migrations
  carries an id (9001) the current binary doesn't know about. The
  runner must leave the alien row alone AND still apply its own
  known migrations. Without this, forward-then-backward upgrades or
  running two daemon versions against the same state dir would
  either fail or start destructively reinterpreting rows.

  TestDropColumnIfExistsIsIdempotent — pins the "run twice, no harm"
  property. A daemon restart after migration 2 succeeded on a fresh
  install must not fail because the column is already gone.
  dropColumnIfExists is what makes that idempotent.

internal/store/store_test.go:

  TestOpenRejectsCorruptDB — writes garbage to state.db, Open must
  error cleanly (not panic, not silently overwrite). Also verifies
  the garbage bytes are untouched so the operator can hand the
  file to a recovery tool.

  TestOpenReadOnlyRejectsMissingDB — the doctor path must not
  silently create an empty DB when none exists; that would make
  "no VMs yet" and "your state is missing" indistinguishable.

Package function coverage nudged 39.1% → 40.1%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

internal/store/migrations_test.go

@@ -299,6 +299,124 @@ func TestOpenReadOnlyRefusesWrites(t *testing.T) {
 	}
 }
 
+// TestRunMigrationsIgnoresUnknownAppliedIDs simulates an older banger
+// opening a DB that was written by a newer version: schema_migrations
+// carries rows with ids the current binary's migrations slice doesn't
+// know about. The runner must leave those rows alone and still apply
+// any of its own known migrations that haven't been recorded yet.
+//
+// Without this behaviour, upgrading forward then downgrading back
+// (or running two daemon versions against the same state dir) would
+// either fail outright or start destructively reinterpreting rows.
+func TestRunMigrationsIgnoresUnknownAppliedIDs(t *testing.T) {
+	db := openRawDB(t)
+
+	// Bootstrap schema_migrations and pre-seed a row for a migration
+	// id the current binary doesn't know. Use a high id so it's
+	// clearly outside our slice.
+	if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations (
+		id INTEGER PRIMARY KEY,
+		name TEXT NOT NULL,
+		applied_at TEXT NOT NULL
+	)`); err != nil {
+		t.Fatalf("seed schema_migrations: %v", err)
+	}
+	if _, err := db.Exec(
+		"INSERT INTO schema_migrations (id, name, applied_at) VALUES (?, ?, ?)",
+		9001, "from-the-future", "2099-01-01T00:00:00Z",
+	); err != nil {
+		t.Fatalf("seed future migration row: %v", err)
+	}
+
+	if err := runMigrations(db); err != nil {
+		t.Fatalf("runMigrations: %v", err)
+	}
+
+	// The alien row is untouched.
+	var name string
+	if err := db.QueryRow("SELECT name FROM schema_migrations WHERE id = 9001").Scan(&name); err != nil {
+		t.Fatalf("alien migration row disappeared: %v", err)
+	}
+	if name != "from-the-future" {
+		t.Fatalf("alien row name = %q, want 'from-the-future'", name)
+	}
+
+	// Every known migration in our slice was applied — their rows
+	// should exist too.
+	for _, m := range migrations {
+		var got string
+		if err := db.QueryRow("SELECT name FROM schema_migrations WHERE id = ?", m.id).Scan(&got); err != nil {
+			t.Fatalf("migration %d not recorded despite unknown alien row: %v", m.id, err)
+		}
+	}
+}
+
+// TestDropColumnIfExistsIsIdempotent pins the "run twice, no harm"
+// property. A daemon that restarts after a successful migration 2
+// on a fresh install shouldn't fail because the column is already
+// gone. migrateDropDeadImageColumns calls dropColumnIfExists, which
+// must silently succeed when the column is absent.
+func TestDropColumnIfExistsIsIdempotent(t *testing.T) {
+	db := openRawDB(t)
+	// Set up a tiny table with a known column we're going to drop.
+	if _, err := db.Exec(`CREATE TABLE throwaway (keeper TEXT, victim TEXT)`); err != nil {
+		t.Fatalf("CREATE: %v", err)
+	}
+
+	run := func(label string) error {
+		tx, err := db.Begin()
+		if err != nil {
+			t.Fatalf("%s Begin: %v", label, err)
+		}
+		if err := dropColumnIfExists(tx, "throwaway", "victim"); err != nil {
+			_ = tx.Rollback()
+			return err
+		}
+		return tx.Commit()
+	}
+
+	if err := run("first"); err != nil {
+		t.Fatalf("first dropColumnIfExists: %v", err)
+	}
+	// Second call against a table that no longer has the column.
+	if err := run("second"); err != nil {
+		t.Fatalf("second dropColumnIfExists (column already gone): %v", err)
+	}
+
+	// The keeper column must still be there; victim is gone.
+	rows, err := db.Query("PRAGMA table_info(throwaway)")
+	if err != nil {
+		t.Fatalf("PRAGMA: %v", err)
+	}
+	defer rows.Close()
+	var haveKeeper, haveVictim bool
+	for rows.Next() {
+		var (
+			cid       int
+			name      string
+			valueType string
+			notNull   int
+			defaultV  sql.NullString
+			pk        int
+		)
+		if err := rows.Scan(&cid, &name, &valueType, &notNull, &defaultV, &pk); err != nil {
+			t.Fatalf("scan: %v", err)
+		}
+		switch name {
+		case "keeper":
+			haveKeeper = true
+		case "victim":
+			haveVictim = true
+		}
+	}
+	if !haveKeeper {
+		t.Fatal("keeper column disappeared — dropColumnIfExists is too aggressive")
+	}
+	if haveVictim {
+		t.Fatal("victim column survived — dropColumnIfExists didn't actually drop")
+	}
+}
+
 func TestRunMigrationsRejectsDuplicateID(t *testing.T) {
 	db := openRawDB(t)
 	orig := migrations

internal/store/store_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"errors"
+	"os"
 	"path/filepath"
 	"reflect"
 	"strconv"
@@ -319,6 +320,58 @@ func TestStoreConfiguresSQLitePragmasOnPooledConnections(t *testing.T) {
 	}
 }
 
+// TestOpenRejectsCorruptDB pins the actionable-error contract when
+// state.db exists on disk but isn't a valid SQLite file. Users can
+// hit this after a disk-full crash mid-write, a copy that truncated,
+// or accidental manual editing. banger must surface the error
+// cleanly so the operator can delete-and-retry — never panic, never
+// silently overwrite, never leak a partially-opened sql.DB handle.
+func TestOpenRejectsCorruptDB(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, "state.db")
+	garbage := []byte("this is definitely not a sqlite database")
+	if err := os.WriteFile(path, garbage, 0o600); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+
+	s, err := Open(path)
+	if err == nil {
+		_ = s.Close()
+		t.Fatal("Open: want error on corrupt DB file")
+	}
+
+	// The garbage bytes must still be there — Open must not have
+	// overwritten the file mid-attempt. A user recovering from a
+	// mid-write crash needs that invariant to hand the file to a
+	// tool like sqlite3_analyzer.
+	got, readErr := os.ReadFile(path)
+	if readErr != nil {
+		t.Fatalf("ReadFile: %v", readErr)
+	}
+	if string(got) != string(garbage) {
+		t.Fatalf("Open touched the garbage file: got %q, want %q", string(got), string(garbage))
+	}
+}
+
+// TestOpenReadOnlyRejectsMissingDB pins the "no silent creation"
+// contract for the doctor path: OpenReadOnly against a path that
+// doesn't exist must error, not create an empty DB that later reads
+// would mistake for "no VMs yet."
+func TestOpenReadOnlyRejectsMissingDB(t *testing.T) {
+	t.Parallel()
+	missing := filepath.Join(t.TempDir(), "never-existed.db")
+	s, err := OpenReadOnly(missing)
+	if err == nil {
+		_ = s.Close()
+		t.Fatal("OpenReadOnly: want error when the DB file doesn't exist")
+	}
+	if _, statErr := os.Stat(missing); !os.IsNotExist(statErr) {
+		t.Fatalf("OpenReadOnly silently created %q (stat err = %v)", missing, statErr)
+	}
+}
+
 func openTestStore(t *testing.T) *Store {
 	t.Helper()
 	store, err := Open(filepath.Join(t.TempDir(), "state.db"))