From 5fcca2060f91a8b772aeb326f471087649c00dc3 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 6 Jul 2026 17:16:18 +0000 Subject: [PATCH] feat(db_store): decouple migration from store construction Remove auto-migration from DbStore.__init__; add explicit check_migrations() and migrate() methods. The CLI now checks both stores on every startup and prompts the user to confirm before migrating. supervise.prepare() calls .migrate() directly now that __init__ no longer does it implicitly. Co-Authored-By: Claude Sonnet 4.6 --- bot_bottle/cli/__init__.py | 18 +++++++++++++ bot_bottle/db_store.py | 25 ++++++++++++++--- bot_bottle/supervise.py | 4 +-- tests/unit/test_cli_dispatch.py | 6 +++++ tests/unit/test_supervise.py | 4 +++ tests/unit/test_supervise_cli.py | 4 +++ tests/unit/test_supervise_edge.py | 42 +++++++++++++++++++---------- tests/unit/test_supervise_server.py | 4 +++ 8 files changed, 88 insertions(+), 19 deletions(-) diff --git a/bot_bottle/cli/__init__.py b/bot_bottle/cli/__init__.py index 879d067..f97ff53 100644 --- a/bot_bottle/cli/__init__.py +++ b/bot_bottle/cli/__init__.py @@ -7,8 +7,11 @@ from __future__ import annotations import sys +from ..audit_store import AuditStore +from ..db_store import DbVersionError from ..log import Die, die, error from ..manifest import ManifestError +from ..queue_store import QueueStore from ._common import PROG from . import list as _list_mod from .cleanup import cmd_cleanup @@ -74,6 +77,21 @@ def main(argv: list[str] | None = None) -> int: if handler is None: usage() die(f"unknown command: {command}") + queue_store = QueueStore("") + audit_store = AuditStore() + if not queue_store.check_migrations() or not audit_store.check_migrations(): + sys.stderr.write("bot-bottle: database schema is out of date\n") + sys.stderr.write("Migrate now? [y/N] ") + sys.stderr.flush() + try: + answer = sys.stdin.readline().strip().lower() + except EOFError: + answer = "" + if answer != "y": + error("migration required — re-run and confirm to migrate") + return 1 + queue_store.migrate() + audit_store.migrate() try: return handler(rest) or 0 except ManifestError as e: diff --git a/bot_bottle/db_store.py b/bot_bottle/db_store.py index 7cc1849..3d24bc7 100644 --- a/bot_bottle/db_store.py +++ b/bot_bottle/db_store.py @@ -11,6 +11,10 @@ except ImportError: from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module +class DbVersionError(Exception): + """Raised when the on-disk schema is behind the current migration list.""" + + class DbStore: """Base for SQLite-backed stores. Subclasses resolve db_path then call super().__init__.""" @@ -18,14 +22,29 @@ class DbStore: self.db_path = db_path self._migrations = migrations self.db_path.parent.mkdir(parents=True, exist_ok=True) - self._init() def _connect(self) -> sqlite3.Connection: conn = sqlite3.connect(self.db_path) conn.row_factory = sqlite3.Row return conn - def _init(self) -> None: + def check_migrations(self) -> bool: + """Return True if the DB is fully up-to-date, False if migration is needed.""" + if not self.db_path.exists(): + return False + try: + with self._connect() as conn: + row = conn.execute( + "SELECT version FROM schema_versions WHERE module = ?", + (self._migrations.schema_key,), + ).fetchone() + except sqlite3.OperationalError: + return False + version = row[0] if row else 0 + return version == len(self._migrations.migrations) + + def migrate(self) -> None: + """Apply any pending migrations and set permissions on the DB file.""" with self._connect() as conn: self._migrations.apply(conn) self._chmod() @@ -37,4 +56,4 @@ class DbStore: pass -__all__ = ["DbStore"] +__all__ = ["DbStore", "DbVersionError"] diff --git a/bot_bottle/supervise.py b/bot_bottle/supervise.py index ba00b50..38e3993 100644 --- a/bot_bottle/supervise.py +++ b/bot_bottle/supervise.py @@ -357,8 +357,8 @@ class Supervise(ABC): must be set by the launch step before .start runs.""" del stage_dir db_path = host_db_path() - QueueStore(slug) - AuditStore(db_path) + QueueStore(slug).migrate() + AuditStore(db_path).migrate() return SupervisePlan( slug=slug, db_path=db_path, diff --git a/tests/unit/test_cli_dispatch.py b/tests/unit/test_cli_dispatch.py index 4acbc37..06d18f6 100644 --- a/tests/unit/test_cli_dispatch.py +++ b/tests/unit/test_cli_dispatch.py @@ -12,11 +12,17 @@ from unittest.mock import patch import bot_bottle.cli as climod from bot_bottle.cli import main +from bot_bottle.db_store import DbStore from bot_bottle.log import Die from bot_bottle.manifest import ManifestError class TestMainDispatch(unittest.TestCase): + def setUp(self) -> None: + patcher = patch.object(DbStore, "check_migrations", return_value=True) + self._mock_check = patcher.start() + self.addCleanup(patcher.stop) + def test_no_args_prints_usage_returns_2(self) -> None: with patch("sys.stderr", io.StringIO()): self.assertEqual(2, main([])) diff --git a/tests/unit/test_supervise.py b/tests/unit/test_supervise.py index f41ceba..348a69b 100644 --- a/tests/unit/test_supervise.py +++ b/tests/unit/test_supervise.py @@ -8,6 +8,8 @@ from datetime import datetime, timezone from pathlib import Path from bot_bottle import supervise +from bot_bottle.audit_store import AuditStore +from bot_bottle.queue_store import QueueStore from bot_bottle.supervise import ( AuditEntry, Proposal, @@ -113,6 +115,7 @@ class TestQueueIO(unittest.TestCase): self._tmp = tempfile.TemporaryDirectory(prefix="bot-bottle-supervise-test.") self._home_patch = self._patch_home(Path(self._tmp.name)) self.slug = "dev" + QueueStore("").migrate() def tearDown(self): self._home_patch() @@ -222,6 +225,7 @@ class TestAuditLog(unittest.TestCase): def setUp(self): self._tmp = tempfile.TemporaryDirectory(prefix="bot-bottle-supervise-audit.") self._home_patch = self._patch_home(Path(self._tmp.name)) + AuditStore().migrate() def tearDown(self): self._home_patch() diff --git a/tests/unit/test_supervise_cli.py b/tests/unit/test_supervise_cli.py index 836a221..bcf10a0 100644 --- a/tests/unit/test_supervise_cli.py +++ b/tests/unit/test_supervise_cli.py @@ -12,7 +12,9 @@ from pathlib import Path from unittest.mock import patch from bot_bottle import supervise +from bot_bottle.audit_store import AuditStore from bot_bottle.cli import supervise as supervise_cli +from bot_bottle.queue_store import QueueStore from bot_bottle.supervise import ( Proposal, STATUS_APPROVED, @@ -59,6 +61,8 @@ class _FakeHomeMixin: supervise.bot_bottle_root = fake_root # type: ignore[assignment] self._restore_home = lambda: setattr(supervise, "bot_bottle_root", original) + QueueStore("").migrate() + AuditStore().migrate() def _teardown_fake_home(self): self._restore_home() diff --git a/tests/unit/test_supervise_edge.py b/tests/unit/test_supervise_edge.py index 2f69920..3cde845 100644 --- a/tests/unit/test_supervise_edge.py +++ b/tests/unit/test_supervise_edge.py @@ -45,19 +45,22 @@ class TestPathHelpers(unittest.TestCase): class TestReadMalformed(unittest.TestCase): def test_read_proposal_missing_row(self) -> None: with tempfile.TemporaryDirectory() as d: - with patch.dict("os.environ", {"HOME": d}), \ - self.assertRaises(FileNotFoundError): - read_proposal("slug", "p") + with patch.dict("os.environ", {"HOME": d}): + QueueStore("slug").migrate() + with self.assertRaises(FileNotFoundError): + read_proposal("slug", "p") def test_read_response_missing_row(self) -> None: with tempfile.TemporaryDirectory() as d: - with patch.dict("os.environ", {"HOME": d}), \ - self.assertRaises(FileNotFoundError): - read_response("slug", "p") + with patch.dict("os.environ", {"HOME": d}): + QueueStore("slug").migrate() + with self.assertRaises(FileNotFoundError): + read_response("slug", "p") def test_list_pending_reads_db_only(self) -> None: with tempfile.TemporaryDirectory() as d: with patch.dict("os.environ", {"HOME": d}): + QueueStore("slug").migrate() supervise.write_proposal(_proposal()) pending = list_pending_proposals("slug") self.assertEqual(1, len(pending)) @@ -66,6 +69,7 @@ class TestReadMalformed(unittest.TestCase): def test_list_pending_skips_when_response_present(self) -> None: with tempfile.TemporaryDirectory() as d: with patch.dict("os.environ", {"HOME": d}): + QueueStore("slug").migrate() p = _proposal() supervise.write_proposal(p) supervise.write_response("slug", supervise.Response( @@ -79,15 +83,17 @@ class TestReadMalformed(unittest.TestCase): class TestWaitForResponse(unittest.TestCase): def test_missing_response_times_out(self) -> None: with tempfile.TemporaryDirectory() as d: - with patch.dict("os.environ", {"HOME": d}), \ - self.assertRaises(TimeoutError): - wait_for_response("slug", "p", deadline=time.monotonic()) + with patch.dict("os.environ", {"HOME": d}): + QueueStore("slug").migrate() + with self.assertRaises(TimeoutError): + wait_for_response("slug", "p", deadline=time.monotonic()) def test_empty_db_response_does_not_count(self) -> None: with tempfile.TemporaryDirectory() as d: - with patch.dict("os.environ", {"HOME": d}), \ - self.assertRaises(TimeoutError): - wait_for_response("slug", "p", deadline=time.monotonic()) + with patch.dict("os.environ", {"HOME": d}): + QueueStore("slug").migrate() + with self.assertRaises(TimeoutError): + wait_for_response("slug", "p", deadline=time.monotonic()) class TestReadAuditEntries(unittest.TestCase): @@ -99,6 +105,7 @@ class TestReadAuditEntries(unittest.TestCase): def test_reads_entries_from_db(self) -> None: with tempfile.TemporaryDirectory() as home, \ patch.dict("os.environ", {"HOME": home}): + AuditStore().migrate() write_audit_entry(AuditEntry( timestamp="t", bottle_slug="slug", @@ -142,6 +149,7 @@ class TestStoreGuardBranches(unittest.TestCase): with tempfile.TemporaryDirectory() as d: db = Path(d) / "q.db" store = QueueStore("key", db_path=db) + store.migrate() self.assertTrue(db.is_file()) self.assertEqual(db, store.db_path) @@ -149,6 +157,7 @@ class TestStoreGuardBranches(unittest.TestCase): with tempfile.TemporaryDirectory() as d: db = Path(d) / "q.db" store = QueueStore("key", db_path=db) + store.migrate() db.unlink() self.assertEqual([], store.list_pending_proposals()) @@ -156,6 +165,7 @@ class TestStoreGuardBranches(unittest.TestCase): with tempfile.TemporaryDirectory() as d: db = Path(d) / "q.db" store = QueueStore("key", db_path=db) + store.migrate() db.unlink() self.assertEqual([], store.list_all_pending_proposals()) @@ -163,27 +173,31 @@ class TestStoreGuardBranches(unittest.TestCase): with tempfile.TemporaryDirectory() as d: db = Path(d) / "q.db" store = QueueStore("key", db_path=db) + store.migrate() db.unlink() store.archive_proposal("anything") # must not raise def test_queue_store_chmod_oserror_is_swallowed(self): with tempfile.TemporaryDirectory() as d: db = Path(d) / "q.db" + store = QueueStore("key", db_path=db) with patch("pathlib.Path.chmod", side_effect=OSError("ro")): - QueueStore("key", db_path=db) # must not raise + store.migrate() # must not raise def test_audit_store_missing_db_read_returns_empty(self): with tempfile.TemporaryDirectory() as d: db = Path(d) / "a.db" store = AuditStore(db_path=db) + store.migrate() db.unlink() self.assertEqual([], store.read_audit_entries("egress", "slug")) def test_audit_store_chmod_oserror_is_swallowed(self): with tempfile.TemporaryDirectory() as d: db = Path(d) / "a.db" + store = AuditStore(db_path=db) with patch("pathlib.Path.chmod", side_effect=OSError("ro")): - AuditStore(db_path=db) # must not raise + store.migrate() # must not raise if __name__ == "__main__": diff --git a/tests/unit/test_supervise_server.py b/tests/unit/test_supervise_server.py index e41894f..51db278 100644 --- a/tests/unit/test_supervise_server.py +++ b/tests/unit/test_supervise_server.py @@ -17,6 +17,8 @@ from unittest.mock import patch # bare name `supervise`. sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent / "bot_bottle")) import supervise as _sv # noqa: E402 # type: ignore +import queue_store as _qs # noqa: E402 # type: ignore +import audit_store as _as # noqa: E402 # type: ignore from bot_bottle import supervise_server # noqa: E402 from bot_bottle.supervise_server import ( @@ -267,6 +269,8 @@ class TestHandleToolsCall(unittest.TestCase): self._tmp = tempfile.TemporaryDirectory(prefix="supervise-server-test.") self._home_patch = self._patch_home(Path(self._tmp.name)) self.config = ServerConfig(bottle_slug="dev") + _qs.QueueStore("dev").migrate() + _as.AuditStore().migrate() def tearDown(self): self._home_patch()