diff --git a/CHANGELOG.md b/CHANGELOG.md index d6d0493..d02c3f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,32 @@ All notable changes to Curator are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.1.2] — 2026-07-28 + +### Added +- **The ledger is backed up before a schema migration.** Opening `.curator/` with a newer + Curator applies pending migrations automatically; it now copies the ledger to + `.curator/archive/…-pre-migration.sqlite` first. The copy uses SQLite's online backup + API rather than a file copy, so data still sitting in the write-ahead log is preserved. + A ledger with nothing to migrate — the usual case, since every command opens it — is not + copied. +- **`curator doctor` reports pending migrations and the newest backup.** It lists what the + next open will apply without applying it, and prints a ready-to-paste `cp` command to + restore the most recent backup. The backup lookup reads the directory, not the ledger, so + it still answers when the ledger will not open. + +### Fixed +- **A failed backup is never offered as a restore point.** `sqlite3.connect` creates the + destination up front, so a backup interrupted by a full disk, an I/O error, or a killed + process left a truncated file carrying the real name. Being the newest, it became what + `curator doctor` advertised as the ledger to restore — and copying it over a live ledger + would have destroyed it. Backups are now written under a name the restore path ignores and + renamed into place only once complete, and an unusable file is never advertised. +- **A failed migration no longer leaves a half-migrated ledger.** Pending migrations and + their `schema_version` rows now share one transaction, so a failure rolls the schema + changes back with it instead of stopping in an intermediate state and raises + `CuratorStateError`. Previously each migration committed on its own. + ## [0.1.1] — 2026-07-28 ### Fixed @@ -54,5 +80,6 @@ Serial single-writer (local `flock`, no cross-host coordination); the decisions/ — not the provider transcript — are the system of record; macOS primary, Linux in CI, Windows via WSL2 only. +[0.1.2]: https://github.com/JasonZQH/CURATOR/releases/tag/v0.1.2 [0.1.1]: https://github.com/JasonZQH/CURATOR/releases/tag/v0.1.1 [0.1.0]: https://github.com/JasonZQH/CURATOR/releases/tag/v0.1.0 diff --git a/README.md b/README.md index 5790ec3..3961516 100644 --- a/README.md +++ b/README.md @@ -70,11 +70,11 @@ curl -fsSL https://raw.githubusercontent.com/JasonZQH/CURATOR/main/install.sh | Or with pipx / uv: ```bash -pipx install "git+https://github.com/JasonZQH/CURATOR.git@v0.1.1" -uvx --from "git+https://github.com/JasonZQH/CURATOR.git@v0.1.1" curator +pipx install "git+https://github.com/JasonZQH/CURATOR.git@v0.1.2" +uvx --from "git+https://github.com/JasonZQH/CURATOR.git@v0.1.2" curator ``` -> Pin a released tag (`@v0.1.1`) for a reproducible install, or drop it to track `main`. +> Pin a released tag (`@v0.1.2`) for a reproducible install, or drop it to track `main`. > On Windows, run inside WSL2. **Open it** — from any project directory: @@ -106,8 +106,9 @@ Small requests start immediately; `/gate on` reviews the goal proposal first. | Version | Status | What it is | |---|---|---| -| **v0.1.1** | Current | Patch on Phase 0. A Codex tool call is counted once rather than once per lifecycle event, so the activity block no longer reads ~2× high. Docs numbering folded onto a single `v0.1.x` roadmap. | -| v0.1.0 | Previous | Phase 0 — local · single-writer · sequential. `Goal → writer → deterministic verifier → fresh-context reviewer → human confirm`, on a durable SQLite ledger. Opt-in `/resume stash` for the clean-tree guard. | +| **v0.1.2** | Current | Migration safety. Upgrading backs the ledger up before any schema migration touches it, migrations apply as one all-or-nothing transaction, and `curator doctor` shows what is pending plus how to restore. | +| v0.1.1 | Previous | Patch on Phase 0. A Codex tool call is counted once rather than once per lifecycle event, so the activity block no longer reads ~2× high. Docs numbering folded onto a single `v0.1.x` roadmap. | +| v0.1.0 | | Phase 0 — local · single-writer · sequential. `Goal → writer → deterministic verifier → fresh-context reviewer → human confirm`, on a durable SQLite ledger. Opt-in `/resume stash` for the clean-tree guard. | v0.1.0 highlights: full-screen first-run trust & setup, keyboard-selectable slash commands and proposal actions, PM/Engineer/Reviewer seat labels, persistent history with Tab completion and @@ -138,7 +139,7 @@ Full history in [CHANGELOG.md](CHANGELOG.md). | `curator provider add ` | Detect and register a provider CLI (`claude-code` / `codex`) | | `curator provider list` | List configured provider profiles | | `curator status` | Show current project state | -| `curator doctor` | Run environment and readiness checks | +| `curator doctor` | Run environment and readiness checks, including pending schema migrations | | `curator reset` | Archive the ledger and clear runtime state (`--hard` removes `.curator/`) | | `curator contract validate` | Validate editable role contracts | | `curator --version` | Print the version | @@ -157,6 +158,16 @@ Full history in [CHANGELOG.md](CHANGELOG.md). TUI adds Up/Down history, Tab completion, Shift+Enter/Ctrl+J continuation lines, Esc interruption, and two-stage Ctrl+C shutdown. +**Upgrades and your ledger.** Opening `.curator/` with a newer Curator applies any pending +schema migrations automatically. Before it touches anything, the ledger is copied to +`.curator/archive/…-pre-migration.sqlite`, and the migration runs as a single transaction — +if it fails, the ledger is left exactly as it was. `curator doctor` shows what is pending +before it runs and, afterwards, prints the newest backup with the command to restore it: + +```bash +cp .curator/archive/curator--pre-migration.sqlite .curator/curator.sqlite +``` + ## Known limitations Phase 0 is deliberately narrow. Where a claim above could be read more broadly than the code diff --git a/pyproject.toml b/pyproject.toml index cfc29f4..e0f28f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "hatchling.build" [project] name = "curator" -version = "0.1.1" +version = "0.1.2" description = "Local-first coding-agent workbench with real provider orchestration." readme = { file = "README.md", content-type = "text/markdown" } requires-python = ">=3.11" diff --git a/src/curator/__init__.py b/src/curator/__init__.py index 4882ed1..025766d 100644 --- a/src/curator/__init__.py +++ b/src/curator/__init__.py @@ -1,3 +1,3 @@ """Expose package-level metadata for Curator Phase 0.""" -__version__ = "0.1.1" +__version__ = "0.1.2" diff --git a/src/curator/cli.py b/src/curator/cli.py index d7e15c2..39b965e 100644 --- a/src/curator/cli.py +++ b/src/curator/cli.py @@ -21,6 +21,7 @@ from curator.rendering.terminal import ( render_contract_validation_report, render_doctor_report, + render_migration_outcome, render_status_report, ) from curator.providers.setup import add_provider_profile, resolve_provider_name @@ -66,6 +67,13 @@ def _echo_init_summary(created_files_count: int, skipped_files_count: int) -> No typer.echo(f"Skipped existing files: {skipped_files_count}") +def _echo_migration(outcome) -> None: + """Print the migration notice when opening the ledger actually migrated it.""" + message = render_migration_outcome(outcome) + if message: + typer.echo(message) + + def _ensure_curator_state(root: Path, yes: bool) -> None: """Create Curator state when requested or raise for interactive approval.""" if not yes: @@ -232,10 +240,11 @@ def provider_add_command( raise typer.Exit(1) connection = connect_database(paths.database) try: - initialize_database(connection) + migration = initialize_database(connection) result = add_provider_profile(connection, name) finally: connection.close() + _echo_migration(migration) typer.echo(result.message) if not result.created: raise typer.Exit(1) @@ -255,10 +264,11 @@ def provider_list_command() -> None: return connection = connect_database(paths.database) try: - initialize_database(connection) + migration = initialize_database(connection) profiles = load_provider_profiles(connection) finally: connection.close() + _echo_migration(migration) if not profiles: typer.echo("Providers:\n- none (run curator provider add )") return diff --git a/src/curator/diagnostics/doctor.py b/src/curator/diagnostics/doctor.py index cf46e40..fcecc4b 100644 --- a/src/curator/diagnostics/doctor.py +++ b/src/curator/diagnostics/doctor.py @@ -8,7 +8,8 @@ from curator.core.paths import build_curator_paths from curator.diagnostics.models import DoctorCheck, DoctorReport from curator.providers.profiles import resolve_runtime_mode -from curator.state.db import connect_database +from curator.state.backup import latest_pre_migration_backup +from curator.state.db import connect_database, pending_migrations def _check_existing_path(path: Path, ok_detail: str, missing_detail: str) -> DoctorCheck: @@ -56,10 +57,56 @@ def inspect_project_health(project_root: Path | str) -> DoctorReport: "package": DoctorCheck(status="ok", detail=f"curator {__version__}"), "state": state_check, "database": database_check, + "migrations": _migrations_check(paths.database) if initialized else _NO_LEDGER_CHECK, + "backup": _backup_check(paths.curator_dir), }, ) +_NO_LEDGER_CHECK = DoctorCheck(status="missing", detail="No ledger yet.") + + +def _migrations_check(database: Path) -> DoctorCheck: + """Report which migrations the next open will apply, without applying them.""" + try: + connection = connect_database(database) + try: + pending = pending_migrations(connection) + finally: + connection.close() + except sqlite3.DatabaseError as error: + return DoctorCheck(status="fail", detail=f"Cannot read the ledger: {error}") + + if not pending: + return DoctorCheck(status="ok", detail="Schema is current.") + + versions = ", ".join(str(version) for version in pending) + return DoctorCheck( + status="pending", + detail=( + f"{len(pending)} migration(s) pending ({versions}); the next command that opens " + "the ledger applies them, backing it up to .curator/archive/ first." + ), + ) + + +def _backup_check(curator_dir: Path) -> DoctorCheck: + """Point at the newest pre-migration backup and how to restore it. + + Reads the directory rather than the ledger, so this still answers when the ledger will + not open — which is when someone actually needs it. + """ + backup = latest_pre_migration_backup(curator_dir) + if backup is None: + return DoctorCheck(status="none", detail="No pre-migration backup taken yet.") + + database = curator_dir / "curator.sqlite" + return DoctorCheck( + status="ok", + detail=f"Newest pre-migration backup: {backup}\n restore: cp {backup} {database}", + ) + + def _resolve_mode(database: Path) -> str: """Return the runtime mode label for an existing database.""" try: diff --git a/src/curator/init/reset.py b/src/curator/init/reset.py index d138d2b..ba45506 100644 --- a/src/curator/init/reset.py +++ b/src/curator/init/reset.py @@ -2,11 +2,12 @@ import shutil from dataclasses import dataclass, field -from datetime import UTC, datetime +from datetime import datetime from pathlib import Path from curator.core.paths import build_curator_paths from curator.core.schema import CuratorPaths +from curator.state.backup import archive_stamp, ledger_archive_dir @dataclass(frozen=True) @@ -67,10 +68,9 @@ def reset_curator_state( archived_database = None if paths.database.exists(): - stamp = (now or datetime.now(UTC)).strftime("%Y%m%dT%H%M%SZ") - archive_dir = paths.curator_dir / "archive" + archive_dir = ledger_archive_dir(paths.curator_dir) archive_dir.mkdir(parents=True, exist_ok=True) - archived_database = archive_dir / f"curator-{stamp}.sqlite" + archived_database = archive_dir / f"curator-{archive_stamp(now)}.sqlite" shutil.move(str(paths.database), str(archived_database)) removed_paths = [] diff --git a/src/curator/rendering/terminal.py b/src/curator/rendering/terminal.py index 6de6708..a2a9dfe 100644 --- a/src/curator/rendering/terminal.py +++ b/src/curator/rendering/terminal.py @@ -20,8 +20,29 @@ def render_doctor_report(report: DoctorReport) -> str: f"State: {report.checks['state'].status}", f"Database: {report.checks['database'].status}", f"Mode: {report.mode}", - f"Recommended next step: {report.recommended_next_step}", ] + migrations = report.checks.get("migrations") + if migrations is not None and migrations.status != "missing": + lines.append(f"Migrations: {migrations.status} — {migrations.detail}") + backup = report.checks.get("backup") + if backup is not None and backup.status == "ok": + lines.append(f"Backup: {backup.detail}") + lines.append(f"Recommended next step: {report.recommended_next_step}") + return "\n".join(lines) + + +def render_migration_outcome(outcome) -> str: + """Render what a just-applied migration did, or "" when nothing was migrated. + + Migrating a ledger is not something to do silently: the user should learn that their + schema moved and where the copy of the old one is, at the moment it happens. + """ + if outcome is None: + return "" + versions = ", ".join(str(version) for version in outcome.applied) + lines = [f"Migrated ledger: schema v{outcome.from_version} → v{outcome.to_version} ({versions})."] + if outcome.backup is not None: + lines.append(f"Previous ledger backed up to {outcome.backup}") return "\n".join(lines) diff --git a/src/curator/state/backup.py b/src/curator/state/backup.py new file mode 100644 index 0000000..5ca4a60 --- /dev/null +++ b/src/curator/state/backup.py @@ -0,0 +1,96 @@ +"""Copy the Curator ledger aside before a schema migration rewrites it.""" + +import sqlite3 +from datetime import UTC, datetime +from pathlib import Path + +# Migration backups and reset archives are both "a ledger as it was", so they share one +# directory and one timestamp format. The suffix is what tells them apart on disk. +_ARCHIVE_DIR_NAME = "archive" +_STAMP_FORMAT = "%Y%m%dT%H%M%SZ" +PRE_MIGRATION_SUFFIX = "-pre-migration" + + +def ledger_archive_dir(curator_dir: Path) -> Path: + """Return the directory holding superseded copies of the ledger.""" + return Path(curator_dir) / _ARCHIVE_DIR_NAME + + +def archive_stamp(now: datetime | None = None) -> str: + """Return the UTC timestamp used to name an archived ledger.""" + return (now or datetime.now(UTC)).strftime(_STAMP_FORMAT) + + +def backup_ledger( + connection: sqlite3.Connection, + curator_dir: Path, + now: datetime | None = None, +) -> Path: + """Copy the open ledger into the archive directory and return the backup path. + + Uses SQLite's online backup API rather than copying the file. The ledger runs in WAL + mode, so a file copy can miss pages still sitting in the write-ahead log and produce a + backup that is silently short of the data it is supposed to preserve. + """ + directory = ledger_archive_dir(curator_dir) + directory.mkdir(parents=True, exist_ok=True) + + destination_path = _free_backup_path(directory, archive_stamp(now)) + # Write under a name the restore path ignores, then publish by renaming. sqlite3.connect + # creates the file up front, so a backup that dies half way — disk full, I/O error, + # killed process — would otherwise leave a truncated file carrying the real name. That + # file is the newest, so it becomes what doctor offers as the restore point, and copying + # it over a live ledger destroys the ledger. Only a completed backup gets the real name. + partial_path = destination_path.with_suffix(".partial") + try: + destination = sqlite3.connect(partial_path) + try: + connection.backup(destination) + finally: + destination.close() + partial_path.replace(destination_path) + except BaseException: + partial_path.unlink(missing_ok=True) + raise + return destination_path + + +def latest_pre_migration_backup(curator_dir: Path) -> Path | None: + """Return the newest pre-migration backup, or None when none has been taken. + + Reads the directory rather than the ledger, so it still answers when the ledger itself + will not open — which is exactly when someone is looking for a backup. + """ + directory = ledger_archive_dir(curator_dir) + if not directory.is_dir(): + return None + + # An empty file is never a usable ledger, and this path tells someone what to copy over + # a live one, so anything that cannot be a backup is skipped rather than offered. + backups = [ + path + for path in directory.glob(f"curator-*{PRE_MIGRATION_SUFFIX}.sqlite") + if path.stat().st_size > 0 + ] + if not backups: + return None + # By mtime, not by name: a same-second backup is disambiguated with an index that + # sorts before the plain name, so lexicographic order would report the oldest. + return max(backups, key=lambda path: path.stat().st_mtime) + + +def _free_backup_path(directory: Path, stamp: str) -> Path: + """Return a backup path that does not already exist. + + Two migrations inside the same second would otherwise resolve to one filename and the + second would overwrite the first — discarding the older ledger state. + """ + candidate = directory / f"curator-{stamp}{PRE_MIGRATION_SUFFIX}.sqlite" + if not candidate.exists(): + return candidate + + for index in range(2, 100): + candidate = directory / f"curator-{stamp}-{index}{PRE_MIGRATION_SUFFIX}.sqlite" + if not candidate.exists(): + return candidate + raise FileExistsError(f"cannot find a free backup name in {directory}") diff --git a/src/curator/state/db.py b/src/curator/state/db.py index 6a64b84..47666ed 100644 --- a/src/curator/state/db.py +++ b/src/curator/state/db.py @@ -1,13 +1,26 @@ """Manage SQLite connections and database initialization.""" import sqlite3 +from dataclasses import dataclass from datetime import UTC, datetime from pathlib import Path from typing import Callable +from curator.core.errors import CuratorStateError +from curator.state.backup import backup_ledger from curator.state.migrations import phase0_schema_sql +@dataclass(frozen=True) +class MigrationOutcome: + """Describe the schema migrations one initialization actually applied.""" + + applied: tuple[int, ...] + from_version: int + to_version: int + backup: Path | None = None + + class CuratorConnection(sqlite3.Connection): """Track Curator-owned transaction nesting without mutating sqlite3.Connection.""" @@ -96,25 +109,88 @@ def _ensure_memory_entry_learning_columns(connection: sqlite3.Connection) -> Non ) -def _apply_versioned_migrations(connection: sqlite3.Connection) -> None: - """Run numbered migrations that are not yet recorded in schema_version.""" - applied = { - row["version"] - for row in connection.execute("select version from schema_version").fetchall() - } - for version, migration in VERSIONED_MIGRATIONS: - if version in applied: - continue +def applied_migrations(connection: sqlite3.Connection) -> set[int]: + """Return the migration versions this ledger has already recorded. + + A ledger old enough to predate the schema_version table has recorded nothing, which is + a valid state to migrate from rather than an error — and this is called from read-only + callers like doctor that never create the table. + """ + try: + rows = connection.execute("select version from schema_version").fetchall() + except sqlite3.OperationalError: + return set() + return {row["version"] for row in rows} + + +def pending_migrations(connection: sqlite3.Connection) -> list[int]: + """Return the migration versions this ledger has not applied yet, in order.""" + already = applied_migrations(connection) + return [version for version, _ in VERSIONED_MIGRATIONS if version not in already] + + +def _apply_versioned_migrations(connection: sqlite3.Connection, pending: list[int]) -> None: + """Apply pending migrations as one all-or-nothing unit. + + Every migration and its schema_version row share a single transaction: SQLite rolls + DDL back with everything else, so a failure half way through leaves the ledger exactly + as it was rather than in a state no version of Curator is written to read. + """ + migrations = dict(VERSIONED_MIGRATIONS) + connection.begin_transaction() + try: + for version in pending: + migrations[version](connection) + connection.execute( + "insert into schema_version (version, applied_at) values (?, ?)", + (version, datetime.now(UTC).isoformat()), + ) + connection.commit_transaction() + except Exception as error: + connection.rollback_transaction() + raise CuratorStateError(f"schema migration failed and was rolled back: {error}") from error + + +def _ledger_database_path(connection: sqlite3.Connection) -> Path | None: + """Return the file backing this connection, or None for an in-memory ledger.""" + for row in connection.execute("pragma database_list").fetchall(): + if row["name"] == "main" and row["file"]: + return Path(row["file"]) + return None + + +def _ledger_has_content(connection: sqlite3.Connection) -> bool: + """Report whether this ledger already holds objects worth preserving.""" + row = connection.execute("select count(*) as count from sqlite_master").fetchone() + return bool(row["count"]) + + +def initialize_database(connection: sqlite3.Connection) -> MigrationOutcome | None: + """Create the Phase 0 SQLite tables and apply pending migrations. + + Returns what was migrated, or None when the ledger was already current — which is the + common case, since every command that opens the ledger lands here. An existing ledger + with work to do is copied to .curator/archive/ first, before anything writes to it. + """ + pending = pending_migrations(connection) + previous = max(applied_migrations(connection), default=0) + + backup: Path | None = None + if pending and _ledger_has_content(connection): + database_path = _ledger_database_path(connection) + if database_path is not None: + backup = backup_ledger(connection, database_path.parent) - migration(connection) - connection.execute( - "insert into schema_version (version, applied_at) values (?, ?)", - (version, datetime.now(UTC).isoformat()), - ) - - -def initialize_database(connection: sqlite3.Connection) -> None: - """Create the Phase 0 SQLite tables and apply pending migrations.""" connection.executescript(phase0_schema_sql()) - _apply_versioned_migrations(connection) + if pending: + _apply_versioned_migrations(connection, pending) connection.commit() + + if not pending: + return None + return MigrationOutcome( + applied=tuple(pending), + from_version=previous, + to_version=max(pending), + backup=backup, + ) diff --git a/tests/test_migration_safety.py b/tests/test_migration_safety.py new file mode 100644 index 0000000..61d440e --- /dev/null +++ b/tests/test_migration_safety.py @@ -0,0 +1,252 @@ +"""Verify a schema migration backs the ledger up, is atomic, and stays visible. + +Every command that opens the ledger runs migrations, so the guarantees under test are: +the old ledger is preserved before anything writes, a failure leaves the ledger exactly as +it was, and nothing migrates without a way to see it coming. +""" + +import sqlite3 + +import pytest + +from curator.core.errors import CuratorStateError +from curator.core.paths import build_curator_paths +from curator.diagnostics.doctor import inspect_project_health +from curator.state.backup import ( + backup_ledger, + latest_pre_migration_backup, + ledger_archive_dir, +) +from curator.state.db import ( + connect_database, + initialize_database, + pending_migrations, +) + + +def _open(tmp_path): + """Return a connection to this project's ledger.""" + return connect_database(build_curator_paths(tmp_path).database) + + +def _backups(tmp_path): + """Return every pre-migration backup on disk for this project.""" + directory = ledger_archive_dir(build_curator_paths(tmp_path).curator_dir) + return sorted(directory.glob("*-pre-migration.sqlite")) if directory.is_dir() else [] + + +def _schema_rows(connection): + """Return the schema_version table as comparable tuples.""" + return [ + (row["version"], row["applied_at"]) + for row in connection.execute("select version, applied_at from schema_version") + ] + + +def _add_migration(monkeypatch, version, migration): + """Append one migration to the registry for the duration of a test.""" + from curator.state import db + + monkeypatch.setattr( + db, "VERSIONED_MIGRATIONS", (*db.VERSIONED_MIGRATIONS, (version, migration)) + ) + + +def test_a_fresh_ledger_is_not_backed_up(tmp_path): + """Verify creating a ledger writes no backup — there is nothing to preserve.""" + connection = _open(tmp_path) + + outcome = initialize_database(connection) + + assert outcome is not None and outcome.applied == (1, 2) + assert outcome.backup is None + assert _backups(tmp_path) == [] + connection.close() + + +def test_an_up_to_date_ledger_reports_nothing_and_writes_no_backup(tmp_path): + """Verify the common case — every command opens the ledger — stays free of copies.""" + connection = _open(tmp_path) + initialize_database(connection) + + for _ in range(10): + assert initialize_database(connection) is None + + assert _backups(tmp_path) == [] + connection.close() + + +def test_a_pending_migration_backs_the_ledger_up_first(tmp_path, monkeypatch): + """Verify an existing ledger is copied aside before a new migration touches it.""" + connection = _open(tmp_path) + initialize_database(connection) + _add_migration( + monkeypatch, + 3, + lambda conn: conn.execute("alter table memory_entries add column probe text"), + ) + + outcome = initialize_database(connection) + + assert outcome is not None and outcome.applied == (3,) + assert outcome.from_version == 2 and outcome.to_version == 3 + assert outcome.backup is not None and outcome.backup.exists() + assert _backups(tmp_path) == [outcome.backup] + + # The backup is a real ledger, and it predates the migration. + restored = sqlite3.connect(outcome.backup) + columns = {row[1] for row in restored.execute("pragma table_info(memory_entries)")} + assert "probe" not in columns + restored.close() + connection.close() + + +def test_a_failed_migration_leaves_the_ledger_untouched(tmp_path, monkeypatch): + """Verify a migration that raises rolls back its DDL and its schema_version row.""" + connection = _open(tmp_path) + initialize_database(connection) + before_schema = _schema_rows(connection) + before_columns = {row["name"] for row in connection.execute("pragma table_info(memory_entries)")} + + def _doomed(conn): + """Change the schema, then fail — the change must not survive.""" + conn.execute("alter table memory_entries add column half_applied text") + raise RuntimeError("migration exploded") + + _add_migration(monkeypatch, 3, _doomed) + + with pytest.raises(CuratorStateError): + initialize_database(connection) + + assert _schema_rows(connection) == before_schema # version 3 was never recorded + after_columns = {row["name"] for row in connection.execute("pragma table_info(memory_entries)")} + assert after_columns == before_columns # the DDL rolled back with it + + backups = _backups(tmp_path) + assert len(backups) == 1 # and the pre-migration copy is still there to fall back on + sqlite3.connect(backups[0]).close() + connection.close() + + +def test_the_backup_captures_data_still_sitting_in_the_wal(tmp_path): + """Verify the online backup API is used, not a file copy. + + The ledger runs in WAL mode, so a committed row can live in the -wal file until a + checkpoint. Copying curator.sqlite alone would silently lose it. + """ + connection = _open(tmp_path) + initialize_database(connection) + connection.execute("create table wal_probe (value text)") + connection.execute("insert into wal_probe values ('kept')") + connection.commit() + + wal = build_curator_paths(tmp_path).database.with_name("curator.sqlite-wal") + assert wal.exists() and wal.stat().st_size > 0 # the row really is in the WAL + + backup = backup_ledger(connection, build_curator_paths(tmp_path).curator_dir) + + restored = sqlite3.connect(backup) + assert [row[0] for row in restored.execute("select value from wal_probe")] == ["kept"] + restored.close() + connection.close() + + +def test_pending_migrations_treats_a_ledger_without_schema_version_as_unmigrated(tmp_path): + """Verify a ledger predating the schema_version table is migratable, not an error.""" + connection = _open(tmp_path) + + assert pending_migrations(connection) == [1, 2] # no schema_version table exists yet + connection.close() + + +def test_doctor_reports_pending_migrations_without_applying_them(tmp_path, monkeypatch): + """Verify the dry-run really is dry — doctor must never migrate the ledger.""" + connection = _open(tmp_path) + initialize_database(connection) + before = _schema_rows(connection) + connection.close() + + _add_migration(monkeypatch, 3, lambda conn: conn.execute("select 1")) + + report = inspect_project_health(tmp_path) + inspect_project_health(tmp_path) + + assert report.checks["migrations"].status == "pending" + assert "3" in report.checks["migrations"].detail + + connection = _open(tmp_path) + assert _schema_rows(connection) == before + assert pending_migrations(connection) == [3] + connection.close() + + +def test_doctor_points_at_the_newest_backup_with_a_restore_command(tmp_path, monkeypatch): + """Verify recovery never requires remembering a timestamp.""" + connection = _open(tmp_path) + initialize_database(connection) + _add_migration(monkeypatch, 3, lambda conn: conn.execute("select 1")) + outcome = initialize_database(connection) + connection.close() + + check = inspect_project_health(tmp_path).checks["backup"] + + assert check.status == "ok" + assert str(outcome.backup) in check.detail + assert "cp " in check.detail and "curator.sqlite" in check.detail + + +def test_a_failed_backup_is_never_offered_as_a_restore_point(tmp_path, monkeypatch): + """Verify a backup that dies half way cannot become what doctor tells you to restore. + + sqlite3.connect creates the destination up front, so a failure mid-copy used to leave a + truncated file carrying the real name. Being newest, it shadowed every good backup — and + the restore command copies it over the live ledger, destroying it. + """ + from curator.state.db import CuratorConnection + + curator_dir = build_curator_paths(tmp_path).curator_dir + connection = _open(tmp_path) + initialize_database(connection) + good = backup_ledger(connection, curator_dir) + + def _explode(self, target, **kwargs): + """Fail the way a full disk does — after the destination file already exists.""" + raise sqlite3.OperationalError("disk I/O error") + + monkeypatch.setattr(CuratorConnection, "backup", _explode, raising=False) + with pytest.raises(sqlite3.OperationalError): + backup_ledger(connection, curator_dir) + monkeypatch.undo() + + assert _backups(tmp_path) == [good] # the failed attempt left nothing behind + assert latest_pre_migration_backup(curator_dir) == good + assert not list(ledger_archive_dir(curator_dir).glob("*.partial")) + connection.close() + + +def test_an_empty_backup_file_is_not_offered_as_a_restore_point(tmp_path): + """Verify a zero-byte file in the archive never becomes the advertised restore point.""" + curator_dir = build_curator_paths(tmp_path).curator_dir + connection = _open(tmp_path) + initialize_database(connection) + good = backup_ledger(connection, curator_dir) + + stray = ledger_archive_dir(curator_dir) / "curator-29990101T000000Z-pre-migration.sqlite" + stray.touch() # newer by name and mtime, but empty + + assert latest_pre_migration_backup(curator_dir) == good + connection.close() + + +def test_latest_backup_is_the_newest_one_not_the_last_alphabetically(tmp_path): + """Verify same-second backups do not make an older copy look like the newest.""" + curator_dir = build_curator_paths(tmp_path).curator_dir + connection = _open(tmp_path) + initialize_database(connection) + + first = backup_ledger(connection, curator_dir) + second = backup_ledger(connection, curator_dir) # same second -> indexed name + + assert first != second + assert latest_pre_migration_backup(curator_dir) == second + connection.close()