From 0a415cdc742c59e06a69b3bb5ca55e580abb0fc4 Mon Sep 17 00:00:00 2001 From: mohamed zwanski Date: Thu, 22 Oct 2026 21:12:33 +0100 Subject: [PATCH] Fix #79: Fall back from WAL when the filesystem can't support it SQLite accepted PRAGMA journal_mode=WAL on filesystems where the -shm shared-memory index can't be mmapped (WSL /mnt/c, Docker/VM shared folders, FUSE, eCryptfs), then failed the next query with 'disk I/O error', killing the orchestrator and MCP server. - Probe WAL on a throwaway database in the DB directory before using it - Detect more unsafe filesystems (9p, drvfs, virtiofs, fuse.*, ...) and match the most specific mount point instead of any string prefix - Convert a database left in WAL mode using EXCLUSIVE locking, which keeps the WAL index in heap memory, so it can be opened again - Add AUTOFORGE_SQLITE_JOURNAL_MODE override Co-Authored-By: Claude Opus 5.5 --- .env.example | 7 ++ api/database.py | 180 +++++++++++++++++++++++++++++----- test_database_journal_mode.py | 164 +++++++++++++++++++++++++++++++ 3 files changed, 328 insertions(+), 23 deletions(-) create mode 100644 test_database_journal_mode.py diff --git a/.env.example b/.env.example index ed163bacf..4eb974581 100644 --- a/.env.example +++ b/.env.example @@ -17,6 +17,13 @@ # Example: EXTRA_READ_PATHS=/Volumes/Data/dev,/Users/shared/libs # EXTRA_READ_PATHS= +# SQLite Journal Mode (Optional) +# AutoForge uses WAL mode for features.db and automatically falls back to DELETE mode +# on filesystems where WAL fails (network drives, WSL /mnt/c, Docker/VM shared folders). +# Set this to force a mode if you still see "sqlite3.OperationalError: disk I/O error". +# Allowed: WAL, DELETE, TRUNCATE, PERSIST +# AUTOFORGE_SQLITE_JOURNAL_MODE=DELETE + # Google Cloud Vertex AI Configuration (Optional) # To use Claude via Vertex AI on Google Cloud Platform, uncomment and set these variables. # Requires: gcloud CLI installed and authenticated (run: gcloud auth application-default login) diff --git a/api/database.py b/api/database.py index 523ea2243..267af8464 100644 --- a/api/database.py +++ b/api/database.py @@ -5,6 +5,8 @@ SQLite database schema for feature storage using SQLAlchemy. """ +import os +import sqlite3 import sys from datetime import datetime, timezone from pathlib import Path @@ -260,18 +262,59 @@ def _migrate_add_testing_columns(engine) -> None: pass +# Filesystems where WAL's shared-memory index (mmap of the -shm file) is known to +# be broken or unsafe. Includes network filesystems and host-shared mounts used by +# WSL (/mnt/c is 9p or drvfs) and container runtimes (virtiofs, grpcfuse, ...). +_WAL_UNSAFE_FS_TYPES = frozenset({ + "nfs", "nfs4", "cifs", "smb3", "smbfs", "afs", "ceph", "glusterfs", "lustre", + "9p", "drvfs", "virtiofs", "vboxsf", "vmhgfs", "prl_fs", "ecryptfs", +}) + +# Env var to force a journal mode (e.g. DELETE) and skip auto-detection +JOURNAL_MODE_ENV_VAR = "AUTOFORGE_SQLITE_JOURNAL_MODE" +_VALID_JOURNAL_MODES = ("WAL", "DELETE", "TRUNCATE", "PERSIST") + +# Cache of WAL probe results, keyed by database directory +_wal_support_cache: dict[str, bool] = {} + + +def _get_mount_fs_type(path_str: str) -> Optional[str]: + """Return the filesystem type of the most specific mount containing path_str.""" + try: + with open("/proc/mounts", "r") as f: + mounts = f.read() + except OSError: + return None + + best_mount = "" + best_type: Optional[str] = None + for line in mounts.splitlines(): + parts = line.split() + if len(parts) < 3: + continue + # /proc/mounts escapes spaces and other special characters as octal + mount_point = parts[1].replace("\\040", " ").replace("\\011", "\t").replace("\\134", "\\") + prefix = mount_point.rstrip("/") + "/" + if path_str == mount_point or path_str.startswith(prefix): + if len(mount_point) >= len(best_mount): + best_mount = mount_point + best_type = parts[2] + return best_type + + def _is_network_path(path: Path) -> bool: - """Detect if path is on a network filesystem. + """Detect if path is on a filesystem where WAL mode is unsafe. WAL mode doesn't work reliably on network filesystems (NFS, SMB, CIFS) - and can cause database corruption. This function detects common network - path patterns so we can fall back to DELETE mode. + or host-shared mounts (WSL /mnt/c, Docker/VM shared folders, FUSE) and + can cause "disk I/O error" failures or database corruption. This function + detects common patterns so we can fall back to DELETE mode. Args: path: The path to check Returns: - True if the path appears to be on a network filesystem + True if the path appears to be on such a filesystem """ path_str = str(path.resolve()) @@ -291,26 +334,114 @@ def _is_network_path(path: Path) -> bool: except (AttributeError, OSError): pass else: - # Unix: Check mount type via /proc/mounts or mount command - try: - with open("/proc/mounts", "r") as f: - mounts = f.read() - # Check each mount point to find which one contains our path - for line in mounts.splitlines(): - parts = line.split() - if len(parts) >= 3: - mount_point = parts[1] - fs_type = parts[2] - # Check if path is under this mount point and if it's a network FS - if path_str.startswith(mount_point): - if fs_type in ("nfs", "nfs4", "cifs", "smbfs", "fuse.sshfs"): - return True - except (FileNotFoundError, PermissionError): - pass + # Unix: check the type of the mount that contains the path + fs_type = _get_mount_fs_type(path_str) + if fs_type is not None: + # fuse.* covers sshfs, rclone, gcsfuse, Docker Desktop's grpcfuse, etc. + # (fuseblk, used by local ntfs-3g, is deliberately not matched) + if fs_type in _WAL_UNSAFE_FS_TYPES or fs_type.startswith("fuse."): + return True return False +def _probe_wal_support(db_dir: Path) -> bool: + """Check whether WAL mode actually works in db_dir. + + Some filesystems accept ``PRAGMA journal_mode=WAL`` but then fail on the + first read with ``disk I/O error`` because the -shm file cannot be + memory-mapped. Exercise WAL on a throwaway database to find out before + touching the real one. + """ + cache_key = db_dir.as_posix() + if cache_key in _wal_support_cache: + return _wal_support_cache[cache_key] + + probe = db_dir / f".wal_probe_{os.getpid()}.db" + supported = False + conns: list[sqlite3.Connection] = [] + try: + writer = sqlite3.connect(str(probe), timeout=5, isolation_level=None) + conns.append(writer) + mode = writer.execute("PRAGMA journal_mode=WAL").fetchone() + if mode and str(mode[0]).lower() == "wal": + writer.execute("CREATE TABLE IF NOT EXISTS probe (x INTEGER)") + writer.execute("INSERT INTO probe VALUES (1)") + # A second connection forces use of the shared-memory index + reader = sqlite3.connect(str(probe), timeout=5, isolation_level=None) + conns.append(reader) + supported = reader.execute("SELECT COUNT(*) FROM probe").fetchone()[0] == 1 + except sqlite3.Error: + supported = False + finally: + for c in conns: + try: + c.close() + except sqlite3.Error: + pass + for suffix in ("", "-wal", "-shm", "-journal"): + try: + Path(str(probe) + suffix).unlink(missing_ok=True) + except OSError: + pass + + _wal_support_cache[cache_key] = supported + return supported + + +def _is_wal_database(db_path: Path) -> bool: + """Return True if the database file header says it is in WAL mode.""" + try: + with open(db_path, "rb") as f: + header = f.read(20) + except OSError: + return False + # Bytes 18/19 are the file format write/read versions: 2 means WAL + return len(header) == 20 and header.startswith(b"SQLite format 3\x00") and header[18] == 2 + + +def _convert_wal_to_rollback(db_path: Path, journal_mode: str) -> None: + """Switch an existing WAL database to a rollback journal mode. + + A database left in WAL mode on a filesystem that can't mmap the -shm file + can't even be opened normally. With EXCLUSIVE locking SQLite keeps the WAL + index in heap memory instead, so it can checkpoint and leave WAL mode. + """ + try: + conn = sqlite3.connect(str(db_path), timeout=30, isolation_level=None) + try: + conn.execute("PRAGMA locking_mode=EXCLUSIVE") + conn.execute(f"PRAGMA journal_mode={journal_mode}") + finally: + conn.close() + except sqlite3.Error as e: + print(f"[database] Could not switch {db_path} out of WAL mode: {e}", file=sys.stderr) + + +def _choose_journal_mode(project_dir: Path, db_path: Path) -> str: + """Decide which SQLite journal mode to use for a project database.""" + override = os.environ.get(JOURNAL_MODE_ENV_VAR, "").strip().upper() + if override: + if override in _VALID_JOURNAL_MODES: + return override + print( + f"[database] Ignoring invalid {JOURNAL_MODE_ENV_VAR}={override!r} " + f"(expected one of {', '.join(_VALID_JOURNAL_MODES)})", + file=sys.stderr, + ) + + if _is_network_path(project_dir): + return "DELETE" + if not _probe_wal_support(db_path.parent): + print( + f"[database] WAL mode is not supported on the filesystem at {db_path.parent}; " + "using DELETE journal mode instead", + file=sys.stderr, + ) + return "DELETE" + return "WAL" + + def _migrate_add_human_input_columns(engine) -> None: """Add human input columns to existing databases that don't have them.""" with engine.connect() as conn: @@ -414,9 +545,12 @@ def create_database(project_dir: Path) -> tuple: db_path.parent.mkdir(parents=True, exist_ok=True) # Choose journal mode based on filesystem type - # WAL mode doesn't work reliably on network filesystems and can cause corruption - is_network = _is_network_path(project_dir) - journal_mode = "DELETE" if is_network else "WAL" + # WAL mode doesn't work reliably on network/shared filesystems and can fail with + # "disk I/O error" or cause corruption, so verify it works before using it + journal_mode = _choose_journal_mode(project_dir, db_path) + if journal_mode != "WAL" and _is_wal_database(db_path): + # A previous run may have left the database in WAL mode + _convert_wal_to_rollback(db_path, journal_mode) engine = create_engine(db_url, connect_args={ "check_same_thread": False, diff --git a/test_database_journal_mode.py b/test_database_journal_mode.py new file mode 100644 index 000000000..2a519a288 --- /dev/null +++ b/test_database_journal_mode.py @@ -0,0 +1,164 @@ +""" +Tests for SQLite journal mode selection in api/database.py. + +Covers the fallback from WAL to DELETE mode on filesystems where WAL's +shared-memory index doesn't work (issue #79: "disk I/O error" on +PRAGMA table_info when starting the orchestrator). +""" + +import sqlite3 +from pathlib import Path +from unittest import mock + +import pytest + +from api import database +from api.database import ( + JOURNAL_MODE_ENV_VAR, + Feature, + _get_mount_fs_type, + _is_network_path, + _is_wal_database, + _probe_wal_support, + create_database, + dispose_engine, +) + +FAKE_MOUNTS = """\ +/dev/sda1 / ext4 rw,relatime 0 0 +C:\\134 /mnt/c 9p rw,noatime 0 0 +server:/export /mnt/nfs nfs4 rw 0 0 +/dev/sdb1 /mnt/c/local\\040disk ext4 rw 0 0 +sshfs#host: /home/me/remote fuse.sshfs rw 0 0 +/dev/sdc1 /media/usb fuseblk rw 0 0 +""" + + +@pytest.fixture(autouse=True) +def _clean_state(monkeypatch): + monkeypatch.delenv(JOURNAL_MODE_ENV_VAR, raising=False) + database._wal_support_cache.clear() + yield + database._wal_support_cache.clear() + + +def _journal_mode(engine) -> str: + with engine.connect() as conn: + raw = conn.connection.dbapi_connection + assert raw is not None + return str(raw.execute("PRAGMA journal_mode").fetchone()[0]).lower() + + +def _patch_mounts(content: str): + return mock.patch("builtins.open", mock.mock_open(read_data=content)) + + +class TestMountDetection: + def test_longest_mount_point_wins(self): + with _patch_mounts(FAKE_MOUNTS): + assert _get_mount_fs_type("/mnt/c/Users/me/project") == "9p" + assert _get_mount_fs_type("/mnt/c/local disk/project") == "ext4" + assert _get_mount_fs_type("/home/me/project") == "ext4" + + def test_prefix_must_match_whole_component(self): + with _patch_mounts(FAKE_MOUNTS): + # /mnt/nfs-local is not under /mnt/nfs + assert _get_mount_fs_type("/mnt/nfs-local/project") == "ext4" + + @pytest.mark.parametrize( + "path,expected", + [ + ("/mnt/c/Users/me/project", True), # WSL Windows drive (9p) + ("/mnt/nfs/project", True), + ("/home/me/remote/project", True), # fuse.sshfs + ("/media/usb/project", False), # fuseblk (local ntfs-3g) + ("/home/me/project", False), + ("/mnt/c/local disk/project", False), + ], + ) + def test_is_network_path(self, path, expected): + if database.sys.platform == "win32": + pytest.skip("Unix mount detection") + with mock.patch.object(Path, "resolve", lambda self: self), _patch_mounts(FAKE_MOUNTS): + assert _is_network_path(Path(path)) is expected + + +class TestWalProbe: + def test_probe_succeeds_on_local_disk_and_cleans_up(self, tmp_path): + assert _probe_wal_support(tmp_path) is True + assert list(tmp_path.iterdir()) == [] + + def test_probe_fails_when_sqlite_errors(self, tmp_path): + real_connect = sqlite3.connect + calls = {"n": 0} + + def flaky_connect(*args, **kwargs): + calls["n"] += 1 + if calls["n"] == 2: + raise sqlite3.OperationalError("disk I/O error") + return real_connect(*args, **kwargs) + + with mock.patch.object(database.sqlite3, "connect", flaky_connect): + assert _probe_wal_support(tmp_path) is False + assert list(tmp_path.iterdir()) == [] + + def test_probe_result_is_cached(self, tmp_path): + assert _probe_wal_support(tmp_path) is True + with mock.patch.object(database.sqlite3, "connect", side_effect=AssertionError("not cached")): + assert _probe_wal_support(tmp_path) is True + + +class TestCreateDatabaseJournalMode: + def test_uses_wal_on_local_disk(self, tmp_path): + engine, _ = create_database(tmp_path) + try: + assert _journal_mode(engine) == "wal" + finally: + dispose_engine(tmp_path) + + def test_falls_back_to_delete_when_wal_unsupported(self, tmp_path): + with mock.patch.object(database, "_probe_wal_support", return_value=False): + engine, _ = create_database(tmp_path) + try: + assert _journal_mode(engine) == "delete" + finally: + dispose_engine(tmp_path) + + def test_env_override(self, tmp_path, monkeypatch): + monkeypatch.setenv(JOURNAL_MODE_ENV_VAR, "delete") + with mock.patch.object(database, "_probe_wal_support", side_effect=AssertionError("probe skipped")): + engine, _ = create_database(tmp_path) + try: + assert _journal_mode(engine) == "delete" + finally: + dispose_engine(tmp_path) + + def test_invalid_env_override_is_ignored(self, tmp_path, monkeypatch): + monkeypatch.setenv(JOURNAL_MODE_ENV_VAR, "bogus; DROP TABLE features") + engine, _ = create_database(tmp_path) + try: + assert _journal_mode(engine) == "wal" + finally: + dispose_engine(tmp_path) + + def test_existing_wal_database_is_converted_and_keeps_data(self, tmp_path): + # First run on a filesystem where WAL "works" leaves the DB in WAL mode + _, session_maker = create_database(tmp_path) + with session_maker() as session: + session.add(Feature(priority=1, category="c", name="kept", description="d", steps=[])) + session.commit() + dispose_engine(tmp_path) + + db_path = database.get_database_path(tmp_path) + assert _is_wal_database(db_path) + + # Next run detects that WAL is unusable and must switch the file over + with mock.patch.object(database, "_probe_wal_support", return_value=False): + engine, session_maker = create_database(tmp_path) + try: + assert _journal_mode(engine) == "delete" + assert not _is_wal_database(db_path) + with session_maker() as session: + assert [f.name for f in session.query(Feature).all()] == ["kept"] + finally: + dispose_engine(tmp_path)