Skip to content

fix(events): route aemit through dependency-aware dispatch - #8916

Open
xuan (yuee3) wants to merge 1 commit into
crewAIInc:mainfrom
yuee3:fix/aemit-dependency-path
Open

xuan (yuee3) wants to merge 1 commit into
crewAIInc:mainfrom
yuee3:fix/aemit-dependency-path

Conversation

@yuee3

@yuee3 xuan (yuee3) commented Nov 1, 2026 •

Copy link
Copy Markdown

Fixes #8913

Problem

CrewAIEventsBus.aemit() never consulted _handler_dependencies, so events emitted through the async entry point always bypassed dependency-aware dispatch and went straight to asyncio.gather over all registered async handlers. The other two dispatch entry points (emit() and replay()) both check has_dependencies and route to _emit_with_dependencies(); aemit() was the only one that did not.

Consequences for events emitted via aemit():

  1. Async depends_on chains execute concurrently and out of order instead of level by level.
  2. Sync handlers that async handlers depend on are never executed at all, while the dependent async handlers still run.

Fix

In aemit(), read has_dependencies inside the existing r_locked() block (same pattern as emit()), and when true, call await self._emit_with_dependencies(...) directly (aemit already runs on the event loop thread, so no run_coroutine_threadsafe is needed). The no-dependency behavior is unchanged: only async handlers run, concurrently.

Relation to #8716

Completely independent: #8716 fixes mixed sync/async handler future tracking in emit() (flush() semantics). This PR only touches aemit()'s dispatch path — a different function and a different failure mode.

Verification

  • New tests in lib/crewai/tests/events/test_depends.py:
    • test_aemit_respects_async_dependency_chain — async dependency chain executes in depends_on order under aemit (failed before the fix, passes after).
    • test_aemit_runs_sync_dependency_handlers — sync handler that an async handler depends on is executed under aemit (failed before the fix, passes after).
    • test_aemit_ignores_dependencies renamed to test_aemit_without_dependencies_runs_async_only to reflect preserved no-dependency semantics.
  • test_depends.py: 12/12 passed.
  • Full tests/events suite: only pre-existing environment failures (litellm not installed, lancedb ColumnOrdering import error); the failure set is identical with and without this change (29 failed / 166 passed both ways, same files), so no regressions.

Note

Medium Risk
Changes core event dispatch ordering for async emit paths; wrong ordering could affect listeners that rely on depends_on, but the change aligns with existing emit/replay behavior.

Overview
CrewAIEventsBus.aemit() now matches emit() and replay() when handlers use depends_on: it checks _handler_dependencies and awaits _emit_with_dependencies() instead of always asyncio.gather-ing async handlers.

That fixes async depends_on chains running out of order under aemit, and sync prerequisite handlers being skipped while dependents still ran. When no dependencies are registered, behavior is unchanged—only async handlers run, concurrently.

Tests rename the old “ignores dependencies” case and add coverage for async chains and sync→async dependency under aemit.

Reviewed by Cursor Bugbot for commit 2bc43f5. Bugbot is set up for automated code reviews on this repo. Configure here.

aemit() never consulted _handler_dependencies, so events emitted via
the async entry point bypassed dependency-aware dispatch: async
depends_on chains ran concurrently out of order, and sync handlers that
async handlers depended on never executed. Route aemit through
_emit_with_dependencies when dependencies are registered, matching the
emit() and replay() dispatch paths.
@strix-security

strix-security Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Strix Security Review

✅ No open security findings.

PR summary: Makes the async event-bus entry point `CrewAIEventsBus.aemit()` honor `depends_on` handler registrations, routing dependency-aware events through the same level-by-level dispatcher that `emit()` and `replay()` already use, while leaving the no-dependency async-only path unchanged. Adds two tests covering async dependency ordering and sync dependency execution under `aemit`.

Review summary

The diff is a 10-line dispatch fix in CrewAIEventsBus.aemit() plus tests: async emits now consult _handler_dependencies and route dependent event types through _emit_with_dependencies(), matching emit() and replay(). I traced the new call path (locking, executor initialization, level-by-level sync/async execution), checked for duplicate or re-entrant handler execution, event-loop blocking, and whether the newly-executed sync handlers reach any privileged side effect, and confirmed that _handler_dependencies is populated only by explicit developer depends_on registrations (none exist in library code) and that aemit has no production callers — so no attacker-controlled input reaches the changed dispatch logic. No security issues were found in the changed code.

Ruled out (2)
  • Can calling `_emit_with_dependencies()` from `aemit()` on the event-loop thread deadlock, block the loop, or race the runtime lock/executor?
    The dependency dispatcher never uses run_coroutine_threadsafe or awaits a future from the loop thread (sync levels are submitted to `_sync_executor` and awaited via `run_in_executor`), the read lock is released before the call and `RWLock` read re-acquisition cannot block, and `_ensure_executor_ini…
  • Do the sync handlers newly executed by `aemit()` cause duplicate execution, swallowed-control bypass, or privileged side effects on attacker-controlled events?
    The dependency branch returns before the pre-existing `_acall_handlers` call and registration keeps a handler in exactly one of the sync/async sets, so nothing runs twice; `_handler_dependencies` is only populated by explicit `depends_on=` registrations, of which there are none in library code, and…

Review 1 on this PR · 2bc43f5 · requested Nov 1 13:54 UTC when the PR was opened


Reviewed by Strix
Re-run review · Configure security review settings

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] aemit() bypasses the dependency-aware dispatch path: sync dependencies never run and async depends_on chains execute out of order

1 participant