Repository navigation
The cross-tree module loader in the slm health test needs a manual stub for every new backend import #14538
Description
The failure shape
autobot-slm-backend/slm/agent/health_collector_state_change_test.py loads a backend module by file path and hand-stubs its imports:
stubs = {
"constants": MagicMock(),
"constants.ttl_constants": MagicMock(TTL_7_DAYS=...),
"autobot_shared.ssot_config": MagicMock(config=MagicMock()),
...
}
with patch.dict(sys.modules, stubs):
spec = importlib.util.spec_from_file_location(...)Every top-level import the backend module makes must appear in that dict. Add one over in autobot-backend, and this test fails with:
E ModuleNotFoundError: No module named 'services.gateway'
That reads as a broken checkout, not a missing stub — and it is worse than it looks, because under the slm rootdir services resolves to the slm package, so a module that exists perfectly well in its own tree is genuinely absent here.
Why it is worth fixing rather than living with
This already cost a full CI cycle on #14334: the failure appeared only in the sharded CI run, never from the repo root, so it was initially misattributed to #14111's shard flakiness. Reproducing it required knowing to run with the slm rootdir.
The cost lands on whoever edits notification_service.py next, who has no reason to know this file exists.
Options
- An import hook that auto-stubs any absent
services.*/constants.*during the load, rather than a hand-maintained list - Or make the loader insert the backend root on
sys.pathfor the duration, so the imports resolve for real - Or invert it: expose the enum and template the test actually needs from a module with no heavy imports, so no cross-tree load is required at all
The third is the smallest surface if the test only needs NotificationEvent.SERVICE_FAILED and render_template() — worth checking whether the cross-tree load is load-bearing at all.
Acceptance criteria
- Adding a new top-level import to
autobot-backend/services/notification_service.pydoes not require editing this test - A regression test demonstrates that: add a throwaway import, confirm the health test still passes
- If the loader survives, its failure names the stub list as the cause rather than surfacing a bare
ModuleNotFoundError
Provenance
Found while root-causing a red shard on #14334 (#14270). Confirmed by code review of the same PR, which agreed the stub added there is the right minimal fix for that test and that the mechanism itself is the debt.
Activity
test(slm): give cross-tree loader stubs a real __spec__ for Python 3.…
test(slm): fix redis health test pollution and the cross-tree stub lo…
mrveiss commentedon Aug 19, 2026
Delivered by PR #14576, squash-merged as
f1c42c7e7and verified inorigin/Dev_new_gui.The hand-maintained
sys.modulesstub dict is replaced by_exec_with_auto_stub, which execs the target and onModuleNotFoundErrorstubs only the exact missing name, scoped toservices/constantsroots and refusing anything under a caller-suppliedforbiddenprefix.The fail-open surface was reviewed explicitly and narrowed.
forbiddenoriginally defaulted to(), so a future caller who forgot it got no protection against stubbing its own subject under test — the safety was opt-in rather than default. It is now a required parameter with every call site updated. Cost nothing to close, and would have been invisible later.What remains inherent to the design, stated rather than hidden: any genuinely-missing
services.*/constants.*import in the transitive graph gets stubbed, including a typo or a real breakage. The mechanism cannot distinguish "absent because it belongs to the other backend" from "absent because someone broke it" — but the hand-list it replaces had the same blind spot, so this is not a regression. It is bounded (max_attempts=25), fails loud on an empty name, and restores every touched key to its exact prior value on all exit paths including a raising test.That last point matters more than it looks: this repo has a leak guard that fails an entire shard with zero test failures when a synthetic stub survives. An earlier iteration popped a real
autobot_shared.logging_managerand the guard caught it — which is evidence both the guard and the restore-to-exact-value design are working.Both copies remain byte-identical (
b2d85682878aa4ae), verified in base after merge. Nothing prevents that drifting again, and the drift this PR fixed was silent — misattributed to shard flakiness for a full CI cycle. That guard is filed as #14584, cross-linked to the canonical-source umbrella #13916.