fix(errors): stop masking query-level errors as 'database_not_connected'

[cdeust]

What

_classify_error routed exceptions to the PostgreSQL setup guide (database_not_connected) when their text matched any of "operationalerror", "role", "does not exist", or a bare "timeout". Those substrings match ordinary query-level failures, not connection failures.

The worst offender: sqlite3.OperationalError's class name is literally OperationalError, so type(exc).__name__.lower() == "operationalerror". Every SQLite query error — a FTS5 syntax error, no such table, database is locked — matched and was handed a brew install postgresql@17 guide. On the SQLite backend that guide is doubly wrong, and it buried the real error.

This is how it was found: while running the harness-comparison benchmark on a real SQLite store, a FTS5 syntax error in get_causal_chain (fixed in #447) came back to the client as "Cortex could not connect to PostgreSQL" — sending diagnosis in exactly the wrong direction.

The other keywords were similarly broad: "role" matched control/payroll/role-based; "does not exist" matched column "x" does not exist (a query/schema bug); bare "timeout" matched statement/lock timeouts.

Fix

Tighten the database_not_connected bucket to unambiguous connection/auth phrases only:

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

Genuine create-db/create-role first-run conditions stay fully actionable from their own honest OperationalError: ... FATAL: database "cortex" does not exist text, which the fall-through (type(exc).__name__, str(exc)) returns verbatim — precision over hand-holding, so a real bug is never masked.

Tests

Added TestQueryErrorsAreNotMaskedAsDbNotConnected: FTS5 syntax error, no such table, column "x" does not exist, a role substring in unrelated text, and a locked-db error all fall through to <Type>: <message>; a companion test asserts the seven genuine connection failures still get the setup guide. The pre-existing test_generic_connection_refused_still_classified_as_db_not_connected continues to pass unchanged.

Gates: ruff check ✓ · ruff format --check ✓ · craftsmanship gate ✓ · pytest tests_py/server/test_tool_error_handler_classification.py → 13 passed.

Pairs with #447 (the FTS5 crash this bug was hiding).

🤖 Generated with Claude Code

---

Generated by Claude Code

[cdeust]

ZETETIC-REVIEW: REQUEST_CHANGES

Independent review (fresh-context reviewer, read-only via gh, head 7a0e1db).

Sound and kept: the masking diagnosis is correct (SQLite OperationalError class-name substring match + bare role/does not exist/timeout routing everything to the PostgreSQL guide), the fix resolves the exact reported FTS5/SQLite case, the truth-table test covers it, no keyword collisions across the four buckets, no bare except introduced, CI green on all required checks.

Blocking (one item):

• mcp_server/tool_error_handler.py:84-142 — _classify_error grew from 42 to 59 lines: §4.2 hard cap (50) at High stakes (file imported by 7 production modules registering ~50 tools), no ADR. CI's craftsmanship gate missed it; a green automated gate is not a verdict. Fix (mechanical, Fowler Extract Variable/Constant): hoist the missing_extension and database_not_connected keyword lists into module-level named constants and attach the 16-line rationale comment to the constants — drops the function well under the cap without losing content.

Non-blocking:

• The "truth-table" claim is not exhaustive: no such host, connection reset, timeout expired (3 of the 10 database_not_connected phrases) have no test — add them while touching the file.
• Pre-existing, untouched: bare "extension" substring in the missing_extension bucket is broad — awareness only.

[cdeust]

ZETETIC-REVIEW: APPROVE

Delta re-review of commit e74ee3a (same fresh-context reviewer as the prior REQUEST_CHANGES):

• §4.2 finding CLOSED: _classify_error re-derived independently at 22 lines (was 58). Clean Extract Variable — _MISSING_EXTENSION_PHRASES (3) and _CONNECTION_FAILURE_PHRASES (10) constants diffed byte-for-byte against the former inline lists: identical content, order, and comments. Behavior-preserving confirmed, not just claimed.
• Truth-table gap CLOSED: the three missing phrases (no such host, connection reset, timeout expired) added — 10/10 coverage of the constant.
• Baseline-prune AUDITED: the .craftsmanship-baseline.json diff removes exactly the one now-stale _classify_error method-size entry; the separate safe_handler entry and every other entry untouched; no threshold or config field changed. No rule configured around.
• CI: all checks green on e74ee3a, including Craftsmanship Gate and CI Green.

Green AND verified — mergeable.