From ceb0b9cf5f30360ab83bc1b097f747149a9c38d9 Mon Sep 17 00:00:00 2001 From: Mike Nolet Date: Sat, 2 May 2026 08:45:23 +0200 Subject: [PATCH 1/2] fix(test-infra): openInboundDb honors in-memory test DB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit initTestSessionDb() creates an in-memory inbound singleton, but openInboundDb() always opened the hardcoded /workspace/inbound.db path. Every test that exercised getPendingMessages — directly, or via test fixtures that load data through it (e.g. poll-loop.test.ts:29 loads formatter test rows via getPendingMessages) — failed with SQLITE_CANTOPEN under `bun test` outside a real container. Baseline on main: 34 pass, 25 fail across 6 files. After this fix: 59 pass, 0 fail. In test mode, openInboundDb returns the in-memory singleton. The singleton's .close() is no-op'd in initTestSessionDb so caller try/finally cleanup doesn't tear down the shared DB; closeSessionDb invokes the saved original close to do the real teardown. Production behavior is unchanged — _inboundIsTest only flips inside initTestSessionDb, which is never called outside the test runner. Co-Authored-By: Claude Opus 4.7 (1M context) --- container/agent-runner/src/db/connection.ts | 27 ++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/container/agent-runner/src/db/connection.ts b/container/agent-runner/src/db/connection.ts index 3ca44a8..ac563fa 100644 --- a/container/agent-runner/src/db/connection.ts +++ b/container/agent-runner/src/db/connection.ts @@ -27,6 +27,13 @@ const DEFAULT_HEARTBEAT_PATH = '/workspace/.heartbeat'; let _inbound: Database | null = null; let _outbound: Database | null = null; let _heartbeatPath: string = DEFAULT_HEARTBEAT_PATH; +// True when initTestSessionDb() set _inbound to an in-memory DB. Used by +// openInboundDb() so tests don't try to open the missing /workspace path. +let _inboundIsTest = false; +// Saved real close() for the in-memory inbound singleton. We no-op the +// public .close() during tests so caller try/finally doesn't tear down +// the shared DB; closeSessionDb() invokes this to do the real teardown. +let _inboundOriginalClose: (() => void) | null = null; /** * Avoid all cached db reads; open inbound.db read-only with mmap and page cache disabled. @@ -42,6 +49,12 @@ let _heartbeatPath: string = DEFAULT_HEARTBEAT_PATH; * Cost is microseconds per query, so safe for universal use. */ export function openInboundDb(): Database { + // In test mode the inbound DB is an in-memory singleton — there is no + // file at DEFAULT_INBOUND_PATH. Return the singleton directly; its + // .close() was no-op'd in initTestSessionDb so caller try/finally + // cleanup doesn't tear down the shared DB. + if (_inboundIsTest && _inbound) return _inbound; + const db = new Database(DEFAULT_INBOUND_PATH, { readonly: true }); db.exec('PRAGMA busy_timeout = 5000'); db.exec('PRAGMA mmap_size = 0'); @@ -171,6 +184,12 @@ export function clearStaleProcessingAcks(): void { /** For tests — creates in-memory DBs with the session schemas. */ export function initTestSessionDb(): { inbound: Database; outbound: Database } { _inbound = new Database(':memory:'); + _inboundIsTest = true; + // No-op .close() so callers using openInboundDb()'s try/finally pattern + // don't tear down our shared singleton. closeSessionDb() does the real + // teardown via the saved original. + _inboundOriginalClose = _inbound.close.bind(_inbound); + _inbound.close = () => {}; _inbound.exec('PRAGMA foreign_keys = ON'); _inbound.exec(` CREATE TABLE messages_in ( @@ -244,8 +263,14 @@ export function initTestSessionDb(): { inbound: Database; outbound: Database } { } export function closeSessionDb(): void { - _inbound?.close(); + if (_inboundOriginalClose) { + _inboundOriginalClose(); + _inboundOriginalClose = null; + } else { + _inbound?.close(); + } _inbound = null; + _inboundIsTest = false; _outbound?.close(); _outbound = null; } From 6d6584d1207e7ae55ab20c068c11be65a7a58426 Mon Sep 17 00:00:00 2001 From: gavrielc Date: Tue, 5 May 2026 16:02:10 +0300 Subject: [PATCH 2/2] fix(test-infra): openInboundDb honors in-memory test DB openInboundDb() always opened /workspace/inbound.db which doesn't exist in CI. In test mode, return a thin wrapper over the in-memory singleton that delegates prepare/exec but no-ops close(), so callers' try/finally cleanup doesn't destroy the shared DB mid-test. One flag (_testMode), no monkey-patching, no saved-close bookkeeping. Co-Authored-By: Claude Opus 4.6 (1M context) --- container/agent-runner/src/db/connection.ts | 45 +++++++-------------- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/container/agent-runner/src/db/connection.ts b/container/agent-runner/src/db/connection.ts index ac563fa..871e43a 100644 --- a/container/agent-runner/src/db/connection.ts +++ b/container/agent-runner/src/db/connection.ts @@ -27,34 +27,29 @@ const DEFAULT_HEARTBEAT_PATH = '/workspace/.heartbeat'; let _inbound: Database | null = null; let _outbound: Database | null = null; let _heartbeatPath: string = DEFAULT_HEARTBEAT_PATH; -// True when initTestSessionDb() set _inbound to an in-memory DB. Used by -// openInboundDb() so tests don't try to open the missing /workspace path. -let _inboundIsTest = false; -// Saved real close() for the in-memory inbound singleton. We no-op the -// public .close() during tests so caller try/finally doesn't tear down -// the shared DB; closeSessionDb() invokes this to do the real teardown. -let _inboundOriginalClose: (() => void) | null = null; +let _testMode = false; /** - * Avoid all cached db reads; open inbound.db read-only with mmap and page cache disabled. - * + * Avoid all cached db reads; open inbound.db read-only with mmap and page cache disabled. + * * Use this (not getInboundDb) for readers that need to see host-written rows * promptly — e.g. messages_in polling. Caller must .close() the returned * connection (try/finally). * * Needed for mounts where host writes don't reliably invalidate * SQLite's caches: virtiofs (Colima, Lima, Podman Machine, Apple - * Container), NFS. - * + * Container), NFS. + * * Cost is microseconds per query, so safe for universal use. */ export function openInboundDb(): Database { - // In test mode the inbound DB is an in-memory singleton — there is no - // file at DEFAULT_INBOUND_PATH. Return the singleton directly; its - // .close() was no-op'd in initTestSessionDb so caller try/finally - // cleanup doesn't tear down the shared DB. - if (_inboundIsTest && _inbound) return _inbound; - + // In test mode return a thin wrapper over the in-memory singleton. + // Callers do try/finally { db.close() } — the wrapper no-ops close() + // so the singleton survives for the rest of the test. + if (_testMode && _inbound) { + const db = _inbound; + return { prepare: (sql: string) => db.prepare(sql), exec: (sql: string) => db.exec(sql), close: () => {} } as unknown as Database; + } const db = new Database(DEFAULT_INBOUND_PATH, { readonly: true }); db.exec('PRAGMA busy_timeout = 5000'); db.exec('PRAGMA mmap_size = 0'); @@ -183,13 +178,8 @@ export function clearStaleProcessingAcks(): void { /** For tests — creates in-memory DBs with the session schemas. */ export function initTestSessionDb(): { inbound: Database; outbound: Database } { + _testMode = true; _inbound = new Database(':memory:'); - _inboundIsTest = true; - // No-op .close() so callers using openInboundDb()'s try/finally pattern - // don't tear down our shared singleton. closeSessionDb() does the real - // teardown via the saved original. - _inboundOriginalClose = _inbound.close.bind(_inbound); - _inbound.close = () => {}; _inbound.exec('PRAGMA foreign_keys = ON'); _inbound.exec(` CREATE TABLE messages_in ( @@ -263,14 +253,9 @@ export function initTestSessionDb(): { inbound: Database; outbound: Database } { } export function closeSessionDb(): void { - if (_inboundOriginalClose) { - _inboundOriginalClose(); - _inboundOriginalClose = null; - } else { - _inbound?.close(); - } + _inbound?.close(); _inbound = null; - _inboundIsTest = false; + _testMode = false; _outbound?.close(); _outbound = null; }