From 267b8447b4e08129999c5214b0db941b8cf47778 Mon Sep 17 00:00:00 2001 From: Caspian Zhao Date: Fri, 14 Aug 2026 09:13:31 -0700 Subject: [PATCH] fix(memory): never bind-mount the live knowledge DB WAL trio into forum containers Forum-phase containers used to bind-mount the live knowledge DB plus its -wal/-shm sidecars read-write, and the in-container MCP server opened them with read_only=True. On macOS (Docker Desktop) that combination corrupts the authoritative host store: - In WAL mode, even a read-only SQLite connection WRITES to the -shm wal-index: readers update read-marks and, when their view of the header looks invalid, grab the recovery locks and REBUILD the wal-index. - A VirtioFS bind mount shares neither POSIX advisory locks nor a coherent shared mmap between the macOS host and the Linux VM. Host fcntl locks are invisible in the container and vice versa, and each side has its own page cache of the -shm file, lazily synced underneath the other's live mapping. - A container "reader" therefore silently clobbers the host writer's wal-index mid-transaction. Subsequent host WAL appends/checkpoint backfills use poisoned frame->page mappings and copy page images to wrong offsets in the main DB file: "sqlite3.DatabaseError: database disk image is malformed" (autopsies show cross-page transplants, e.g. an index leaf at the `generations` table root and a copy of page 1 at page 13), or a host SIGBUS inside walIndexAppend/walIndexReadHdr when the mapping is invalidated outright ("FS pagein error"). The failure probability scales with the number of concurrent forum containers and WAL churn, which is why 15-workstream campaigns died at the gen-1-to-2 boundary (first forum-heavy phase transition) while 3-task runs usually survived, and why strategies without forum phases (whose containers never mount the DB files) never corrupted. Reproduced synthetically with zero LLM calls: a host KnowledgeStore under forum-drain-shaped load dies within seconds alongside 15 (or even 1) containers that merely OPEN the bind-mounted trio read-only - including with a plain sqlite3 mode=ro connection and no ksi code in the container. The same load with no containers, at the same rate and duration, is clean. Fix - the live WAL trio never crosses the container boundary: - _store_common.snapshot_db_for_container_mount() produces a standalone point-in-time copy via VACUUM INTO on its own read-only connection (consistent WAL read snapshot, safe next to the writer thread; output is a single rollback-journal file with no sidecars). - container_host materializes that snapshot for per_task_forum / cross_task_forum payloads (same lifecycle as the per-task memory snapshot: created in the knowledge-DB dir, unlinked after the run) and stamps payload.knowledge.db_is_snapshot. On snapshot failure the container runs with NO knowledge DB (visible degradation) - it never falls back to mounting the live files. - container_mounts.ts mounts the snapshot as a single read-only file for forum containers and stops mounting the live knowledge/runtime-audit DB files there entirely. Forum WRITES are unaffected: they already flow through the forum_bus JSONL mount and are drained into SQLite host-side by the single writer. - mcp_server opens a snapshot-backed knowledge store with the new KnowledgeStore(read_only=True, immutable=True) (KNOWLEDGE_DB_IMMUTABLE=1 from the runner): immutable=1 is truthful there and makes SQLite skip locking and wal-index shared memory entirely. Freshness is unchanged in practice: the knowledge DB only gains rows at drain/phase boundaries (before the next container launch and snapshot), and mid-round forum traffic travels via the ForumBus JSONL, not the DB. Validation: tests/memory/test_forum_container_db_snapshot.py and tests/runtime/test_container_host.py::TestForumKnowledgeDbSnapshot fail on the previous code and pass now; the synthetic 15-container repro is clean for the full duration in the fixed topology (read-only immutable snapshot mount) while the live-trio topology still dies in seconds; a previously always-fatal 15-task ARC-1 forum campaign (3 generations) completes with zero "malformed" errors and a clean PRAGMA integrity_check. Claude-Session: https://claude.ai/code/session_01KNTYDZGTNzY3Yrg2wB7ixj --- .../src/anthropic_direct_forum.ts | 7 +- .../agent-runner/src/memory_mcp_env.ts | 7 +- .../agent-runner/src/query_config.ts | 9 +- .../agent-runner/src/shared_types.ts | 12 ++ runtime_runner/src/container_mounts.ts | 46 +++-- runtime_runner/src/main.ts | 3 + runtime_runner/src/shared_types.ts | 12 ++ runtime_runner/src/types.ts | 6 + src/ksi/memory/_store_common.py | 40 ++++ src/ksi/memory/knowledge_store.py | 13 ++ src/ksi/memory/mcp_server.py | 12 +- src/ksi/runtime/container_host.py | 82 ++++++++- .../test_forum_container_db_snapshot.py | 141 ++++++++++++++ tests/runtime/test_container_host.py | 174 +++++++++++++++++- 14 files changed, 536 insertions(+), 28 deletions(-) create mode 100644 tests/memory/test_forum_container_db_snapshot.py diff --git a/runtime_runner/agent-runner/src/anthropic_direct_forum.ts b/runtime_runner/agent-runner/src/anthropic_direct_forum.ts index 9a50db4..4da3c03 100644 --- a/runtime_runner/agent-runner/src/anthropic_direct_forum.ts +++ b/runtime_runner/agent-runner/src/anthropic_direct_forum.ts @@ -251,7 +251,12 @@ export function buildMemoryMcpEnv( const env: Record = { PATH: process.env.PATH || '/usr/local/bin:/usr/bin:/bin', HOME: process.env.HOME || '/home/node', - KNOWLEDGE_DB_PATH: `/app/memory-db/${dbFile}`, + // dbFile may be '' when a forum snapshot failed to build host-side; the + // MCP server then runs without a knowledge store (degraded, never live). + KNOWLEDGE_DB_PATH: dbFile ? `/app/memory-db/${dbFile}` : '', + // Snapshot DBs never change while mounted: immutable=1 lets SQLite skip + // all locking/wal-index access (required for safety on macOS bind mounts). + KNOWLEDGE_DB_IMMUTABLE: containerInput.memoryMcp.dbIsSnapshot ? '1' : '', MEMORY_SNAPSHOT_PATH: snapshotFile ? `/app/memory-db/${snapshotFile}` : '', MCP_TOOLSET: 'forum', FORUM_GENERATION: String(containerInput.memoryMcp.forumGeneration ?? 0), diff --git a/runtime_runner/agent-runner/src/memory_mcp_env.ts b/runtime_runner/agent-runner/src/memory_mcp_env.ts index a8d2850..8b7f308 100644 --- a/runtime_runner/agent-runner/src/memory_mcp_env.ts +++ b/runtime_runner/agent-runner/src/memory_mcp_env.ts @@ -17,7 +17,12 @@ export function buildOpenAIMemoryMcpEnv( : ''; const memoryToolset = isOpenAIForumPhase(taskSource) ? 'forum' : 'task'; return { - KNOWLEDGE_DB_PATH: `/app/memory-db/${dbFile}`, + // dbFile may be '' when a forum snapshot failed to build host-side; the + // MCP server then runs without a knowledge store (degraded, never live). + KNOWLEDGE_DB_PATH: dbFile ? `/app/memory-db/${dbFile}` : '', + // Snapshot DBs never change while mounted: immutable=1 lets SQLite skip + // all locking/wal-index access (required for safety on macOS bind mounts). + KNOWLEDGE_DB_IMMUTABLE: containerInput.memoryMcp.dbIsSnapshot ? '1' : '', MEMORY_SNAPSHOT_PATH: snapshotFile ? `/app/memory-db/${snapshotFile}` : '', MCP_TOOLSET: memoryToolset, FORUM_GENERATION: String(containerInput.memoryMcp.forumGeneration ?? 0), diff --git a/runtime_runner/agent-runner/src/query_config.ts b/runtime_runner/agent-runner/src/query_config.ts index 8fdd1b2..0945322 100644 --- a/runtime_runner/agent-runner/src/query_config.ts +++ b/runtime_runner/agent-runner/src/query_config.ts @@ -245,7 +245,14 @@ export function buildMcpServerConfig( command: 'python3', args: ['/app/memory/mcp_server.py'], env: { - KNOWLEDGE_DB_PATH: `/app/memory-db/${dbFile}`, + // dbFile may be '' when a forum snapshot failed to build host-side; + // the MCP server then runs without a knowledge store (degraded, never + // live). + KNOWLEDGE_DB_PATH: dbFile ? `/app/memory-db/${dbFile}` : '', + // Snapshot DBs never change while mounted: immutable=1 lets SQLite + // skip all locking/wal-index access (required for safety on macOS + // bind mounts). + KNOWLEDGE_DB_IMMUTABLE: containerInput.memoryMcp.dbIsSnapshot ? '1' : '', MEMORY_SNAPSHOT_PATH: snapshotFile ? `/app/memory-db/${snapshotFile}` : '', MCP_TOOLSET: memoryToolset, FORUM_GENERATION: String(containerInput.memoryMcp.forumGeneration ?? 0), diff --git a/runtime_runner/agent-runner/src/shared_types.ts b/runtime_runner/agent-runner/src/shared_types.ts index 26596fb..952e2c5 100644 --- a/runtime_runner/agent-runner/src/shared_types.ts +++ b/runtime_runner/agent-runner/src/shared_types.ts @@ -23,6 +23,18 @@ export interface MemoryMcpConfig { * directory mount (issue #1009). */ runtimeDbPath?: string; + /** + * True when dbPath points at a standalone point-in-time snapshot of the + * knowledge DB rather than the live file. Set by the Python container host + * for forum task sources: the live WAL trio (db + -wal + -shm) must never + * be bind-mounted into a container, because Docker Desktop's VirtioFS gives + * the container neither the host's POSIX advisory locks nor a coherent view + * of the -shm wal-index — even a read-only in-container SQLite connection + * writes to the wal-index and corrupts the host writer's database. When + * true, container_mounts.ts mounts dbPath as a single read-only file (no + * sidecars, no runtime-audit DB) and the MCP server opens it immutable. + */ + dbIsSnapshot?: boolean; taskId?: string; taskSource?: string; forumGeneration?: number; diff --git a/runtime_runner/src/container_mounts.ts b/runtime_runner/src/container_mounts.ts index ede96ca..1417ff3 100644 --- a/runtime_runner/src/container_mounts.ts +++ b/runtime_runner/src/container_mounts.ts @@ -373,18 +373,40 @@ export function appendMemoryAndArcMounts( // sidecars) rather than the whole per-experiment directory, so an agent // can't enumerate sibling files that land in the same subdirectory. if (forumWritesNeeded) { - // This branch only runs for forum containers, which need the DB - // mounted read-write (ForumBus.append / forum_signal_done INSERT), - // so the mounts are unconditionally read-write here. - addSqliteFileMounts(mounts, input.memoryMcp.dbPath, false, workspaceRuntime, 'Knowledge DB'); - if (input.memoryMcp.runtimeDbPath) { - addSqliteFileMounts( - mounts, - input.memoryMcp.runtimeDbPath, - false, - workspaceRuntime, - 'Runtime-audit DB', - ); + if (input.memoryMcp.dbIsSnapshot) { + // dbPath is a standalone point-in-time snapshot (rollback-journal + // mode, no sidecars) cut by the Python container host. Mount it as + // a single read-only file and mount NOTHING live: bind-mounting a + // live WAL trio into the Docker VM corrupts the host DB on macOS + // (VirtioFS shares neither POSIX locks nor a coherent -shm mmap, + // and WAL readers write to the wal-index) — see + // MemoryMcpConfig.dbIsSnapshot. Forum WRITES don't need the DB: + // they flow through the forum_bus JSONL mount below and are + // drained into SQLite host-side. An empty/missing dbPath means + // snapshot creation failed — run without a knowledge DB rather + // than ever falling back to the live files. + if (input.memoryMcp.dbPath && fs.existsSync(input.memoryMcp.dbPath)) { + addSqliteFileMounts(mounts, input.memoryMcp.dbPath, true, workspaceRuntime, 'Knowledge DB snapshot'); + } else { + logger.warn( + { group: workspaceRuntime.name, dbPath: input.memoryMcp.dbPath }, + 'Knowledge DB snapshot missing — forum container runs without a knowledge DB', + ); + } + } else { + // Legacy path (payloads without db_is_snapshot): live DB mounted + // read-write (ForumBus.append / forum_signal_done INSERT), + // so the mounts are unconditionally read-write here. + addSqliteFileMounts(mounts, input.memoryMcp.dbPath, false, workspaceRuntime, 'Knowledge DB'); + if (input.memoryMcp.runtimeDbPath) { + addSqliteFileMounts( + mounts, + input.memoryMcp.runtimeDbPath, + false, + workspaceRuntime, + 'Runtime-audit DB', + ); + } } } if (input.memoryMcp.snapshotPath) { diff --git a/runtime_runner/src/main.ts b/runtime_runner/src/main.ts index e3672f0..d2dabd2 100644 --- a/runtime_runner/src/main.ts +++ b/runtime_runner/src/main.ts @@ -763,6 +763,9 @@ async function main(): Promise { // bind-mount the runtime-audit DB individually; RUNTIME_DB_PATH // (set below) already assumes it, so both must stay in sync. runtimeDbPath: payload.runtime_audit?.db_path || undefined, + // Forum sources: dbPath is a standalone snapshot, and the live WAL + // trio must never be mounted (see MemoryMcpConfig.dbIsSnapshot). + dbIsSnapshot: Boolean(payload.knowledge.db_is_snapshot), taskId: payload.task?.id || '', taskSource, forumGeneration: coerceOptionalNumber(taskMeta.forum_generation), diff --git a/runtime_runner/src/shared_types.ts b/runtime_runner/src/shared_types.ts index 26596fb..952e2c5 100644 --- a/runtime_runner/src/shared_types.ts +++ b/runtime_runner/src/shared_types.ts @@ -23,6 +23,18 @@ export interface MemoryMcpConfig { * directory mount (issue #1009). */ runtimeDbPath?: string; + /** + * True when dbPath points at a standalone point-in-time snapshot of the + * knowledge DB rather than the live file. Set by the Python container host + * for forum task sources: the live WAL trio (db + -wal + -shm) must never + * be bind-mounted into a container, because Docker Desktop's VirtioFS gives + * the container neither the host's POSIX advisory locks nor a coherent view + * of the -shm wal-index — even a read-only in-container SQLite connection + * writes to the wal-index and corrupts the host writer's database. When + * true, container_mounts.ts mounts dbPath as a single read-only file (no + * sidecars, no runtime-audit DB) and the MCP server opens it immutable. + */ + dbIsSnapshot?: boolean; taskId?: string; taskSource?: string; forumGeneration?: number; diff --git a/runtime_runner/src/types.ts b/runtime_runner/src/types.ts index a5a4e58..30c5208 100644 --- a/runtime_runner/src/types.ts +++ b/runtime_runner/src/types.ts @@ -42,6 +42,12 @@ export interface KsiPayload { disable_memory_tools?: boolean; forum_generation?: number; experiment_name?: string; + /** + * True when db_path is a standalone snapshot cut for a forum container + * (may be "" when snapshot creation failed). The live WAL trio must not + * be mounted — see MemoryMcpConfig.dbIsSnapshot in shared_types.ts. + */ + db_is_snapshot?: boolean; }; runtime_audit?: { db_path: string; diff --git a/src/ksi/memory/_store_common.py b/src/ksi/memory/_store_common.py index 6feb40d..6d728aa 100644 --- a/src/ksi/memory/_store_common.py +++ b/src/ksi/memory/_store_common.py @@ -217,6 +217,46 @@ def _locked_guard( process_lock.release() +def snapshot_db_for_container_mount(src_path: str | Path, dest_path: str | Path) -> None: + """Produce a standalone, self-contained copy of ``src_path`` at ``dest_path``. + + Used by the container runtime to build the read-only knowledge DB that + forum containers mount INSTEAD of the live DB. The live WAL trio + (``db`` + ``-wal`` + ``-shm``) must never cross the Docker VM boundary: on + macOS the bind-mount filesystem (VirtioFS) provides neither cross-boundary + POSIX advisory locks nor coherent shared mmaps, so even a ``mode=ro`` + SQLite connection inside a container silently fights the host writer over + the ``-shm`` wal-index (readers update read-marks and may rebuild the + wal-index) and corrupts the database — see + tests/memory/test_forum_container_db_snapshot.py. + + ``VACUUM INTO`` runs on its own read-only connection: it takes a consistent + WAL read snapshot of the source (safe alongside the store's writer thread, + same-host POSIX locking applies) and writes only to ``dest_path``. The + output is a compact single file in rollback-journal mode — no ``-wal`` / + ``-shm`` sidecars — so a container can open it read-only (even + ``immutable=1``) without ever touching shared WAL state. + + Raises on failure (busy beyond the timeout, disk full); callers decide the + degrade path — they must NOT fall back to mounting the live DB. + """ + src = Path(src_path).resolve() + dest = Path(dest_path) + # VACUUM INTO refuses to overwrite; a stale file at dest (e.g. crashed + # prior run with the same name) must go first. + dest.unlink(missing_ok=True) + conn = sqlite3.connect(f"file:{src}?mode=ro", uri=True, timeout=30.0) + try: + conn.execute("PRAGMA busy_timeout=30000") + conn.execute("VACUUM INTO ?", (str(dest),)) + except BaseException: + # Never leave a half-written snapshot behind for a container to mount. + conn.close() + dest.unlink(missing_ok=True) + raise + conn.close() + + def _wal_checkpoint( *, read_only: bool, diff --git a/src/ksi/memory/knowledge_store.py b/src/ksi/memory/knowledge_store.py index ab52281..4740a50 100644 --- a/src/ksi/memory/knowledge_store.py +++ b/src/ksi/memory/knowledge_store.py @@ -323,6 +323,7 @@ def __init__( db_path: str, *, read_only: bool = False, + immutable: bool = False, default_experiment: str = "default", enable_vec: bool = False, vec_dimensions: int = 768, @@ -333,12 +334,24 @@ def __init__( self._vec_enabled = False self._vec_dimensions = vec_dimensions + # ``immutable`` promises SQLite the file cannot change while open, so + # it skips ALL locking and shared-memory (wal-index) access. That is + # only true for the standalone snapshot files the container runtime + # mounts for forum containers (see + # _store_common.snapshot_db_for_container_mount) — never for a live DB, + # where it would serve stale/torn reads. Requires read_only. + if immutable and not read_only: + raise ValueError("immutable=True requires read_only=True") + self._immutable = bool(immutable) + Path(db_path).parent.mkdir(parents=True, exist_ok=True) KnowledgeStore._cleanup_stale_locks(Path(db_path).parent) self._db_key = str(Path(db_path).resolve()) if self._read_only: uri = f"file:{Path(db_path).resolve()}?mode=ro" + if self._immutable: + uri += "&immutable=1" self._conn = sqlite3.connect(uri, uri=True, check_same_thread=False, timeout=30.0) else: self._conn = sqlite3.connect(db_path, check_same_thread=False, timeout=30.0) diff --git a/src/ksi/memory/mcp_server.py b/src/ksi/memory/mcp_server.py index 3ed5abb..a480051 100644 --- a/src/ksi/memory/mcp_server.py +++ b/src/ksi/memory/mcp_server.py @@ -1327,13 +1327,23 @@ def main() -> None: except Exception: pass - # Initialize KnowledgeStore (authoritative swarm memory/state access) + # Initialize KnowledgeStore (authoritative swarm memory/state access). + # KNOWLEDGE_DB_IMMUTABLE=1 is set by the container runner when the mounted + # DB is a standalone point-in-time snapshot (forum containers — the live + # WAL trio never crosses the container boundary; see + # _store_common.snapshot_db_for_container_mount). immutable=1 makes SQLite + # skip locking and wal-index shared memory entirely, which is both correct + # (the snapshot never changes while mounted) and required for safety on + # macOS bind mounts, where even read-only WAL access writes to the shared + # -shm mapping. + knowledge_db_immutable = _env_flag("KNOWLEDGE_DB_IMMUTABLE", False) knowledge_store: KnowledgeStore | None = None if knowledge_db_path and Path(knowledge_db_path).exists(): try: knowledge_store = KnowledgeStore( knowledge_db_path, read_only=True, + immutable=knowledge_db_immutable, default_experiment=memory_experiment or "__mcp__", enable_vec=enable_semantic, ) diff --git a/src/ksi/runtime/container_host.py b/src/ksi/runtime/container_host.py index 3b6d745..6c08076 100644 --- a/src/ksi/runtime/container_host.py +++ b/src/ksi/runtime/container_host.py @@ -17,6 +17,7 @@ from ..benchmarks.polyglot_harness import DEFAULT_POLYGLOT_TIMEOUT_SEC from ..errors import AuthenticationFailure +from ..memory._store_common import snapshot_db_for_container_mount from ..models import TaskSpec from ..prompts import build_execution_prompt, build_task_markdown from ..tasks.registry import resolve_source @@ -1608,20 +1609,46 @@ def _materialize_payload_side_files( seed_package: Any, swebench_container_images: dict[str, str], experiment_name: str, - ) -> tuple[Path | None, bool]: + ) -> tuple[Path | None, bool, Path | None]: """Materialize knowledge / runtime-audit side files into *td* and point *payload* at them. - Returns ``(snapshot_path, snapshot_failed)``: the memory-snapshot path - (so the caller can unlink it after the run) or ``None``, and a flag that - is ``True`` when a memory snapshot WAS available but its ``write_text`` - failed — so the container starts cold (memory-less) and the caller can - stamp ``runtime_meta`` for later analysis. Mutates *payload* - in place: it may set ``payload["knowledge"]`` and - ``payload["runtime_audit"]``. + Returns ``(snapshot_path, snapshot_failed, db_snapshot_path)``: the + memory-snapshot path (so the caller can unlink it after the run) or + ``None``; a flag that is ``True`` when a memory snapshot WAS available + but its ``write_text`` failed — so the container starts cold + (memory-less) and the caller can stamp ``runtime_meta`` for later + analysis; and, for forum task sources only, the standalone knowledge-DB + snapshot file the container mounts instead of the live DB (the caller + unlinks it after the run). Mutates *payload* in place: it may set + ``payload["knowledge"]`` and ``payload["runtime_audit"]``. + + Forum sources and the live DB: forum containers historically + bind-mounted the live knowledge DB (+ ``-wal``/``-shm``) read-write and + opened it ``read_only=True`` from the in-container MCP server. That is + NOT safe on macOS: Docker Desktop bind mounts (VirtioFS) provide + neither cross-VM POSIX advisory locks nor a coherent shared mmap of the + ``-shm`` wal-index, and in WAL mode even a read-only SQLite connection + writes to the wal-index (read-marks, recovery). A container reader + therefore silently clobbers the host writer's live wal-index, which + misdirects WAL appends/checkpoint backfills into wrong page offsets — + "database disk image is malformed" on the authoritative store (or a + host SIGBUS mid-commit). So for ``per_task_forum`` / + ``cross_task_forum`` the payload's ``knowledge.db_path`` now points at + a point-in-time ``VACUUM INTO`` snapshot (single file, rollback-journal + mode, no sidecars) and carries ``db_is_snapshot=True`` so the runner + mounts it read-only and never mounts the live WAL trio. Freshness is + unchanged in practice: forum-round posts travel via the ForumBus JSONL + (not the DB), and the DB itself only gains rows at drain/phase + boundaries — i.e. before the next container launch and snapshot. If + the snapshot cannot be built the container runs with NO knowledge DB + (``db_path`` keeps the absent snapshot path so nothing is mounted while + ForumBus still resolves ``/forum_bus``; the degradation is + logged) — it must never fall back to mounting the live DB. """ snapshot_path: Path | None = None snapshot_failed = False + db_snapshot_path: Path | None = None if self.knowledge_db_path: knowledge_db = Path(self.knowledge_db_path) if not knowledge_db.is_absolute(): @@ -1663,6 +1690,36 @@ def _materialize_payload_side_files( "forum_generation": generation, "experiment_name": experiment_name, } + if source in _FORUM_TASK_SOURCES: + # Forum containers get a standalone read-only snapshot, never + # the live WAL trio — see the docstring above for the + # macOS/VirtioFS corruption mechanism this closes. + db_snapshot_path = knowledge_db.parent / ( + f"knowledge_ro_snapshot_{agent_id}_{uuid.uuid4().hex[:8]}.sqlite" + ) + try: + snapshot_db_for_container_mount(knowledge_db, db_snapshot_path) + payload["knowledge"]["db_path"] = str(db_snapshot_path) + except Exception: + log.warning( + "Failed to snapshot knowledge DB for forum container " + "(task=%s agent=%s); container runs without a knowledge DB " + "— NOT falling back to mounting the live DB", + task.id, + agent_id, + exc_info=True, + ) + # Keep db_path pointing at the (now nonexistent) snapshot: + # the runner then mounts nothing for it, and the MCP server + # finds no knowledge store — but ForumBus still derives its + # JSONL dir from db_path's parent, so forum posts survive + # the degraded launch. The helper never leaves a partial + # file behind, so the path is guaranteed absent. + payload["knowledge"]["db_path"] = str(db_snapshot_path) + db_snapshot_path = None + # Always stamped for forum sources: tells the runner the live + # WAL trio must not be mounted, even when the snapshot failed. + payload["knowledge"]["db_is_snapshot"] = True if snapshot_written and snapshot_path is not None: payload["knowledge"]["snapshot_path"] = str(snapshot_path) if self.runtime_db_path: @@ -1672,7 +1729,7 @@ def _materialize_payload_side_files( runtime_db.parent.mkdir(parents=True, exist_ok=True) runtime_db.touch(exist_ok=True) payload["runtime_audit"] = {"db_path": str(runtime_db)} - return snapshot_path, snapshot_failed + return snapshot_path, snapshot_failed, db_snapshot_path def _build_task_runner_env( self, @@ -2293,7 +2350,7 @@ def run_task( with tempfile.TemporaryDirectory(prefix="ksi-task-") as td, contextlib.ExitStack() as snapshot_guard: payload_path = Path(td) / "payload.json" - snapshot_path, knowledge_snapshot_failed = self._materialize_payload_side_files( + snapshot_path, knowledge_snapshot_failed, forum_db_snapshot_path = self._materialize_payload_side_files( payload=ctx.payload, td=td, task=task, @@ -2314,6 +2371,11 @@ def run_task( # cleans it up. The runner ``finally`` below unlinks # promptly on the normal path; this is the idempotent safety net. snapshot_guard.callback(_unlink_snapshot, snapshot_path) + if forum_db_snapshot_path is not None: + # Same lifecycle as the memory snapshot above: the forum + # knowledge-DB snapshot lives next to the live DB and must not + # outlive the container run it was cut for. + snapshot_guard.callback(_unlink_snapshot, forum_db_snapshot_path) payload_path.write_text(json.dumps(ctx.payload, ensure_ascii=True), encoding="utf-8") # Build the polyglot test-feedback config FIRST (it has no diff --git a/tests/memory/test_forum_container_db_snapshot.py b/tests/memory/test_forum_container_db_snapshot.py new file mode 100644 index 0000000..1146ce8 --- /dev/null +++ b/tests/memory/test_forum_container_db_snapshot.py @@ -0,0 +1,141 @@ +"""Regression tests for the forum-container knowledge-DB snapshot path. + +Root cause being pinned (macOS + Docker Desktop): forum containers used to +bind-mount the LIVE knowledge DB and its ``-wal``/``-shm`` sidecars, and the +in-container MCP server opened them with ``read_only=True``. In WAL mode even a +read-only SQLite connection WRITES to the ``-shm`` wal-index (read-marks, +recovery rebuilds) and takes POSIX locks — but a VirtioFS bind mount shares +neither the host's advisory locks nor a coherent mmap of the ``-shm``. The +container-side "reader" therefore silently fought the host writer over the +wal-index, misdirecting WAL appends/checkpoint backfills into wrong page +offsets: ``sqlite3.DatabaseError: database disk image is malformed`` on the +authoritative store (or a host SIGBUS inside ``walIndexAppend`` / +``walIndexReadHdr``), scaling with workstream count. + +The fix: the live WAL trio never crosses the container boundary. Forum +containers mount a standalone point-in-time snapshot produced by +``_store_common.snapshot_db_for_container_mount`` (``VACUUM INTO`` on its own +read-only connection), and the MCP server opens it ``read_only=True, +immutable=True`` — no locks, no wal-index, no shared state. + +See tests/runtime/test_container_host.py::TestForumKnowledgeDbSnapshot for the +payload/mount-contract side of the same regression. +""" + +from __future__ import annotations + +import sqlite3 + +import pytest + +from ksi.memory._store_common import snapshot_db_for_container_mount +from ksi.memory.knowledge_store import KnowledgeStore + + +def _wal_trio(db_path): + return [db_path, db_path.with_name(db_path.name + "-wal"), db_path.with_name(db_path.name + "-shm")] + + +def test_snapshot_is_standalone_and_complete_while_writer_is_live(tmp_path): + """Snapshot cut while the writer holds the DB open must be a single + self-contained file carrying every committed row — including rows that + still live only in the source's ``-wal``. + """ + db = tmp_path / "exp_knowledge.sqlite" + store = KnowledgeStore(str(db), default_experiment="exp") + try: + for i in range(5): + store.record_post( + task_id="t1", + agent_id="agent-0", + generation=1, + text=f"post {i}", + source_phase="per_task_forum", + ) + # Live WAL trio exists: the writer connection is open and rows are in + # the -wal (autocheckpoint threshold not reached). + for p in _wal_trio(db): + assert p.exists(), f"precondition: live {p.name} missing" + + snap = tmp_path / "knowledge_ro_snapshot_agent-0_deadbeef.sqlite" + snapshot_db_for_container_mount(db, snap) + + # Standalone: single rollback-journal file, no sidecars to share. + assert snap.exists() + assert not snap.with_name(snap.name + "-wal").exists() + assert not snap.with_name(snap.name + "-shm").exists() + header = snap.read_bytes()[:100] + assert header[18] == 1 and header[19] == 1, "snapshot must not be in WAL mode" + finally: + store.close() + + # Complete: all committed rows visible through the store's own read path. + ro = KnowledgeStore(str(snap), read_only=True, immutable=True, default_experiment="exp") + try: + page = ro.query_task("t1", entry_types=["post"], experiment="exp") + assert len(page["discussion"]) == 5 + finally: + ro.close() + + +def test_snapshot_overwrites_stale_dest(tmp_path): + """A stale file at the destination (crashed prior run) must not survive: + ``VACUUM INTO`` refuses to overwrite, so the helper unlinks first. + """ + db = tmp_path / "exp_knowledge.sqlite" + store = KnowledgeStore(str(db), default_experiment="exp") + try: + store.record_insight(task_id="t1", agent_id="agent-0", generation=1, text="x") + snap = tmp_path / "snap.sqlite" + snap.write_bytes(b"stale garbage from a crashed run") + snapshot_db_for_container_mount(db, snap) + conn = sqlite3.connect(f"file:{snap}?mode=ro&immutable=1", uri=True) + try: + assert conn.execute("SELECT COUNT(*) FROM knowledge").fetchone()[0] == 1 + finally: + conn.close() + finally: + store.close() + + +def test_snapshot_failure_leaves_no_partial_file(tmp_path): + """A failed snapshot (source is not a database) must raise AND leave no + dest file behind for a container to mount. + """ + not_a_db = tmp_path / "exp_knowledge.sqlite" + not_a_db.write_bytes(b"this is not a sqlite database") + snap = tmp_path / "snap.sqlite" + with pytest.raises(sqlite3.Error): + snapshot_db_for_container_mount(not_a_db, snap) + assert not snap.exists(), "half-written snapshot must be cleaned up" + + +def test_immutable_requires_read_only(tmp_path): + """``immutable=1`` promises SQLite the file never changes; on a writable + store that would be a lie (stale/torn reads at best). Fail loudly. + """ + with pytest.raises(ValueError, match="immutable=True requires read_only=True"): + KnowledgeStore(str(tmp_path / "k.sqlite"), immutable=True) + + +def test_immutable_open_takes_no_locks_and_creates_no_sidecars(tmp_path): + """The immutable read path must not create ``-wal``/``-shm`` (there is no + shared WAL state to fight over — the property the container mount relies + on). + """ + db = tmp_path / "exp_knowledge.sqlite" + store = KnowledgeStore(str(db), default_experiment="exp") + try: + store.record_post(task_id="t1", agent_id="agent-0", generation=1, text="p") + snap = tmp_path / "snap.sqlite" + snapshot_db_for_container_mount(db, snap) + finally: + store.close() + + ro = KnowledgeStore(str(snap), read_only=True, immutable=True, default_experiment="exp") + try: + ro.query_task("t1", experiment="exp") + assert not snap.with_name(snap.name + "-wal").exists() + assert not snap.with_name(snap.name + "-shm").exists() + finally: + ro.close() diff --git a/tests/runtime/test_container_host.py b/tests/runtime/test_container_host.py index 547ffa7..a075ced 100644 --- a/tests/runtime/test_container_host.py +++ b/tests/runtime/test_container_host.py @@ -553,7 +553,7 @@ def _materialize(self, executor, tmp_path, snapshot): def test_healthy_snapshot_write_reports_no_failure(self, tmp_path): executor = self._make_executor(tmp_path) - (snapshot_path, failed), payload = self._materialize(executor, tmp_path, {"entries": []}) + (snapshot_path, failed, _db_snap), payload = self._materialize(executor, tmp_path, {"entries": []}) assert failed is False assert snapshot_path is not None assert payload["knowledge"].get("snapshot_path") == str(snapshot_path) @@ -563,12 +563,182 @@ def test_failed_snapshot_write_sets_flag_and_omits_path(self, tmp_path): # A non-JSON-serializable snapshot makes the host-side write_text raise, # so the container would start cold. The method must swallow it (the # container still runs) AND signal the failure. - (snapshot_path, failed), payload = self._materialize(executor, tmp_path, {"bad": object()}) + (snapshot_path, failed, _db_snap), payload = self._materialize(executor, tmp_path, {"bad": object()}) assert failed is True # No snapshot_path is threaded into the payload, so the container starts cold. assert "snapshot_path" not in payload.get("knowledge", {}) +class TestForumKnowledgeDbSnapshot: + """Forum containers must mount a standalone snapshot, never the live DB. + + Regression for the macOS/Docker Desktop corruption: bind-mounting the live + knowledge DB (+ ``-wal``/``-shm``) into forum containers let in-container + ``read_only=True`` SQLite connections write to the shared wal-index over a + VirtioFS mount with no cross-VM lock/mmap coherence, corrupting the host + store ("database disk image is malformed" at the gen boundary, scaling + with workstream count). See + tests/memory/test_forum_container_db_snapshot.py for the snapshot/immutable + primitives; this class pins the payload contract that keeps the live WAL + trio out of containers. + """ + + def _make_executor(self, tmp_path, **kw): + defaults = dict( + command=["echo", "dummy"], + working_dir=str(tmp_path), + instruction_path=str(tmp_path / "INSTRUCTION.md"), + agent_workspace_root=str(tmp_path / "workspaces"), + env={ + "MODEL_PROVIDER": "anthropic", + "MODEL_AUTH_MODE": "api", + "MODEL": "claude-sonnet-4-6", + "ANTHROPIC_API_KEY": "sk-test", + }, + ) + defaults.update(kw) + return KsiContainerExecutor(**defaults) + + def _seed_live_db(self, db_path): + from ksi.memory.knowledge_store import KnowledgeStore + + store = KnowledgeStore(str(db_path), default_experiment="test_exp") + try: + store.record_post( + task_id="t1", + agent_id="agent-0", + generation=1, + text="prior post", + source_phase="per_task_forum", + ) + finally: + store.close() + + def _run_forum_task(self, tmp_path, task_source, db_path): + captured = [] + snapshot_state = {} + + def fake_run(cmd, **kw): + with open(cmd[-1]) as f: + payload = json.load(f) + captured.append(payload) + snap = str(payload.get("knowledge", {}).get("db_path", "")) + if snap: + p = Path(snap) + snapshot_state["exists_at_runner_time"] = p.exists() + if p.exists(): + header = p.read_bytes()[:100] + # bytes 18/19: 1 = rollback journal, 2 = WAL + snapshot_state["journal_mode_bytes"] = (header[18], header[19]) + snapshot_state["wal_sidecar"] = p.with_name(p.name + "-wal").exists() + snapshot_state["shm_sidecar"] = p.with_name(p.name + "-shm").exists() + task_id = payload["task"]["id"] + return MagicMock( + returncode=0, + stdout=_runner_stdout(task_id=task_id), + stderr="", + ) + + task = TaskSpec( + id="__forum__g1_r0_agent-0", + prompt="discuss", + metadata={ + "task_source": task_source, + "task_md_override": "discuss", + "forum_generation": 1, + "forum_round": 0, + "forum_agent_id": "agent-0", + "forum_expected_agents": 1, + "forum_task_ids": ["t1"], + }, + ) + with patch( + "ksi.runtime.container_host._run_command_with_backstop", + side_effect=fake_run, + ): + ex = self._make_executor(tmp_path, knowledge_db_path=str(db_path)) + ex.run_task( + generation=1, + agent_id="agent-0", + task=task, + agent_seed_package={}, + experiment_name="test_exp", + ) + assert len(captured) == 1 + return captured[0], snapshot_state + + @pytest.mark.parametrize("task_source", ["per_task_forum", "cross_task_forum"]) + def test_forum_payload_points_at_standalone_snapshot(self, tmp_path, task_source): + db_path = tmp_path / "exp_knowledge.sqlite" + self._seed_live_db(db_path) + + payload, snap_state = self._run_forum_task(tmp_path, task_source, db_path) + + mem = payload["knowledge"] + assert mem.get("db_is_snapshot") is True + # The live DB path must NOT cross the container boundary. + assert mem["db_path"] != str(db_path) + assert mem["db_path"], "snapshot path missing from a healthy forum payload" + # The snapshot existed while the runner ran, standalone: rollback + # journal mode, no -wal/-shm to share with the host writer. + assert snap_state.get("exists_at_runner_time") is True + assert snap_state.get("journal_mode_bytes") == (1, 1) + assert snap_state.get("wal_sidecar") is False + assert snap_state.get("shm_sidecar") is False + # Same lifecycle as the memory snapshot: unlinked after the run. + assert not Path(mem["db_path"]).exists(), "forum DB snapshot leaked after run_task" + + def test_task_source_keeps_live_db_path(self, tmp_path): + """Non-forum sources keep the historical payload: the live path (their + containers never mount the DB files — the mount gate is forum-only). + """ + db_path = tmp_path / "exp_knowledge.sqlite" + self._seed_live_db(db_path) + captured = [] + + def fake_run(cmd, **kw): + with open(cmd[-1]) as f: + captured.append(json.load(f)) + return MagicMock(returncode=0, stdout=_runner_stdout(task_id="t1"), stderr="") + + with patch( + "ksi.runtime.container_host._run_command_with_backstop", + side_effect=fake_run, + ): + ex = self._make_executor(tmp_path, knowledge_db_path=str(db_path)) + ex.run_task( + generation=1, + agent_id="agent-0", + task=TaskSpec(id="t1", prompt="solve"), + agent_seed_package={}, + experiment_name="test_exp", + ) + mem = captured[0]["knowledge"] + assert mem["db_path"] == str(db_path) + assert "db_is_snapshot" not in mem + + def test_snapshot_failure_degrades_without_live_fallback(self, tmp_path): + """If the snapshot cannot be built, the payload must NOT carry the live + DB path. It keeps the (absent) snapshot path so the runner mounts + nothing for it while ForumBus — whose JSONL dir derives from + ``db_path``'s parent — keeps working in the degraded container. + """ + db_path = tmp_path / "exp_knowledge.sqlite" + # Not a SQLite database: VACUUM INTO fails naturally. + db_path.write_bytes(b"this is not a sqlite database") + + payload, snap_state = self._run_forum_task(tmp_path, "per_task_forum", db_path) + + mem = payload["knowledge"] + assert mem.get("db_is_snapshot") is True + # Never the live path; the stamped path must not exist (nothing for the + # runner to mount), but must live in the knowledge-DB dir so ForumBus + # still resolves /forum_bus. + assert mem["db_path"] != str(db_path) + assert snap_state.get("exists_at_runner_time") is False + assert Path(mem["db_path"]).parent == db_path.parent + + class TestRunnerEnvelopeIdentity: def _make_executor(self, tmp_path): return KsiContainerExecutor(