#185 AF-289: reuse one injector in the event delivery worker

[Anonymous]

Originally created by: AnaniyaT

Build the event delivery worker's injector once per process so it stops opening a new database engine and pool for every message.

What's included

• api/domains/events/worker.py — lazily built, lock-guarded process-wide injector; both actor helpers resolve from it. Importing the module still opens no engine. Adds reset_injector() for tests.
• api/tests/unit/test_event_delivery_worker.py — asserts the injector is built once across many messages and not at import.

Required repo config — none.

Notes

• No chart changes, so no chart version bump.
• Pool sizing, postgres max_connections and a TTL on the reconciler jobs are deliberately left for follow-ups.
• Verified against a real postgres: 500 messages through the old code left 13–18 client connections open until a full GC; the new code holds 1.

Testing

• make check-api
• make check-migrations
• make test-api — 1908 passed; test_email_connection_is_refused_when_the_environment_has_no_agent_email_domain fails only because the local .env sets AGENT_EMAIL_DOMAIN, and passes with it unset.

[Anonymous]

Originally posted by: dominykas-aai-labs

1. reset_injector() is public, unused, and re-creates the leak if anyone calls it.
api/domains/events/worker.py:40-44 drops the injector without engine.dispose(), so a production caller would strand exactly the pooled connections this ticket is about. It's also the only non-underscore helper in a module of _get_injector / _processor / _repository. And it's dead code: its single call site (test_event_delivery_worker.py:105) sits one line before importlib.reload(worker), which already re-executes _injector = None.

Pick one mechanism, not both — I'd delete it and keep the reload-based test, since nothing in production needs to reset the injector.

2. Move the reload inside the try.
test_event_delivery_worker.py:94 runs importlib.reload(worker) before try:. If that reload raises, the finally never runs and the worker module keeps the patched create_injector for the rest of the pytest session — monkeypatch teardown restores api.core.utils, not the name the module already imported. Move line 94 to the first statement inside the try.

3. Document the new handler-thread contract.
docs/features/domain-events.md "Delivery worker contract" (lines 90-104) is the authoritative doc, and AGENTS.md step 5 plus review-protocol item 2 require code, tests, and docs to move together when an invariant changes. Handlers being process-wide singletons shared across worker threads is a new rule every future handler author must follow. One sentence near the "Handlers use a formal interface" paragraph:

[Anonymous]

Originally posted by: dominykas-aai-labs

4. Replace the silent handler-registry fallback with no fallback. (pre-existing, but in the function this PR edits)

api/domains/events/worker.py:50-53 swallows every exception from injector.get(EventHandlerRegistry) with no log line and substitutes an empty registry. An empty registry raises UnknownEventHandlerError, which the processor treats as terminal — so a transient failure resolving the registry (it pulls AgentRepository → the engine, and EmailService → EmailClient) permanently dead-letters the delivery instead of retrying it. The line directly above, injector.get(OutboxMessageRepository), correctly lets the same class of failure propagate to dramatiq's retry. That inconsistency is the bug.

[Anonymous]

Originally posted by: AnaniyaT

Addressed in the second commit:

1. reset_injector() removed. The tests reload the module through a fixture instead.
2. The reload now happens inside the fixture's try/finally, so a failed reload still restores the real create_injector.
3. Added a sentence to the "Handlers use a formal interface" paragraph in docs/features/domain-events.md stating handlers are process-wide singletons shared across worker threads, with no per-delivery mutable state and their own sessions per call. Both existing handlers already comply.
4. The silent EventHandlerRegistry fallback is gone; a resolution failure propagates so dramatiq retries. Covered by test_processor_propagates_handler_registry_resolution_failure. The registry only depends on the two handlers, whose email dependencies raise on send rather than on construction, so resolving it can't fail on missing config.

make check-api, make check-migrations, make test-api (1909 passed; same single local-.env email-domain failure as before).