[webmixgamer]

Summary

#3468 (36711e7) already fixed the 40 failures this issue named. Putting the pre-#3468 test_deploy_writable_templates.py back still fails 41 tests: ent666 ×21, ent477 git_refresh_hook ×10, ent477 definitions_endpoint ×6, retention_floor ×4. That is the issue's 40 plus one parametrize added since. #3468 never mentioned this issue, which is why the two weren't linked. This PR closes the class those 40 belonged to.

The class: a test evicts pkg.mod from sys.modules and imports it again. The re-import also rebinds the package attribute pkg.mod to the fresh copy. monkeypatch.delitem, patch.dict and every hand-written snapshot restore put back only the sys.modules entry, so the rest of the run holds two copies of one module. Which copy a caller gets depends on how it spells the import:

• from pkg.mod import x, importlib.import_module and mock.patch read sys.modules;
• from pkg import mod, import pkg.mod as m and monkeypatch.setattr("pkg.mod.x") read the attribute.

A later test patches one copy while the code reads the other, and fails only in a full run. This has happened five times: #3272, #3470, #2572, trinity-enterprise#752 and #2859. A probe over the whole unit island found 64 files that leave such a split.

Changes

• tests/module_split_guard.py (new):
  • scan() finds a split;
  • Guard repairs it;
  • unbind() drops a stale binding;
  • evict(monkeypatch, *names) is the eviction that leaves no split behind. A package takes its loaded submodules with it, so it can't reproduce test isolation: dev push unit suite red since #2487, 13 test_ent615_token_free_remotes failures from tests that reset services.git_service without its submodules #2859.
• tests/unit/conftest.py runs the Guard from pytest_runtest_setup and pytest_runtest_teardown wrappers. The teardown wrapper runs after the item's last teardown, module and class fixtures included.
  • A split whose sys.modules entry is still the pre-test module is repaired: the attribute goes back to its pre-test value.
  • A split where the test replaced the entry fails the file's last test. It is judged once that file's module fixtures have torn down, so a module fixture that restores its own copy (test_fleet_status_resilience) is not flagged. The message names the module and the fix.
  • Splits that exist before the first test (collection-time loaders, the fix(tests): test_audit_log_unit.py — 50 collection ImportErrors when run in full suite #762 class) are left alone. Healing them toward sys.modules broke 4 test_ent692_skill_sources_mcp tests, because the entry there is a test file's private copy.
  • TRINITY_SPLIT_GUARD=off switches the guard off. It is the control arm of the guard's own end-to-end test.
  • The conftest's own _POP_PREFIXES pops now also drop the backend package's binding. Before, from services import task_execution_service (used in pull_coordination_service.py) kept handing out the popped copy.
• tests/lint_sys_modules.py stops recommending raw monkeypatch.delitem. A delitem of a dotted or computed name is now flagged outside tests/unit/, where nothing repairs it (3 root files baselined), and the hint points at evict().
• Two files that left their own copy in sys.modules now use monkeypatch.setitem: test_session_operations (db.sessions) and test_audit_retention_prune (db.audit).
• test_deploy_writable_templates.py: fix(tests): repair tests and fixtures drifted behind merged changes (#3442, #3272) #3468's inline loop becomes evict().
• Test registry entry and a learnings fragment: "restore what the test changed, never pick a side".

The other 62 polluting files need no edit, because the guard repairs them:

The 62 files

test_1081_lease_reaper.py, test_1081_lease_sweep_exclusion.py, test_1081_physical_meter.py, test_1081_pull_endpoints.py, test_1083_db_seams.py, test_1083_lease_reaper.py, test_1115_schedules_summary.py, test_1199_agent_subscription_correlation.py, test_1449_backlog_metadata_scrub.py, test_1474_read_boundary_z.py, test_1482_write_params.py, test_1644_retention_guard.py, test_1704_git_service_plugins.py, test_1714_bulk_sweep_completion_events.py, test_1771a_retention_edges.py, test_1771c_schedules_analytics_edges.py, test_1771c_schedules_analytics_properties.py, test_1771c_schedules_cas_edges.py, test_1804_recovery_closes_activity.py, test_1804_terminal_activity_chain.py, test_1832_duration_clamp.py, test_1853_error_telemetry_salvage.py, test_2036_claude_settings_leak.py, test_2075_detect_git_dir.py, test_2193_token_stats_day_bucket.py, test_2329_pull_delivery_parity.py, test_2352_subscription_failure_kind_predicates.py, test_2392_effect_guard_fail_closed.py, test_2409_headroom_ranked_switch.py, test_2433_db_restamp.py, test_2434_duration_overflow.py, test_2443_headroom_off_loop.py, test_2467_turn_integrity.py, test_2524_fanout_real_schema.py, test_2842_2843_pull_claim_order.py, test_3114_pull_route_interactive.py, test_3114_pull_route_workspace.py, test_3127_pull_chat.py, test_3588_close_cancelled_claimed_activities.py, test_3589_claim_completed_at.py, test_471_subscription_usage_observability.py, test_837_whitelist_grant.py, test_847_entitlement_seam.py, test_882_completed_at_floor.py, test_chat_dispatched_marker.py, test_data_paths_allowlist.py, test_data_paths_gitignore.py, test_ec_backlog_admission_edges.py, test_ec_backlog_admission_properties.py, test_ec_idempotency_edges.py, test_ec_idempotency_properties.py, test_ec_pull_coordination_edges.py, test_ec_pull_coordination_properties.py, test_fleet_status_resilience.py, test_fork_to_own.py, test_github_init_gitignore.py, test_github_init_push.py, test_idempotency.py, test_reset_preserve_state_guardrails.py, test_schedule_soft_delete.py, test_self_hosted_git_url.py, test_slack_file_uploads.py

To find them again, run the island with TRINITY_SPLIT_GUARD=off and a small plugin that calls module_split_guard.scan() after each teardown.

Stated limits (documented in the guard's docstring):

• a module first imported inside a polluting test can keep references to the orphaned copy;
• a test that pops a module without restoring it, outside the conftest's own pops, leaves a dangling attribute;
• the root tests/ tier has no guard. The lint covers raw delitem there.

Test Plan

• Guard tests: cd tests && pytest unit/test_3390_module_split_guard.py, 30 passed. They cover:

  • the rules on private module tables;
  • evict() through the real import system;
  • the wiring end to end: the samples run in a subprocess under the real conftest, then again with the guard off, where the same victim fails.

• Lint tests: pytest unit/test_lint_sys_modules.py, 38 passed, 8 of them new. python tests/lint_sys_modules.py and python tests/lint_root_test_placement.py are both OK.

• Mutations, all red:

  • the conftest never heals;
  • the pop keeps the binding;
  • the heal ignores a pre-existing split;
  • a replaced entry gets healed;
  • no __name__ check;
  • no __spec__ check;
  • evict() without its family;
  • evict() without the attribute (41 failures with the guard off);
  • isinstance instead of type();
  • a verdict after every test instead of at module end.

• Real class: the pre-fix(tests): repair tests and fixtures drifted behind merged changes (#3442, #3272) #3468 polluter plus the 4 victim files, -p no:randomly: 121 passed with the guard, 41 failed with TRINITY_SPLIT_GUARD=off.

• Whole unit island:

  • seeds 12345, 67890 and 99999 (-n 5 --dist loadfile);
  • the issue's own sequential repro (-p no:randomly);
  • CI's exact command on the merged tree.

  Each run fails the same set as the clean-dev baseline: 6 failures and 4 errors from a stale local venv (missing opentelemetry.exporter, payments-py pin), with no guard errors and nothing order-dependent.

Fixes #3390

🤖 Generated with Claude Code

[dolho]

/review: approve, no critical findings

Scope: clean. The PR does what #3390 asks: it closes the whole class of a package attribute left pointing at a different module copy than sys.modules. It merges cleanly with current dev.

What was checked

Changed code	Tested by	Where it runs
module_split_guard.scan / Guard	TestGuard, and TestWiring, which runs a subprocess under the real conftest	tests/unit/conftest.py setup/teardown hooks
unbind / _drop_stale_binding	test_a_popped_backend_module_takes_its_package_binding_with_it	conftest._restore_unit_sys_modules
evict	test_evict_restores_both_halves, through the real import system	test_deploy_writable_templates.py
lint delitem rule	8 new cases in test_lint_sys_modules.py	CI lint and its baseline (a guard)

Results

• Tests, run locally: pytest unit/test_3390_module_split_guard.py unit/test_lint_sys_modules.py -p no:randomly: 68 passed. python tests/lint_sys_modules.py: OK.
• Mutations, both red:
  • A guard that never repairs a split fails 3 tests: test_heals_an_attribute_the_test_rebound, test_puts_back_an_already_split_attribute_the_test_moved, and the end-to-end TestWiring case.
  • A _drop_stale_binding that does nothing fails 1 test: the popped-binding test.

Non-blocking notes

1. Cost per test. Guard.after_test scans all of sys.modules after every test. With main imported that is about 2.2 ms per test (2.8k modules): about 65 s of CPU over the unit island, about 10–15 s of wall time under xdist. Acceptable as it is. A cheap way to cut it: skip the scan when sys.modules is unchanged in size and identity since the before snapshot.
2. A reported split stays in place. self._splits = current keeps a split the guard could not repair, so later files on the same xdist worker inherit it and can fail as a knock-on effect. The guilty file is still named, so this is noise rather than a missed split. It is worth one line in the docstring's stated limits.
3. The end-of-file verdict depends on test order (low confidence). last_in_module assumes a file's tests run one after another. That holds under CI's --dist loadfile with randomly's module shuffle. Under --dist load the verdict would blame a different test in the file, but it would not raise a false positive.
4. Lint baseline is off by one, not caused by this PR. --regenerate-baseline changes tests/unit/test_monitoring_dormant_skip.py from 2 to 1. That drift was already on dev.

No security, SQL, auth or enterprise-docs concerns: the diff touches tests only.

🤖 Generated with Claude Code

[webmixgamer]

@dolho On note 1, skipping the scan when sys.modules is unchanged since before would miss exactly the case the guard is for.

In the #3272 shape, monkeypatch.delitem plus a re-import plus the teardown undo leaves sys.modules identical: the same keys and the same entry objects. Only the parent package attribute moves. _split_guard_sample.py::test_1 asserts that state: sys.modules[_MOD] is original while package.mod is fresh.

A fingerprint of sys.modules alone can't see it, so the per-test scan stays. I agree the cost is acceptable.