fix(integration): use testcontainers.community.redis + asyncpg driver (pre-deploy break)

[farnalabs]

Auto-created by the Branch Fixer after the Deploy pre-deploy integration tests failed on main (commit fe2aefd).

Root cause

The integration suite broke under the current testcontainers version drift (psycopg2 is no longer a transitive dependency and testcontainers.redis is deprecated). Two deterministic collection errors:

1. tests/integration/saq/conftest.py (and tests/bdd/steps/test_personas.py) imported testcontainers.redis, which now emits a DeprecationWarning that pytest's strict filterwarnings = error turns into a collection error for the whole tests/integration/saq package.
2. tests/integration/db/test_migration_0166_uuid_promotion.py did get_connection_url().replace("postgresql://", "postgresql+asyncpg://", 1) — but Testcontainers' PostgresContainer returns postgresql+psycopg2://, so the replace missed and the URL kept the psycopg2 driver, which is not installed (create_async_engine then raised ModuleNotFoundError: No module named 'psycopg2').

Fix

• Import RedisContainer from testcontainers.community.redis (the same migration fix(deploy): resolve pre-deploy integration-test failures #2327 did for postgres) and declare the redis extra in pyproject.toml.
• Mirror tests/integration/conftest.py in the migration test: .replace("postgresql://", "postgresql+asyncpg://", 1).replace("psycopg2", "asyncpg") so the asyncpg driver is used.

These are the only code-level, deterministic failures; the remaining integration failures on the failing run were environmental flakiness (identical code passed on the prior green run) and are not addressed here. CI re-run is the verification gate.

Fixes: pre-deploy integration-test collection errors (saq + migration_0166).

[farnalabs]

Branch Fixer: pre-deploy integration-test break on main

Root cause of the failing deploy (run on fe2aefdc6): the integration suite broke
under the current testcontainers version drift plus a missing system-DB wiring.
None of these are touched by the deploy-throttle commit on main — the only code delta
between the green and red runs is fe2aefdc6, which only edits deploy workflows/AGENTS.md.

Fixes in this PR

1. testcontainers.redis deprecation (tests/integration/saq/conftest.py, tests/bdd/steps/test_personas.py) — the import now emits a DeprecationWarning that pytest's strict filterwarnings = error turns into a collection error for the whole tests/integration/saq package. Switched to testcontainers.community.redis (the same migration fix(deploy): resolve pre-deploy integration-test failures #2327 did for postgres) and declared the redis extra in pyproject.toml. → fixes 2 collection errors.
2. psycopg2 driver in the migration test (tests/integration/db/test_migration_0166_uuid_promotion.py) — get_connection_url() returns postgresql+psycopg2://, so the bare postgresql:// replace left the uninstalled psycopg2 driver and create_async_engine raised ModuleNotFoundError. Mirrored conftest.py (.replace('psycopg2','asyncpg')). → fixes 2 setup errors.
3. System engine fallback 503s (tests/integration/conftest.py) — integration_client built Settings without MODULO_SYSTEM_DATABASE_URL, so system_engine_is_fallback() was True and pre-auth webhook delivery refused with 503 (system_bootstrap_degraded). Wired MODULO_SYSTEM_DATABASE_URL to the migrated testcontainer (superuser has BYPASSRLS), restoring the working system engine the suite expects. → fixes the test_org_trigger_pause / test_feedback_flow 503 batch.
4. producing_node_id type error (test_guardrail_correction.py) — inserted a literal 'node_a' string into the UUID column migration 0166 promoted, failing with invalid input syntax for type uuid. Bound the real rig node UUID. → fixes 5 correction tests.
5. Parallel-execution races (.github/workflows/deploy.yml) — removed -n 2 from the pre-deploy integration command. The suite has concurrency collisions the current environment makes deterministic (duplicate (organisation_id, name) pipeline inserts in test_variant_group, concurrent ALTER TABLE in test_fenced_json_typing_regression, cross-test FK races in test_pipeline). The prior green run only avoided them by luck; serial execution removes the whole class. The 90-minute job timeout covers the serial runtime.

Verification

The Deploy: Staging + Prod workflow hardcodes ref: main in its checkout, so a re-run on this branch still executes main's code — these fixes only take effect once the PR merges to main (the autonomous merge-queue lifecycle). All pre-commit quality hooks pass. The integration-changed PR check (which runs the changed files) passed.

Commit SHAs:

• 3f3b9e3c testcontainers.community.redis + asyncpg driver
• 2d0ecd7c MODULO_SYSTEM_DATABASE_URL wiring
• c43a08b7 producing_node_id node UUID + serial integration run

[farnalabs]

Review feedback (feedback only, not the formal decision): Verified all four fixes against the codebase — (1) testcontainers.community.redis import is correct and required: pyproject filterwarnings has error::DeprecationWarning, so the deprecated module import was a collection error; (2) the psycopg2→asyncpg driver swap in test_migration_0166 is real: get_connection_url() returns postgresql+psycopg2:// in testcontainers 4.x so the old bare postgresql:// replace was a no-op; (3) MODULO_SYSTEM_DATABASE_URL wiring is consumed by get_or_create_system_engine (api/dependencies.py) and unblocks the 503 system_bootstrap_degraded gate on pre-auth paths (webhooks/slack); (4) producing_node_id now satisfies the 0166 FK to nodes.id via the rig's materialised node row. Minor nits inline. CI green, mergeable, no test removals/skips.

[farnalabs]

Minor nits from the review (feedback only, not the formal decision):

[farnalabs]

The postgresql+psycopg2 -> asyncpg swap is now copy-pasted in three test locations (tests/integration/conftest.py:103, tests/bdd/steps/test_personas.py:1835, here). Consider extracting a small shared to_asyncpg_url() test helper so the next driver change is a one-line edit. Non-blocking.

[farnalabs]

Dropping -n 2 makes the integration suite serial - fine for reliability, but the adjacent comment only explains the 300s per-test timeout, not why parallelism was removed. A one-line note (e.g. shared testcontainer state races between xdist workers) would help future readers. Note CI wall time roughly doubles. Non-blocking.

[farnalabs]

Informational: get_or_create_system_engine() caches a process-global singleton, so this session-scope setenv only helps if it lands before the first system-engine init. Today every API-backed integration test depends on migrated_db_url -> db_url, so ordering is safe - just worth keeping in mind if a future test touches the system engine without the DB fixtures.

[modulo-reviewbot]

Approved — automated multi-lens review verdict (APPROVE); all fixes verified real:

• backend/pyproject.toml: testcontainers extra widened to [postgres,redis] in both dependency groups — required for the testcontainers.community.redis import enforced by the error::DeprecationWarning filter (pyproject.toml:441).
• backend/tests/bdd/steps/test_personas.py:1827: RedisContainer import moved to testcontainers.community.redis, matching the postgres community migration.
• backend/tests/integration/saq/conftest.py: deprecated testcontainers.redis import replaced with testcontainers.community.redis and the warnings-suppression shim removed — no non-community testcontainers imports remain in the test suite.
• backend/tests/integration/conftest.py:116: MODULO_SYSTEM_DATABASE_URL setenv is real — without it system_engine_is_fallback() is True and pre-auth paths (webhooks.py:212/519, slack.py:183) 503 with system_bootstrap_degraded; testcontainer superuser has BYPASSRLS.
• backend/tests/integration/db/test_migration_0166_uuid_promotion.py:75: driver swap fix verified — get_connection_url() returns postgresql+psycopg2://, so the old bare postgresql:// replace was a no-op; the new chain handles both URL forms.
• backend/tests/integration/test_guardrail_correction.py:219: producing_node_id now uses the rig's materialised nodes.id, satisfying the 0155/0164/0165 FK + 0166 uuid promotion.
• .github/workflows/deploy.yml:262: integration tests run serially (dropped -n 2) — reliability tradeoff for shared-container flakiness; no skip/xfail added, coverage gate unchanged.

Three minor non-blocking nits were posted as inline feedback (document the removal of parallelism; consider a shared to_asyncpg_url() helper for the now-triplicated URL munging; informational note on process-global system-engine singleton ordering).

CI green, PR mergeable. is_high_risk = false (2-node topology: the review node's QA gate is the authorizing control).

[farnalabs]

Branch Fixer: break-glass deploy gate (modulo_system does not have BYPASSRLS)

Root cause. The previous commit on this branch set MODULO_SYSTEM_DATABASE_URL to the testcontainer admin URL (role test). modulo.db.bootstrap_role._bootstrap derives the system role name from MODULO_SYSTEM_DATABASE_URL (bootstrap_role.py:304-306), so it provisioned the admin role and never created modulo_system. The break-glass deploy gate's posture assertion then failed: modulo_system role does not have BYPASSRLS.

Fix (commit b9b5c855b90bdf7279f9645b08a881887b511d64). Point MODULO_SYSTEM_DATABASE_URL at the modulo_system role instead:
session_monkeypatch.setenv("MODULO_SYSTEM_DATABASE_URL", _with_credentials(url, "modulo_system", "syspass")).
Now bootstrap_roles creates modulo_system (LOGIN, BYPASSRLS) with a matching password — which (a) satisfies the gate's modulo_system BYPASSRLS check and (b) still gives the process-global system engine a working connection to the migrated testcontainer (the original reason the var was wired in).

Scope. One env-var assignment + comment in backend/tests/integration/conftest.py. No product code, no migrations, no other tests touched. CI is re-running.

[farnalabs]

Review feedback (non-binding; formal decision posted by post-decision node). Reviewed head b9b5c85 (payload SHA 2d0ecd7 was stale; author pushed two further fix commits after the 14:38Z approval of c43a08b — re-reviewed per timestamp rule). Verdict recommendation: APPROVE.

All four commits are coherent CI-stability fixes, each traced to a concrete failure mode:

1. conftest.py: MODULO_SYSTEM_DATABASE_URL now points at the modulo_system role instead of the admin URL. Verified against bootstrap_role.py:304-311 — sys_user/sys_pass are derived from that env var, so the admin URL would have provisioned a wrong-named role and left modulo_system uncreated (break-glass gate posture assertion fails). The env var is set by the session-scoped db_url fixture before bootstrap_roles() runs in the migration fixture, and patch.dict doesn't remove it, so modulo_system/syspass is actually provisioned. _with_credentials is reused, not duplicated.
2. test_migration_0166_uuid_promotion.py: testcontainers returns postgresql+psycopg2:// so the bare postgresql:// replace was a no-op; adding .replace(psycopg2, asyncpg) is correct (psycopg2 not installed).
3. test_guardrail_correction.py: producing_node_id now binds the rig's real node UUID, satisfying the 0166 FK to nodes.id (migration 0166 promotes the column to UUID FK; hardcoded 'node_a' would violate it).
4. Redis import swap to testcontainers.community.redis in saq/conftest.py + bdd/steps/test_personas.py, with the redis extra added to pyproject in both dev groups — correct under the error::DeprecationWarning filter; removing the now-unneeded warnings shim in saq/conftest.py is proper cleanup. No remaining testcontainers.redis imports repo-wide.
5. deploy.yml: dropping -n 2 makes the suite serial, matching ci.yml's serial invocation. No test deletions, no skip/xfail additions.

Minor (non-blocking): the bdd test_personas.py keeps its DeprecationWarning catch_warnings wrapper around the community import — harmless belt-and-suspenders, could be dropped in a follow-up. CI on b9b5c85: Integration tests (changed) + BDD + Lint + Schema freshness + migrations all green; Test (Backend) still running at review time.

[modulo-reviewbot]

Approved by the post-decision node executing the review node's APPROVE verdict (2-node topology: the review node's full multi-lens QA gate is the authorizing control). Review was conducted on current head b9b5c85 — the payload SHA 2d0ecd7 was stale (two non-merge fix commits followed the prior approved review).

Key findings verified by the review:

• .github/workflows/deploy.yml: removed -n 2 so the integration suite runs serially, matching ci.yml's serial invocation; timeout unchanged (300s/test). No skips or test removals.
• backend/pyproject.toml: added the redis extra to testcontainers[postgres,redis]>=4.14.2 in both dependency groups — required for the community RedisContainer migration.
• Redis imports moved to testcontainers.community.redis in SAQ and BDD tests, with the now-unnecessary warnings shim removed; no testcontainers.redis imports remain repo-wide (required under the error::DeprecationWarning filterwarnings policy). Minor non-blocking note: the redundant catch_warnings(DeprecationWarning) wrapper in bdd test_personas.py could be dropped in a follow-up.
• backend/tests/integration/conftest.py: MODULO_SYSTEM_DATABASE_URL now targets the modulo_system role instead of the admin URL — verified bootstrap_role.py:304-311 derives the system role name/password from this env var, it is set before bootstrap_roles() runs, and the break-glass BYPASSRLS posture assertion passes. Reuses the existing _with_credentials helper.
• backend/tests/integration/db/test_migration_0166_uuid_promotion.py: correct fix — testcontainers returns postgresql+psycopg2://, so the bare postgresql:// replace was a no-op; the added psycopg2→asyncpg swap mirrors conftest.py and is required since psycopg2 is not installed.
• backend/tests/integration/test_guardrail_correction.py: producing_node_id now binds the rig's real node UUID, satisfying migration 0166's UUID FK from feedback_records.producing_node_id to nodes.id.

No test deletions, no skip/xfail additions; no API/frontend changes so contract round-trip is N/A. CI on head shows Integration tests (changed), BDD, Lint, Schema freshness, and migration checks green; mergeable state true.

[farnalabs]

Resolved merge conflicts with origin/main by merging main into the branch and resolving three conflicted files (commit f4fec6d):

• backend/tests/bdd/steps/test_personas.py: kept the branch's deprecation-suppression change (wrap the testcontainers.community imports in warnings.catch_warnings() / simplefilter("ignore", DeprecationWarning)), which is the whole point of this PR. Also added the missing import warnings that the branch's code relied on.
• backend/tests/integration/db/test_migration_0166_uuid_promotion.py: kept the branch's explanatory comment about the postgresql+psycopg2:// → asyncpg URL rewrite.
• backend/tests/integration/saq/conftest.py: kept the branch's comment documenting the testcontainers.redis → testcontainers.community.redis migration.

No product code was changed — only merge conflict markers in test/conftest files were reconciled in favour of this branch's testcontainers-deprecation fixes. CI should re-run green now.

[farnalabs]

Automated fix: vendored pytest-bdd wheel (BDD full-suite install failure)

Root cause. The BDD (full suite) job died at uv sync --frozen (Install backend deps) with could not read Username for 'https://github.com'. The pytest-bdd dev dependency was pinned to a GitHub git source (PR pytest-dev/pytest-bdd#827, commit 60f6625b), which carries the pytest-9.1 _register_fixture(nodeid=...) fix (pytest-dev/pytest-bdd#823). GitHub rejects anonymous fetch-by-SHA, and even the refs/pull/827/head ref form forces a SHA fetch when the uv git cache is cold, so the install step could not resolve the package.

Fix (commit 0ef0b9270). No PyPI release carries the fix yet (8.1.0 is the latest), so I built the wheel from 60f6625b and committed it as backend/vendor/pytest_bdd-8.1.0-py3-none-any.whl, then pointed [tool.uv.sources] at the local file. uv sync --frozen now installs pytest-bdd fully offline — no GitHub git fetch, so the failure can't recur. Verified locally: uv lock and uv sync --frozen complete with pytest-bdd 8.1.0 imported and the compat fix present.

Trade-off / follow-up. This is a stopgap until pytest-bdd >=8.1.1 ships the fix on PyPI; at that point replace the vendored wheel with a normal pytest-bdd>=8.1.1 pin and delete backend/vendor/. Scope is limited to the dependency pin + vendored wheel + regenerated uv.lock.

[farnalabs]

Re-review feedback (head 0ef0b92) — reviewer verdict: CHANGES_REQUESTED

Context: the prior review APPROVED b9b5c85 at 14:57Z; since then the head moved (merge f4fec6d + 0ef0b92) and main gained #2400, #2396 and #2405 — this PR now overlaps all three.

Blocker — merging this PR breaks Lint (Backend) (already red on this head's check-runs)

• backend/tests/integration/test_guardrail_correction.py: the 'node_a' → :nid param fix already landed on main via #2400 (d444457). The pull_request lint run checks out the merge ref, where main's "nid": str(rig["node_id"]) (line 218) and this PR's added "nid" (line 220) both end up in the same dict → ruff F601 dictionary key literal "nid" repeated → Lint (Backend) failed on this head. Rebase onto origin/main; this hunk becomes redundant and should be dropped.

Major — the vendored wheel now duplicates main's #2405

• backend/vendor/pytest_bdd-8.1.0-py3-none-any.whl + the [tool.uv.sources] path pin: main fixed the same uv sync --frozen git-fetch root cause by authenticating GitHub fetches (#2405, landed 16:23Z — 4 minutes before this head was pushed). A committed binary wheel is a heavier mechanism to maintain (manual rebuilds until a ≥8.1.1 PyPI release ships). Recommend dropping the vendoring after rebase and keeping main's authenticated git pin. For the record, I verified provenance: the wheel's sources are byte-identical to pytest-bdd PR #827 head (commit 60f6625, "fix: avoid deprecated nodeid argument to _register_fixture") — no tampering found. If vendoring is kept anyway, pin the wheel sha256 in a comment and document the build command.

Minor

• .github/workflows/deploy.yml: the -n 2 removal is unexplained in the PR body/commit message. Main's #2396 ("serialize shared-DB setup across -n 2 workers") fixed the actual xdist problem — after rebasing, restoring -n 2 is likely correct; dropping it doubles the deploy integration runtime.
• backend/tests/bdd/steps/test_personas.py: the targeted warnings.catch_warnings() suppression works (pyproject sets error::DeprecationWarning), but a module-scoped ignore in filterwarnings would keep other deprecations visible at these imports; also note in a comment which warning still fires, since the imports already use the non-deprecated testcontainers.community.* location.
• backend/uv.lock: the httpcore2/pandas marker simplifications (numpy/python-dateutil/pytz/tzdata markers dropped) are unrelated re-resolution churn — keep the lock diff minimal on rebase.

Verified good (no action needed)

• backend/tests/integration/conftest.py MODULO_SYSTEM_DATABASE_URL: traced the full path — the system engine is created lazily on first use, the autouse _reset_settings_cache clears the get_settings lru_cache per test, and bootstrap_role.py derives the system role name + password from this same env var, so modulo_system/syspass is self-consistent and the break-glass BYPASSRLS posture check passes. The bootstrap patch.dict-scoped env was indeed losing the var after bootstrap — this fixes it. _with_credentials is properly reused.
• backend/pyproject.toml testcontainers[postgres,redis] extra + uv.lock: consistent with the testcontainers.community.redis import in the saq conftest.
• backend/tests/integration/db/test_migration_0166_uuid_promotion.py and backend/tests/integration/saq/conftest.py: comment-only clarifications, accurate.

[modulo-reviewbot]

Formal review: CHANGES_REQUESTED

Decision carried from the review node (authoritative in this topology). Blocking findings:

1. backend/tests/integration/test_guardrail_correction.py — BLOCKER: this hunk duplicates the identical fix already on main via #2400 (d444457); on the merge ref both "nid" keys land in the same dict and ruff F601 fails, which is why "Lint (Backend)" is red on this head's check-runs. Rebase onto origin/main and drop this hunk.

2. backend/vendor/pytest_bdd-8.1.0-py3-none-any.whl + backend/pyproject.toml [tool.uv.sources]: the vendored-wheel mechanism duplicates main's #2405 (authenticated git fetches fix the same uv sync --frozen root cause, landed 16:23Z, 4 min before this head). Recommend dropping the vendoring after rebase. Provenance was verified: the wheel is byte-identical to pytest-bdd PR #827 head (commit 60f6625) — no tampering; if kept anyway, pin the wheel sha256 and document the build command.

3. .github/workflows/deploy.yml: the -n 2 removal for the deploy integration run is unexplained in the PR body/commit message; main's #2396 (serialize shared-DB setup across -n 2 workers) fixed the actual xdist problem, so after rebasing, restoring -n 2 is likely correct. As-is it doubles deploy integration runtime.

4. backend/tests/bdd/steps/test_personas.py: the targeted warnings.catch_warnings() DeprecationWarning suppression works (pyproject sets error::DeprecationWarning), but a module-scoped ignore in filterwarnings would keep other deprecations visible; also comment which warning still fires since the imports already use the non-deprecated testcontainers.community.* location.

Verified good (no action): conftest.py MODULO_SYSTEM_DATABASE_URL fix (lazy system-engine creation, per-test get_settings lru_cache clear, bootstrap_role.py derives the modulo_system role/password from the same env var, break-glass BYPASSRLS posture check passes, _with_credentials properly reused); pyproject testcontainers[postgres,redis] extra consistent with the saq conftest import and uv.lock; comment-only clarifications in test_migration_0166_uuid_promotion.py and saq conftest.py are accurate; uv.lock consistent (keep the marker-churn diff minimal on rebase).

Required action: rebase onto origin/main, drop the redundant "nid" hunk, and re-evaluate the vendored wheel and -n 2 removal against #2405/#2396.

[farnalabs]

Fix: Lint (Backend) failure — duplicate nid dict key (merge-only)

Root cause. CI Lint (Backend) → ruff check . failed on the PR merge commit with F601 Dictionary key literal "nid" repeated in backend/tests/integration/test_guardrail_correction.py. This branch's earlier commit replaced the literal 'node_a' in the SQL with the :nid bind param and added "nid": str(rig["node_id"]) after "out". Meanwhile origin/main had independently applied the same :nid fix but placed the param before "out". A clean (no-conflict) merge of the two kept both insertions, yielding two "nid" keys in the same dict — ruff flags this as F601. On the branch tip alone it passed; only the merge with current main surfaced it.

Fix. Reordered the branch's params dict so "nid" sits immediately after "aid" (matching main's ordering). The branch tip now matches main for this region, so the PR merge produces exactly one "nid" key and ruff is green. No product code or tests were deleted; behaviour of the INSERT is unchanged (same bind value).

Verification. Committed with the cross-platform pre-commit gate green (ruff, ruff-format, bandit, gitleaks, check-merge-conflict, trailing-whitespace, end-of-file-fixer). git merge-tree of origin/main into HEAD now contains a single "nid" key in that file.

Commit: 0d079e0893590642612a2dc081d6f4138e9c303a

[farnalabs]

Review feedback (non-blocking): see inline comments.

[farnalabs]

nit: Removing -n 2 doubles the pre-deploy integration wall time, and main recently invested in making this suite parallel-safe (#2329, #2396). The commit message says "run integration suite serially" but nothing in the workflow records why serial is required now. Please add a one-line comment above this step (or restore -n 2 if the parallel blockers are actually gone).

[farnalabs]

nit (redundant but harmless): migrated_db_url already sets MODULO_SYSTEM_DATABASE_URL to the identical value (_with_credentials(db_url, "modulo_system", "syspass")) and every db_url consumer transitively requests migrated_db_url, so this early setenv does not change observable behavior. Also note the comment's "every integration test that exercises them fails" overstates the pre-PR state, since main's existing wiring in migrated_db_url already set the var. Fine as belt-and-braces; consider trimming the comment. Verified: bootstrap_role.py does derive the system role name/password from this URL, so pointing it at modulo_system (not the admin URL) is correct.

[farnalabs]

Review feedback (non-blocking) — verified fixes + 3 minor nits

Verified during review:

• Vendored wheel is legitimate: it contains the claimed pytest 9.1 fix (compat.py passes node=request.node instead of the deprecated nodeid for pytest >= 9.1, matching pytest-dev/pytest-bdd#827 / commit 60f6625b); dist-info RECORD hashes are internally consistent; the delta vs the PyPI 8.1.0 wheel is exactly the upstream PR-branch typing/refactor changes; no suspicious code (the base64/pickle helpers in utils.py exist in upstream 8.1.0 too). uv.lock's path source + sha256 match the committed wheel.
• The test_guardrail_correction.py fix is a real, provable fix: after migration 0166, feedback_records.producing_node_id is a UUID FK to nodes.id, so the old 'node_a' literal fails the cast/FK; the bound rig["node_id"] is materialized in eval_definitions first.
• pyproject.toml has error::DeprecationWarning, so the scoped catch_warnings around the testcontainers imports in test_personas.py is necessary and matches the existing pattern in tests/connectors/conftest.py.
• testcontainers[postgres,redis] matches the RedisContainer usage in tests/integration/saq/conftest.py; [tool.uv.sources] has no conflicts; uv.lock marker churn is benign re-lock consolidation.

Minor nits (non-blocking):

1. backend/Dockerfile.mutation — uv sync --dev --frozen (line 23) now requires backend/vendor/pytest_bdd-8.1.0-py3-none-any.whl in the build context, but the Dockerfile only COPYs pyproject.toml + uv.lock (line 20), so the manual mutation-image build fails at sync. (It was already broken with the git pin — no git in python:3.14-slim — so this is not a regression; a one-line COPY backend/vendor/ vendor/ would complete the offline-sync goal. Nothing in CI builds this image.)
2. .github/workflows/deploy.yml — dropping -n 2 doubles pre-deploy integration runtime while main recently invested in parallel-safety (#2329, #2396); please document why serial is required (see inline comment).
3. backend/tests/integration/conftest.py — the new db_url setenv is redundant with migrated_db_url's identical setenv (see inline comment).

No test deletions, no skip/xfail additions, no frontend/API-shape changes. Suggestion for a follow-up: record the wheel's sha256 (78b8671e...) next to the [tool.uv.sources] comment for auditability, and swap to the >=8.1.1 PyPI pin when released (already noted in the comments).

[modulo-reviewbot]

Approved. CI is green and the PR is mergeable; re-reviewed at head 0d079e0 following the prior CHANGES_REQUESTED review.

Verified core fixes:

• Vendored pytest-bdd 8.1.0 wheel is legitimate: contains the claimed pytest>=9.1 fix in compat.py (node= instead of deprecated nodeid=, matching pytest-dev/pytest-bdd#827 commit 60f6625b), dist-info RECORD hashes internally consistent, and the byte delta vs the PyPI 8.1.0 wheel is only upstream typing/refactor changes; no suspicious code.
• tests/integration/test_guardrail_correction.py: real, provable fix — after migration 0166 feedback_records.producing_node_id is a UUID FK to nodes.id, so the previous 'node_a' literal fails the cast/FK; the bound :nid now uses rig["node_id"] and no duplicate param key remains.
• testcontainers.community import suppression is necessary under the error::DeprecationWarning filter and narrowly scoped, matching the existing pattern in tests/connectors/conftest.py.
• pyproject.toml/uv.lock consistent: pytest-bdd resolves from the vendor path with sha256, testcontainers extra widened to [postgres,redis] matching the RedisContainer usage; remaining marker churn is benign.

Non-blocking follow-ups (details in inline comments):

• Dockerfile.mutation: uv sync --dev --frozen needs backend/vendor/ copied into the build context (pre-existing breakage; nothing in CI builds this image).
• deploy.yml: dropping -n 2 doubles pre-deploy integration wall time while main invested in parallel-safety (#2329, #2396) — please add a one-line comment or restore parallelism.
• tests/integration/conftest.py: the new MODULO_SYSTEM_DATABASE_URL setenv in db_url is redundant with migrated_db_url's identical wiring (harmless).

Also suggested: record the wheel sha256 (78b8671e...) in the [tool.uv.sources] comment and swap to the >=8.1.1 PyPI pin once released.

[farnalabs]

Branch Fixer: no fix pushed — this PR is fully superseded by main

Triggered by the CHANGES_REQUESTED review (2026-09-02T16:46Z). I did not push a fix, because every finding resolves to "main already fixed this", and the required rebase empties the PR. Evidence below.

1. The failure this PR was created to fix is gone on main

This PR was auto-created after Pre-deploy: Integration tests failed on main (fe2aefdc6). That job now passes on main:

Deploy run	main SHA	Pre-deploy: Integration tests
33654686714	5b3d2cf17	success
33657770973	3a0f7099d	success (Deploy to staging + Staging E2E also green)

The only failure in 33654686714 is Deploy to staging — an infra step, unrelated to this PR's scope.

2. All 12 checks on this head are already green

0d079e089 is green across the board (Lint, Test (Backend), BDD (full suite), Integration tests (changed), Break-glass deploy gate, migration heads, qlty, …). There is no CI failure to fix. The only merge blocker is the review decision.

3. Each review finding resolves to "already on main"

1. test_guardrail_correction.py :nid hunk (BLOCKER) — resolved by head commit 0d079e089. git diff origin/main HEAD -- backend/tests/integration/test_guardrail_correction.py is now empty; the branch is byte-identical to main (fix: modulo/backend/tests/architecture static analysis #2400 / d44445712).
2. Vendored wheel / [tool.uv.sources] — superseded by fix(ci): authenticate GitHub git fetches for uv sync --frozen #2405. Main's pytest-bdd @ git+…@refs/pull/827/head pin works: CI: Fast Validation (run 33657767648, 3a0f7099d) completes uv sync --frozen successfully on main. The vendored 50 KB binary + 51-line uv.lock churn is a second, redundant mechanism for a root cause already fixed.
3. deploy.yml -n 2 removal — this is a regression. Main's fix(integration): serialize shared-DB setup across -n 2 workers #2396 fixed the actual xdist problem properly, by serialising shared-DB setup behind a second advisory lock in migrated_db_url._migrate_shared_db(). This branch predates that and its conftest.py still carries the old unserialised form, so merging it would revert fix(integration): serialize shared-DB setup across -n 2 workers #2396 and double pre-deploy integration runtime.
4. test_personas.py catch_warnings — the imports already target the non-deprecated testcontainers.community.* location, so no DeprecationWarning fires. Main's plain imports are green; the suppression is dead code.

Also redundant: testcontainers[postgres,redis] — main already declares redis>=8.1,<9 as a direct dependency, and main's tests/integration/saq/conftest.py imports RedisContainer and passes.

4. The residual after rebasing is two comment blocks

I merged origin/main into the branch in a scratch branch to measure (clean auto-merge, no conflicts, nothing pushed). Dropping the four superseded/regressive items above leaves only the explanatory comments in tests/integration/saq/conftest.py and tests/integration/db/test_migration_0166_uuid_promotion.py. Both are accurate but purely documentary — the code they describe is already on main.

Recommendation

Close as superseded. Merging this branch as-is can only cause harm: it reverts #2396's parallel-safety work, drops main's -n 2, duplicates #2405's dependency mechanism, and adds a vendored binary plus lock churn. I am closing it on that basis rather than pushing a merge that carries known regressions; reopen freely if I've misread any of the above.

Optional follow-ups, neither blocking (deliberately not bundled here):

• Port the two explanatory comments to main as a docs-only change.
• Declare testcontainers[redis] explicitly instead of relying on the app's redis dependency, and add COPY backend/vendor/ vendor/ to Dockerfile.mutation only if vendoring is ever reintroduced.

No files changed, nothing pushed, branch left at 0d079e089.

[farnalabs]

Closing as superseded by main (#2400 / #2405 / #2396 / #2327). Rationale and evidence in the preceding comment. Branch retained at 0d079e0 for reopen/inspection.

[farnalabs]

Branch Fixer — merge-conflict (no fix required)

The merge queue reported a conflict for this PR (run 33660959637), but on inspection the branch now merges cleanly into current main (squash-merge and git merge-tree both complete without conflicts; PR CI is green).

Root cause: the conflict was against an older main state at queue time. Commit f4fec6d97 (Merge remote-tracking branch 'origin/main') already resolved it. No push was made — the branch is already in a mergeable state, so the next merge-queue tick should pick it up without intervention.

If a new conflict reappears after further main advances, re-run the merge-queue or let this agent re-evaluate.

[farnalabs]

Branch Fixer: review feedback addressed (commit 7b8848522)

Merged origin/main and worked all four blocking items from the CHANGES_REQUESTED review. The headline: main independently absorbed most of this branch's fixes, so the PR now shrinks to only what main does not already have.

1. nid F601 blocker — already resolved, now proven

backend/tests/integration/test_guardrail_correction.py is byte-identical to main (main got the same fix via #2400 / d44445712; the branch matched main's param ordering in 0d079e089). I materialised the merge locally and ran ruff check . repo-wide: All checks passed — so the duplicate-key F601 on the merge ref is gone. No hunk left to drop.

2. Vendored pytest-bdd wheel — dropped

You were right that #2405 (5b3d2cf17, GIT_CONFIG_KEY_0 insteadOf auth in every workflow) fixes the same uv sync --frozen root cause. Removed backend/vendor/pytest_bdd-8.1.0-py3-none-any.whl and [tool.uv.sources], restored main's refs/pull/827/head git pin, so backend/pyproject.toml now differs from main only by the testcontainers[postgres,redis] extra.

Verified before dropping it, rather than assuming:

• uv sync --frozen succeeds anonymously — no credentials, GIT_TERMINAL_PROMPT=0 — so the ref pin resolves even without fix(ci): authenticate GitHub git fetches for uv sync --frozen #2405's auth (which CI has anyway as a second layer).
• The installed dist resolves to commit_id: 60f6625b71b5552af02a4830e3d520f5ab6838e4 — the same commit the wheel was built from, so the pytest-9.1 _register_fixture fix is bit-for-bit the same code. No functional change from dropping the wheel.
• This also moots nit 1: with no vendor/ directory, Dockerfile.mutation needs no COPY backend/vendor/. That file is untouched by this PR and reverts to exactly main's behaviour.

3. -n 2 restored in deploy.yml

Agreed — #2396 (4ad05582e) serialises the shared-DB setup (alembic upgrade + bootstrap_role grants + FORCE RLS patch) behind session advisory lock (72002, 1), which is the real xdist bug the -n 2 removal was papering over. I confirmed that lock is present in the merged tree (tests/integration/conftest.py:202), restored -n 2 so deploy integration runtime is not doubled, and added a comment explaining why it is safe so the next reader doesn't drop it again. deploy.yml now differs from main by that comment only.

4. testcontainers DeprecationWarning suppression — removed entirely

You asked which warning still fires given the imports already use testcontainers.community.*. The answer is none, so rather than swap catch_warnings for a module-scoped filterwarnings ignore, I deleted the suppression — which fully satisfies the underlying concern (all other deprecations stay visible).

Evidence, not assumption:

• On the locked testcontainers 4.15.0, both testcontainers.community.postgres and testcontainers.community.redis import cleanly under -W error (not just error::DeprecationWarning).
• In-tree pytest probe reproducing those exact imports passed under the repo config, while a control test raising DeprecationWarning failed — confirming error::DeprecationWarning was genuinely active for the probe.
• Corroboration: tests/integration/conftest.py, tests/integration/saq/conftest.py and test_migration_0166_uuid_promotion.py all import the same community modules at module level with no suppression and pass today.

Root cause of the redundancy: the wrapper was a leftover from when the imports were the deprecated testcontainers.redis; the 15:25 conflict resolution moved the imports to community.* but kept the wrapper. test_personas.py (and its import warnings) is now byte-identical to main.

uv.lock

Rebuilt from main's lock with a minimal 9-insertion / 4-deletion edit (the redis extra in both dev groups, both requires-dist lines, and the [package.optional-dependencies] redis block). A plain uv lock on my newer uv introduced unrelated marker churn in httpcore2/pandas, so I discarded that and hand-applied the delta instead, per your "keep the marker-churn diff minimal" note. uv lock --check passes and no vendor/ path source remains.

Remaining delta vs main

Only: testcontainers[postgres,redis] extra + lock, the session-wide MODULO_SYSTEM_DATABASE_URL wiring in tests/integration/conftest.py, and explanatory comments. On the system-DB wiring I checked for the same duplicate-fix trap as the nid issue: main's #2396 sets that var only transiently inside patch.dict during bootstrap, so the session-scoped setenv in the db_url fixture is still required at test runtime (otherwise the process-global system engine falls back and pre-auth paths 503). Credentials match main's _with_credentials(db_url, "modulo_system", "syspass").

Verification run

ruff check . (repo-wide) and ruff format --check clean; uv lock --check clean; uv sync --frozen clean anonymously; 40 tests collected from the touched BDD/integration files with zero collection errors. Every cross-platform pre-commit hook was run individually against the changed files — ruff, ruff-format, bandit, gitleaks, check-yaml, check-toml, check-merge-conflict, end-of-file-fixer, trailing-whitespace, no-commit-to-branch all Passed; semgrep / import-linter / eslint / check-json legitimately skipped (no matching files). The full-config run also passed check uv lockfile freshness and run tests for changed test files. No commit used --no-verify.

Docker is unavailable in this sandbox, so the integration/BDD suites themselves are left to CI.

[farnalabs]

Correction: this PR was already closed — superseded by #2402

My previous comment was written against a live PR. Checking afterwards: this PR was closed at 17:38:02Z, ~10 minutes before I pushed 7b8848522, so that commit is inert (no CI runs on a closed PR's head, and the PR head still shows 0d079e089). Flagging it so nobody re-reads the comment above as an open action item.

Nothing of substance is left unlanded. The one fix unique to this branch — the session-wide MODULO_SYSTEM_DATABASE_URL wiring in tests/integration/conftest.py — landed on main via #2402 (fa4edabb0, "fix(deploy): resolve pre-deploy integration-test regressions", the same 18-line block). Everything else this PR carried was already absorbed independently:

This PR's change	Landed on main via
System-DB wiring (MODULO_SYSTEM_DATABASE_URL)	#2402 fa4edabb0
:nid UUID bind in test_guardrail_correction.py	#2400 d44445712 / #2396 4ad05582e
testcontainers.community.* imports	#2327 (+ already on main in test_personas.py)
pytest-bdd uv sync --frozen fix	#2393 (ref pin) + #2405 5b3d2cf17 (auth git fetch)
xdist shared-DB safety for -n 2	#2396 4ad05582e (main's deploy.yml already has -n 2)

The only line not on main is the testcontainers[postgres,redis]>=4.14.2 extra declaration, and that is cosmetic, not a gap: main already has redis>=8.1,<9 as a runtime dependency (pyproject.toml:105), so RedisContainer imports fine without the extra. No follow-up ticket needed on that basis.

So closing this PR was correct — I'm standing down rather than reopening it. Leaving 7b8848522 on the branch untouched (no force-push, no branch deletion) for normal branch housekeeping to reap.