Compare commits

...

5 Commits

Author SHA1 Message Date
CleverAgents Bot 11de2bddb2 ci: stop master workflow on PR updates
CI / lint (pull_request) Has been cancelled
CI / typecheck (pull_request) Has been cancelled
CI / security (pull_request) Has been cancelled
CI / quality (pull_request) Has been cancelled
CI / unit_tests (pull_request) Has been cancelled
CI / integration_tests (pull_request) Has been cancelled
CI / e2e_tests (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / build (pull_request) Has been cancelled
CI / docker (pull_request) Has been cancelled
CI / helm (pull_request) Has been cancelled
CI / push-validation (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
Remove the stale pull_request trigger from master.yml so PR branch commits do not launch the master workflow.

Maintenance patch for PR #10887.
2026-06-10 20:22:22 -04:00
HAL9000 20a49d92fd fix(actor): catch typer.Exit alongside click.exceptions.Exit in run handlers
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 37s
CI / build (pull_request) Successful in 48s
CI / helm (pull_request) Successful in 50s
CI / lint (pull_request) Successful in 58s
CI / quality (pull_request) Successful in 57s
CI / typecheck (pull_request) Successful in 1m0s
CI / security (pull_request) Successful in 1m12s
CI / e2e_tests (pull_request) Successful in 5m40s
CI / integration_tests (pull_request) Successful in 10m53s
CI / unit_tests (pull_request) Failing after 14m39s
CI / docker (pull_request) Has been skipped
CI / coverage (pull_request) Successful in 23m53s
CI / status-check (pull_request) Failing after 5s
In typer>=0.9.0 typer.Exit is a plain Exception subclass that does not
inherit from click.exceptions.Exit. The run commands in actor.py and
actor_run.py only re-raised click.exceptions.Exit, so a typer.Exit
raised by _resolve_config_files fell through to the generic Exception
handler (exit code 3 instead of 2). The BDD step files also used
click.exceptions.Exit in their except clauses, causing the affected
resolve_config_files scenarios to error out instead of capturing the
expected exit code. Remove unused import click from the two step files.

Refs: #10356
2026-06-07 01:44:20 -04:00
HAL9000 7faab43918 fix(events): add unsubscribe() to mock EventBus test doubles
CI / push-validation (pull_request) Successful in 22s
CI / helm (pull_request) Successful in 32s
CI / build (pull_request) Successful in 36s
CI / lint (pull_request) Successful in 47s
CI / quality (pull_request) Successful in 51s
CI / security (pull_request) Successful in 1m17s
CI / typecheck (pull_request) Successful in 1m25s
CI / benchmark-publish (pull_request) Has been skipped
CI / e2e_tests (pull_request) Successful in 3m32s
CI / integration_tests (pull_request) Failing after 4m15s
CI / unit_tests (pull_request) Failing after 8m18s
CI / docker (pull_request) Has been skipped
CI / coverage (pull_request) Successful in 15m39s
CI / status-check (pull_request) Failing after 2s
The EventBus protocol is @runtime_checkable and gained unsubscribe()
in this PR. Python's isinstance() check requires all protocol methods
to be present, so _MockEventBus and _FailingEventBus in the step files
failed the isinstance(event_bus, EventBus) guard in
FixThenRevalidateOrchestrator.__init__, raising ValidationError before
any scenario step ran — causing 5 errored scenarios.

Added no-op unsubscribe() to:
- _MockEventBus in fix_then_revalidate_coverage_boost_steps.py
- _FailingEventBus in fix_then_revalidate_coverage_boost_steps.py
- _FailingEventBus in fix_then_revalidate_coverage_r3_steps.py

Refs: #10356
2026-06-06 23:43:34 -04:00
HAL9000 73832cd6bb fix(events): remove type: ignore[misc] from protocol unsubscribe stub and fix formatting
CI / helm (pull_request) Successful in 58s
CI / lint (pull_request) Successful in 1m22s
CI / build (pull_request) Successful in 1m14s
CI / quality (pull_request) Successful in 1m47s
CI / typecheck (pull_request) Successful in 1m59s
CI / security (pull_request) Successful in 2m2s
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 20s
CI / integration_tests (pull_request) Successful in 4m1s
CI / e2e_tests (pull_request) Successful in 4m57s
CI / unit_tests (pull_request) Failing after 5m37s
CI / docker (pull_request) Has been skipped
CI / coverage (pull_request) Successful in 14m46s
CI / status-check (pull_request) Failing after 3s
2026-05-05 20:14:11 +00:00
HAL9000 1e9620223f fix(events): add unsubscribe() to EventBus protocol and implementations
CI / lint (pull_request) Failing after 46s
CI / helm (pull_request) Successful in 29s
CI / push-validation (pull_request) Successful in 31s
CI / build (pull_request) Successful in 48s
CI / typecheck (pull_request) Successful in 1m16s
CI / quality (pull_request) Successful in 1m22s
CI / security (pull_request) Successful in 1m29s
CI / coverage (pull_request) Has been skipped
CI / benchmark-publish (pull_request) Has been skipped
CI / e2e_tests (pull_request) Successful in 4m3s
CI / integration_tests (pull_request) Successful in 4m25s
CI / unit_tests (pull_request) Failing after 4m38s
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 5s
Add unsubscribe() method to EventBus protocol and both implementations
(ReactiveEventBus, LoggingEventBus) to prevent handler reference memory
leaks in long-running processes.

Previously, once a handler was registered via subscribe() it could never
be removed. Bound-method handlers hold strong references to their owning
objects, preventing garbage collection even after the owning component
is shut down.

The new unsubscribe() method removes the handler from _subscriptions
using contextlib.suppress(ValueError) for a clean no-op when the handler
was never registered. This releases the strong reference, allowing the
owning object to be garbage collected.

TDD regression scenarios tagged @tdd_issue @tdd_issue_10354 verify:
- Unsubscribed handlers are not called on subsequent emit() calls
- Unsubscribing a non-registered handler is a no-op (no exception)
- Owning objects are garbage collected after unsubscription

ISSUES CLOSED: #10356
2026-04-28 07:24:30 +00:00
13 changed files with 238 additions and 11 deletions
-2
View File
@@ -3,8 +3,6 @@ name: CI
on:
push:
branches: [master, develop]
pull_request:
branches: [master, develop]
vars:
docker_prefix: "http://harbor.cleverthis.com/docker/"
+12
View File
@@ -5,6 +5,18 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
## [Unreleased]
### Fixed
- **EventBus unsubscribe() — memory leak fix** (#10356): Added `unsubscribe()` method
to the `EventBus` protocol and both implementations (`ReactiveEventBus`,
`LoggingEventBus`). Previously, once a handler was registered via `subscribe()` it
could never be removed, causing bound-method handler references to keep owning
objects alive indefinitely in long-running processes. The new `unsubscribe()` method
removes the handler from `_subscriptions`; unsubscribing a handler that was never
registered is a no-op. TDD regression scenarios tagged `@tdd_issue @tdd_issue_10354`
verify unsubscription behaviour and garbage-collection of owning objects.
### Changed
- **Diagnostics spec examples expanded to all 9 providers** (#5320): Updated the
+49
View File
@@ -163,3 +163,52 @@ Feature: EventBus protocol and domain event emission
Scenario: Container wires event_bus into DecisionService
When I resolve decision_service from the DI container
Then the decision_service should have an event_bus attribute
# ---------------------------------------------------------------------------
# unsubscribe() — TDD issue #10354 / bug fix #10356
# ---------------------------------------------------------------------------
@tdd_issue @tdd_issue_10354
Scenario: ReactiveEventBus supports unsubscribing a handler
Given a ReactiveEventBus
When I subscribe to "plan.created" events
And I unsubscribe the handler from "plan.created" events
And I emit a "plan.created" DomainEvent
Then the handler should have received 0 event
@tdd_issue @tdd_issue_10354
Scenario: LoggingEventBus supports unsubscribing a handler
Given a LoggingEventBus
When I subscribe to "plan.created" events
And I unsubscribe the handler from "plan.created" events
And I emit a "plan.created" DomainEvent
Then the handler should have received 0 event
@tdd_issue @tdd_issue_10354
Scenario: Unsubscribing a non-registered handler is a no-op for ReactiveEventBus
Given a ReactiveEventBus
Then unsubscribing a handler that was never registered should not raise
@tdd_issue @tdd_issue_10354
Scenario: Unsubscribing a non-registered handler is a no-op for LoggingEventBus
Given a LoggingEventBus
Then unsubscribing a handler that was never registered should not raise
@tdd_issue @tdd_issue_10354
Scenario: Unsubscribed ReactiveEventBus handler does not prevent garbage collection
Given a ReactiveEventBus
When I subscribe a bound method handler for "plan.created" events
And I unsubscribe the bound method handler from "plan.created" events
And I delete the owning component reference
Then the component should be garbage collected
@tdd_issue @tdd_issue_10354
Scenario: Unsubscribed LoggingEventBus handler does not prevent garbage collection
Given a LoggingEventBus
When I subscribe a bound method handler for "plan.created" events
And I unsubscribe the bound method handler from "plan.created" events
And I delete the owning component reference
Then the component should be garbage collected
Scenario: EventBus Protocol unsubscribe stub is executable
Then the EventBus Protocol unsubscribe stub should be callable
@@ -15,7 +15,6 @@ from types import SimpleNamespace
from typing import Any
from unittest.mock import AsyncMock, MagicMock, patch
import click
import typer
import yaml
from behave import then, when
@@ -165,7 +164,7 @@ def step_resolve_with_no_config_data(context: Any) -> None:
try:
resolve_config_files("local/empty-actor", [])
context.resolve_exit_code = 0
except (SystemExit, click.exceptions.Exit) as exc:
except (SystemExit, typer.Exit) as exc:
context.resolve_exit_code = getattr(
exc, "exit_code", getattr(exc, "code", 1)
)
@@ -209,7 +208,7 @@ def step_resolve_unknown_actor_directly(context: Any) -> None:
try:
resolve_config_files("nonexistent/actor", [])
context.resolve_exit_code = 0
except (SystemExit, click.exceptions.Exit) as exc:
except (SystemExit, typer.Exit) as exc:
context.resolve_exit_code = getattr(
exc, "exit_code", getattr(exc, "code", 1)
)
@@ -260,7 +259,7 @@ def step_resolve_with_empty_config_blob(context: Any) -> None:
try:
resolve_config_files("local/empty-blob-actor", [])
context.resolve_exit_code = 0
except (SystemExit, click.exceptions.Exit) as exc:
except (SystemExit, typer.Exit) as exc:
context.resolve_exit_code = getattr(
exc, "exit_code", getattr(exc, "code", 1)
)
@@ -356,7 +355,7 @@ def step_resolve_with_unserializable_config_blob(context: Any) -> None:
try:
resolve_config_files("local/bad-blob-actor", [])
context.resolve_exit_code = 0
except (SystemExit, click.exceptions.Exit) as exc:
except (SystemExit, typer.Exit) as exc:
context.resolve_exit_code = getattr(
exc, "exit_code", getattr(exc, "code", 1)
)
@@ -6,7 +6,6 @@ import contextlib
from typing import Any
from unittest.mock import MagicMock, patch
import click
import typer
from behave import then, when
@@ -49,7 +48,7 @@ def step_resolve_with_control_character_name(context: Any) -> None:
try:
resolve_config_files(actor_name, [])
context.resolve_exit_code = 0
except (SystemExit, click.exceptions.Exit) as exc:
except (SystemExit, typer.Exit) as exc:
context.resolve_exit_code = getattr(
exc, "exit_code", getattr(exc, "code", 1)
)
+80
View File
@@ -477,3 +477,83 @@ def step_decision_service_has_event_bus(ctx: Context) -> None:
"DecisionService resolved from DI has no event_bus attribute"
)
assert svc.event_bus is not None, "DecisionService.event_bus should not be None"
# ---------------------------------------------------------------------------
# unsubscribe() steps — TDD issue #10354 / bug fix #10356
# ---------------------------------------------------------------------------
@when('I unsubscribe the handler from "{et}" events')
def step_unsubscribe_handler(ctx: Context, et: str) -> None:
assert ctx.collectors, "No collectors registered — subscribe first"
ctx.bus.unsubscribe(EventType(et), ctx.collectors[0])
@then("unsubscribing a handler that was never registered should not raise")
def step_unsubscribe_never_registered(ctx: Context) -> None:
raised = False
try:
ctx.bus.unsubscribe(EventType.PLAN_CREATED, lambda e: None)
except Exception:
raised = True
assert not raised, (
"Expected no exception when unsubscribing an unregistered handler"
)
class _GCComponent:
"""Minimal component whose bound method can be subscribed as a handler."""
def __init__(self) -> None:
self.received: list[DomainEvent] = []
def handle(self, event: DomainEvent) -> None:
self.received.append(event)
@when('I subscribe a bound method handler for "{et}" events')
def step_subscribe_bound_method(ctx: Context, et: str) -> None:
import weakref
ctx.gc_component = _GCComponent()
ctx.gc_component_ref: weakref.ref[_GCComponent] = weakref.ref(ctx.gc_component)
ctx.bound_handler = ctx.gc_component.handle
ctx.bus.subscribe(EventType(et), ctx.bound_handler)
@when('I unsubscribe the bound method handler from "{et}" events')
def step_unsubscribe_bound_method(ctx: Context, et: str) -> None:
ctx.bus.unsubscribe(EventType(et), ctx.bound_handler)
@when("I delete the owning component reference")
def step_delete_component_ref(ctx: Context) -> None:
import gc
del ctx.gc_component
del ctx.bound_handler
gc.collect()
@then("the component should be garbage collected")
def step_component_garbage_collected(ctx: Context) -> None:
assert ctx.gc_component_ref() is None, (
"Component was NOT garbage collected — the bus still holds a strong reference"
)
@then("the EventBus Protocol unsubscribe stub should be callable")
def step_protocol_unsubscribe_stub_callable(ctx: Context) -> None:
"""Exercise Protocol unsubscribe stub directly to ensure 100% coverage."""
obj = type(
"_BareImpl",
(),
{
"emit": lambda self, event: None,
"subscribe": lambda self, et, h: None,
"unsubscribe": lambda self, et, h: None,
},
)()
obj.unsubscribe(EventType.PLAN_CREATED, lambda e: None)
@@ -547,6 +547,13 @@ class _MockEventBus:
) -> None:
pass # Not needed for this test
def unsubscribe(
self,
event_type: EventType,
handler: Any,
) -> None:
pass # Not needed for this test
class _FailingEventBus:
"""Mock EventBus whose emit always raises."""
@@ -561,6 +568,13 @@ class _FailingEventBus:
) -> None:
pass
def unsubscribe(
self,
event_type: EventType,
handler: Any,
) -> None:
pass
# ---------------------------------------------------------------------------
# Given steps for new review-fix scenarios
@@ -118,6 +118,13 @@ class _FailingEventBus:
) -> None:
pass
def unsubscribe(
self,
event_type: EventType,
handler: Any,
) -> None:
pass
# ---------------------------------------------------------------------------
# Given steps
+1 -1
View File
@@ -185,7 +185,7 @@ def run(
except UnsafeConfigurationError as exc:
typer.echo(f"Error: {exc}", err=True)
raise typer.Exit(code=1) from exc
except click.exceptions.Exit:
except (click.exceptions.Exit, typer.Exit):
raise
except CleverAgentsError as exc:
typer.echo(f"Error: {exc}", err=True)
+1 -1
View File
@@ -159,7 +159,7 @@ def run(
except UnsafeConfigurationError as exc:
typer.echo(f"Error: {exc}", err=True)
raise typer.Exit(code=1) from exc
except click.exceptions.Exit:
except (click.exceptions.Exit, typer.Exit):
raise
except CleverAgentsError as exc:
typer.echo(f"Error: {exc}", err=True)
@@ -13,6 +13,7 @@ Based on:
from __future__ import annotations
import contextlib
from collections.abc import Callable
import structlog
@@ -105,5 +106,30 @@ class LoggingEventBus:
raise TypeError("handler must be callable")
self._subscriptions.setdefault(event_type, []).append(handler)
def unsubscribe(
self,
event_type: EventType,
handler: Callable[[DomainEvent], None],
) -> None:
"""Remove *handler* from the list of subscribers for *event_type*.
If *handler* was not registered for *event_type*, this is a no-op
no exception is raised. This makes it safe to call during component
teardown without tracking whether :meth:`subscribe` was ever called.
Removing a handler releases the strong reference held by
``_subscriptions``, allowing the owning object to be garbage
collected once all other references are dropped.
Args:
event_type: The :class:`EventType` the handler was registered for.
handler: The callable to remove.
"""
handlers = self._subscriptions.get(event_type)
if handlers is None:
return
with contextlib.suppress(ValueError):
handlers.remove(handler)
__all__ = ["LoggingEventBus"]
@@ -49,5 +49,23 @@ class EventBus(Protocol):
"""
...
def unsubscribe(
self,
event_type: EventType,
handler: Callable[[DomainEvent], None],
) -> None:
"""Remove a previously registered handler for *event_type*.
If *handler* was not registered for *event_type*, this is a no-op
(no exception is raised). This allows callers to safely call
``unsubscribe`` during component teardown without tracking whether
``subscribe`` was ever called.
Args:
event_type: The :class:`EventType` the handler was registered for.
handler: The callable to remove.
"""
...
__all__ = ["EventBus"]
@@ -163,6 +163,31 @@ class ReactiveEventBus:
raise TypeError("handler must be callable")
self._subscriptions.setdefault(event_type, []).append(handler)
def unsubscribe(
self,
event_type: EventType,
handler: Callable[[DomainEvent], None],
) -> None:
"""Remove *handler* from the list of subscribers for *event_type*.
If *handler* was not registered for *event_type*, this is a no-op
no exception is raised. This makes it safe to call during component
teardown without tracking whether :meth:`subscribe` was ever called.
Removing a handler releases the strong reference held by
``_subscriptions``, allowing the owning object to be garbage
collected once all other references are dropped.
Args:
event_type: The :class:`EventType` the handler was registered for.
handler: The callable to remove.
"""
handlers = self._subscriptions.get(event_type)
if handlers is None:
return
with contextlib.suppress(ValueError):
handlers.remove(handler)
@property
def stream(self) -> Observable:
"""Read-only observable stream for advanced RxPY operators.