diff --git a/.craftsmanship-baseline.json b/.craftsmanship-baseline.json index b3b63055..a0ebc17f 100644 --- a/.craftsmanship-baseline.json +++ b/.craftsmanship-baseline.json @@ -5517,11 +5517,6 @@ "kind": "method-size", "detail": "classify_write_class" }, - { - "file": "mcp_server/tool_error_handler.py", - "kind": "method-size", - "detail": "_classify_error" - }, { "file": "mcp_server/tool_error_handler.py", "kind": "method-size", diff --git a/mcp_server/tool_error_handler.py b/mcp_server/tool_error_handler.py index e7206956..e33c344d 100644 --- a/mcp_server/tool_error_handler.py +++ b/mcp_server/tool_error_handler.py @@ -80,6 +80,41 @@ async def tool_remember(...) -> dict: "Then restart Claude Code." ) +_MISSING_EXTENSION_PHRASES = [ + 'type "vector" does not exist', + "extension", + "pg_trgm", +] + +# Connection/auth failures ONLY — each phrase below is unambiguous about a +# server that is unreachable, still starting, or refusing credentials. +# +# Deliberately NOT here (issue: SQLite query errors masked as +# "PostgreSQL not connected"): the exception CLASS name "operationalerror", +# a bare "does not exist", a bare "role", and a bare "timeout". Those match +# ordinary query-level failures — a FTS5 "syntax error", "no such table", +# a column that "does not exist", a statement/lock "timeout" — none of +# which are connection problems. On the SQLite backend they are never +# connection problems, yet the base class ``sqlite3.OperationalError`` set +# ``type(exc).__name__.lower() == "operationalerror"`` and every one of +# them got the PostgreSQL ``brew install`` guide, burying the real error +# (a FTS5 syntax error in get_causal_chain surfaced exactly this way). +# An error that is genuinely a create-db/create-role setup step is still +# fully actionable from its own honest ``OperationalError: ... FATAL: +# database "cortex" does not exist`` text, which the fall-through returns. +_CONNECTION_FAILURE_PHRASES = [ + "connection refused", + "could not connect", + "could not translate host name", + "no such host", + "connection reset", + "server closed the connection", + "the database system is starting up", + "password authentication failed", + "connection timed out", + "timeout expired", # psycopg connect-timeout wording +] + def _classify_error(exc: Exception) -> tuple[str, str]: """Classify an exception into a user-friendly category and message.""" @@ -96,30 +131,10 @@ def _classify_error(exc: Exception) -> tuple[str, str]: if "explicit database_url unreachable" in exc_lower: return "explicit_database_url_unreachable", str(exc) - if any( - kw in exc_lower - for kw in [ - 'type "vector" does not exist', - "extension", - "pg_trgm", - ] - ): + if any(kw in exc_lower for kw in _MISSING_EXTENSION_PHRASES): return "missing_extension", _EXTENSION_GUIDE - if any( - kw in exc_lower - for kw in [ - "connection refused", - "could not connect", - "no such host", - "connection reset", - "does not exist", - "operationalerror", - "role", - "password authentication", - "timeout", - ] - ): + if any(kw in exc_lower for kw in _CONNECTION_FAILURE_PHRASES): return "database_not_connected", _DB_SETUP_GUIDE return type(exc).__name__, str(exc) diff --git a/tests_py/server/test_tool_error_handler_classification.py b/tests_py/server/test_tool_error_handler_classification.py index 8c2a796c..a0e5e3c6 100644 --- a/tests_py/server/test_tool_error_handler_classification.py +++ b/tests_py/server/test_tool_error_handler_classification.py @@ -69,6 +69,69 @@ def test_unrecognized_error_falls_through_unclassified(self): assert message == "some unrelated application error" +class TestQueryErrorsAreNotMaskedAsDbNotConnected: + """Regression: SQLite query-level OperationalErrors were classified as + 'database_not_connected' (PostgreSQL setup guide) because the exception + CLASS name is 'OperationalError' and the old keyword list matched + 'operationalerror'. A FTS5 syntax error in get_causal_chain surfaced this + way — the real error was buried under a 'brew install postgresql' guide, + doubly wrong on the SQLite backend. Query errors must fall through to + their honest ': '.""" + + def test_fts5_syntax_error_not_masked(self): + import sqlite3 + + exc = sqlite3.OperationalError('fts5: syntax error near ""__main__""') + error_type, message = _classify_error(exc) + assert error_type == "OperationalError" + assert "fts5: syntax error" in message + + def test_no_such_table_not_masked(self): + import sqlite3 + + exc = sqlite3.OperationalError("no such table: memories") + error_type, message = _classify_error(exc) + assert error_type == "OperationalError" + assert "no such table" in message + + def test_column_does_not_exist_not_masked(self): + # bare "does not exist" used to route a query bug to the DB guide. + exc = Exception('column "heat" does not exist') + error_type, _ = _classify_error(exc) + assert error_type == "Exception" + + def test_word_containing_role_not_masked(self): + # bare "role" matched substrings like "control"/"payroll" — gone. + exc = ValueError("invalid role assignment in access control policy") + error_type, _ = _classify_error(exc) + assert error_type == "ValueError" + + def test_statement_timeout_not_masked(self): + # a lock/statement timeout is not a connection failure. + import sqlite3 + + exc = sqlite3.OperationalError("database is locked") + error_type, _ = _classify_error(exc) + assert error_type == "OperationalError" + + def test_genuine_connection_failures_still_classified(self): + for msg in [ + "connection refused", + "could not connect to server", + "could not translate host name", + "no such host", + "connection reset", + "server closed the connection unexpectedly", + "the database system is starting up", + "password authentication failed for user", + "connection timed out", + "timeout expired", + ]: + error_type, message = _classify_error(RuntimeError(msg)) + assert error_type == "database_not_connected", msg + assert "PostgreSQL" in message + + class TestSafeHandlerErrorPath: """safe_handler must raise ToolError, not return an error dict, and must log the underlying exception (with traceback) before