fix(tests): re-emit events until reactive automation triggers#22517
fix(tests): re-emit events until reactive automation triggers#22517devin-ai-integration[bot] wants to merge 2 commits into
Conversation
The reactive automation integration test emitted a single burst of events 1s after creating the automation, assuming the trigger services had loaded it by then. Automations are loaded via a Postgres NOTIFY (or a periodic reconcile), and a reactive trigger only counts events that arrive after it is loaded. Under CI load the load can lag past the fixed 1s wait, so the burst is silently dropped and the automation never fires, timing out at 60s. Keep emitting events until the automation fires (bounded by the existing timeout) instead of relying on a fixed wait. Also catch builtin TimeoutError (what anyio.fail_after raises on 3.10) rather than asyncio.TimeoutError. Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e6747367c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await listener | ||
| except asyncio.TimeoutError: | ||
| async with get_events_client() as events: | ||
| while not listener.done(): |
There was a problem hiding this comment.
Await the listener result after it completes
If wait_for_event finishes with an exception, for example because the websocket disconnects before delivering the triggered event, listener.done() becomes true and this loop exits without ever awaiting or inspecting the task result, so the reactive automation test can pass despite never observing the automation event. Please await listener after the loop, or check the done result returned by asyncio.wait, so subscriber failures still fail the test.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in cdd117e. Added await listener after the emit loop so a subscriber failure (e.g. websocket disconnect) surfaces its exception instead of the test passing just because the task finished.
Co-Authored-By: bot_apk <apk@cognition.ai>
Summary
integration-tests/test_automation_assessments.py::test_reactive_automationflaked onmainwithTimeoutError(failed run, commit0926dad).Root cause — a load race, not the subscriber. The test created the automation, slept a fixed
1s(insidecreate_or_replace_automation), then emitted a single burst of 5 events. But a newly-created automation isn't necessarily loaded into the trigger services the instant the API call returns — loading happens via a PostgresNOTIFY(listen_for_automation_changes) or the periodicreconcile_automations(~5s). A reactive trigger only counts events that arrive after it is loaded (find_interested_triggersreturns nothing otherwise, and events are never replayed). Under CI load the load can lag past the1swait, so the burst is silently dropped, the automation never fires, and the listener times out at 60s.The subscriber side is not the culprit: the
/events/outwebsocket backfills matching events (wants_backfilldefaults toTrue), so thetriggeredevent would be delivered even if the subscription registered slightly late.Fix
Keep emitting the triggering events until the automation actually fires, instead of relying on a single burst landing after the automation is loaded (still bounded by the existing 60s
anyio.fail_after):Also catches builtin
TimeoutError(whatanyio.fail_afterraises on Python 3.10, the CI version) rather thanasyncio.TimeoutError— on 3.10asyncio.TimeoutError is not TimeoutError, so the oldexceptnever matched and the rawTimeoutErrorescaped (matching the CI traceback).Validation
Reproduced the exact CI failure and confirmed the fix by injecting an 8s automation-load lag into the server:
FAILED ... Reactive automation did not trigger in 60s(61.98s)1 passed in 3.46sWithout the injected lag:
test_reactive_automationpasses 20/20 (--flake-runs=20), and the fulltest_automation_assessments.pymodule passes (4 passed).Scope kept to the failing reactive test.
Checklist
Link to Devin session: https://app.devin.ai/sessions/16e3d30e206c4b3e9b80fca076a9d174