bug: read-only knowledge store connection always fails due to DuckDB same-file constraint

[mickume]

Summary

Starting af code always emits a noisy WARNING + full traceback because _setup_infrastructure tries to open a read-only DuckDB connection to the same knowledge store file that already has an active read-write connection. DuckDB does not allow mixed read_only configurations on the same database file within a single process.

Reproduction

af code

Observe in the output:

[WARNING] agentfox.engine.run: Failed to open read-only knowledge store for context assembly; falling back to main connection
…
_duckdb.ConnectionException: Connection Error: Can't open a connection to same database file with a different configuration than existing connections

Root Cause

In packages/agentfox/agentfox/engine/run.py, _setup_infrastructure:

• Line 109: opens a read-write connection — open_knowledge_store(config.knowledge, read_only=False)
• Line 118: attempts a read-only connection to the same file — open_knowledge_store(config.knowledge, read_only=True)

DuckDB (1.5.4) enforces that all in-process connections to a given file share the same read_only flag. The second call always fails.

The pattern was introduced in commit a053f869 for requirement 06-REQ-7.3 ("ensure assemble_context never holds a write lock"). The fallback on lines 119–123 catches the error and reuses the main connection, so the application runs — but:

1. The traceback is alarming and confusing to users.
2. The original intent (lock-free reads during context assembly) is silently defeated.

Impact

• Every af code invocation prints a multi-line traceback that looks like a crash.
• The read-only separation goal of 06-REQ-7.3 is never achieved in practice.

Suggested Fix

Replace the separate read_only=True connection with a cursor from the existing read-write connection:

# Current (always fails):
context_knowledge_db = open_knowledge_store(config.knowledge, read_only=True)

# Proposed:
context_cursor = knowledge_db.connection.cursor()

DuckDB supports multiple cursors on a single connection, allowing concurrent reads without a separate connection object. The context assembly code would receive the cursor (or a thin wrapper) instead of a full KnowledgeDB instance.

This satisfies the spirit of 06-REQ-7.3 — context reads don't contend with write queries — without violating DuckDB's same-file configuration constraint.

Files Involved

• packages/agentfox/agentfox/engine/run.py — _setup_infrastructure (primary fix site)
• packages/agentfox/agentfox/knowledge/db.py — may need a helper to expose a cursor wrapper

[mickume]

look for similar patterns in other parts of the codebase.

[mickume]

Starting fix session on branch fix/627-bug-read-only-knowledge-store-connection-always-fails-due-to-duckdb-same-file-constraint... (run: 20260626_140241_b8fd39)

[mickume]

Triage Report

Summary: In packages/agentfox/agentfox/engine/run.py, _setup_infrastructure originally opened a primary read_only=False connection (line 109) then tried a second read_only=True connection to the same file (commit a053f869). DuckDB 1.5.4 raises ConnectionException for mixed read_only configurations on the same in-process file; the try/except fallback emitted a WARNING with full traceback on every af code invocation and silently reused the main connection, defeating 06-REQ-7.3. A subsequent partial fix (commit 85f2e38b) changed the second call to read_only=False and removed the try/except, eliminating the exception, but failed to update test_duckdb_reader_writer_smoke.py::TestOrchestratorStartupSmoke::test_session_receives_read_only_conn, which still asserts call_log[1] is True — causing it to fail. The proper fix is to replace the second open_knowledge_store call with knowledge_db.connection.cursor(), which avoids a second connection entirely and satisfies the spirit of 06-REQ-7.3.

Affected files:

• packages/agentfox/agentfox/engine/run.py
• packages/agentfox/agentfox/knowledge/db.py
• packages/agentfox/tests/integration/test_duckdb_reader_writer_smoke.py
• packages/agentfox/tests/test_assemble_context_readonly.py

Acceptance Criteria

AC-1: _setup_infrastructure calls open_knowledge_store exactly once — the cursor approach eliminates the second connection call entirely.

• Preconditions: A mock AgentFoxConfig is available; open_knowledge_store, DuckDBSink, SinkDispatcher, FoxKnowledgeProvider, AgentTraceSink, and create_platform_safe are all patched.
• Expected: open_knowledge_store is called exactly once with read_only=False; no second call is made.
• Assertion: Patch agentfox.engine.run.open_knowledge_store, call _setup_infrastructure(mock_config), assert mock_open.call_count == 1 and mock_open.call_args.kwargs['read_only'] is False.

AC-2: The context connection in infra is derived from the primary connection (a cursor), not a second KnowledgeDB instance, and is distinct from infra['knowledge_db'].connection.

• Preconditions: _setup_infrastructure has returned infra without raising.
• Expected: infra['context_knowledge_db'].connection is not infra['knowledge_db'].connection, and infra['context_knowledge_db'] is not infra['knowledge_db'].
• Assertion: Assert both identity checks hold: the context connection object and the main connection object are distinct Python objects.

AC-3: No WARNING log message is emitted by _setup_infrastructure on a successful run — the DuckDB ConnectionException path is no longer reachable.

• Preconditions: A valid mock config is provided; all infrastructure dependencies are patched to succeed.
• Expected: logger.warning is never called within _setup_infrastructure during a normal startup.
• Assertion: Patch agentfox.engine.run.logger and call _setup_infrastructure(mock_config); assert logger.warning.call_count == 0.

AC-4: The stale smoke test TestOrchestratorStartupSmoke::test_session_receives_read_only_conn is updated and passes — it must reflect the cursor-based implementation rather than asserting call_log[1] is True.

• Preconditions: The full test suite is executed with make test.
• Expected: All tests in test_duckdb_reader_writer_smoke.py pass, including test_session_receives_read_only_conn.
• Assertion: Run make test; assert zero failures in test_duckdb_reader_writer_smoke.py::TestOrchestratorStartupSmoke.

AC-5: A DuckDB cursor obtained from the primary connection satisfies the conn parameter of assemble_context — read-only SELECT queries execute correctly through it.

• Preconditions: An in-memory or tmp_path-based DuckDB with the full schema applied is open with read_only=False.
• Expected: Calling assemble_context(spec_dir=..., task_group=1, conn=knowledge_db.connection.cursor(), ...) returns a non-empty string without raising any exception.
• Assertion: Construct a KnowledgeDB, call .open(), derive cursor = db.connection.cursor(), call assemble_context(..., conn=cursor), assert the return value is a str and no exception is raised.

(run: 20260626_140241_b8fd39)

[mickume]

Fix Review Report

Overall verdict: PASS

Summary: All 5 acceptance criteria pass. The fix replaces the second open_knowledge_store(read_only=True) call (which always failed due to DuckDB's same-file configuration constraint) with a cursor-based ContextKnowledgeDB wrapper. open_knowledge_store is now called exactly once. The WARNING log and traceback are eliminated. Tests have been updated to reflect the cursor approach. The 8 test failures in make test are all pre-existing (6 on main baseline, 2 additional are xdist-flaky and pass in isolation). Lint is clean on all changed files.

Per-criterion verdicts

• ✅ AC-1 (NS-REQ-1.1): PASS
  • Evidence: In _setup_infrastructure (run.py lines 90–173), open_knowledge_store is called exactly once at line 109 with read_only=False. The second call that previously existed has been replaced by ContextKnowledgeDB(knowledge_db.connection.cursor()) at line 117. Grep for open_knowledge_store in run.py confirms only one call site (line 109) and one import (line 26). The test test_session_receives_read_only_conn asserts len(call_log) == 1 and call_log[0] is False, and it passes.
• ✅ AC-2 (NS-REQ-2.1): PASS
  • Evidence: Line 117 creates context_knowledge_db = ContextKnowledgeDB(knowledge_db.connection.cursor()). ContextKnowledgeDB (db.py lines 150–176) wraps a DuckDB cursor and exposes it via the .connection property. The cursor is a distinct Python object from knowledge_db.connection. Tests in both test_duckdb_reader_writer_smoke.py (lines 219–222) and test_assemble_context_readonly.py (lines 343–351) assert: (1) infra['context_knowledge_db'] is not infra['knowledge_db'], (2) isinstance(infra['context_knowledge_db'], ContextKnowledgeDB), and (3) infra['context_knowledge_db'].connection is mock_cursor (distinct from main connection). All pass.
• ✅ AC-3 (NS-REQ-3.1): PASS
  • Evidence: Grep for logger.warning in run.py shows no calls within _setup_infrastructure (lines 90–173). The old try/except fallback with logger.warning('Failed to open read-only knowledge store...') has been completely removed. The only logger call in _setup_infrastructure is logger.debug at line 163 (platform creation failure, unrelated). The DuckDB ConnectionException path is no longer reachable because there is no second open_knowledge_store call.
• ✅ AC-4 (NS-REQ-4.1): PASS
  • Evidence: All tests in test_duckdb_reader_writer_smoke.py pass (6/6 in TestOrchestratorStartupSmoke), including test_session_receives_read_only_conn. The test has been updated to assert: (1) exactly 1 open_knowledge_store call, (2) isinstance(context_knowledge_db, ContextKnowledgeDB), (3) context_knowledge_db.connection is mock_cursor. Full output: 18 passed in 1.40s across both test files.
• ✅ AC-5 (NS-REQ-5.1): PASS
  • Evidence: DuckDB cursors are duckdb.DuckDBPyConnection instances (verified: isinstance(db.cursor(), duckdb.DuckDBPyConnection) is True). assemble_context accepts conn: duckdb.DuckDBPyConnection, so a cursor satisfies the type. The ContextKnowledgeDB.connection property returns the cursor directly, which is what gets passed to assemble_context at runtime. The existing test test_assemble_context_with_read_only_conn confirms assemble_context executes SELECT queries and returns a str without exceptions. The session_lifecycle.py type annotation has been updated to accept KnowledgeDB | ContextKnowledgeDB | None (line 241).

(run: 20260626_140241_b8fd39)

[mickume]

Fix complete on branch fix/627-bug-read-only-knowledge-store-connection-always-fails-due-to-duckdb-same-file-constraint. Changes have been merged into main. (run: 20260626_140241_b8fd39)