fix(sqlite): isolate transactions per request (HC-CORTEX-002)

[cdeust]

Summary

Fixes the locally reproduced cross-request SQLite transaction interference tracked by HC-CORTEX-002.

The previous fallback store shared one process-wide connection with thread checks disabled. A concurrent acknowledged request could therefore commit unfinished writes from a rejected supersession. This change gives active execution threads separate native handles, tracks the exact handles used by a handler and nested offloads, finalizes unfinished work at the request boundary, quarantines failed cleanup, and releases non-anchor request handles. It also freezes relative database paths at construction and fails closed if the last named-memory keeper cannot roll back.

This is deliberately a draft. It proves the local correction and regression slice only. HC-CORTEX-002 remains pending until the preregistered SQLite ladder has run twice and the matched PostgreSQL reference cell has been published.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would change existing behavior)
• Refactor (no functional change; rules/coding-standards.md compliance)
• Documentation only
• Audit-finding closure: local regression slice for HC-CORTEX-002; capstone verdict still pending

Test plan

• All existing tests pass on every backend. SQLite/local suite passes; the PostgreSQL benchmark cell is unavailable on this host and remains mandatory.
• New tests added for concurrent supersede/insert isolation, reused workers, nested offload, unnamed handlers, cleanup failure, named-memory failure, connection lifecycle, relative paths, FTS/vector parity, integrity, foreign keys, and close/reopen persistence.
• Mutation survival check: adversarial review falsified and closed per-thread-only isolation, wrong-handle cleanup, no-op nested scopes, unnamed inline execution, rollback reuse, post-close reuse, per-request handle retention, relative-path drift, and silent named-memory recreation.
• Manual verification: deterministic baseline reproduced the rejected row, FTS row, and supersession edge before the fix; the corrected oracle observes zero rejected rows and one acknowledged row.

Frozen local evidence

• Base: 8f5ae3b
• Candidate: 9faa80d
• Focused regression files: 15 passed
• Affected SQLite/handler suite: 214 passed
• Full suite: 7,669 passed plus 123 subtests; two existing multiprocessing fork deprecation warnings; 214.17 seconds
• Ruff 0.16.0 format and lint: passed
• Pyright: zero diagnostics
• Craftsmanship gate: passed
• Documentation-claims gate: passed

Audit notes

• Engineering review: two independent read-only adversarial reviews; zero remaining blocker in the declared cross-request scope.
• Popper-style falsification: six successive counterexamples expanded the regression boundary before the diff was frozen.
• Outstanding deferred findings:
  • Client cancellation or transport loss is indeterminate because cancelling an asyncio offload does not stop its native worker. The benchmark must wait for quiescence and reconcile storage; it must never score this as a rejection.
  • Handler-wide atomicity for writes already committed by a store method requires a separate explicit unit-of-work design.
  • Direct manually-created threads outside the propagated request context remain retained until store close.
  • Two clean SQLite ladder runs and the PostgreSQL reference cell are required before this draft is ready.

Coding-standards compliance

• §2.2 Layer dependency direction preserved.
• §3.2 No untyped boundary added in production code.
• §4.1 No production file exceeds 500 lines.
• §4.2 No production function exceeds 50 lines.
• §4.4 No production function has more than four parameters.
• §7 Local reasoning preserved.
• §8 No unsupported numeric threshold added.
• §9 No dead code or untracked TODO added.

Breaking changes

None declared. Nested safe-handler scopes now fail closed because independent nested request transactions are unsupported; registered MCP handlers do not use that pattern.

Screenshots / logs

No UI surface changed. Raw benchmark artifacts will be published by harness-comparison after preregistration, not embedded post hoc in this source PR.

Reviewer checklist

• CHANGELOG.md updated.
• ADR-0055 added with sources, alternatives, consequences, non-claims, and verification obligations.
• No secrets, credentials, or PII in the diff.
• CI passes on the latest commit.

[cdeust]

HC-CORTEX-002 sealed evidence — readiness decision

The preregistered 18-cell protocol (2026-08-30-hc-cortex-002-v1, frozen 2026-09-01T17:31:11Z) has been executed and published as a sealed, independently verified release:

• Release: https://github.com/cdeust/harness-comparison/tree/main/artifacts/hc-cortex-002-release-20260901-r2 (seal VERIFIED, 203 content-addressed artifacts, raw-input set SHA-256 8e1a35799deb9e2bef382e935295cdb2ebdaa0f7c81ed9110bdc1cb437c1f762, recomputed byte-exactly by the read-only verifier)
• Dossier: https://github.com/cdeust/harness-comparison/blob/main/issues/Cortex/scalability/sqlite-transaction-isolation.md
• Baseline 8f5ae3b87b6969f3abcb3736859febfdab69304a, candidate 9faa80d3f36b1c7fd05edb4aca8202448a79fb27 (this PR's head), bound in the release's execution-manifest.json.

From scoring/scoring.json only:

Predicate	Result
studyVerdict	PASS
causalContrast	PASS, blocked (RED) -> proven (GREEN); baseline fails fault_rollback_state, fault_retry_choreography, marker_exactly_once_and_rejected_zero, memory_count, fts_count, vector_count, post_load_health_is_read_only; candidate fails none
candidateConformance	PASS, 17/17 required cells
SQLite ladder	two repetitions (r1, r2) at C1/C2/C4/C5, all proven
PostgreSQL reference (17.9)	two repetitions (r1, r2) at C1/C2/C4/C5, all proven
descriptiveSaturation	SQLite OBSERVED (onset C5, both repetitions); PostgreSQL NOT_OBSERVED up to C5 — descriptive only, no maturity score, no performance winner

The two readiness conditions this PR set for itself ("two clean SQLite ladder runs" and "the matched PostgreSQL reference cell") are met by the sealed evidence. Non-claims stay as declared: macOS host only, same-user local execution, no maturity score, no winner.

Decision: promoting this draft to ready-for-review. Merge still requires its own review verdict on this repository; the harness evidence closes the capstone condition, not the code review.

[cdeust]

ZETETIC-REVIEW: APPROVE

Revue de code — PR #452 « fix(sqlite): isolate transactions per request (HC-CORTEX-002) »

Commit de tête revu : c6503f7e (branche fix/hc-cortex-002-sqlite-transaction-isolation, merge de origin/main dans le candidat noté 9faa80d3 — mcp_server/ est octet-pour-octet identique à 9faa80d3, seul CHANGELOG.md diffère par résolution de conflit).

Verdict

APPROVE. Aucun constat bloquant. Deux issue-candidates non bloquants (dette préexistante, ni introduite ni aggravée par ce diff) et un nit couvert par une clause de non-garantie explicite de l'ADR-0055.

Périmètre revu

Diff git diff origin/main...HEAD (15 fichiers, 1293 insertions / 52 suppressions) :
mcp_server/handlers/request_transaction.py (nouveau), mcp_server/infrastructure/sqlite_connection_registry.py (nouveau), mcp_server/infrastructure/sqlite_request_scope.py (nouveau), sqlite_compat.py, sqlite_store.py, sqlite_store_entities.py, sqlite_store_receipts.py, sqlite_store_relationships.py, sqlite_store_stats.py, mcp_server/tool_error_handler.py, docs/adr/ADR-0055-sqlite-connection-per-execution-thread.md, CHANGELOG.md, et 3 nouveaux fichiers de test sous tests_py/infrastructure/.

Calibration des enjeux (Move 7)

Classification : High. Le diff touche la primitive de concurrence/transaction de la couche de stockage et le chemin de composition MCP central (tool_error_handler.py, traversé par les ~52 outils enregistrés). Critère : « Modifies … concurrency primitives (locks, transactions, async coordination) ». Profondeur appliquée : Moves 1–6 complets.

Vérification du câblage handler → production (exigée explicitement)

tool_error_handler.py::_run_coroutine_on_thread (ligne ~152-166) enveloppe loop.run_until_complete(handler_fn(args)) dans with handler_transaction_scope(): — import mcp_server.handlers.request_transaction.handler_transaction_scope (ligne 63).

safe_handler — la fonction unique par laquelle tous les enregistrements d'outils MCP passent (docstring : « Usage in tool registries: … result = await safe_handler(remember.handler, …) ») — appelle désormais systématiquement asyncio.to_thread(_run_coroutine_on_thread, handler_fn, args) :

• chemin nommé (tool_name fourni), ligne ~222-224 : result = await asyncio.to_thread(_run_coroutine_on_thread, handler_fn, args) sous le sémaphore d'admission ;
• chemin non nommé (ancien comportement : await handler_fn(args) en ligne, SANS offload ni isolation), ligne ~226 : remplacé par result = await asyncio.to_thread(_run_coroutine_on_thread, handler_fn, args).

Conclusion : le correctif est bien câblé jusqu'au chemin de production — vérifié par lecture complète du fichier tool_error_handler.py, pas seulement du diff, conformément au devoir de collecte de preuve. Ce n'est pas un mécanisme construit mais non appelé.

mcp_server/infrastructure/sqlite_connection_registry.py et sqlite_request_scope.py n'importent ni core/ ni handlers/ (stdlib uniquement + inter-import mutuel autorisé) → aucune violation de couche (docs/module-inventory.md § Dependency Rules). mcp_server/handlers/request_transaction.py importe infrastructure.sqlite_request_scope — sens handlers→infrastructure autorisé.

Constats détaillés (file:line, sévérité, confiance)

Bloquants

Aucun.

Should-fix

Aucun.

Nits

• mcp_server/infrastructure/sqlite_connection_registry.py:22-33 (méthode connection()) — sévérité : nit, confiance : moyenne. Fenêtre de course étroite entre la libération du verrou self._lock et l'appel (hors verrou) à register_request_connection(self, connection) : un close() concurrent du registre dans cette fenêtre pourrait faire enregistrer une connexion déjà fermée dans le ContextVar de la requête. Non reproduit par un test. Couvert explicitement par l'ADR-0055 (« Shutdown assumes the store is quiescent; closing a handle while a request is using it remains outside the lifecycle contract ») — accepté consciemment, pas une omission silencieuse. Suggestion : documenter ce cas directement dans le docstring de connection().

Issue-candidates (dette préexistante, ni introduite ni aggravée par ce diff — non bloquant, périmètre de revue = le diff)

• mcp_server/infrastructure/sqlite_store.py (1039 lignes au total) — sévérité : issue-candidate, confiance : haute (mesuré par wc -l). Très au-dessus du plafond local de 300 lignes (CLAUDE.md) et du plafond dur §4.1 (500). Dette préexistante : le diff n'ajoute/ne retire net que ~50 lignes localisées dans __init__/_maybe_load_vec/commentaires. Candidat naturel pour refactorer, hors périmètre de ce diff.
• Compromis de performance documenté par les auteurs — sévérité : issue-candidate, confiance : haute (lecture directe de _finalize_request_connections + ADR). release_request_connection est appelé pour chaque connexion touchée à chaque requête, y compris sur un thread de pool réutilisé — donc le motif « une connexion par thread » se comporte en pratique comme « une connexion par requête » pour les workers poolés (fermeture/réouverture sqlite3 native à chaque appel d'outil). Compromis assumé et documenté dans l'ADR-0055 (« Non-anchor request handles are then released, so inactive per-request executors do not accumulate connections » ; échelle de charge explicitement reportée — « this ADR does not invent a throughput threshold »). Recommandé de suivre via le banc de charge déjà annoncé (ADR § Verification) avant tout futur audit de performance ; hand-off Knuth si une régression de débit est mesurée.

Audit SOLID (Move 2) — synthèse

• DIP : sqlite_compat.py introduit le Protocol SqliteConnectionLike ; les mixins (SqliteEntityMixin, SqliteReceiptsMixin, SqliteRelationshipMixin, SqliteStatsMixin) typent _raw_conn sur ce Protocol plutôt que sur sqlite3.Connection concret — conforme §1.5/§5.1.
• LSP : ThreadLocalSqliteConnection reproduit fidèlement l'API sqlite3.Connection utilisée sans affaiblir de postcondition.
• SRP/OCP : chaque nouveau fichier a une responsabilité unique et nommée ; aucune branche conditionnelle par type ajoutée pour un cas spécial.

Adéquation des tests (Move 4)

15 tests neufs (4 dans test_sqlite_connection_registry.py, 8 dans test_sqlite_request_transaction.py, 3 dans test_sqlite_transaction_isolation.py), assertions fortes (comptages exacts de lignes, types/messages d'exception exacts, PRAGMA integrity_check/foreign_key_check, persistance après fermeture/réouverture). Chemins couverts : succès apparent avec transaction non validée rejeté ; travail imbriqué via asyncio.to_thread finalisé par sa poignée native exacte ; portées imbriquées de safe_handler refusées avant exécution du handler interne ; appels concurrents non nommés isolés ; échec de rollback → quarantaine + erreur originale préservée ; échec de rollback sur l'ancre en mémoire → registre invalidé ; chemin relatif figé contre un changement de cwd ; reproduction déterministe du scénario RED original de l'ADR (supersede rejeté concurrent à un insert acquitté).

Jugées mutation-résistantes par raisonnement qualitatif (pas de mutateur configuré dans le dépôt) : les assertions sur compteurs exacts (0/1) et types d'exception exacts résisteraient à des mutations plausibles (inversion de dirty > 0, inversion de preserve_anchor, permutation rollback/release).

Vérification adversariale (CR-4, avant tout APPROVE)

1. Faux positifs/sur-ajustement : sans objet (code de plomberie, invariants physiques vérifiés directement).
2. Cas manqués : fenêtre de course identifiée (voir nit ci-dessus), couverte par une clause de non-garantie explicite de l'ADR — pas une omission silencieuse.
3. Robustesse/entrées adverses : sans objet (aucune entrée externe non fiable dans ce diff).
4. Adéquation des tests : jugée forte (voir Move 4).

Aucun constat bloquant confirmé sur les quatre focales → APPROVE licite.

Move 0 — Réconciliation et anti-esquive (§13.2 + §14)

grep -i "pre-existing|unrelated|out of scope" sur le diff : toutes les occurrences de « unrelated » décrivent le défaut corrigé lui-même (opérations concurrentes non liées partageant une transaction), aucune n'écarte une défaillance constatée. Le CHANGELOG énonce honnêtement ce qui n'est pas couvert (banc de charge cross-backend, « tracked separately and is not claimed by this correction ») — déclaration de périmètre, pas esquive. Les deux contrôles passent.

Sécurité et hygiène (Move 6)

Aucune entrée utilisateur non validée dans ce diff (plomberie interne de connexion). Aucun secret, aucune donnée sensible journalisée. Portée de commit unique (isolation transactionnelle). ADR-0055 sourcé (citations précises de la documentation Python sqlite3 et sqlite.org pour chaque décision).

Sorties brutes des gates (exécutées dans le worktree /private/tmp/cortex-hc-cortex-002, HEAD = c6503f7)

$ git -C /private/tmp/cortex-hc-cortex-002 rev-parse HEAD
c6503f7e5180dcd81826a9feca14e302196a5d09

$ .venv/bin/python scripts/check_craftsmanship.py
Craftsmanship gate: OK

$ /opt/homebrew/bin/ruff check .
All checks passed!

$ /opt/homebrew/bin/ruff format --check .
1251 files already formatted

$ .venv/bin/python -m pytest -q -p no:cacheprovider \
    tests_py/infrastructure/test_sqlite_connection_registry.py \
    tests_py/infrastructure/test_sqlite_request_transaction.py \
    tests_py/infrastructure/test_sqlite_transaction_isolation.py
...............                                                          [100%]
15 passed in 1.70s   (exécution 1)
...............                                                          [100%]
15 passed in 1.39s   (exécution 2)
...............                                                          [100%]
15 passed in 1.26s   (exécution 3)
...............                                                          [100%]
15 passed in 1.19s   (exécution 4)

Aucune instabilité observée sur 4 exécutions consécutives des trois fichiers cibles (3 exigées, 1 exécution supplémentaire de contrôle).

$ .venv/bin/python -m pytest -q -p no:cacheprovider tests_py/infrastructure tests_py/handlers \
    -k "sqlite or request_transaction or tool_error"
........................................................................ [ 33%]
........................................................................ [ 67%]
....................................................................     [100%]
212 passed, 1906 deselected in 22.41s

Hand-offs

• refactorer — découpage de sqlite_store.py (dette de taille préexistante, hors périmètre de ce diff).
• Knuth — si le banc de charge annoncé dans l'ADR-0055 (« HC-CORTEX-002 load ladder ») révèle une régression de débit due au coût d'ouverture de connexion par requête.