Skip to content

fix(mcp): recover from DatabaseError, stale files, and transaction lock leaks (fixes #286) - #287

Merged
Coding-Dev-Tools merged 3 commits into
mainfrom
fix/issue-286-long-lived-mcp-db-error-recovery
Oct 26, 2026
Merged

Coding-Dev-Tools merged 3 commits into
mainfrom
fix/issue-286-long-lived-mcp-db-error-recovery

Conversation

@Coding-Dev-Tools

Copy link
Copy Markdown
Owner

Summary

Fixes #286 and addresses comment #286 (comment).

Root Cause Analysis

  1. Generic (DatabaseError) in Log: mcp_server.py:_err() logged only type(exc).name. In Python's sqlite3, SQLITE_CORRUPT, SQLITE_NOTADB, SQLITE_EMPTY, and SQLITE_FORMAT all raise sqlite3.DatabaseError directly, which collapsed into (DatabaseError) without identifying the underlying SQLite error code or name.
  2. Permanent Stale/Poisoned State: A long-lived engraphis-mcp stdio server kept a single Store.conn open indefinitely. When a separate daily import or sync process ran (modifying or checkpointing WAL, or replacing the file on POSIX via os.replace), Store.conn either lost synchronization, held an open transaction pinning an older WAL read point, or held an fd to an unlinked inode (st_ino mismatch). Once SQLite returns SQLITE_CORRUPT on a connection, that connection remains permanently broken until closed and re-opened.
  3. E_NOT_FOUND on get_memory: When the database file was replaced or a read transaction remained pinned, get_memory read from the stale snapshot/old inode where newly imported memories did not exist, raising ValidationError(no memory with id ...) which Smart MCP mapped to E_NOT_FOUND.
  4. Lock Stranding on Rollback Failure: In _SerializedConnection._finish(), if rollback() raised an exception while in_transaction was true, keep_pin was set to True, leaking the _lock acquire and deadlocking subsequent worker threads in AnyIO's threadpool with 'store write lock timeout — a transaction appears stuck'.

Changes Implemented

  • engraphis/core/store.py:
    • _finish(): fixed keep_pin = bool(self._raw.in_transaction) and not is_rollback so a failed rollback never leaves a transaction pin stranded.
    • reset_pin(): clears transaction pin state and releases _lock.
    • replace_raw(new_raw): safely closes old SQLite connection and installs new raw connection under lock.
    • Store.init(): tracks self._opened_identity = (st_dev, st_ino) for file-backed databases.
    • Store.is_stale(): checks whether the on-disk file was replaced by another process.
    • Store.reconnect(): cleanly re-opens connection, re-applies all pragmas (foreign_keys=ON, secure_delete=ON, synchronous, journal_mode=WAL, engraphis_unicode_lower), replaces conn._raw, and refreshes _read_snapshot_pool.
    • Store.check_health(): liveness probe (SELECT 1, PRAGMA data_version, is_stale()) with auto-reconnection.
    • Store.get_memory(), get_memories(), and _select_memory_records(): auto-reconnect if stale before read, and if sqlite3.DatabaseError is encountered, auto-reconnect and retry once.
  • engraphis/service.py:
    • Added MemoryService.check_health().
    • Updated inspect(): if _check_owns() fails with ValidationError, checks self.store.is_stale() and reconnects/re-checks before failing.
  • engraphis/mcp_server.py:
    • Added _safe_exception_marker(exc): extracts safe, sanitized SQLite error names (SQLITE_CORRUPT, SQLITE_BUSY, SQLITE_LOCKED, SQLITE_IOERR, SQLITE_NOTADB, write_lock_timeout) without leaking queries, paths, credentials, or memory text.
    • Updated _err(exc): logs MCP tool operation failed (DatabaseError: SQLITE_CORRUPT) instead of bare (DatabaseError), and on database errors triggers background store health check / reconnection.
    • Updated _classify_gateway_exception(): maps SQLITE_BUSY and SQLITE_LOCKED to E_RETRYABLE, and triggers reconnect on database errors.
    • Updated engraphis_recall_context(), engraphis_recall(), engraphis_recall_grounded(), and engraphis_get_memory(): auto-reconnect and retry once on database error.
  • tests/test_mcp_database_error_recovery.py:
    • Added 9 unit and regression tests covering safe exception attribution, secret redaction, lock release on rollback failure, file replacement detection, auto-reconnect on read, health checks, and Smart gateway error classification.

Verification

  • ruff check .: All checks passed.
  • python scripts/check_commercial_manifest.py: OK.
  • python scripts/externalize_dashboard_assets.py: OK.
  • python -m eval.grounded: 100% (11/11).
  • pytest tests/test_mcp_server.py tests/test_smart_mcp_gateway.py tests/test_sqlite_durability.py tests/test_mcp_database_error_recovery.py: 221 passed, 1 skipped.

…ck leaks

- Add safe exception marker extraction in mcp_server logging to attribute
  underlying SQLite errors (SQLITE_CORRUPT, SQLITE_BUSY, SQLITE_LOCKED,
  SQLITE_IOERR, SQLITE_NOTADB, write_lock_timeout) without leaking SQL,
  paths, credentials, or memory content (Issue #286).
- Prevent transaction lock leak in _SerializedConnection._finish when
  rollback fails, avoiding 60s timeout deadlocks across threadpool workers.
- Add Store.is_stale() and Store.reconnect() to detect when the SQLite
  database file was replaced or modified out-of-process and cleanly
  re-initialize connection pragmas, unicode function, and snapshot pools.
- Add auto-recovery and single-retry loops on DatabaseError/OperationalError
  in Store memory fetchers, engraphis_recall_context, engraphis_recall,
  engraphis_recall_grounded, and engraphis_get_memory.
- Add comprehensive regression and unit tests in
  tests/test_mcp_database_error_recovery.py.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-26T00:53:21.903825Z
We couldn’t complete this review. Comment @codex review to try again.
8c54169 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Coding-Dev-Tools
Coding-Dev-Tools merged commit a80f178 into main Oct 26, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Long-lived MCP server: all tool calls fail with (DatabaseError) until the server is restarted

1 participant