From 8d2769bf71981c0bba0ef9158f7932d2f4689b10 Mon Sep 17 00:00:00 2001 From: GISCE Bot Date: Thu, 8 Oct 2026 10:39:57 +0200 Subject: [PATCH 1/2] fix: require explicit SQLite migration preflight Co-authored-by: ecarreras --- docs/operations.md | 14 ++- docs/persistence.md | 7 +- src/github_agent_bridge/autoupdate.py | 67 +++++++++------ src/github_agent_bridge/cli.py | 59 +++++++++++-- .../persistence/__init__.py | 14 ++- .../persistence/database.py | 37 +++++++- src/github_agent_bridge/persistence/jobs.py | 21 +++++ src/github_agent_bridge/queue.py | 32 ++++--- .../sql/migrations/__init__.py | 72 ++++++++++++++++ tests/test_autoupdate.py | 19 ++--- tests/test_cli_migrations.py | 85 +++++++++++++++++++ tests/test_mcp.py | 2 +- tests/test_queue.py | 25 ++++-- tests/test_webhook.py | 4 +- 14 files changed, 383 insertions(+), 75 deletions(-) create mode 100644 tests/test_cli_migrations.py diff --git a/docs/operations.md b/docs/operations.md index a3a49af5..2960c57b 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -302,14 +302,22 @@ their version, name, checksum, and application time in `schema_migrations`. `schema.sql` remains the current snapshot for fresh databases; migrations make existing databases converge on that snapshot and perform required backfills. Applied migration files are immutable: a checksum mismatch or a database newer -than the installed package aborts startup instead of guessing. For an explicit -operator run outside autoupdate (after independently ensuring the queue is -quiet), use: +than the installed package aborts startup instead of guessing. Ordinary queue, +CLI, dashboard and webhook startup validates migration history without applying +pending steps or the rolling schema snapshot. For an explicit operator run +outside autoupdate, use: ```bash gab --db ~/.local/state/github-agent-bridge/bridge.sqlite3 migrate-db ``` +`migrate-db` refuses to run while any `pending`, `running` or +`waiting_approval` job exists, even if the executor is paused. For an existing +database it creates an online backup before applying the snapshot and versioned +steps; use `--backup-dir` to choose the destination. A failed migration restores +that backup before returning an error. Fresh database creation remains allowed +without a backup because there is no prior state to preserve. + Dashboard admins can run the same first-step workflow from the autoupdate notice: `Check now` refreshes and records the plan, `Apply update` runs the immediate safe subset, and `Complete reload` runs the recorded deferred reload once the diff --git a/docs/persistence.md b/docs/persistence.md index 5d8fffac..5145ad66 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -25,9 +25,10 @@ domain repositories, the service owns one transaction and passes that connection to every participating repository; repositories must not open a nested independent transaction. -Backup and restore are deliberately excluded. They use SQLite's backup API in -`autoupdate.py` and remain specialized database operations rather than -repository methods. +Backup and restore are deliberately excluded from repositories. The specialized +`backup_sqlite_database()` and `restore_sqlite_database()` operations use +SQLite's backup API and are shared by autoupdate and the explicit migration +command. Executor heartbeat writes, acknowledgement claims and streamed session-event writes treat `SQLITE_BUSY` and `SQLITE_LOCKED` as transient contention after the diff --git a/src/github_agent_bridge/autoupdate.py b/src/github_agent_bridge/autoupdate.py index d15ebec2..e5e9ed21 100644 --- a/src/github_agent_bridge/autoupdate.py +++ b/src/github_agent_bridge/autoupdate.py @@ -11,13 +11,20 @@ from . import __version__ from .models import utc_now +from .persistence import ( + ACTIVE_JOB_STATUSES, + Database, + StateRepository, + active_job_counts, + backup_sqlite_database, + restore_sqlite_database, +) from .policy import Policy from .queue import JobQueue CommandRunner = Callable[[Sequence[str], Path | None], subprocess.CompletedProcess[str]] UPDATE_STATE_KEY = "autoupdate" -ACTIVE_JOB_STATUSES = ("pending", "running", "waiting_approval") RISKY_PATH_PREFIXES = ( "src/github_agent_bridge/cli.py", "src/github_agent_bridge/dispatch.py", @@ -256,6 +263,28 @@ def update_queue_state(queue: JobQueue) -> dict[str, Any]: } +def database_queue_state( + db: str | Path, + *, + migration_preflight: bool = False, +) -> dict[str, Any]: + """Inspect queue state without triggering schema initialization or migration.""" + database = Database(db) + counts = active_job_counts(database) + executor_paused = StateRepository(database).executor_paused() + active_statuses = ( + ACTIVE_JOB_STATUSES + if migration_preflight or not executor_paused + else ("running",) + ) + active_counts = {status: int(counts.get(status, 0)) for status in active_statuses} + return { + "active_counts": active_counts, + "active_total": sum(active_counts.values()), + "executor_paused": executor_paused, + } + + def _service_plan_restarts_executor(service_plan: dict[str, Any]) -> bool: units = service_plan.get("units") if isinstance(service_plan.get("units"), dict) else {} executor_unit = str(units.get("executor") or DEFAULT_SYSTEMD_UNITS["executor"]) @@ -309,32 +338,12 @@ def _command_result( def _backup_sqlite_db(db: str | Path, backup_dir: str | Path | None = None) -> dict[str, Any]: - db_path = Path(db).expanduser() backup_root = Path(backup_dir).expanduser() if backup_dir else DEFAULT_BACKUP_DIR - backup_root.mkdir(parents=True, exist_ok=True) - timestamp = utc_now().replace(":", "").replace("-", "") - backup_path = backup_root / f"{db_path.stem}-{timestamp}.sqlite3" - with sqlite3.connect(db_path) as source, sqlite3.connect(backup_path) as target: - source.backup(target) - return { - "path": str(backup_path), - "created_at": utc_now(), - "source": str(db_path), - "size_bytes": backup_path.stat().st_size, - } + return backup_sqlite_database(db, backup_root) def _restore_sqlite_db(db: str | Path, backup_path: str | Path) -> dict[str, Any]: - db_path = Path(db).expanduser() - backup = Path(backup_path).expanduser() - with sqlite3.connect(backup) as source, sqlite3.connect(db_path) as target: - source.backup(target) - return { - "restored_at": utc_now(), - "source": str(backup), - "target": str(db_path), - "size_bytes": db_path.stat().st_size, - } + return restore_sqlite_database(db, backup_path) def default_migration_command(db: str | Path, *, python_bin: str = sys.executable) -> list[str]: @@ -565,7 +574,7 @@ def apply_update_plan( migration_state: dict[str, Any] = {} service_plan = plan.get("service_plan") if isinstance(plan.get("service_plan"), dict) else {} queue_info = ( - update_queue_state(JobQueue(db)) + database_queue_state(db, migration_preflight=bool(migration_files)) if db is not None else plan.get("queue") if isinstance(plan.get("queue"), dict) else {} ) @@ -612,7 +621,7 @@ def apply_update_plan( return result if migration_files and run_migrations: - queue_info = update_queue_state(JobQueue(db)) + queue_info = database_queue_state(db, migration_preflight=True) result["queue"] = queue_info if int(queue_info.get("active_total") or 0): migration_state["status"] = "deferred" @@ -796,12 +805,9 @@ def plan_update( systemd_units: dict[str, str] | None = None, runner: CommandRunner = _default_runner, ) -> dict[str, Any]: - queue = JobQueue(db) repo_path = Path(repo_dir).expanduser().resolve() current_tag = f"v{installed_version.lstrip('v')}" release = ReleaseInfo(tag_name=target_tag, source="explicit_target") if target_tag else latest_release(repo, gh_bin=gh_bin, runner=runner) - queue_state = update_queue_state(queue) - active_total = int(queue_state["active_total"]) warnings: list[str] = [] try: @@ -812,6 +818,11 @@ def plan_update( classification = classify_changed_files(files) up_to_date = release.tag_name == current_tag migration_required = bool(classification["migration_files"]) + queue_state = database_queue_state( + db, + migration_preflight=migration_required, + ) + active_total = int(queue_state["active_total"]) executor_reload_pending = False dashboard_restart_allowed = False executor_restart_allowed = False diff --git a/src/github_agent_bridge/cli.py b/src/github_agent_bridge/cli.py index 21447d9a..a40a6856 100644 --- a/src/github_agent_bridge/cli.py +++ b/src/github_agent_bridge/cli.py @@ -7,13 +7,20 @@ import os import re import shlex +import sqlite3 import subprocess import sys from pathlib import Path from . import feedback from .actors import backfill_trigger_actors -from .autoupdate import apply_update_plan, complete_pending_reload, plan_update, record_update_plan +from .autoupdate import ( + DEFAULT_BACKUP_DIR, + apply_update_plan, + complete_pending_reload, + plan_update, + record_update_plan, +) from .cancellation import cancel_running_job from .dashboard_data import inspect_db_read_only, list_jobs from .dispatch import FEEDBACK_LEARNING_RULES, GitHubClient, OpenClawDispatcher, RunMode, prompt_rule @@ -23,9 +30,16 @@ from .mcp import authenticate_token, create_token, list_tokens, revoke_token, serve_stdio from .observability import DEFAULT_PROCESS_SAMPLE_RETENTION_SECONDS, configure_sentry from .parser import decode_header_value, extract_body_text, is_github_notification_message, parse_auth_results +from .persistence import ( + Database, + active_job_counts, + backup_sqlite_database, + restore_sqlite_database, +) from .policy import Policy, validate_policy_file from .queue import JobQueue from .reader import ImapConfig, ImapReader, imap_mailbox_arg +from .sql.migrations import migration_history DEFAULT_DB = os.path.expanduser("~/.local/state/github-agent-bridge/bridge.sqlite3") DEFAULT_POLICY = os.path.expanduser("~/.config/github-agent-bridge/policy.json") @@ -91,16 +105,41 @@ def cmd_init_db(args: argparse.Namespace) -> int: def cmd_migrate_db(args: argparse.Namespace) -> int: - queue = JobQueue(args.db) + db_path = Path(args.db).expanduser() + backup: dict | None = None + if db_path.exists(): + counts = active_job_counts(Database(db_path)) + active_total = sum(counts.values()) + if active_total: + print( + "migration blocked: active jobs must be resolved first " + + json.dumps(counts, sort_keys=True), + file=sys.stderr, + ) + return 2 + backup = backup_sqlite_database( + db_path, + Path(args.backup_dir).expanduser(), + ) + try: + queue = JobQueue(db_path, migrate=True) + except Exception as exc: + rollback_error = "" + if backup is not None: + try: + restore_sqlite_database(db_path, backup["path"]) + except sqlite3.Error as restore_exc: + rollback_error = f"; rollback failed: {restore_exc}" + print(f"migration failed: {exc}{rollback_error}", file=sys.stderr) + return 1 with queue.connect() as con: - rows = con.execute( - "SELECT version,name,checksum,applied_at FROM schema_migrations ORDER BY version" - ).fetchall() + migrations = migration_history(con) print( json.dumps( { "database": str(queue.path), - "migrations": [dict(row) for row in rows], + "backup": backup, + "migrations": migrations, }, ensure_ascii=False, ) @@ -485,7 +524,13 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument("--policy", default=None) sub = p.add_subparsers(required=True) s = sub.add_parser("init-db"); s.set_defaults(func=cmd_init_db) - s = sub.add_parser("migrate-db", help="apply pending versioned SQLite migrations"); s.set_defaults(func=cmd_migrate_db) + s = sub.add_parser("migrate-db", help="back up the database and apply pending versioned SQLite migrations") + s.add_argument( + "--backup-dir", + default=os.getenv("GITHUB_AGENT_BRIDGE_AUTOUPDATE_BACKUP_DIR", str(DEFAULT_BACKUP_DIR)), + help="directory for the required pre-migration SQLite backup", + ) + s.set_defaults(func=cmd_migrate_db) s = sub.add_parser("validate-policy", help="validate a policy file against the published schema") s.add_argument("--policy", required=True) s.set_defaults(func=cmd_validate_policy) diff --git a/src/github_agent_bridge/persistence/__init__.py b/src/github_agent_bridge/persistence/__init__.py index 9a7d4cfd..3db45d1f 100644 --- a/src/github_agent_bridge/persistence/__init__.py +++ b/src/github_agent_bridge/persistence/__init__.py @@ -2,7 +2,13 @@ from .acknowledgements import AcknowledgementClaim, AcknowledgementRepository from .commit_statuses import CommitStatusClaim, CommitStatusRepository -from .database import ClosingConnection, Database, TransactionMode +from .database import ( + ClosingConnection, + Database, + TransactionMode, + backup_sqlite_database, + restore_sqlite_database, +) from .feedback import ( FeedbackEvent, FeedbackProposal, @@ -10,7 +16,7 @@ FeedbackRule, ) from .ingestion import IngestionRepository, IngestionRequest, IngestionResult -from .jobs import JobRepository, job_from_row +from .jobs import ACTIVE_JOB_STATUSES, JobRepository, active_job_counts, job_from_row from .mcp_tokens import McpToken, McpTokenCredential, McpTokenRepository from .observability import ObservabilityAlert, ObservabilityRepository, ProcessSample from .runtime import RuntimeProcess, RuntimeRepository @@ -21,6 +27,7 @@ __all__ = [ "AcknowledgementClaim", "AcknowledgementRepository", + "ACTIVE_JOB_STATUSES", "CommitStatusClaim", "CommitStatusRepository", "ClosingConnection", @@ -48,5 +55,8 @@ "WebhookRepository", "WebPushRepository", "WebPushSubscription", + "active_job_counts", + "backup_sqlite_database", "job_from_row", + "restore_sqlite_database", ] diff --git a/src/github_agent_bridge/persistence/database.py b/src/github_agent_bridge/persistence/database.py index e8af300c..b86ab041 100644 --- a/src/github_agent_bridge/persistence/database.py +++ b/src/github_agent_bridge/persistence/database.py @@ -2,9 +2,10 @@ import sqlite3 from contextlib import contextmanager +from datetime import datetime, timezone from enum import Enum from pathlib import Path -from typing import Iterator +from typing import Any, Iterator class TransactionMode(str, Enum): @@ -109,3 +110,37 @@ def _configure(self, con: sqlite3.Connection, *, read_only: bool) -> None: journal_mode = str(con.execute("PRAGMA journal_mode").fetchone()[0]).lower() if journal_mode != "wal": con.execute("PRAGMA journal_mode=WAL") + + +def backup_sqlite_database( + path: str | Path, + backup_dir: str | Path, +) -> dict[str, Any]: + """Create a consistent online SQLite backup for an operator action.""" + source_path = Path(path).expanduser() + backup_root = Path(backup_dir).expanduser() + backup_root.mkdir(parents=True, exist_ok=True) + timestamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%S%fZ") + backup_path = backup_root / f"{source_path.stem}-{timestamp}.sqlite3" + with sqlite3.connect(source_path) as source, sqlite3.connect(backup_path) as target: + source.backup(target) + return { + "path": str(backup_path), + "created_at": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), + "source": str(source_path), + "size_bytes": backup_path.stat().st_size, + } + + +def restore_sqlite_database(path: str | Path, backup_path: str | Path) -> dict[str, Any]: + """Restore a SQLite database from an operator-created backup.""" + target_path = Path(path).expanduser() + source_path = Path(backup_path).expanduser() + with sqlite3.connect(source_path) as source, sqlite3.connect(target_path) as target: + source.backup(target) + return { + "restored_at": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), + "source": str(source_path), + "target": str(target_path), + "size_bytes": target_path.stat().st_size, + } diff --git a/src/github_agent_bridge/persistence/jobs.py b/src/github_agent_bridge/persistence/jobs.py index 7ac982f2..c3b15c24 100644 --- a/src/github_agent_bridge/persistence/jobs.py +++ b/src/github_agent_bridge/persistence/jobs.py @@ -10,6 +10,27 @@ from .runtime import RuntimeRepository from .state import StateRepository +ACTIVE_JOB_STATUSES = ("pending", "running", "waiting_approval") + + +def active_job_counts(database: Database) -> dict[str, int]: + """Read migration-blocking job counts without constructing a queue.""" + with database.read_only() as con: + jobs_exists = con.execute( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name='jobs'" + ).fetchone() + if jobs_exists is None: + return {status: 0 for status in ACTIVE_JOB_STATUSES} + placeholders = ",".join("?" for _ in ACTIVE_JOB_STATUSES) + rows = con.execute( + f"SELECT status,count(*) count FROM jobs " + f"WHERE status IN ({placeholders}) GROUP BY status", + ACTIVE_JOB_STATUSES, + ).fetchall() + counts = {status: 0 for status in ACTIVE_JOB_STATUSES} + counts.update({str(row["status"]): int(row["count"]) for row in rows}) + return counts + def job_from_row(row: sqlite3.Row | None) -> Job | None: """Map one persistence row to the public queue job DTO.""" diff --git a/src/github_agent_bridge/queue.py b/src/github_agent_bridge/queue.py index ee48b6bf..d272367a 100644 --- a/src/github_agent_bridge/queue.py +++ b/src/github_agent_bridge/queue.py @@ -24,7 +24,7 @@ from . import feedback from .actors import trigger_actor_details_for_enqueue, trigger_actor_details_from_notification from .intent_classifier import ParserResult, classify_notification_with_llm, should_classify_with_llm -from .sql.migrations import apply_migrations +from .sql.migrations import apply_migrations, validate_migrations SCHEMA_PACKAGE = "github_agent_bridge.sql" @@ -67,10 +67,13 @@ def canonical_event_key( class JobQueue: - def __init__(self, path: str | Path): + def __init__(self, path: str | Path, *, migrate: bool = False): self.path = Path(path).expanduser() self.database = Database(self.path) - self.init() + if migrate: + self.migrate() + else: + self.init() self.acknowledgements = AcknowledgementRepository(self.database) self.commit_statuses = CommitStatusRepository(self.database) self.runtime = RuntimeRepository(self.database) @@ -90,18 +93,21 @@ def __init__(self, path: str | Path): ) def connect(self) -> sqlite3.Connection: - initialize = not self.path.exists() - con = self.database.read_write() - try: - if initialize: - self._initialize_database(con) - except Exception: - con.close() - raise - return con + if not self.path.exists(): + self.init() + return self.database.read_write() def init(self) -> None: - with self.connect() as con: + if self.path.exists(): + with self.database.read_only() as con: + validate_migrations(con) + return + with self.database.read_write() as con: + self._initialize_database(con) + + def migrate(self) -> None: + """Initialize or migrate a database through an explicit operator path.""" + with self.database.read_write() as con: self._initialize_database(con) def _ensure_initialized(self) -> None: diff --git a/src/github_agent_bridge/sql/migrations/__init__.py b/src/github_agent_bridge/sql/migrations/__init__.py index 0199551f..b4f04fe1 100644 --- a/src/github_agent_bridge/sql/migrations/__init__.py +++ b/src/github_agent_bridge/sql/migrations/__init__.py @@ -17,6 +17,10 @@ class MigrationError(RuntimeError): """Raised when migration history is inconsistent or a step cannot run.""" +class MigrationRequiredError(MigrationError): + """Raised when an existing database needs an explicit migration run.""" + + @dataclass(frozen=True) class Migration: version: int @@ -71,6 +75,74 @@ def _ensure_history_table(con: sqlite3.Connection) -> None: ) +def validate_migrations( + con: sqlite3.Connection, + migrations: Sequence[Migration] | None = None, +) -> tuple[int, ...]: + """Validate migration history without mutating the database. + + Ordinary bridge processes use this read-only check. Only the explicit + migration command is allowed to create history or apply pending steps. + """ + history_exists = con.execute( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name='schema_migrations'" + ).fetchone() + if history_exists is None: + raise MigrationRequiredError( + "database has no migration history; run `gab migrate-db` before starting services" + ) + + steps = tuple(migrations if migrations is not None else load_migrations()) + versions = [migration.version for migration in steps] + if versions != sorted(set(versions)): + raise MigrationError("migration versions must be unique and ordered") + + rows = con.execute( + "SELECT version,name,checksum FROM schema_migrations ORDER BY version" + ).fetchall() + applied = {int(row[0]): (str(row[1]), str(row[2])) for row in rows} + known_versions = set(versions) + unknown_versions = sorted(set(applied) - known_versions) + if unknown_versions: + raise MigrationError( + "database contains migrations newer than this package: " + + ", ".join(str(version) for version in unknown_versions) + ) + + pending: list[int] = [] + for migration in steps: + recorded = applied.get(migration.version) + if recorded is None: + pending.append(migration.version) + elif recorded != (migration.name, migration.checksum): + raise MigrationError( + f"migration {migration.version} does not match recorded name/checksum" + ) + if pending: + raise MigrationRequiredError( + "database has pending migrations " + + ", ".join(str(version) for version in pending) + + "; run `gab migrate-db` before starting services" + ) + return tuple(sorted(applied)) + + +def migration_history(con: sqlite3.Connection) -> list[dict[str, object]]: + """Return the audited migration history through the persistence boundary.""" + return [ + { + "version": int(row["version"]), + "name": str(row["name"]), + "checksum": str(row["checksum"]), + "applied_at": str(row["applied_at"]), + } + for row in con.execute( + "SELECT version,name,checksum,applied_at " + "FROM schema_migrations ORDER BY version" + ).fetchall() + ] + + def apply_migrations( con: sqlite3.Connection, migrations: Sequence[Migration] | None = None, diff --git a/tests/test_autoupdate.py b/tests/test_autoupdate.py index 53906ed2..82fa9864 100644 --- a/tests/test_autoupdate.py +++ b/tests/test_autoupdate.py @@ -260,7 +260,7 @@ def test_migration_update_is_deferred_while_jobs_are_active(tmp_path, monkeypatc assert plan["service_plan"]["deferred"][0]["unit"] == "github-agent-bridge.service" -def test_paused_executor_only_counts_running_jobs_as_active(tmp_path, monkeypatch): +def test_migration_counts_pending_jobs_while_executor_is_paused(tmp_path, monkeypatch): monkeypatch.setattr("github_agent_bridge.actors.github_actor_details_for_context", lambda ctx, *, gh_bin="gh": None) db = tmp_path / "bridge.sqlite3" q = JobQueue(db) @@ -278,12 +278,12 @@ def test_paused_executor_only_counts_running_jobs_as_active(tmp_path, monkeypatc ) assert plan["queue"] == { - "active_counts": {"running": 0}, - "active_total": 0, + "active_counts": {"pending": 1, "running": 0, "waiting_approval": 0}, + "active_total": 1, "executor_paused": True, } - assert plan["decision"] == "stage_full_reload" - assert plan["executor_restart_allowed"] is True + assert plan["decision"] == "defer_migration" + assert plan["executor_restart_allowed"] is False def test_paused_executor_still_counts_running_jobs_as_active(tmp_path, monkeypatch): @@ -305,7 +305,7 @@ def test_paused_executor_still_counts_running_jobs_as_active(tmp_path, monkeypat ) assert plan["queue"] == { - "active_counts": {"running": 1}, + "active_counts": {"pending": 0, "running": 1, "waiting_approval": 0}, "active_total": 1, "executor_paused": True, } @@ -482,11 +482,10 @@ def test_apply_update_plan_blocks_migration_execution_while_jobs_are_active(tmp_ assert state["migration"]["status"] == "deferred" -def test_apply_update_plan_rechecks_live_queue_after_executor_resumes(tmp_path, monkeypatch): +def test_apply_update_plan_rechecks_live_queue_while_executor_is_paused(tmp_path, monkeypatch): monkeypatch.setattr("github_agent_bridge.actors.github_actor_details_for_context", lambda ctx, *, gh_bin="gh": None) db = tmp_path / "bridge.sqlite3" q = JobQueue(db) - enqueue_job(q) q.pause_executor("upgrade window") plan = plan_update( db, @@ -498,7 +497,7 @@ def test_apply_update_plan_rechecks_live_queue_after_executor_resumes(tmp_path, ), ) assert plan["queue"]["active_total"] == 0 - q.resume_executor() + enqueue_job(q) calls: list[list[str]] = [] execution = apply_update_plan( @@ -512,7 +511,7 @@ def test_apply_update_plan_rechecks_live_queue_after_executor_resumes(tmp_path, assert execution["queue"] == { "active_counts": {"pending": 1, "running": 0, "waiting_approval": 0}, "active_total": 1, - "executor_paused": False, + "executor_paused": True, } assert load_update_state(q)["queue"] == execution["queue"] assert calls == [] diff --git a/tests/test_cli_migrations.py b/tests/test_cli_migrations.py new file mode 100644 index 00000000..660224b8 --- /dev/null +++ b/tests/test_cli_migrations.py @@ -0,0 +1,85 @@ +import sqlite3 + +import pytest + +from github_agent_bridge import cli +from github_agent_bridge.models import Notification +from github_agent_bridge.policy import Policy +from github_agent_bridge.queue import JobQueue +from github_agent_bridge.sql.migrations import MigrationRequiredError + + +def _enqueue_waiting_approval(queue: JobQueue) -> None: + notification = Notification( + uid=1, + message_id="", + subject="Re: [gisce/github-agent-bridge] migration", + from_addr="Edu ", + body=( + "@giscebot review " + "https://github.com/gisce/github-agent-bridge/issues/260#issuecomment-1" + ), + auth={"spf": True, "dkim": True, "dmarc": True}, + ) + policy = Policy( + trusted_orgs={"gisce"}, + bot_logins={"giscebot"}, + ) + job, _ = queue.enqueue(notification, policy) + assert job is not None + with queue.connect() as con: + con.execute( + "UPDATE jobs SET status='waiting_approval' WHERE id=?", + (job.id,), + ) + + +def test_migrate_db_blocks_active_jobs_without_mutating_legacy_schema( + tmp_path, + capsys, +): + db = tmp_path / "bridge.sqlite3" + queue = JobQueue(db) + _enqueue_waiting_approval(queue) + with queue.connect() as con: + con.execute("DROP TABLE schema_migrations") + con.execute("DROP TABLE job_commit_statuses") + + with pytest.raises(MigrationRequiredError, match="no migration history"): + JobQueue(db) + + backup_dir = tmp_path / "backups" + result = cli.main( + ["--db", str(db), "migrate-db", "--backup-dir", str(backup_dir)] + ) + + assert result == 2 + assert "waiting_approval" in capsys.readouterr().err + assert not backup_dir.exists() + with sqlite3.connect(db) as con: + tables = { + row[0] + for row in con.execute( + "SELECT name FROM sqlite_master WHERE type='table'" + ) + } + assert "schema_migrations" not in tables + assert "job_commit_statuses" not in tables + + +def test_migrate_db_backs_up_quiet_database_before_applying(tmp_path, capsys): + db = tmp_path / "bridge.sqlite3" + queue = JobQueue(db) + with queue.connect() as con: + con.execute("DELETE FROM schema_migrations WHERE version=2") + con.execute("DROP TABLE job_commit_statuses") + + backup_dir = tmp_path / "backups" + result = cli.main( + ["--db", str(db), "migrate-db", "--backup-dir", str(backup_dir)] + ) + + assert result == 0 + assert len(list(backup_dir.glob("*.sqlite3"))) == 1 + assert '"backup":' in capsys.readouterr().out + JobQueue(db) diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 9258c217..393111b9 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -68,7 +68,7 @@ def test_existing_mcp_tokens_table_is_upgraded_before_user_index(tmp_path): ) con.close() - JobQueue(db) + JobQueue(db, migrate=True) con = sqlite3.connect(db) columns = {row[1] for row in con.execute("PRAGMA table_info(mcp_tokens)")} diff --git a/tests/test_queue.py b/tests/test_queue.py index d33647f6..237d0d73 100644 --- a/tests/test_queue.py +++ b/tests/test_queue.py @@ -10,7 +10,7 @@ from github_agent_bridge.intent_classifier import IntentClassification from github_agent_bridge.policy import FeedbackLearning, IntentClassifier, Policy from github_agent_bridge.queue import JobQueue -from github_agent_bridge.sql.migrations import load_migrations +from github_agent_bridge.sql.migrations import MigrationRequiredError, load_migrations BODY1 = "@pilipilisbot one https://github.com/gisce/erp/pull/1#issuecomment-10" BODY2 = "@pilipilisbot two https://github.com/gisce/erp/pull/1#issuecomment-11" @@ -150,12 +150,12 @@ def test_connect_recreates_missing_parent_directory(tmp_path): ).fetchone()[0] == 1 -def test_init_adds_quarantine_schema_to_existing_database(tmp_path): +def test_explicit_migration_adds_quarantine_schema_to_existing_database(tmp_path): db = tmp_path / "q.sqlite3" with sqlite3.connect(db) as con: con.execute("CREATE TABLE state (key TEXT PRIMARY KEY, value TEXT NOT NULL)") - JobQueue(db) + JobQueue(db, migrate=True) with sqlite3.connect(db) as con: tables = { @@ -194,7 +194,7 @@ def test_fresh_database_records_packaged_migrations(tmp_path): assert all(row[3].endswith("Z") for row in rows) -def test_versioned_migration_adds_columns_to_existing_database(tmp_path): +def test_queue_requires_explicit_migration_for_existing_database(tmp_path): db = tmp_path / "q.sqlite3" q = JobQueue(db) with q.connect() as con: @@ -202,7 +202,20 @@ def test_versioned_migration_adds_columns_to_existing_database(tmp_path): con.execute("ALTER TABLE jobs DROP COLUMN trigger_actor_avatar_url") con.execute("DELETE FROM schema_migrations WHERE version=1") - JobQueue(db) + with pytest.raises(MigrationRequiredError, match="pending migrations 1"): + JobQueue(db) + + with sqlite3.connect(db) as con: + columns = {row[1] for row in con.execute("PRAGMA table_info(jobs)")} + applied = con.execute( + "SELECT count(*) FROM schema_migrations WHERE version=1" + ).fetchone()[0] + + assert "trigger_actor" not in columns + assert "trigger_actor_avatar_url" not in columns + assert applied == 0 + + JobQueue(db, migrate=True) with sqlite3.connect(db) as con: columns = {row[1] for row in con.execute("PRAGMA table_info(jobs)")} @@ -739,7 +752,7 @@ def test_init_backfills_only_the_known_legacy_interval_as_estimated(tmp_path): ), ) - JobQueue(db) + JobQueue(db, migrate=True) JobQueue(db) with sqlite3.connect(db) as con: diff --git a/tests/test_webhook.py b/tests/test_webhook.py index 7196559c..e4c049e4 100644 --- a/tests/test_webhook.py +++ b/tests/test_webhook.py @@ -1718,6 +1718,7 @@ def test_existing_webhook_receipt_schema_is_migrated_for_hook_inventory(tmp_path "action TEXT,event_key TEXT,repository TEXT,payload_hash TEXT NOT NULL,status TEXT NOT NULL," "duplicate_count INTEGER NOT NULL DEFAULT 0,created_at TEXT NOT NULL)" ) + JobQueue(db, migrate=True) payload = issue_comment_payload() client = TestClient(create_app(DashboardConfig(db=db, require_auth=False, webhook_secrets=(SECRET,)))) @@ -1756,9 +1757,10 @@ def test_existing_webhook_hook_targets_are_backfilled_from_github_api_url(tmp_pa "https://api.github.com/orgs/gisce/hooks/43", "2026-10-05T15:29:45+00:00", ), - ) + ) con.execute("DELETE FROM schema_migrations WHERE version=1") + JobQueue(db, migrate=True) client = TestClient(create_app(DashboardConfig(db=db, require_auth=False, webhook_secrets=(SECRET,)))) hooks = {hook["id"]: hook for hook in client.get("/api/webhooks/github/hooks").json()["hooks"]} From 9841f33d8861b5689d9871b6738a3b69fed7821a Mon Sep 17 00:00:00 2001 From: GISCE Bot Date: Thu, 8 Oct 2026 13:08:15 +0200 Subject: [PATCH 2/2] fix: handle missing database during update planning Treat an absent SQLite file as an empty queue without creating or migrating it. Co-authored-by: pilipilisbot <265267145+pilipilisbot@users.noreply.github.com> --- src/github_agent_bridge/autoupdate.py | 6 ++++++ tests/test_autoupdate.py | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/github_agent_bridge/autoupdate.py b/src/github_agent_bridge/autoupdate.py index e5e9ed21..54c83f7e 100644 --- a/src/github_agent_bridge/autoupdate.py +++ b/src/github_agent_bridge/autoupdate.py @@ -270,6 +270,12 @@ def database_queue_state( ) -> dict[str, Any]: """Inspect queue state without triggering schema initialization or migration.""" database = Database(db) + if not database.path.exists(): + return { + "active_counts": {status: 0 for status in ACTIVE_JOB_STATUSES}, + "active_total": 0, + "executor_paused": False, + } counts = active_job_counts(database) executor_paused = StateRepository(database).executor_paused() active_statuses = ( diff --git a/tests/test_autoupdate.py b/tests/test_autoupdate.py index 82fa9864..8feff97f 100644 --- a/tests/test_autoupdate.py +++ b/tests/test_autoupdate.py @@ -145,6 +145,25 @@ def test_update_plan_noops_when_release_matches_installed_version(tmp_path, monk assert plan["executor_reload_pending"] is False +def test_update_plan_treats_missing_database_as_an_empty_queue(tmp_path): + db = tmp_path / "missing.sqlite3" + + plan = plan_update( + db, + repo_dir=tmp_path, + installed_version="1.2.3", + runner=release_runner("v1.2.3", []), + ) + + assert plan["decision"] == "noop" + assert plan["queue"] == { + "active_counts": {"pending": 0, "running": 0, "waiting_approval": 0}, + "active_total": 0, + "executor_paused": False, + } + assert not db.exists() + + def test_dashboard_only_update_can_stage_while_jobs_are_active(tmp_path, monkeypatch): monkeypatch.setattr("github_agent_bridge.actors.github_actor_details_for_context", lambda ctx, *, gh_bin="gh": None) db = tmp_path / "bridge.sqlite3"