From 255c43a8fecb5841099ee8d1b45f659be8b3c90b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 29 Aug 2026 23:28:03 -0400 Subject: [PATCH] fix(tests): the rule-history fixture must not delete a book the embedder is still writing (#3241) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration lane failed on a deadlock, not an assertion — 103 passed, and the one error was in teardown: `DELETE FROM rulebooks` blocked against another process holding a lock on a rule row. `update_rule` fires a detached asyncio.create_task(upsert_rule_embedding(...)) that opens its OWN connection and UPDATEs the rule it just saved. The teardown's rulebook delete cascade-locks that same row, and Postgres resolves the cycle by killing one of them. The sibling test_integration_rule_verification never hit this because it calls update_rule but never deletes its rulebook. Cleanup moves to setup, which runs on a fresh loop after the previous test's loop has closed and cancelled whatever it left in flight. That also keeps the #3240 constraint intact: no database call after a yield. Co-Authored-By: Claude Opus 5 --- tests/test_integration_rule_versions.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/test_integration_rule_versions.py b/tests/test_integration_rule_versions.py index 27475fd..ffd84bf 100644 --- a/tests/test_integration_rule_versions.py +++ b/tests/test_integration_rule_versions.py @@ -35,10 +35,13 @@ OWNER_USERNAME = "rule_history_owner" async def constraint(): """One rule carrying a check, with no history yet. - Cleanup is in this fixture's own teardown, never in an autouse one: - `_dispose_engine` arrives through usefixtures, so it tears down BEFORE an - autouse fixture would, and a database call after that point orphans a - pooled connection and breaks the NEXT test to touch Postgres (#3240). + CLEANED UP AT SETUP, NOT TEARDOWN, and that is forced. `update_rule` fires + a detached `asyncio.create_task(upsert_rule_embedding(...))` that opens + its own connection and UPDATEs the rule row. A teardown that deleted the + rulebook would race it: the delete cascade-locks the rule the embedding + task is writing, and Postgres kills one of them with a deadlock. Purging + at setup instead runs on a fresh loop, after the previous test's loop + closed and cancelled whatever it left in flight. """ async with async_session() as s: owner = await ensure_user(s, OWNER_USERNAME) @@ -60,13 +63,7 @@ async def constraint(): verify_with="read the workflow's shell setting", ) - yield {"uid": uid, "rule_id": rule.id} - - async with async_session() as s: - row = await s.get(Rulebook, book.id) - if row is not None: - await s.delete(row) - await s.commit() + return {"uid": uid, "rule_id": rule.id} async def _versions(rule_id: int) -> list[RuleVersion]: