Skip to content

Implement bubus-rust for Rust users#9

Draft
pirate wants to merge 4 commits intomainfrom
codex/implement-bubus-rust-equivalent
Draft

Implement bubus-rust for Rust users#9
pirate wants to merge 4 commits intomainfrom
codex/implement-bubus-rust-equivalent

Conversation

@pirate
Copy link
Copy Markdown
Member

@pirate pirate commented Feb 20, 2026

Motivation

  • Provide a Rust parity of the Python/TypeScript typed-event contract so handlers and their return values have stronger compile-time guarantees.
  • Make it ergonomic to emit/find events and register handlers with Rust types for payload and result shapes.

Description

  • Added a new typed module with an EventSpec trait that declares type Payload, type Result, and const EVENT_TYPE, and a TypedEvent<E> wrapper around BaseEvent that decodes typed payloads and first-results.
  • Extended EventBus with typed APIs: emit_typed<E: EventSpec>, on_typed<E, F, Fut>(...) (accepts Fn(TypedEvent<E>) -> Future<Output = Result<E::Result, String>>), and find_typed<E: EventSpec>(...).
  • Exported the typed module in the crate root (src/lib.rs) and added a small helper payload_map_from_value.
  • Added tests/test_typed_events.rs to validate typed handler registration/emit roundtrip and typed find behavior.

Testing

  • Ran formatting and the full Rust test suite with cargo fmt and cargo test.
  • All tests including the new typed tests (tests/test_typed_events.rs) passed successfully.

Codex Task


Open with Devin

Summary by cubic

Implements a typed event bus in Rust with compile-time payload/result types and an async runtime that mirrors Python/TypeScript semantics. Removes raw emit and addresses runtime coordination around serial locks, queue wakeups, and find waiters to ensure correct ordering and completion.

  • New Features

    • Strongly typed API: EventSpec and TypedEvent with EventBus.emit/emit_with_options, on_typed, and find_typed.
    • Runtime: async queue with global-serial, bus-serial, and parallel modes; handler serial/parallel and all/first completion; queue-jump support.
    • Lineage and discovery: parent/child tracking; find supports past or future with optional parent filter; bounded history with drop or reject; timeouts with parent timeout canceling children.
  • Migration

    • Replace any raw emit with bus.emit::(TypedEvent::::new(payload)) or emit_with_options(..., queue_jump).
    • Register handlers with bus.on_typed::(...).

Written for commit 73d10ed. Summary will update on new commits.

@pirate pirate changed the title Add strict typed event/handler APIs for bubus-rust Implement bubus-rust for Rust users Feb 20, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44f134e256

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

inner.event_completed_at = Some(now_iso());
}
}
event.mark_completed();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Defer completion until all pending bus dispatches finish

process_event_inner decrements event_pending_bus_count and then unconditionally calls event.mark_completed(), so a BaseEvent emitted multiple times (for example to two buses, or twice on the same bus) is marked completed after the first dispatch finishes. That lets wait_completed() return while other dispatches are still executing and can even cause later handlers to move the status back to Started, which breaks completion semantics and can corrupt downstream coordination.

Useful? React with 👍 / 👎.

.event_path
.push(format!("{}#{}", self.name, &self.id[0..4]));
CURRENT_EVENT_ID.with(|id| inner.event_parent_id = id.borrow().clone());
CURRENT_HANDLER_ID.with(|id| inner.event_emitted_by_handler_id = id.borrow().clone());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Record child event IDs when emitting from a handler

cancel_children relies on EventResult.event_children to find descendants to cancel, but emit_with_options only stores parent metadata (event_parent_id / event_emitted_by_handler_id) and never appends the emitted child ID to the active handler result. In parent-timeout scenarios, this leaves every event_children list empty, so cancellation never propagates to emitted child events and they keep running after the parent has timed out.

Useful? React with 👍 / 👎.

pub fn first_result(&self) -> Option<E::Result> {
let results: HashMap<String, crate::event_result::EventResult> =
self.inner.inner.lock().event_results.clone();
for result in results.values() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make TypedEvent::first_result deterministic

TypedEvent::first_result iterates HashMap::values() and returns the first successful entry, but HashMap order is not stable. With multiple successful handlers for the same typed event, this method can return different results across runs, creating flaky behavior for consumers that expect a consistent “first” result.

Useful? React with 👍 / 👎.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 12 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="bubus-rust/src/event_bus.rs">

<violation number="1" location="bubus-rust/src/event_bus.rs:188">
P1: Custom agent: **Make sure READMEs, docs/, and examples/ are up-to-date with implementation**

The `emit` → `emit_raw` rename breaks the Quickstart example in `bubus-rust/README.md` (line 34: `bus.emit(event.clone())`). The new generic `emit<E: EventSpec>` in `typed.rs` takes `E::Payload`, not `Arc<BaseEvent>`, so the existing README snippet will not compile. Update the README to use `bus.emit_raw(event.clone())` or rewrite the example to use the typed API.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

}
}

pub fn emit_raw(&self, event: Arc<BaseEvent>) -> Arc<BaseEvent> {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Make sure READMEs, docs/, and examples/ are up-to-date with implementation

The emitemit_raw rename breaks the Quickstart example in bubus-rust/README.md (line 34: bus.emit(event.clone())). The new generic emit<E: EventSpec> in typed.rs takes E::Payload, not Arc<BaseEvent>, so the existing README snippet will not compile. Update the README to use bus.emit_raw(event.clone()) or rewrite the example to use the typed API.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/event_bus.rs, line 188:

<comment>The `emit` → `emit_raw` rename breaks the Quickstart example in `bubus-rust/README.md` (line 34: `bus.emit(event.clone())`). The new generic `emit<E: EventSpec>` in `typed.rs` takes `E::Payload`, not `Arc<BaseEvent>`, so the existing README snippet will not compile. Update the README to use `bus.emit_raw(event.clone())` or rewrite the example to use the typed API.</comment>

<file context>
@@ -185,11 +185,11 @@ impl EventBus {
 
-    pub fn emit(&self, event: Arc<BaseEvent>) -> Arc<BaseEvent> {
-        self.emit_with_options(event, false)
+    pub fn emit_raw(&self, event: Arc<BaseEvent>) -> Arc<BaseEvent> {
+        self.emit_raw_with_options(event, false)
     }
</file context>
Fix with Cubic

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

29 issues found across 25 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="bubus-rust/tests/test_eventbus_dispatch_defaults.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_dispatch_defaults.rs:25">
P1: Custom agent: **Make sure concurrency options work correctly and consistently**

This test doesn't actually verify bus default handler settings. It sets explicit event-level overrides (`Some(EventHandlerConcurrencyMode::Serial)` and `Some(EventHandlerCompletionMode::All)`) which bypass the bus-default fallback logic in `process_event_inner` (the `.unwrap_or(self.event_handler_concurrency)` path is never hit). Additionally, with only one handler, the assertion `event_results.len() == 1` passes regardless of concurrency/completion mode.

To test bus defaults, remove the event-level overrides (leave them as `None`) and register multiple handlers to distinguish between `All` vs `First` completion and `Serial` vs `Parallel` concurrency behavior.</violation>
</file>

<file name="bubus-rust/src/typed.rs">

<violation number="1" location="bubus-rust/src/typed.rs:54">
P2: `HashMap` iteration order is non-deterministic in Rust. `first_result()` claims to return the "first" result, but with multiple successful handlers it will return an arbitrary one. Consider using an `IndexMap` or a `BTreeMap` (keyed by something orderable like insertion index or timestamp) to preserve insertion order, matching Python's `dict` ordering semantics.</violation>

<violation number="2" location="bubus-rust/src/typed.rs:94">
P1: Custom agent: **Make sure concurrency options work correctly and consistently**

`find_typed` drops the `child_of` parameter, hardcoding it to `None`. This makes it impossible for typed API users to find events by parent-child relationship — a core feature for nested event tracking and concurrency correctness. The `child_of` parameter should be exposed, accepting an `Option<TypedEvent<_>>` or `Option<Arc<BaseEvent>>`.</violation>
</file>

<file name="bubus-rust/src/lock_manager.rs">

<violation number="1" location="bubus-rust/src/lock_manager.rs:7">
P0: Custom agent: **Make sure concurrency options work correctly and consistently**

`ReentrantLock` wraps `parking_lot::Mutex` which is **not** reentrant — it will deadlock if the same thread re-acquires it. This lock is used for `GLOBAL_SERIAL_LOCK` and `bus_serial_lock` in the event bus's serial concurrency modes. When a handler in `BusSerial`/`GlobalSerial` mode emits and awaits a child event (queue-jumping), re-entering this lock on the same thread will deadlock instead of re-entering correctly.

Use `parking_lot::ReentrantMutex<()>` instead of `parking_lot::Mutex<()>` — it explicitly supports recursive locking from the same thread without deadlocking, which is what the struct name promises and what the concurrency model requires.</violation>
</file>

<file name="bubus-rust/src/base_event.rs">

<violation number="1" location="bubus-rust/src/base_event.rs:21">
P1: Bug: `now_iso()` does not produce ISO 8601 timestamps. It formats raw epoch seconds and nanoseconds (e.g. `"1708444800.123456789Z"`), not a proper ISO 8601 datetime (e.g. `"2024-02-20T12:00:00.123456789Z"`). This breaks cross-language compatibility with the Python/TS implementations which use real ISO datetime strings.

Consider using the `chrono` crate (or `time` crate) — e.g. `chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Nanos, true)`.</violation>

<violation number="2" location="bubus-rust/src/base_event.rs:112">
P2: Custom agent: **Make sure concurrency options work correctly and consistently**

`mark_completed()` calls `self.completed.notify(usize::MAX)` while still holding the `parking_lot::Mutex` guard. All woken tasks in `wait_completed()` will immediately contend on this lock, causing unnecessary contention in high-throughput scenarios. Drop the guard before notifying to avoid the thundering-herd contention window.</violation>
</file>

<file name="bubus-rust/src/event_bus.rs">

<violation number="1" location="bubus-rust/src/event_bus.rs:105">
P1: Custom agent: **Make sure concurrency options work correctly and consistently**

**Memory leak**: `start_loop` spawns a thread that captures `bus: Arc<Self>`, creating a strong reference that only releases when `stop()` is called. If users let the bus fall out of scope without calling `stop()`, the thread keeps running forever holding the `Arc`, preventing the bus and all its state (handlers, event history, queued events) from being dropped. The rule requires memory to be freed automatically without manual `stop()` calls. Consider using a `Weak<Self>` reference in the spawned thread and breaking the loop when the weak reference can no longer be upgraded.</violation>

<violation number="2" location="bubus-rust/src/event_bus.rs:109">
P1: Bug: The event loop sleeps 1ms when the queue is **not** empty, adding unnecessary latency before processing every queued event. This condition appears inverted — sleeping should occur when the queue is empty (if at all), not when there's work to do. Since the empty-queue case is already handled by `queue_notify.listen().await` below, this entire sleep block can be removed.</violation>

<violation number="3" location="bubus-rust/src/event_bus.rs:205">
P1: `emit_with_options` sets `event_parent_id` and `event_emitted_by_handler_id` on the child event, but never records the child's event ID in the parent handler's `EventResult.event_children`. Since `cancel_children` iterates `result.event_children` to find descendants to cancel, the list is always empty and cancellation never propagates to child events on parent timeout. After setting the parent metadata, the emitted child's ID should be appended to the current handler's `EventResult.event_children` in the parent event.</violation>

<violation number="4" location="bubus-rust/src/event_bus.rs:229">
P2: TOCTOU race: `history_order` is locked separately for `len()` and `pop_front()` in the same loop iteration. Another thread can modify the deque between these two lock acquisitions. Use a single lock guard for both operations.</violation>

<violation number="5" location="bubus-rust/src/event_bus.rs:293">
P1: Bug: `?` operator causes `find_in_history` to abort the entire search if any event ID is missing from the events map. If an event was evicted from the map but its ID is still in `history_order`, this returns `None` prematurely instead of continuing to check subsequent entries. Replace `?` with a guard that skips the missing entry.</violation>
</file>

<file name="bubus-rust/README.md">

<violation number="1" location="bubus-rust/README.md:22">
P2: Quickstart example won't compile: `event_bus::new(...)` and `base_event::new(...)` are not free functions. `new` is an associated function on `EventBus` and `BaseEvent` respectively. The imports and constructor calls need to reference the struct types directly.</violation>
</file>

<file name="bubus-rust/src/event_result.rs">

<violation number="1" location="bubus-rust/src/event_result.rs:21">
P2: Field `timeout` should be named `handler_timeout` to match the TS/Python JSON schema (which uses `handler_timeout`). As-is, JSON serialization will produce `"timeout"` instead of `"handler_timeout"`, breaking cross-language event JSON compatibility. Additionally, this field is redundant with the already-present `self.handler.handler_timeout`.</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_timeout.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_timeout.rs:105">
P1: Custom agent: **Test quality checker**

Weak assertion `assert!(has_error || is_completed)` makes this test a no-op: it passes regardless of whether the parent timeout actually cancelled the child. A child that completed successfully (no error) means the timeout propagation **didn't work**, yet the test would still pass. Per the test quality rule, tests must assert that side effects and output values are *behaviorally correct* — not just that some result exists. The assertion should specifically verify the child has an error result with the expected abort/cancellation status, matching what the Python and TS tests enforce.</violation>
</file>

<file name="bubus-rust/tests/test_event_history_store.rs">

<violation number="1" location="bubus-rust/tests/test_event_history_store.rs:17">
P1: Custom agent: **Test quality checker**

These history store tests have weak/trivial assertions that don't verify behavioral correctness. The rule requires that "tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct" — not just that counts match or that UUIDs contain hyphens.

Specific issues:
1. `assert!(history.iter().any(|id| id.contains('-')))` is always true for any UUID and tests nothing about which events were retained. The test must verify that `evt_0` was evicted and `evt_1`/`evt_2` are the surviving entries.
2. The second test doesn't verify the history state after rejection (e.g., `assert_eq!(bus.event_history_ids().len(), 1)`) or that the first event is still retrievable.
3. Neither test registers handlers, so the "event completed" semantics being tested are only the no-handler fast path — the tests should also cover history behavior when handlers are actually processing events.</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_find.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_find.rs:8">
P0: Custom agent: **Test quality checker**

Severely lacking test parity with Python/TS. This file has only 2 trivial tests (37 lines) while the Python find tests have ~1600 lines and TS has ~767+ lines covering: no-match returns None, timeout expiration, `where`/filter predicates, `past` time-window filtering, `child_of` relationships, concurrent waiters, wildcard `*` patterns, metadata field filters, `past+future` combination behavior, bus-scoped history isolation, in-progress event visibility, `max_history_size=0`, and default parameter behavior. All of these critical edge cases are completely untested in Rust.</violation>

<violation number="2" location="bubus-rust/tests/test_eventbus_find.rs:17">
P1: Custom agent: **Test quality checker**

Near-"fake" test: `test_find_future_waits_for_new_event` only asserts `found.is_some()` without verifying any properties of the found event (e.g., `event_type`, `event_id`). The TS equivalent verifies `found_event.event_type === 'ParentEvent'` and the Python equivalent verifies `found.event_id == dispatched.event_id`. At minimum, assert the found event's `event_type` matches `"future_event"` to confirm the correct event was returned rather than just that *something* was returned.</violation>
</file>

<file name="bubus-rust/tests/test_event_handler_ids.rs">

<violation number="1" location="bubus-rust/tests/test_event_handler_ids.rs:5">
P2: Custom agent: **Test quality checker**

This test duplicates coverage of `compute_handler_id` that already exists in `test_ids.rs`. Per the test quality rule: *"tests must be organized well in files for each class/pattern being tested and deduped, I prefer a few longer tests that check everything about a given method or behavior to 10 separate tiny ones that overlap."*

Merge this `Some(handler_file_path)` case into the existing `test_ids.rs` file alongside the existing `None` case test, and assert against a hardcoded expected UUID (cross-validated with Python/TS) rather than re-deriving the expected value using the same algorithm as the implementation.</violation>
</file>

<file name="bubus-rust/tests/event_bus_tests.rs">

<violation number="1" location="bubus-rust/tests/event_bus_tests.rs:53">
P1: Custom agent: **Test quality checker**

This test only asserts the count of results (`len() == 2`) without verifying the actual result values or that the handlers truly ran in parallel. Per the test quality rule: *"no 'fake' tests that simply check if attrs/methods are present or that calls run without error, tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."*

The test should:
1. Assert that the result values contain `json!(1)` and `json!(2)`
2. Verify parallel execution by checking that total elapsed time is closer to 20ms than 40ms (proving handlers ran concurrently, not serially)</violation>
</file>

<file name="bubus-rust/tests/test_event_result.rs">

<violation number="1" location="bubus-rust/tests/test_event_result.rs:7">
P1: Custom agent: **Test quality checker**

This is a "fake" test that only checks two default attribute values (`status` and `timeout`) without asserting behavioral correctness. Per the test quality rule: *"no 'fake' tests that simply check if attrs/methods are present or that calls run without error, tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."*

This is the **only** test for `EventResult` in the entire codebase, yet it doesn't verify:
- All default field values (`event_id`, `started_at`, `result`, `error`, `completed_at`, `event_children`)
- Status transitions (`Pending` → `Started` → `Completed`/`Error`) that occur during event processing
- Result/error value setting after handler execution
- Timeout enforcement behavior
- `event_children` tracking

This test should be expanded into a comprehensive behavioral test that covers the full `EventResult` lifecycle through the `EventBus` APIs, asserting all mutations and side effects at each stage.</violation>
</file>

<file name="bubus-rust/tests/test_typed_events.rs">

<violation number="1" location="bubus-rust/tests/test_typed_events.rs:29">
P1: Custom agent: **Test quality checker**

Typed event tests are critically thin — only 2 happy-path tests with no coverage of error paths, complex types, multiple handlers, or result type enforcement. The TS counterpart (`event_result_typed_results.test.ts`) has 9 tests (275 lines) covering invalid handler results, complex nested schemas, roundtrip serialization, and schema enforcement. Per the rule: "tests must be bulletproof and hit every edge case." At minimum, add tests for:
- Handler returning `Err(...)` and verifying the error propagates correctly
- Typed result schema enforcement (wrong result type from handler → error)
- Complex nested payload/result types
- Multiple typed handlers on the same event
- Serialization roundtrip of typed events</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_on_off.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_on_off.rs:13">
P1: Custom agent: **Test quality checker**

Test only asserts `event_results.len()` but never verifies the actual handler result value or result status. The handler returns `Ok(json!("ok"))` but this value is never checked — a bug that corrupts or drops result values would go undetected. Per the test quality rule, all output values must be asserted as behaviorally correct, not just counted.</violation>
</file>

<file name="bubus-rust/tests/test_ids.rs">

<violation number="1" location="bubus-rust/tests/test_ids.rs:21">
P1: Custom agent: **Test quality checker**

This test is self-referential: it claims to verify Python/TS compatibility (`compatible_with_python_ts`) but only checks that `compute_handler_id` matches the same Rust `Uuid::new_v5` call it wraps — it would pass even if the namespace or seed format diverged from the other implementations. Hardcode a known expected UUID string from the Python or TS implementation to actually verify cross-language parity. Additionally, the `EventHandler::from_callable` block only asserts `get_version_num() == 5` (a shallow version check), which violates the rule against "fake" tests that don't assert behavioral correctness.

Per the test quality rule: _"no 'fake' tests that simply check if attrs/methods are present or that calls run without error, tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."_</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_locking.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_locking.rs:8">
P1: Custom agent: **Test quality checker**

`test_queue_jump` is flaky: the handler completes in nanoseconds with no delay, so the background event loop can dequeue and fully process `event1` before `event2` is even emitted with queue-jump. The `assert!(event2_started <= event1_started)` then fails due to this race condition.

The Python parity test (`test_queue_jump_awaited_child_preempts_queued_sibling_on_same_bus`) avoids this by using a behavioral order-tracking list and async sleeps to create deterministic windows. This test should add a delay inside the handler (e.g., `thread::sleep(Duration::from_millis(15))`) to ensure both events are enqueued before processing completes, and should assert actual execution order rather than relying on timestamp comparisons.</violation>
</file>

<file name="bubus-rust/tests/test_event_handler_first.rs">

<violation number="1" location="bubus-rust/tests/test_event_handler_first.rs:16">
P0: Custom agent: **Test quality checker**

This test file has drastically insufficient coverage compared to the TS parity test (`bubus-ts/tests/event_handler_first.test.ts`, ~20 tests, 549+ lines). The TS tests cover First+Parallel, handler cancellation, error propagation, undefined/falsy edge cases, retry patterns, child event cleanup, and more. This single happy-path test does not meet the rule's requirement for equal test coverage across implementations.

Missing test scenarios include at minimum:
- `first` with parallel handler concurrency
- Cancellation of remaining handlers after first result
- Serial mode skipping undefined results and using the next handler
- All handlers returning errors
- No handlers registered
- Falsy return values (0, empty string, false) treated as valid results</violation>

<violation number="2" location="bubus-rust/tests/test_event_handler_first.rs:21">
P2: Custom agent: **Test quality checker**

`thread::sleep` inside an async block blocks the executor thread rather than yielding. This is a known anti-pattern in async Rust. If a regression causes this handler to execute, it will block the entire thread, potentially causing intermittent test hangs. Use an async sleep instead (e.g., `tokio::time::sleep`).</violation>

<violation number="3" location="bubus-rust/tests/test_event_handler_first.rs:34">
P1: Custom agent: **Test quality checker**

The test doesn't verify that the second handler was NOT called. The TS equivalent uses a `second_handler_called` flag and asserts it remains `false`. Only checking `results.len() == 1` is insufficient — the second handler could execute (a behavioral bug) with its result silently discarded, and this test would still pass. Use an `Arc<AtomicBool>` or similar to track and assert the second handler was never invoked.</violation>
</file>

<file name="bubus-rust/tests/test_base_event.rs">

<violation number="1" location="bubus-rust/tests/test_base_event.rs:14">
P1: Custom agent: **Test quality checker**

This roundtrip test is essentially a no-op — it only proves `serde` is self-consistent, not that the serialized JSON is correct. It violates the rule's requirement that "tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct" and is effectively a "fake" test. The test should assert concrete values: verify `event_type` is `"test_event"`, payload contains `{"value": 1}`, `event_status` is `"Pending"`, `event_id` is a valid UUIDv7, timestamps are present, and the JSON field names match the Python/TS wire format (e.g., `event_type`, `event_results`, `event_created_at`).</violation>

<violation number="2" location="bubus-rust/tests/test_base_event.rs:22">
P1: Custom agent: **Test quality checker**

This test doesn't verify the timestamp side effects of `mark_started()` / `mark_completed()` — `event_started_at` and `event_completed_at` are never asserted to be `Some(...)` after the respective calls. The rule prohibits "fake" tests that only check calls run without error. Add assertions like:
```rust
assert!(event.inner.lock().event_started_at.is_some());
assert!(event.inner.lock().event_completed_at.is_some());

Also verify event_created_at is set on construction and that the payload is preserved through state transitions.


</details>

<details>
<summary>Architecture diagram</summary>

```mermaid
sequenceDiagram
    participant User as User Code
    participant Bus as EventBus
    participant Loop as Runtime Loop (Thread)
    participant Store as Event History/Queue
    participant Handler as EventHandler (Typed)
    participant Event as BaseEvent (Shared State)

    Note over User,Event: NEW: Strongly-Typed Event Flow

    User->>Bus: on_typed<E>(handler_fn)
    Bus->>Bus: NEW: Wrap handler_fn in TypedEvent decoder
    Bus->>Bus: Register handler for E::EVENT_TYPE

    User->>Bus: emit_typed<E>(Payload)
    Bus->>Event: NEW: TypedEvent::new(Payload)
    Event->>Event: Serialize Payload to JSON Map
    Bus->>Store: NEW: register_in_history(Arc<BaseEvent>)
    Bus->>Store: push_back(event)
    Bus->>Loop: notify()

    loop Event Processing
        Loop->>Store: pop_front()
        Store-->>Loop: BaseEvent
        
        Loop->>Event: mark_started()
        
        alt EventConcurrencyMode::BusSerial
            Loop->>Bus: Acquire bus_serial_lock
        end

        Loop->>Bus: Get handlers for event_type
        
        loop For each handler
            Loop->>Handler: Execute callable(event)
            Handler->>Event: NEW: TypedEvent::from_base_event(event)
            Handler->>Handler: Decode JSON Payload to E::Payload
            Handler->>User: Invoke handler_fn(TypedEvent<E>)
            User-->>Handler: Return Result<E::Result, String>
            Handler->>Event: NEW: Update event_results with JSON Result
        end

        opt All/First Handlers Finished
            Loop->>Event: mark_completed()
            Event-->>User: notify wait_completed()
        end

        alt EventConcurrencyMode::BusSerial
            Loop->>Bus: Release bus_serial_lock
        end
    end

    Note over User,Event: NEW: Typed Result Retrieval
    User->>Event: wait_completed()
    User->>Event: NEW: first_result()
    Event->>Event: NEW: Decode JSON Result to E::Result
    Event-->>User: E::Result

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


#[derive(Default, Clone)]
pub struct ReentrantLock {
lock: Arc<Mutex<()>>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Custom agent: Make sure concurrency options work correctly and consistently

ReentrantLock wraps parking_lot::Mutex which is not reentrant — it will deadlock if the same thread re-acquires it. This lock is used for GLOBAL_SERIAL_LOCK and bus_serial_lock in the event bus's serial concurrency modes. When a handler in BusSerial/GlobalSerial mode emits and awaits a child event (queue-jumping), re-entering this lock on the same thread will deadlock instead of re-entering correctly.

Use parking_lot::ReentrantMutex<()> instead of parking_lot::Mutex<()> — it explicitly supports recursive locking from the same thread without deadlocking, which is what the struct name promises and what the concurrency model requires.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/lock_manager.rs, line 7:

<comment>`ReentrantLock` wraps `parking_lot::Mutex` which is **not** reentrant — it will deadlock if the same thread re-acquires it. This lock is used for `GLOBAL_SERIAL_LOCK` and `bus_serial_lock` in the event bus's serial concurrency modes. When a handler in `BusSerial`/`GlobalSerial` mode emits and awaits a child event (queue-jumping), re-entering this lock on the same thread will deadlock instead of re-entering correctly.

Use `parking_lot::ReentrantMutex<()>` instead of `parking_lot::Mutex<()>` — it explicitly supports recursive locking from the same thread without deadlocking, which is what the struct name promises and what the concurrency model requires.</comment>

<file context>
@@ -0,0 +1,29 @@
+
+#[derive(Default, Clone)]
+pub struct ReentrantLock {
+    lock: Arc<Mutex<()>>,
+}
+
</file context>

@@ -0,0 +1,37 @@
use std::{thread, time::Duration};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Custom agent: Test quality checker

Severely lacking test parity with Python/TS. This file has only 2 trivial tests (37 lines) while the Python find tests have ~1600 lines and TS has ~767+ lines covering: no-match returns None, timeout expiration, where/filter predicates, past time-window filtering, child_of relationships, concurrent waiters, wildcard * patterns, metadata field filters, past+future combination behavior, bus-scoped history isolation, in-progress event visibility, max_history_size=0, and default parameter behavior. All of these critical edge cases are completely untested in Rust.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_eventbus_find.rs, line 8:

<comment>Severely lacking test parity with Python/TS. This file has only 2 trivial tests (37 lines) while the Python find tests have ~1600 lines and TS has ~767+ lines covering: no-match returns None, timeout expiration, `where`/filter predicates, `past` time-window filtering, `child_of` relationships, concurrent waiters, wildcard `*` patterns, metadata field filters, `past+future` combination behavior, bus-scoped history isolation, in-progress event visibility, `max_history_size=0`, and default parameter behavior. All of these critical edge cases are completely untested in Rust.</comment>

<file context>
@@ -0,0 +1,37 @@
+use serde_json::{json, Map};
+
+#[test]
+fn test_find_past_match_returns_event() {
+    let bus = EventBus::new(Some("FindBus".to_string()));
+    bus.on("work", "h1", |_event| async move { Ok(json!("ok")) });
</file context>

@@ -0,0 +1,41 @@
use std::{sync::Arc, thread, time::Duration};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Custom agent: Test quality checker

This test file has drastically insufficient coverage compared to the TS parity test (bubus-ts/tests/event_handler_first.test.ts, ~20 tests, 549+ lines). The TS tests cover First+Parallel, handler cancellation, error propagation, undefined/falsy edge cases, retry patterns, child event cleanup, and more. This single happy-path test does not meet the rule's requirement for equal test coverage across implementations.

Missing test scenarios include at minimum:

  • first with parallel handler concurrency
  • Cancellation of remaining handlers after first result
  • Serial mode skipping undefined results and using the next handler
  • All handlers returning errors
  • No handlers registered
  • Falsy return values (0, empty string, false) treated as valid results
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_first.rs, line 16:

<comment>This test file has drastically insufficient coverage compared to the TS parity test (`bubus-ts/tests/event_handler_first.test.ts`, ~20 tests, 549+ lines). The TS tests cover First+Parallel, handler cancellation, error propagation, undefined/falsy edge cases, retry patterns, child event cleanup, and more. This single happy-path test does not meet the rule's requirement for equal test coverage across implementations.

Missing test scenarios include at minimum:
- `first` with parallel handler concurrency
- Cancellation of remaining handlers after first result
- Serial mode skipping undefined results and using the next handler
- All handlers returning errors
- No handlers registered
- Falsy return values (0, empty string, false) treated as valid results</comment>

<file context>
@@ -0,0 +1,41 @@
+}
+
+#[test]
+fn test_event_handler_first_serial_stops_after_first_success() {
+    let bus = EventBus::new(Some("BusFirstSerial".to_string()));
+
</file context>

let event = mk_event("work");
{
let mut inner = event.inner.lock();
inner.event_handler_concurrency = Some(EventHandlerConcurrencyMode::Serial);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Make sure concurrency options work correctly and consistently

This test doesn't actually verify bus default handler settings. It sets explicit event-level overrides (Some(EventHandlerConcurrencyMode::Serial) and Some(EventHandlerCompletionMode::All)) which bypass the bus-default fallback logic in process_event_inner (the .unwrap_or(self.event_handler_concurrency) path is never hit). Additionally, with only one handler, the assertion event_results.len() == 1 passes regardless of concurrency/completion mode.

To test bus defaults, remove the event-level overrides (leave them as None) and register multiple handlers to distinguish between All vs First completion and Serial vs Parallel concurrency behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_eventbus_dispatch_defaults.rs, line 25:

<comment>This test doesn't actually verify bus default handler settings. It sets explicit event-level overrides (`Some(EventHandlerConcurrencyMode::Serial)` and `Some(EventHandlerCompletionMode::All)`) which bypass the bus-default fallback logic in `process_event_inner` (the `.unwrap_or(self.event_handler_concurrency)` path is never hit). Additionally, with only one handler, the assertion `event_results.len() == 1` passes regardless of concurrency/completion mode.

To test bus defaults, remove the event-level overrides (leave them as `None`) and register multiple handlers to distinguish between `All` vs `First` completion and `Serial` vs `Parallel` concurrency behavior.</comment>

<file context>
@@ -0,0 +1,33 @@
+    let event = mk_event("work");
+    {
+        let mut inner = event.inner.lock();
+        inner.event_handler_concurrency = Some(EventHandlerConcurrencyMode::Serial);
+        inner.event_handler_completion = Some(EventHandlerCompletionMode::All);
+    }
</file context>

})
}

pub async fn find_typed<E: EventSpec>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Make sure concurrency options work correctly and consistently

find_typed drops the child_of parameter, hardcoding it to None. This makes it impossible for typed API users to find events by parent-child relationship — a core feature for nested event tracking and concurrency correctness. The child_of parameter should be exposed, accepting an Option<TypedEvent<_>> or Option<Arc<BaseEvent>>.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/typed.rs, line 94:

<comment>`find_typed` drops the `child_of` parameter, hardcoding it to `None`. This makes it impossible for typed API users to find events by parent-child relationship — a core feature for nested event tracking and concurrency correctness. The `child_of` parameter should be exposed, accepting an `Option<TypedEvent<_>>` or `Option<Arc<BaseEvent>>`.</comment>

<file context>
@@ -0,0 +1,109 @@
+        })
+    }
+
+    pub async fn find_typed<E: EventSpec>(
+        &self,
+        past: bool,
</file context>

pub fn first_result(&self) -> Option<E::Result> {
let results: HashMap<String, crate::event_result::EventResult> =
self.inner.inner.lock().event_results.clone();
for result in results.values() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: HashMap iteration order is non-deterministic in Rust. first_result() claims to return the "first" result, but with multiple successful handlers it will return an arbitrary one. Consider using an IndexMap or a BTreeMap (keyed by something orderable like insertion index or timestamp) to preserve insertion order, matching Python's dict ordering semantics.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/typed.rs, line 54:

<comment>`HashMap` iteration order is non-deterministic in Rust. `first_result()` claims to return the "first" result, but with multiple successful handlers it will return an arbitrary one. Consider using an `IndexMap` or a `BTreeMap` (keyed by something orderable like insertion index or timestamp) to preserve insertion order, matching Python's `dict` ordering semantics.</comment>

<file context>
@@ -0,0 +1,109 @@
+    pub fn first_result(&self) -> Option<E::Result> {
+        let results: HashMap<String, crate::event_result::EventResult> =
+            self.inner.inner.lock().event_results.clone();
+        for result in results.values() {
+            if result.error.is_none() {
+                if let Some(value) = &result.result {
</file context>

pub status: EventResultStatus,
pub event_id: String,
pub handler: EventHandler,
pub timeout: Option<f64>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Field timeout should be named handler_timeout to match the TS/Python JSON schema (which uses handler_timeout). As-is, JSON serialization will produce "timeout" instead of "handler_timeout", breaking cross-language event JSON compatibility. Additionally, this field is redundant with the already-present self.handler.handler_timeout.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/event_result.rs, line 21:

<comment>Field `timeout` should be named `handler_timeout` to match the TS/Python JSON schema (which uses `handler_timeout`). As-is, JSON serialization will produce `"timeout"` instead of `"handler_timeout"`, breaking cross-language event JSON compatibility. Additionally, this field is redundant with the already-present `self.handler.handler_timeout`.</comment>

<file context>
@@ -0,0 +1,44 @@
+    pub status: EventResultStatus,
+    pub event_id: String,
+    pub handler: EventHandler,
+    pub timeout: Option<f64>,
+    pub started_at: Option<String>,
+    pub result: Option<Value>,
</file context>

let current_size = self.runtime.history_order.lock().len();
if current_size >= max_size {
if self.runtime.max_history_drop {
while self.runtime.history_order.lock().len() >= max_size {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: TOCTOU race: history_order is locked separately for len() and pop_front() in the same loop iteration. Another thread can modify the deque between these two lock acquisitions. Use a single lock guard for both operations.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/event_bus.rs, line 229:

<comment>TOCTOU race: `history_order` is locked separately for `len()` and `pop_front()` in the same loop iteration. Another thread can modify the deque between these two lock acquisitions. Use a single lock guard for both operations.</comment>

<file context>
@@ -0,0 +1,699 @@
+            let current_size = self.runtime.history_order.lock().len();
+            if current_size >= max_size {
+                if self.runtime.max_history_drop {
+                    while self.runtime.history_order.lock().len() >= max_size {
+                        if let Some(oldest) = self.runtime.history_order.lock().pop_front() {
+                            self.runtime.events.lock().remove(&oldest);
</file context>

@@ -0,0 +1,32 @@
use bubus_rust::id::compute_handler_id;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Test quality checker

This test duplicates coverage of compute_handler_id that already exists in test_ids.rs. Per the test quality rule: "tests must be organized well in files for each class/pattern being tested and deduped, I prefer a few longer tests that check everything about a given method or behavior to 10 separate tiny ones that overlap."

Merge this Some(handler_file_path) case into the existing test_ids.rs file alongside the existing None case test, and assert against a hardcoded expected UUID (cross-validated with Python/TS) rather than re-deriving the expected value using the same algorithm as the implementation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_ids.rs, line 5:

<comment>This test duplicates coverage of `compute_handler_id` that already exists in `test_ids.rs`. Per the test quality rule: *"tests must be organized well in files for each class/pattern being tested and deduped, I prefer a few longer tests that check everything about a given method or behavior to 10 separate tiny ones that overlap."*

Merge this `Some(handler_file_path)` case into the existing `test_ids.rs` file alongside the existing `None` case test, and assert against a hardcoded expected UUID (cross-validated with Python/TS) rather than re-deriving the expected value using the same algorithm as the implementation.</comment>

<file context>
@@ -0,0 +1,32 @@
+use uuid::Uuid;
+
+#[test]
+fn test_compute_handler_id_matches_uuidv5_seed_algorithm() {
+    let eventbus_id = "0195f6ac-9f10-7e4b-bf69-fb33c68ca13e";
+    let handler_name = "tests.handlers.handle_work";
</file context>


bus.on("work", "first", |_event| async move { Ok(json!("winner")) });
bus.on("work", "second", |_event| async move {
thread::sleep(Duration::from_millis(20));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Test quality checker

thread::sleep inside an async block blocks the executor thread rather than yielding. This is a known anti-pattern in async Rust. If a regression causes this handler to execute, it will block the entire thread, potentially causing intermittent test hangs. Use an async sleep instead (e.g., tokio::time::sleep).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_first.rs, line 21:

<comment>`thread::sleep` inside an async block blocks the executor thread rather than yielding. This is a known anti-pattern in async Rust. If a regression causes this handler to execute, it will block the entire thread, potentially causing intermittent test hangs. Use an async sleep instead (e.g., `tokio::time::sleep`).</comment>

<file context>
@@ -0,0 +1,41 @@
+
+    bus.on("work", "first", |_event| async move { Ok(json!("winner")) });
+    bus.on("work", "second", |_event| async move {
+        thread::sleep(Duration::from_millis(20));
+        Ok(json!("late"))
+    });
</file context>

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

29 issues found across 25 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="bubus-rust/tests/test_eventbus_dispatch_defaults.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_dispatch_defaults.rs:25">
P1: Custom agent: **Make sure concurrency options work correctly and consistently**

This test doesn't actually verify bus default handler settings. It sets explicit event-level overrides (`Some(EventHandlerConcurrencyMode::Serial)` and `Some(EventHandlerCompletionMode::All)`) which bypass the bus-default fallback logic in `process_event_inner` (the `.unwrap_or(self.event_handler_concurrency)` path is never hit). Additionally, with only one handler, the assertion `event_results.len() == 1` passes regardless of concurrency/completion mode.

To test bus defaults, remove the event-level overrides (leave them as `None`) and register multiple handlers to distinguish between `All` vs `First` completion and `Serial` vs `Parallel` concurrency behavior.</violation>
</file>

<file name="bubus-rust/src/typed.rs">

<violation number="1" location="bubus-rust/src/typed.rs:54">
P2: `HashMap` iteration order is non-deterministic in Rust. `first_result()` claims to return the "first" result, but with multiple successful handlers it will return an arbitrary one. Consider using an `IndexMap` or a `BTreeMap` (keyed by something orderable like insertion index or timestamp) to preserve insertion order, matching Python's `dict` ordering semantics.</violation>

<violation number="2" location="bubus-rust/src/typed.rs:94">
P1: Custom agent: **Make sure concurrency options work correctly and consistently**

`find_typed` drops the `child_of` parameter, hardcoding it to `None`. This makes it impossible for typed API users to find events by parent-child relationship — a core feature for nested event tracking and concurrency correctness. The `child_of` parameter should be exposed, accepting an `Option<TypedEvent<_>>` or `Option<Arc<BaseEvent>>`.</violation>
</file>

<file name="bubus-rust/src/lock_manager.rs">

<violation number="1" location="bubus-rust/src/lock_manager.rs:7">
P0: Custom agent: **Make sure concurrency options work correctly and consistently**

`ReentrantLock` wraps `parking_lot::Mutex` which is **not** reentrant — it will deadlock if the same thread re-acquires it. This lock is used for `GLOBAL_SERIAL_LOCK` and `bus_serial_lock` in the event bus's serial concurrency modes. When a handler in `BusSerial`/`GlobalSerial` mode emits and awaits a child event (queue-jumping), re-entering this lock on the same thread will deadlock instead of re-entering correctly.

Use `parking_lot::ReentrantMutex<()>` instead of `parking_lot::Mutex<()>` — it explicitly supports recursive locking from the same thread without deadlocking, which is what the struct name promises and what the concurrency model requires.</violation>
</file>

<file name="bubus-rust/src/base_event.rs">

<violation number="1" location="bubus-rust/src/base_event.rs:21">
P1: Bug: `now_iso()` does not produce ISO 8601 timestamps. It formats raw epoch seconds and nanoseconds (e.g. `"1708444800.123456789Z"`), not a proper ISO 8601 datetime (e.g. `"2024-02-20T12:00:00.123456789Z"`). This breaks cross-language compatibility with the Python/TS implementations which use real ISO datetime strings.

Consider using the `chrono` crate (or `time` crate) — e.g. `chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Nanos, true)`.</violation>

<violation number="2" location="bubus-rust/src/base_event.rs:112">
P2: Custom agent: **Make sure concurrency options work correctly and consistently**

`mark_completed()` calls `self.completed.notify(usize::MAX)` while still holding the `parking_lot::Mutex` guard. All woken tasks in `wait_completed()` will immediately contend on this lock, causing unnecessary contention in high-throughput scenarios. Drop the guard before notifying to avoid the thundering-herd contention window.</violation>
</file>

<file name="bubus-rust/src/event_bus.rs">

<violation number="1" location="bubus-rust/src/event_bus.rs:105">
P1: Custom agent: **Make sure concurrency options work correctly and consistently**

**Memory leak**: `start_loop` spawns a thread that captures `bus: Arc<Self>`, creating a strong reference that only releases when `stop()` is called. If users let the bus fall out of scope without calling `stop()`, the thread keeps running forever holding the `Arc`, preventing the bus and all its state (handlers, event history, queued events) from being dropped. The rule requires memory to be freed automatically without manual `stop()` calls. Consider using a `Weak<Self>` reference in the spawned thread and breaking the loop when the weak reference can no longer be upgraded.</violation>

<violation number="2" location="bubus-rust/src/event_bus.rs:109">
P1: Bug: The event loop sleeps 1ms when the queue is **not** empty, adding unnecessary latency before processing every queued event. This condition appears inverted — sleeping should occur when the queue is empty (if at all), not when there's work to do. Since the empty-queue case is already handled by `queue_notify.listen().await` below, this entire sleep block can be removed.</violation>

<violation number="3" location="bubus-rust/src/event_bus.rs:205">
P1: `emit_with_options` sets `event_parent_id` and `event_emitted_by_handler_id` on the child event, but never records the child's event ID in the parent handler's `EventResult.event_children`. Since `cancel_children` iterates `result.event_children` to find descendants to cancel, the list is always empty and cancellation never propagates to child events on parent timeout. After setting the parent metadata, the emitted child's ID should be appended to the current handler's `EventResult.event_children` in the parent event.</violation>

<violation number="4" location="bubus-rust/src/event_bus.rs:229">
P2: TOCTOU race: `history_order` is locked separately for `len()` and `pop_front()` in the same loop iteration. Another thread can modify the deque between these two lock acquisitions. Use a single lock guard for both operations.</violation>

<violation number="5" location="bubus-rust/src/event_bus.rs:293">
P1: Bug: `?` operator causes `find_in_history` to abort the entire search if any event ID is missing from the events map. If an event was evicted from the map but its ID is still in `history_order`, this returns `None` prematurely instead of continuing to check subsequent entries. Replace `?` with a guard that skips the missing entry.</violation>
</file>

<file name="bubus-rust/README.md">

<violation number="1" location="bubus-rust/README.md:22">
P2: Quickstart example won't compile: `event_bus::new(...)` and `base_event::new(...)` are not free functions. `new` is an associated function on `EventBus` and `BaseEvent` respectively. The imports and constructor calls need to reference the struct types directly.</violation>
</file>

<file name="bubus-rust/src/event_result.rs">

<violation number="1" location="bubus-rust/src/event_result.rs:21">
P2: Field `timeout` should be named `handler_timeout` to match the TS/Python JSON schema (which uses `handler_timeout`). As-is, JSON serialization will produce `"timeout"` instead of `"handler_timeout"`, breaking cross-language event JSON compatibility. Additionally, this field is redundant with the already-present `self.handler.handler_timeout`.</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_timeout.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_timeout.rs:105">
P1: Custom agent: **Test quality checker**

Weak assertion `assert!(has_error || is_completed)` makes this test a no-op: it passes regardless of whether the parent timeout actually cancelled the child. A child that completed successfully (no error) means the timeout propagation **didn't work**, yet the test would still pass. Per the test quality rule, tests must assert that side effects and output values are *behaviorally correct* — not just that some result exists. The assertion should specifically verify the child has an error result with the expected abort/cancellation status, matching what the Python and TS tests enforce.</violation>
</file>

<file name="bubus-rust/tests/test_event_history_store.rs">

<violation number="1" location="bubus-rust/tests/test_event_history_store.rs:17">
P1: Custom agent: **Test quality checker**

These history store tests have weak/trivial assertions that don't verify behavioral correctness. The rule requires that "tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct" — not just that counts match or that UUIDs contain hyphens.

Specific issues:
1. `assert!(history.iter().any(|id| id.contains('-')))` is always true for any UUID and tests nothing about which events were retained. The test must verify that `evt_0` was evicted and `evt_1`/`evt_2` are the surviving entries.
2. The second test doesn't verify the history state after rejection (e.g., `assert_eq!(bus.event_history_ids().len(), 1)`) or that the first event is still retrievable.
3. Neither test registers handlers, so the "event completed" semantics being tested are only the no-handler fast path — the tests should also cover history behavior when handlers are actually processing events.</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_find.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_find.rs:8">
P0: Custom agent: **Test quality checker**

Severely lacking test parity with Python/TS. This file has only 2 trivial tests (37 lines) while the Python find tests have ~1600 lines and TS has ~767+ lines covering: no-match returns None, timeout expiration, `where`/filter predicates, `past` time-window filtering, `child_of` relationships, concurrent waiters, wildcard `*` patterns, metadata field filters, `past+future` combination behavior, bus-scoped history isolation, in-progress event visibility, `max_history_size=0`, and default parameter behavior. All of these critical edge cases are completely untested in Rust.</violation>

<violation number="2" location="bubus-rust/tests/test_eventbus_find.rs:17">
P1: Custom agent: **Test quality checker**

Near-"fake" test: `test_find_future_waits_for_new_event` only asserts `found.is_some()` without verifying any properties of the found event (e.g., `event_type`, `event_id`). The TS equivalent verifies `found_event.event_type === 'ParentEvent'` and the Python equivalent verifies `found.event_id == dispatched.event_id`. At minimum, assert the found event's `event_type` matches `"future_event"` to confirm the correct event was returned rather than just that *something* was returned.</violation>
</file>

<file name="bubus-rust/tests/test_event_handler_ids.rs">

<violation number="1" location="bubus-rust/tests/test_event_handler_ids.rs:5">
P2: Custom agent: **Test quality checker**

This test duplicates coverage of `compute_handler_id` that already exists in `test_ids.rs`. Per the test quality rule: *"tests must be organized well in files for each class/pattern being tested and deduped, I prefer a few longer tests that check everything about a given method or behavior to 10 separate tiny ones that overlap."*

Merge this `Some(handler_file_path)` case into the existing `test_ids.rs` file alongside the existing `None` case test, and assert against a hardcoded expected UUID (cross-validated with Python/TS) rather than re-deriving the expected value using the same algorithm as the implementation.</violation>
</file>

<file name="bubus-rust/tests/event_bus_tests.rs">

<violation number="1" location="bubus-rust/tests/event_bus_tests.rs:53">
P1: Custom agent: **Test quality checker**

This test only asserts the count of results (`len() == 2`) without verifying the actual result values or that the handlers truly ran in parallel. Per the test quality rule: *"no 'fake' tests that simply check if attrs/methods are present or that calls run without error, tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."*

The test should:
1. Assert that the result values contain `json!(1)` and `json!(2)`
2. Verify parallel execution by checking that total elapsed time is closer to 20ms than 40ms (proving handlers ran concurrently, not serially)</violation>
</file>

<file name="bubus-rust/tests/test_event_result.rs">

<violation number="1" location="bubus-rust/tests/test_event_result.rs:7">
P1: Custom agent: **Test quality checker**

This is a "fake" test that only checks two default attribute values (`status` and `timeout`) without asserting behavioral correctness. Per the test quality rule: *"no 'fake' tests that simply check if attrs/methods are present or that calls run without error, tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."*

This is the **only** test for `EventResult` in the entire codebase, yet it doesn't verify:
- All default field values (`event_id`, `started_at`, `result`, `error`, `completed_at`, `event_children`)
- Status transitions (`Pending` → `Started` → `Completed`/`Error`) that occur during event processing
- Result/error value setting after handler execution
- Timeout enforcement behavior
- `event_children` tracking

This test should be expanded into a comprehensive behavioral test that covers the full `EventResult` lifecycle through the `EventBus` APIs, asserting all mutations and side effects at each stage.</violation>
</file>

<file name="bubus-rust/tests/test_typed_events.rs">

<violation number="1" location="bubus-rust/tests/test_typed_events.rs:29">
P1: Custom agent: **Test quality checker**

Typed event tests are critically thin — only 2 happy-path tests with no coverage of error paths, complex types, multiple handlers, or result type enforcement. The TS counterpart (`event_result_typed_results.test.ts`) has 9 tests (275 lines) covering invalid handler results, complex nested schemas, roundtrip serialization, and schema enforcement. Per the rule: "tests must be bulletproof and hit every edge case." At minimum, add tests for:
- Handler returning `Err(...)` and verifying the error propagates correctly
- Typed result schema enforcement (wrong result type from handler → error)
- Complex nested payload/result types
- Multiple typed handlers on the same event
- Serialization roundtrip of typed events</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_on_off.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_on_off.rs:13">
P1: Custom agent: **Test quality checker**

Test only asserts `event_results.len()` but never verifies the actual handler result value or result status. The handler returns `Ok(json!("ok"))` but this value is never checked — a bug that corrupts or drops result values would go undetected. Per the test quality rule, all output values must be asserted as behaviorally correct, not just counted.</violation>
</file>

<file name="bubus-rust/tests/test_ids.rs">

<violation number="1" location="bubus-rust/tests/test_ids.rs:21">
P1: Custom agent: **Test quality checker**

This test is self-referential: it claims to verify Python/TS compatibility (`compatible_with_python_ts`) but only checks that `compute_handler_id` matches the same Rust `Uuid::new_v5` call it wraps — it would pass even if the namespace or seed format diverged from the other implementations. Hardcode a known expected UUID string from the Python or TS implementation to actually verify cross-language parity. Additionally, the `EventHandler::from_callable` block only asserts `get_version_num() == 5` (a shallow version check), which violates the rule against "fake" tests that don't assert behavioral correctness.

Per the test quality rule: _"no 'fake' tests that simply check if attrs/methods are present or that calls run without error, tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."_</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_locking.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_locking.rs:8">
P1: Custom agent: **Test quality checker**

`test_queue_jump` is flaky: the handler completes in nanoseconds with no delay, so the background event loop can dequeue and fully process `event1` before `event2` is even emitted with queue-jump. The `assert!(event2_started <= event1_started)` then fails due to this race condition.

The Python parity test (`test_queue_jump_awaited_child_preempts_queued_sibling_on_same_bus`) avoids this by using a behavioral order-tracking list and async sleeps to create deterministic windows. This test should add a delay inside the handler (e.g., `thread::sleep(Duration::from_millis(15))`) to ensure both events are enqueued before processing completes, and should assert actual execution order rather than relying on timestamp comparisons.</violation>
</file>

<file name="bubus-rust/tests/test_event_handler_first.rs">

<violation number="1" location="bubus-rust/tests/test_event_handler_first.rs:16">
P0: Custom agent: **Test quality checker**

This test file has drastically insufficient coverage compared to the TS parity test (`bubus-ts/tests/event_handler_first.test.ts`, ~20 tests, 549+ lines). The TS tests cover First+Parallel, handler cancellation, error propagation, undefined/falsy edge cases, retry patterns, child event cleanup, and more. This single happy-path test does not meet the rule's requirement for equal test coverage across implementations.

Missing test scenarios include at minimum:
- `first` with parallel handler concurrency
- Cancellation of remaining handlers after first result
- Serial mode skipping undefined results and using the next handler
- All handlers returning errors
- No handlers registered
- Falsy return values (0, empty string, false) treated as valid results</violation>

<violation number="2" location="bubus-rust/tests/test_event_handler_first.rs:21">
P2: Custom agent: **Test quality checker**

`thread::sleep` inside an async block blocks the executor thread rather than yielding. This is a known anti-pattern in async Rust. If a regression causes this handler to execute, it will block the entire thread, potentially causing intermittent test hangs. Use an async sleep instead (e.g., `tokio::time::sleep`).</violation>

<violation number="3" location="bubus-rust/tests/test_event_handler_first.rs:34">
P1: Custom agent: **Test quality checker**

The test doesn't verify that the second handler was NOT called. The TS equivalent uses a `second_handler_called` flag and asserts it remains `false`. Only checking `results.len() == 1` is insufficient — the second handler could execute (a behavioral bug) with its result silently discarded, and this test would still pass. Use an `Arc<AtomicBool>` or similar to track and assert the second handler was never invoked.</violation>
</file>

<file name="bubus-rust/tests/test_base_event.rs">

<violation number="1" location="bubus-rust/tests/test_base_event.rs:14">
P1: Custom agent: **Test quality checker**

This roundtrip test is essentially a no-op — it only proves `serde` is self-consistent, not that the serialized JSON is correct. It violates the rule's requirement that "tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct" and is effectively a "fake" test. The test should assert concrete values: verify `event_type` is `"test_event"`, payload contains `{"value": 1}`, `event_status` is `"Pending"`, `event_id` is a valid UUIDv7, timestamps are present, and the JSON field names match the Python/TS wire format (e.g., `event_type`, `event_results`, `event_created_at`).</violation>

<violation number="2" location="bubus-rust/tests/test_base_event.rs:22">
P1: Custom agent: **Test quality checker**

This test doesn't verify the timestamp side effects of `mark_started()` / `mark_completed()` — `event_started_at` and `event_completed_at` are never asserted to be `Some(...)` after the respective calls. The rule prohibits "fake" tests that only check calls run without error. Add assertions like:
```rust
assert!(event.inner.lock().event_started_at.is_some());
assert!(event.inner.lock().event_completed_at.is_some());

Also verify event_created_at is set on construction and that the payload is preserved through state transitions.


</details>

<details>
<summary>Architecture diagram</summary>

```mermaid
sequenceDiagram
    participant User as User Code
    participant Bus as EventBus
    participant Loop as Runtime Loop (Thread)
    participant Store as Event History/Queue
    participant Handler as EventHandler (Typed)
    participant Event as BaseEvent (Shared State)

    Note over User,Event: NEW: Strongly-Typed Event Flow

    User->>Bus: on_typed<E>(handler_fn)
    Bus->>Bus: NEW: Wrap handler_fn in TypedEvent decoder
    Bus->>Bus: Register handler for E::EVENT_TYPE

    User->>Bus: emit_typed<E>(Payload)
    Bus->>Event: NEW: TypedEvent::new(Payload)
    Event->>Event: Serialize Payload to JSON Map
    Bus->>Store: NEW: register_in_history(Arc<BaseEvent>)
    Bus->>Store: push_back(event)
    Bus->>Loop: notify()

    loop Event Processing
        Loop->>Store: pop_front()
        Store-->>Loop: BaseEvent
        
        Loop->>Event: mark_started()
        
        alt EventConcurrencyMode::BusSerial
            Loop->>Bus: Acquire bus_serial_lock
        end

        Loop->>Bus: Get handlers for event_type
        
        loop For each handler
            Loop->>Handler: Execute callable(event)
            Handler->>Event: NEW: TypedEvent::from_base_event(event)
            Handler->>Handler: Decode JSON Payload to E::Payload
            Handler->>User: Invoke handler_fn(TypedEvent<E>)
            User-->>Handler: Return Result<E::Result, String>
            Handler->>Event: NEW: Update event_results with JSON Result
        end

        opt All/First Handlers Finished
            Loop->>Event: mark_completed()
            Event-->>User: notify wait_completed()
        end

        alt EventConcurrencyMode::BusSerial
            Loop->>Bus: Release bus_serial_lock
        end
    end

    Note over User,Event: NEW: Typed Result Retrieval
    User->>Event: wait_completed()
    User->>Event: NEW: first_result()
    Event->>Event: NEW: Decode JSON Result to E::Result
    Event-->>User: E::Result

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


#[derive(Default, Clone)]
pub struct ReentrantLock {
lock: Arc<Mutex<()>>,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Custom agent: Make sure concurrency options work correctly and consistently

ReentrantLock wraps parking_lot::Mutex which is not reentrant — it will deadlock if the same thread re-acquires it. This lock is used for GLOBAL_SERIAL_LOCK and bus_serial_lock in the event bus's serial concurrency modes. When a handler in BusSerial/GlobalSerial mode emits and awaits a child event (queue-jumping), re-entering this lock on the same thread will deadlock instead of re-entering correctly.

Use parking_lot::ReentrantMutex<()> instead of parking_lot::Mutex<()> — it explicitly supports recursive locking from the same thread without deadlocking, which is what the struct name promises and what the concurrency model requires.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/lock_manager.rs, line 7:

<comment>`ReentrantLock` wraps `parking_lot::Mutex` which is **not** reentrant — it will deadlock if the same thread re-acquires it. This lock is used for `GLOBAL_SERIAL_LOCK` and `bus_serial_lock` in the event bus's serial concurrency modes. When a handler in `BusSerial`/`GlobalSerial` mode emits and awaits a child event (queue-jumping), re-entering this lock on the same thread will deadlock instead of re-entering correctly.

Use `parking_lot::ReentrantMutex<()>` instead of `parking_lot::Mutex<()>` — it explicitly supports recursive locking from the same thread without deadlocking, which is what the struct name promises and what the concurrency model requires.</comment>

<file context>
@@ -0,0 +1,29 @@
+
+#[derive(Default, Clone)]
+pub struct ReentrantLock {
+    lock: Arc<Mutex<()>>,
+}
+
</file context>
Fix with Cubic

@@ -0,0 +1,37 @@
use std::{thread, time::Duration};
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Custom agent: Test quality checker

Severely lacking test parity with Python/TS. This file has only 2 trivial tests (37 lines) while the Python find tests have ~1600 lines and TS has ~767+ lines covering: no-match returns None, timeout expiration, where/filter predicates, past time-window filtering, child_of relationships, concurrent waiters, wildcard * patterns, metadata field filters, past+future combination behavior, bus-scoped history isolation, in-progress event visibility, max_history_size=0, and default parameter behavior. All of these critical edge cases are completely untested in Rust.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_eventbus_find.rs, line 8:

<comment>Severely lacking test parity with Python/TS. This file has only 2 trivial tests (37 lines) while the Python find tests have ~1600 lines and TS has ~767+ lines covering: no-match returns None, timeout expiration, `where`/filter predicates, `past` time-window filtering, `child_of` relationships, concurrent waiters, wildcard `*` patterns, metadata field filters, `past+future` combination behavior, bus-scoped history isolation, in-progress event visibility, `max_history_size=0`, and default parameter behavior. All of these critical edge cases are completely untested in Rust.</comment>

<file context>
@@ -0,0 +1,37 @@
+use serde_json::{json, Map};
+
+#[test]
+fn test_find_past_match_returns_event() {
+    let bus = EventBus::new(Some("FindBus".to_string()));
+    bus.on("work", "h1", |_event| async move { Ok(json!("ok")) });
</file context>
Fix with Cubic

@@ -0,0 +1,41 @@
use std::{sync::Arc, thread, time::Duration};
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Custom agent: Test quality checker

This test file has drastically insufficient coverage compared to the TS parity test (bubus-ts/tests/event_handler_first.test.ts, ~20 tests, 549+ lines). The TS tests cover First+Parallel, handler cancellation, error propagation, undefined/falsy edge cases, retry patterns, child event cleanup, and more. This single happy-path test does not meet the rule's requirement for equal test coverage across implementations.

Missing test scenarios include at minimum:

  • first with parallel handler concurrency
  • Cancellation of remaining handlers after first result
  • Serial mode skipping undefined results and using the next handler
  • All handlers returning errors
  • No handlers registered
  • Falsy return values (0, empty string, false) treated as valid results
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_first.rs, line 16:

<comment>This test file has drastically insufficient coverage compared to the TS parity test (`bubus-ts/tests/event_handler_first.test.ts`, ~20 tests, 549+ lines). The TS tests cover First+Parallel, handler cancellation, error propagation, undefined/falsy edge cases, retry patterns, child event cleanup, and more. This single happy-path test does not meet the rule's requirement for equal test coverage across implementations.

Missing test scenarios include at minimum:
- `first` with parallel handler concurrency
- Cancellation of remaining handlers after first result
- Serial mode skipping undefined results and using the next handler
- All handlers returning errors
- No handlers registered
- Falsy return values (0, empty string, false) treated as valid results</comment>

<file context>
@@ -0,0 +1,41 @@
+}
+
+#[test]
+fn test_event_handler_first_serial_stops_after_first_success() {
+    let bus = EventBus::new(Some("BusFirstSerial".to_string()));
+
</file context>
Fix with Cubic

let event = mk_event("work");
{
let mut inner = event.inner.lock();
inner.event_handler_concurrency = Some(EventHandlerConcurrencyMode::Serial);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Make sure concurrency options work correctly and consistently

This test doesn't actually verify bus default handler settings. It sets explicit event-level overrides (Some(EventHandlerConcurrencyMode::Serial) and Some(EventHandlerCompletionMode::All)) which bypass the bus-default fallback logic in process_event_inner (the .unwrap_or(self.event_handler_concurrency) path is never hit). Additionally, with only one handler, the assertion event_results.len() == 1 passes regardless of concurrency/completion mode.

To test bus defaults, remove the event-level overrides (leave them as None) and register multiple handlers to distinguish between All vs First completion and Serial vs Parallel concurrency behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_eventbus_dispatch_defaults.rs, line 25:

<comment>This test doesn't actually verify bus default handler settings. It sets explicit event-level overrides (`Some(EventHandlerConcurrencyMode::Serial)` and `Some(EventHandlerCompletionMode::All)`) which bypass the bus-default fallback logic in `process_event_inner` (the `.unwrap_or(self.event_handler_concurrency)` path is never hit). Additionally, with only one handler, the assertion `event_results.len() == 1` passes regardless of concurrency/completion mode.

To test bus defaults, remove the event-level overrides (leave them as `None`) and register multiple handlers to distinguish between `All` vs `First` completion and `Serial` vs `Parallel` concurrency behavior.</comment>

<file context>
@@ -0,0 +1,33 @@
+    let event = mk_event("work");
+    {
+        let mut inner = event.inner.lock();
+        inner.event_handler_concurrency = Some(EventHandlerConcurrencyMode::Serial);
+        inner.event_handler_completion = Some(EventHandlerCompletionMode::All);
+    }
</file context>
Fix with Cubic

})
}

pub async fn find_typed<E: EventSpec>(
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Make sure concurrency options work correctly and consistently

find_typed drops the child_of parameter, hardcoding it to None. This makes it impossible for typed API users to find events by parent-child relationship — a core feature for nested event tracking and concurrency correctness. The child_of parameter should be exposed, accepting an Option<TypedEvent<_>> or Option<Arc<BaseEvent>>.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/typed.rs, line 94:

<comment>`find_typed` drops the `child_of` parameter, hardcoding it to `None`. This makes it impossible for typed API users to find events by parent-child relationship — a core feature for nested event tracking and concurrency correctness. The `child_of` parameter should be exposed, accepting an `Option<TypedEvent<_>>` or `Option<Arc<BaseEvent>>`.</comment>

<file context>
@@ -0,0 +1,109 @@
+        })
+    }
+
+    pub async fn find_typed<E: EventSpec>(
+        &self,
+        past: bool,
</file context>
Fix with Cubic

pub status: EventResultStatus,
pub event_id: String,
pub handler: EventHandler,
pub timeout: Option<f64>,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Field timeout should be named handler_timeout to match the TS/Python JSON schema (which uses handler_timeout). As-is, JSON serialization will produce "timeout" instead of "handler_timeout", breaking cross-language event JSON compatibility. Additionally, this field is redundant with the already-present self.handler.handler_timeout.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/event_result.rs, line 21:

<comment>Field `timeout` should be named `handler_timeout` to match the TS/Python JSON schema (which uses `handler_timeout`). As-is, JSON serialization will produce `"timeout"` instead of `"handler_timeout"`, breaking cross-language event JSON compatibility. Additionally, this field is redundant with the already-present `self.handler.handler_timeout`.</comment>

<file context>
@@ -0,0 +1,44 @@
+    pub status: EventResultStatus,
+    pub event_id: String,
+    pub handler: EventHandler,
+    pub timeout: Option<f64>,
+    pub started_at: Option<String>,
+    pub result: Option<Value>,
</file context>
Fix with Cubic

@@ -0,0 +1,32 @@
use bubus_rust::id::compute_handler_id;
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Test quality checker

This test duplicates coverage of compute_handler_id that already exists in test_ids.rs. Per the test quality rule: "tests must be organized well in files for each class/pattern being tested and deduped, I prefer a few longer tests that check everything about a given method or behavior to 10 separate tiny ones that overlap."

Merge this Some(handler_file_path) case into the existing test_ids.rs file alongside the existing None case test, and assert against a hardcoded expected UUID (cross-validated with Python/TS) rather than re-deriving the expected value using the same algorithm as the implementation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_ids.rs, line 5:

<comment>This test duplicates coverage of `compute_handler_id` that already exists in `test_ids.rs`. Per the test quality rule: *"tests must be organized well in files for each class/pattern being tested and deduped, I prefer a few longer tests that check everything about a given method or behavior to 10 separate tiny ones that overlap."*

Merge this `Some(handler_file_path)` case into the existing `test_ids.rs` file alongside the existing `None` case test, and assert against a hardcoded expected UUID (cross-validated with Python/TS) rather than re-deriving the expected value using the same algorithm as the implementation.</comment>

<file context>
@@ -0,0 +1,32 @@
+use uuid::Uuid;
+
+#[test]
+fn test_compute_handler_id_matches_uuidv5_seed_algorithm() {
+    let eventbus_id = "0195f6ac-9f10-7e4b-bf69-fb33c68ca13e";
+    let handler_name = "tests.handlers.handle_work";
</file context>
Fix with Cubic


bus.on("work", "first", |_event| async move { Ok(json!("winner")) });
bus.on("work", "second", |_event| async move {
thread::sleep(Duration::from_millis(20));
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Test quality checker

thread::sleep inside an async block blocks the executor thread rather than yielding. This is a known anti-pattern in async Rust. If a regression causes this handler to execute, it will block the entire thread, potentially causing intermittent test hangs. Use an async sleep instead (e.g., tokio::time::sleep).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_first.rs, line 21:

<comment>`thread::sleep` inside an async block blocks the executor thread rather than yielding. This is a known anti-pattern in async Rust. If a regression causes this handler to execute, it will block the entire thread, potentially causing intermittent test hangs. Use an async sleep instead (e.g., `tokio::time::sleep`).</comment>

<file context>
@@ -0,0 +1,41 @@
+
+    bus.on("work", "first", |_event| async move { Ok(json!("winner")) });
+    bus.on("work", "second", |_event| async move {
+        thread::sleep(Duration::from_millis(20));
+        Ok(json!("late"))
+    });
</file context>
Fix with Cubic

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40f46eab2b

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +487 to +490
if handler_completion == EventHandlerCompletionMode::First
&& self.has_winner(&event)
{
break;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wait for all spawned handlers before marking event complete

In parallel handler mode with event_handler_completion == First, this loop breaks as soon as one winner is observed, which drops the remaining JoinHandles without joining or cancelling them. Those handler threads keep running after process_event_inner continues to event.mark_completed(), so wait_completed()/wait_until_idle() can report completion while loser handlers are still executing and mutating event_results.

Useful? React with 👍 / 👎.

Comment on lines +229 to +232
while self.runtime.history_order.lock().len() >= max_size {
if let Some(oldest) = self.runtime.history_order.lock().pop_front() {
self.runtime.events.lock().remove(&oldest);
} else {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prevent history trim from dropping active events

register_in_history evicts the oldest entry whenever capacity is reached, but it never checks whether that event is still Pending/Started. Because events are inserted into history at emit time, bursts with max_history_drop=true can remove in-flight events from runtime.events, which then makes wait_until_idle and parent/child traversal operate on incomplete state and return incorrect results while work is still running.

Useful? React with 👍 / 👎.

Comment on lines +254 to +258
if past {
let child_of_event_id = child_of
.as_ref()
.map(|event| event.inner.lock().event_id.clone());
if let Some(matched) = self.find_in_history(pattern, child_of_event_id.as_deref()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Register future waiters before checking past history

When past and future are both enabled, find scans history first and only registers a future waiter afterward. A matching event emitted in that gap is missed (it was not in the past scan and no waiter existed yet), causing find to block until timeout even though the event already exists in history.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 11 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="bubus-rust/src/typed.rs">

<violation number="1" location="bubus-rust/src/typed.rs:74">
P1: Custom agent: **Make sure READMEs, docs/, and examples/ are up-to-date with implementation**

The `bubus-rust/README.md` quickstart example is broken and won't compile after this change. `bus.emit()` now requires `E: EventSpec` and takes `E::Payload`, but the README still passes an `Arc<BaseEvent>`. The old raw emit API (`emit_raw`) was removed, leaving only typed `emit` methods. The README needs to be updated to demonstrate the new typed API (`EventSpec` trait, `TypedEvent`, `emit`, `emit_with_options`, `emit_existing`, `emit_existing_with_options`).</violation>
</file>

<file name="bubus-rust/tests/test_eventbus_locking.rs">

<violation number="1" location="bubus-rust/tests/test_eventbus_locking.rs:20">
P1: Custom agent: **Test quality checker**

Tests mix typed emission (`bus.emit::<QEvent>(...)`) with untyped handler registration (`bus.on("q", "h", ...)`) and declare `type Result = EmptyResult` on both `QEvent` and `WorkEvent`, even though the handlers return `serde_json::Value` / `json!(1)`. This defeats the purpose of the typed API and violates the strict-typing requirement. Use `bus.on_typed::<QEvent, _, _>(...)` / `bus.on_typed::<WorkEvent, _, _>(...)` with correctly-typed `Result` associated types that match what the handlers actually return (e.g., `serde_json::Value` or a dedicated result struct), rather than a misleading `EmptyResult`.</violation>
</file>

<file name="bubus-rust/tests/test_event_handler_first.rs">

<violation number="1" location="bubus-rust/tests/test_event_handler_first.rs:19">
P1: Custom agent: **Test quality checker**

Test introduces typed `EventSpec` with `type Result = WorkResult` but handlers use untyped `bus.on()` returning `json!("winner")` (a bare string, not a `WorkResult` object). The type contract is decorative and incorrect — calling `first_result()` would panic. Per the test quality rule: "tests must use and ideally enforce proper strict types for everything, including event types, return value types." Use `bus.on_typed::<WorkEvent>(...)` with handlers that return `Ok(WorkResult { value: "winner".into() })` and assert via the typed `first_result()` API instead of reaching into `emitted.inner.inner.lock()`.</violation>
</file>

<file name="bubus-rust/tests/event_bus_tests.rs">

<violation number="1" location="bubus-rust/tests/event_bus_tests.rs:64">
P1: Custom agent: **Test quality checker**

This test only asserts the *count* of handler results (`len() == 2`) but never verifies the actual result *values* (`json!(1)` and `json!(2)`). Per the test quality rule, tests must assert that all output values are behaviorally correct — a length-only check would still pass if both handlers returned identical or wrong values. Add assertions that verify the actual result payloads contain the expected `1` and `2`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

TypedEvent::from_base_event(emitted)
}

pub fn emit_with_options<E: EventSpec>(
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Make sure READMEs, docs/, and examples/ are up-to-date with implementation

The bubus-rust/README.md quickstart example is broken and won't compile after this change. bus.emit() now requires E: EventSpec and takes E::Payload, but the README still passes an Arc<BaseEvent>. The old raw emit API (emit_raw) was removed, leaving only typed emit methods. The README needs to be updated to demonstrate the new typed API (EventSpec trait, TypedEvent, emit, emit_with_options, emit_existing, emit_existing_with_options).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/typed.rs, line 74:

<comment>The `bubus-rust/README.md` quickstart example is broken and won't compile after this change. `bus.emit()` now requires `E: EventSpec` and takes `E::Payload`, but the README still passes an `Arc<BaseEvent>`. The old raw emit API (`emit_raw`) was removed, leaving only typed `emit` methods. The README needs to be updated to demonstrate the new typed API (`EventSpec` trait, `TypedEvent`, `emit`, `emit_with_options`, `emit_existing`, `emit_existing_with_options`).</comment>

<file context>
@@ -67,10 +67,33 @@ impl<E: EventSpec> TypedEvent<E> {
         TypedEvent::from_base_event(emitted)
     }
 
+    pub fn emit_with_options<E: EventSpec>(
+        &self,
+        payload: E::Payload,
</file context>
Fix with Cubic

@@ -0,0 +1,110 @@
use std::{thread, time::Duration};
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Test quality checker

Tests mix typed emission (bus.emit::<QEvent>(...)) with untyped handler registration (bus.on("q", "h", ...)) and declare type Result = EmptyResult on both QEvent and WorkEvent, even though the handlers return serde_json::Value / json!(1). This defeats the purpose of the typed API and violates the strict-typing requirement. Use bus.on_typed::<QEvent, _, _>(...) / bus.on_typed::<WorkEvent, _, _>(...) with correctly-typed Result associated types that match what the handlers actually return (e.g., serde_json::Value or a dedicated result struct), rather than a misleading EmptyResult.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_eventbus_locking.rs, line 20:

<comment>Tests mix typed emission (`bus.emit::<QEvent>(...)`) with untyped handler registration (`bus.on("q", "h", ...)`) and declare `type Result = EmptyResult` on both `QEvent` and `WorkEvent`, even though the handlers return `serde_json::Value` / `json!(1)`. This defeats the purpose of the typed API and violates the strict-typing requirement. Use `bus.on_typed::<QEvent, _, _>(...)` / `bus.on_typed::<WorkEvent, _, _>(...)` with correctly-typed `Result` associated types that match what the handlers actually return (e.g., `serde_json::Value` or a dedicated result struct), rather than a misleading `EmptyResult`.</comment>

<file context>
@@ -1,8 +1,34 @@
+struct EmptyPayload {}
+#[derive(Clone, Serialize, Deserialize)]
+struct EmptyResult {}
+struct QEvent;
+impl EventSpec for QEvent {
+    type Payload = QPayload;
</file context>
Fix with Cubic

value: String,
}
struct WorkEvent;
impl EventSpec for WorkEvent {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Test quality checker

Test introduces typed EventSpec with type Result = WorkResult but handlers use untyped bus.on() returning json!("winner") (a bare string, not a WorkResult object). The type contract is decorative and incorrect — calling first_result() would panic. Per the test quality rule: "tests must use and ideally enforce proper strict types for everything, including event types, return value types." Use bus.on_typed::<WorkEvent>(...) with handlers that return Ok(WorkResult { value: "winner".into() }) and assert via the typed first_result() API instead of reaching into emitted.inner.inner.lock().

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_first.rs, line 19:

<comment>Test introduces typed `EventSpec` with `type Result = WorkResult` but handlers use untyped `bus.on()` returning `json!("winner")` (a bare string, not a `WorkResult` object). The type contract is decorative and incorrect — calling `first_result()` would panic. Per the test quality rule: "tests must use and ideally enforce proper strict types for everything, including event types, return value types." Use `bus.on_typed::<WorkEvent>(...)` with handlers that return `Ok(WorkResult { value: "winner".into() })` and assert via the typed `first_result()` API instead of reaching into `emitted.inner.inner.lock()`.</comment>

<file context>
@@ -1,15 +1,25 @@
+    value: String,
+}
+struct WorkEvent;
+impl EventSpec for WorkEvent {
+    type Payload = EmptyPayload;
+    type Result = WorkResult;
</file context>
Fix with Cubic

}
let emitted = bus.emit_existing(event);
block_on(emitted.wait_completed());
assert_eq!(emitted.inner.inner.lock().event_results.len(), 2);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Test quality checker

This test only asserts the count of handler results (len() == 2) but never verifies the actual result values (json!(1) and json!(2)). Per the test quality rule, tests must assert that all output values are behaviorally correct — a length-only check would still pass if both handlers returned identical or wrong values. Add assertions that verify the actual result payloads contain the expected 1 and 2.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/event_bus_tests.rs, line 64:

<comment>This test only asserts the *count* of handler results (`len() == 2`) but never verifies the actual result *values* (`json!(1)` and `json!(2)`). Per the test quality rule, tests must assert that all output values are behaviorally correct — a length-only check would still pass if both handlers returned identical or wrong values. Add assertions that verify the actual result payloads contain the expected `1` and `2`.</comment>

<file context>
@@ -42,14 +53,14 @@ fn test_parallel_handler_concurrency() {
-    assert_eq!(event.inner.lock().event_results.len(), 2);
+    let emitted = bus.emit_existing(event);
+    block_on(emitted.wait_completed());
+    assert_eq!(emitted.inner.inner.lock().event_results.len(), 2);
     bus.stop();
 }
</file context>
Suggested change
assert_eq!(emitted.inner.inner.lock().event_results.len(), 2);
let results = emitted.inner.inner.lock().event_results.clone();
assert_eq!(results.len(), 2);
let mut values: Vec<serde_json::Value> = results.values().map(|r| r.result.clone().unwrap()).collect();
values.sort_by_key(|v| v.as_i64().unwrap());
assert_eq!(values, vec![json!(1), json!(2)]);
Fix with Cubic

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 187bc68cf1

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +240 to +241
} else {
return false;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle max_history_size=0 without rejecting emits

When max_history_size is Some(0) and max_history_drop is false, current_size >= max_size is always true and this branch returns false, so enqueue_base_with_options immediately marks the event completed without dispatching handlers. That makes zero-history mode silently drop all work instead of running events and then pruning completed history entries (which process_event_inner already tries to do at line 530), so users can lose every emit under this configuration.

Useful? React with 👍 / 👎.

Comment on lines +208 to +209
CURRENT_EVENT_ID.with(|id| inner.event_parent_id = id.borrow().clone());
CURRENT_HANDLER_ID.with(|id| inner.event_emitted_by_handler_id = id.borrow().clone());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve existing parent metadata when re-emitting events

These assignments always overwrite event_parent_id and event_emitted_by_handler_id from thread-local context, even for already-linked events being forwarded. Re-emitting an event from inside its own handler can set event_parent_id to the event’s own ID, which can trap event_is_child_of_ids in an endless parent walk and also corrupt ancestry checks/cancellation for forwarded events that already had a valid parent chain.

Useful? React with 👍 / 👎.

Comment on lines +262 to +263
if let Some(matched) = self.find_in_history(pattern, child_of_event_id.as_deref()) {
return Some(matched);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make find past+future lookup atomic

With past=true and future=Some(...), find checks history first and only later registers a future waiter. A matching event emitted in that gap is missed by both mechanisms (not in the past scan yet, and no waiter registered), so find can incorrectly time out even though a matching event was emitted during the call, causing flaky synchronization under concurrent producers.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 12 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="bubus-rust/tests/test_typed_events.rs">

<violation number="1" location="bubus-rust/tests/test_typed_events.rs:39">
P1: Custom agent: **Make sure READMEs, docs/, and examples/ are up-to-date with implementation**

The `bubus-rust/README.md` Quickstart shows `bus.emit(event.clone())` using an untyped API that no longer exists as a public method. The actual API (as demonstrated by this test change) is `bus.emit::<AddEvent>(TypedEvent::<AddEvent>::new(...))`. The README needs to be updated to show the typed emit pattern and document the new `EventSpec`, `TypedEvent`, `on_typed`, and `find_typed` APIs — otherwise the Quickstart example won't compile.</violation>
</file>

<file name="bubus-rust/tests/event_bus_tests.rs">

<violation number="1" location="bubus-rust/tests/event_bus_tests.rs:33">
P1: Custom agent: **Test quality checker**

Test uses typed emit API (`bus.emit::<WorkEvent>`) but the handler is registered with untyped `bus.on()` and returns `json!("ok")` — a string that is incompatible with `WorkEvent::Result = WorkResult { value: i64 }`. The assertion also checks raw JSON instead of using `event.first_result()`. Per the test quality rule, tests must "use and ideally enforce proper strict types for everything, including event types, return value types." This test should use `on_typed` and assert via `first_result()` to actually validate the typed contract, or at minimum the handler should return a value compatible with the declared result type.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@@ -0,0 +1,58 @@
use bubus_rust::{
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Make sure READMEs, docs/, and examples/ are up-to-date with implementation

The bubus-rust/README.md Quickstart shows bus.emit(event.clone()) using an untyped API that no longer exists as a public method. The actual API (as demonstrated by this test change) is bus.emit::<AddEvent>(TypedEvent::<AddEvent>::new(...)). The README needs to be updated to show the typed emit pattern and document the new EventSpec, TypedEvent, on_typed, and find_typed APIs — otherwise the Quickstart example won't compile.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_typed_events.rs, line 39:

<comment>The `bubus-rust/README.md` Quickstart shows `bus.emit(event.clone())` using an untyped API that no longer exists as a public method. The actual API (as demonstrated by this test change) is `bus.emit::<AddEvent>(TypedEvent::<AddEvent>::new(...))`. The README needs to be updated to show the typed emit pattern and document the new `EventSpec`, `TypedEvent`, `on_typed`, and `find_typed` APIs — otherwise the Quickstart example won't compile.</comment>

<file context>
@@ -36,7 +36,7 @@ fn test_on_typed_and_emit_typed_roundtrip() {
     });
 
-    let event = bus.emit::<AddEvent>(AddPayload { a: 4, b: 9 });
+    let event = bus.emit::<AddEvent>(TypedEvent::<AddEvent>::new(AddPayload { a: 4, b: 9 }));
     block_on(event.wait_completed());
 
</file context>
Fix with Cubic

fn test_emit_and_handler_result() {
let bus = EventBus::new(Some("BusA".to_string()));
bus.on("work", "h1", |_event| async move { Ok(json!("ok")) });
let event = bus.emit::<WorkEvent>(TypedEvent::<WorkEvent>::new(WorkPayload { value: 1 }));
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Test quality checker

Test uses typed emit API (bus.emit::<WorkEvent>) but the handler is registered with untyped bus.on() and returns json!("ok") — a string that is incompatible with WorkEvent::Result = WorkResult { value: i64 }. The assertion also checks raw JSON instead of using event.first_result(). Per the test quality rule, tests must "use and ideally enforce proper strict types for everything, including event types, return value types." This test should use on_typed and assert via first_result() to actually validate the typed contract, or at minimum the handler should return a value compatible with the declared result type.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/event_bus_tests.rs, line 33:

<comment>Test uses typed emit API (`bus.emit::<WorkEvent>`) but the handler is registered with untyped `bus.on()` and returns `json!("ok")` — a string that is incompatible with `WorkEvent::Result = WorkResult { value: i64 }`. The assertion also checks raw JSON instead of using `event.first_result()`. Per the test quality rule, tests must "use and ideally enforce proper strict types for everything, including event types, return value types." This test should use `on_typed` and assert via `first_result()` to actually validate the typed contract, or at minimum the handler should return a value compatible with the declared result type.</comment>

<file context>
@@ -30,7 +30,7 @@ impl EventSpec for WorkEvent {
     let bus = EventBus::new(Some("BusA".to_string()));
     bus.on("work", "h1", |_event| async move { Ok(json!("ok")) });
-    let event = bus.emit::<WorkEvent>(WorkPayload { value: 1 });
+    let event = bus.emit::<WorkEvent>(TypedEvent::<WorkEvent>::new(WorkPayload { value: 1 }));
     block_on(event.wait_completed());
 
</file context>
Fix with Cubic

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73d10ed5a6

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +540 to +542
if timed_out {
break;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Continue serial handlers after a single timeout

Breaking out of the serial handler loop on any timed_out result means one handler timeout prevents later handlers from running, even when completion mode is All. A concrete case is a short event_handler_timeout on the first handler: it returns true from run_handler_with_context, this branch breaks, and subsequent handlers never execute, so the event loses valid results that should still be collected.

Useful? React with 👍 / 👎.

Comment on lines +551 to +555
let mut join_handles = Vec::new();
for handler in handlers {
let bus = self.clone();
let event_clone = event.clone();
join_handles.push(thread::spawn(move || {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply first-completion policy in parallel mode

When handler concurrency is parallel, this branch launches every handler immediately and waits for all of them, regardless of event_handler_completion. If event_handler_completion is First, the event still behaves like All (no early winner, no loser cancellation), which breaks first-result semantics and can keep expensive handlers running unnecessarily.

Useful? React with 👍 / 👎.

Comment on lines +718 to +721
let call_result = if let Some(timeout_secs) = timeout {
rx.recv_timeout(Duration::from_secs_f64(timeout_secs))
.map_err(|_| "timeout".to_string())
} else {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop timed-out handlers from continuing in background

A timeout here only means the receiver stopped waiting; the spawned handler thread keeps running with no cancellation path. If that background handler later emits child events, it can do so after the parent has already timed out and run cancel_children, allowing post-timeout child work to continue even though the parent is considered completed/error.

Useful? React with 👍 / 👎.

@pirate pirate marked this pull request as draft February 26, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant