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(