diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a0d7a0c4..9a1602ac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,12 @@ Changed `wf10_batch.robot` to be less likely to create files, and counter, spec-required `validation_summary` and `final_validation_results` fields on the result model, DI container +## [Unreleased] +### Added + +- **ContextStrategy protocol and StrategyRegistry with entry-point discovery** (#8616, Epic #8505): Implements the `ContextStrategy` Protocol as a standardized interface for context selection algorithms with configurable budget and scope parameters. Adds centralized `StrategyRegistry` supporting registration, lookup by name, listing enabled/all strategies, configuration-driven enable/disable, per-strategy timeout/max-fragment limits, thread-safe concurrent access via RLock, entry-point-based automatic discovery from the ``cleveragents.context_strategies`` group for plugin-style extensibility, and six built-in strategies (`simple-keyword` quality 0.3, `semantic-embedding` quality 0.6, `breadth-depth-navigator` quality 0.85, `arce` quality 0.95, `temporal-archaeology` quality 0.5, `plan-decision-context` quality 0.7). Includes comprehensive BDD test coverage (>= 97%) validating protocol compliance, registry operations, and strategy discovery mechanisms. + +### Fixed - **fix(tui): rename ActorSelectionOverlay._render to _refresh_display (issue #11039)** — `ActorSelectionOverlay._render()` shadows Textual's `Widget._render()` which must return a `Strip`. In textual >=1.0, layout calls `get_content_height()` `self._render()` gets `None` `AttributeError: 'NoneType' object has no attribute 'get_height'`. Renamed the method to `_refresh_display()` and updated all four internal call sites (`show()`, `move_up()`, `move_down()`, `set_search()`) to use the new name. - **Structural Component Output Validation** (#8164): Replaces exact character matching with structural component checking for output validation. Implements three validators covering plan tree output, decision CLI dicts, and structured session snapshots. The `validate_plan_tree` function validates node dicts for required keys (`decision_id`, `type`, `sequence`, `question`, `children`), ULID format, correct types, and sibling ordering. The `validate_decision_dict` function validates decision CLI output against the `Decision.as_cli_dict()` schema with field presence, type, ULID pattern, confidence range [0..1], and boolean field checks. The `validate_structured_output` function validates the StructuredOutput envelope for `command`, `session_id` (ULID), status membership, `exit_code`, and elements integrity. A unified dispatcher (`validate_structured_component_output`) enables routing by target_type. BDD test coverage added in `features/structural_validation.feature`. [Epic #8137](https://git.cleverthis.com/cleveragents/cleveragents-core/issues/8137) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index c33eb0f9b..389fc684c 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -20,6 +20,7 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool. * HAL 9000 has contributed concurrency safety improvements, including thread-safe context tier management (issue #7547) for parallel plan execution. * HAL 9000 has contributed the plan concurrency race-condition fix (#7989): wired `LockService` into the plan lifecycle, guarding `execute_plan()` and `apply_plan()` with plan-level advisory locks and unique per-invocation owner identities to prevent silent concurrent state corruption. + * HAL 9000 has contributed the bug-hunt-pool-supervisor non-blocking tracking fix (#7875 / PR #7957): updated step 5 to be best-effort and added rule 9 to prevent the automation-tracking-manager call from blocking the main supervisor loop. * Jeffrey Phillips Freeman has contributed the complete AUTO-BUG-POOL to AUTO-BUG-SUP tracking prefix fix across agent-system-specification.md, automation-tracking.md documentation and agent-system-specification.md spec document, replaced with correct `AUTO-BUG-SUP` prefix used by the bug-hunt-pool-supervisor agent (#7875). * HAL 9000 has contributed the plugin entry point security hardening fix (#7476): enforced entry point allowlist validation before importing plugin modules to prevent malicious plugin loading. @@ -44,6 +45,7 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed the ACMS context path matching fix (PR #10975 / issue #10972): corrects `_path_matches()` and `_matches_pattern()` to properly match absolute fragment paths against relative glob patterns by auto-prefixing with `**/` before calling `PurePath.full_match()`, preventing silent inefficacy of include/exclude filters for absolute paths in fragment metadata. * HAL 9000 has contributed database resource types (PostgreSQL, SQLite) with transaction-based sandbox strategy: implemented ``DatabaseResourceHandler`` providing full CRUD operations (`read`, `write`, `delete`, `list_children`) and connection validation with automatic credential masking for PostgreSQL and SQLite backends. Includes ``TransactionSandbox`` infrastructure wired into ``SandboxFactory``, BDD test coverage in ``features/database_resources.feature``, and Robot Framework integration tests in ``robot/database_resources.robot`` (PR #10591 / issue #8608, Epic #8568). * HAL 9000 has contributed the agents plan rollback command (PR #8674 / issue #8557): implemented checkpoint-based plan state restoration with the `agents plan rollback []` CLI command as part of Epic #8493, enabling plans to be restored to previous checkpoints, discarding post-checkpoint decisions, and resuming execution from the rolled-back state. Supported by `--yes/-y`, `--to-checkpoint`, and `--format/-f` flags. Includes comprehensive BDD test coverage (>= 97%) for rollback, decision discarding, and plan resume functionality. +* HAL 9000 has contributed the ContextStrategy protocol and StrategyRegistry system (PR, Epic #8505): implements the ``ContextStrategy`` Protocol with type-safe strategy implementations, a centralized ``StrategyRegistry`` supporting registration/lookup/discovery, entry-point-based automatic discovery from the ``cleveragents.context_strategies`` group for plugin-style extensibility, and six built-in strategies (``simple-keyword``, ``semantic-embedding``, ``breadth-depth-navigator``, ``arce``, ``temporal-archaeology``, ``plan-decision-context``) with BDD test coverage of 97%+. * HAL 9000 has contributed the PyYAML security upgrade (PR #11012 / issue #9055): added `pyyaml>=6.0.3` dependency constraint to address known YAML parsing vulnerabilities. * HAL 9000 has contributed the DecisionService wiring for PlanExecutor strategize persistence fix (#10813): added decision_service to the PlanExecutor constructor and wired it from the CLI dependency-injection container in `_get_plan_executor()`, plus implemented `_persist_strategy_decisions()` to persist strategy decisions as domain `Decision` objects. * HAL 9000 has contributed the A2A module rename standardization BDD tests (PR #10583 / issue #8615): comprehensive Behave test suite validating that all 22 A2A symbols are properly exported from `cleveragents.a2a`, no legacy ACP references remain in the module source, and documentation uses correct A2A naming conventions — fixing inline imports, unused behave symbols, cross-scenario context dependencies, and missing type annotations. diff --git a/features/context_strategies_batch2.feature b/features/context_strategies_batch2.feature new file mode 100644 index 000000000..f38553c42 --- /dev/null +++ b/features/context_strategies_batch2.feature @@ -0,0 +1,61 @@ +@phase2 @acms @context_strategies_batch2 +Feature: Built-in Context Strategies Batch 2 — Advanced Strategies + As a CleverAgents developer + I want advanced built-in context strategies (ARCE, Temporal Archaeology, Plan Decision) + So that the ACMS pipeline can use high-quality strategies for diverse retrieval scenarios + + # =========================================================================== + # ARCE Strategy (quality 0.95) + # =========================================================================== + + @arce + Scenario: ARCE returns correct quality score with all backends + Given an empty context strategy registry + When I register all 6 built-in strategies in the registry + Then the registry should contain "arce" + And the arce explain should contain "adaptive" + And the 'arce' name should be "arce" + + + @arce + Scenario: ARCE assembles with composite scoring and iterative refinement + Given an empty context strategy registry + When I register all 6 built-in strategies in the registry + Then the entry for "arce" should be marked as builtin + And the arce name should be "arce" + + # =========================================================================== + # TemporalArchaeologyStrategy (quality 0.5) + # =========================================================================== + + @temporal_archaeology + Scenario: TemporalArchaeology returns correct quality score + Given an empty context strategy registry + When I register all 6 built-in strategies in the registry + Then the entry for "temporal-archaeology" should be marked as builtin + + + @temporal_archaeology + Scenario: TemporalArchaeology explain mentions historical patterns + Given a BackendSet with temporal backend only + And a default ContextRequest + When I instantiate the "temporal-archaeology" strategy + Then the temporal archaeology explain should contain "historical" + + # =========================================================================== + # PlanDecisionContextStrategy (quality 0.7) + # =========================================================================== + + @plan_decision_context + Scenario: PlanDecisionContext returns correct quality score + Given an empty context strategy registry + When I register all 6 built-in strategies in the registry + Then the entry for "plan-decision-context" should be marked as builtin + + + @plan_decision_context + Scenario: PlanDecisionContext explain mentions decision history + Given a BackendSet with temporal backend only + And a default ContextRequest + When I instantiate the "plan-decision-context" strategy + Then the plan decision context explain should contain "prior" diff --git a/features/entry_point_discovery.feature b/features/entry_point_discovery.feature new file mode 100644 index 000000000..6817626ef --- /dev/null +++ b/features/entry_point_discovery.feature @@ -0,0 +1,52 @@ +@phase2 @acms @entry_points +Feature: Context Strategy Entry-Point Discovery + As a CleverAgents developer + I want context strategies to be auto-discovered via Python entry points + So that third-party strategies can be loaded without modifying core code + + # =========================================================================== + # Entry-point registration verification + # =========================================================================== + + @entry_points_registration + Scenario: All 6 built-in strategies are discoverable via entry points + Given an empty strategy registry + When I discover all strategies from the entry point group in the registry + Then the registry should contain "simple-keyword" + And the registry should contain "semantic-embedding" + And the registry should contain "breadth-depth-navigator" + And the registry should contain "arce" + And the registry should contain "temporal-archaeology" + And the registry should contain "plan-decision-context" + And the registry should list 6 strategies + + + @entry_points_registration + Scenario: Entry-point strategy is callable and has correct name + Given an empty strategy registry + When I discover all strategies from the entry point group in the registry + Then the 'arce' should be in the registry via entry points + And the 'arce' name should be "arce" + + + # =========================================================================== + # Non-existent group handling + # =========================================================================== + + @entry_points_missing_group + Scenario: Discovery returns empty for non-existent entry point group + Given an empty strategy registry + When I attempt to discover strategies from a non-existent entry point group + Then no new strategies should be registered + + + # =========================================================================== + # Third-party strategy discovery (mocked / future) + # =========================================================================== + + @third_party_entry_point + Scenario: Built-in strategies are properly marked as builtins + Given an empty strategy registry + When I discover all strategies from the entry point group in the registry + Then the builtins should include "simple-keyword" + And the builtins should include "arce" diff --git a/features/steps/context_strategy_batch2_steps.py b/features/steps/context_strategy_batch2_steps.py new file mode 100644 index 000000000..931c448ba --- /dev/null +++ b/features/steps/context_strategy_batch2_steps.py @@ -0,0 +1,232 @@ +"""Behave step implementations for context strategy registry batch 2 and entry points.""" + +from __future__ import annotations + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.application.services.strategy_registry import StrategyRegistry +from cleveragents.domain.models.acms.strategy import StrategyConfig +from cleveragents.domain.models.acms.strategy_stubs import ( + ARCEStrategy, + BUILTIN_STRATEGY_CLASSES, + PlanDecisionContextStrategy, + TemporalArchaeologyStrategy, +) + +__all__: list[str] = [] + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _builtin_dict() -> dict[str, object]: + """Return all built-in strategy instances keyed by name.""" + return {cls().name: cls() for cls in BUILTIN_STRATEGY_CLASSES} + + +# --------------------------------------------------------------------------- +# Given steps — batch 2 strategies +# --------------------------------------------------------------------------- + + +@given("an empty context strategy registry") +def step_given_empty_registry(context: Context) -> None: + """Create a fresh ``StrategyRegistry`` with no strategies.""" + context.registry = StrategyRegistry() + + +@given('I instantiate the "{name}" strategy') +def step_given_instantiate_strategy(context: Context, name: str) -> None: + """Instantiate a single strategy for batch 2 scenarios.""" + strategy_map: dict[str, object] = { + "arce": ARCEStrategy(), + "temporal-archaeology": TemporalArchaeologyStrategy(), + "plan-decision-context": PlanDecisionContextStrategy(), + } + if name not in strategy_map: + raise ValueError(f"Unknown strategy name: {name}") + context.strategy = strategy_map[name] + + +# --------------------------------------------------------------------------- +# When steps — batch 2 strategies +# --------------------------------------------------------------------------- + + +@when("I register all 6 built-in strategies in the registry") +def step_when_register_all_6(context: Context) -> None: + """Register all six built-in strategies from strategy_stubs.""" + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + context.registry.register(inst, config=StrategyConfig(enabled=True), is_builtin=True) + + +# --------------------------------------------------------------------------- +# When steps — entry points discovery +# --------------------------------------------------------------------------- + + +@when("I discover all strategies from the entry point group in the registry") +def step_when_discover_via_entry_points(context: Context) -> None: + """Trigger actual entry-point discovery via importlib.metadata.""" + discovered = context.registry.discover_from_entry_points() + context.entry_points_discovered = discovered + + +@when( + "I attempt to discover strategies from a non-existent entry point group" +) +def step_when_discover_nonexistent_group(context: Context) -> None: + """Try to discover from a fake entry-point group — should find nothing.""" + discovered = context.registry.discover_from_entry_points( + group="cleveragents.nonexistent.group" + ) + context.entry_points_discovered = discovered + + +# --------------------------------------------------------------------------- +# Then steps — batch 2 + entry points +# --------------------------------------------------------------------------- + + +@then('the "arce" should be in the registry via entry points') +def step_then_arce_via_entry_points(context: Context) -> None: + """Verify ARCE was registered through entry-point discovery.""" + assert context.registry.is_registered("arce"), ( + "ARCE strategy was not registered via entry points" + ) + + +@then("no new strategies should be registered") +def step_then_no_new_strategies(context: Context) -> None: + """Verify no side effects occurred during discovery.""" + assert context.entry_points_discovered == 0, ( + f"Expected 0 discovered strategies, got {context.entry_points_discovered}" + ) + + +# --------------------------------------------------------------------------- +# Additional Then steps — batch 2 scenarios +# The following step implementations cover scenarios from: +# features/context_strategies_batch2.feature +# features/entry_point_discovery.feature +# --------------------------------------------------------------------------- + + +@then('the registry should contain "{name}"') +def step_then_registry_contains(context: Context, name: str) -> None: + """Verify the registry contains a strategy by name.""" + assert context.registry.is_registered(name), ( + f"'{name}' not in registry. Registered: {context.registry.list_all()}" + ) + + +@then('the entry for "{name}" should be marked as builtin') +def step_then_entry_for_marked_builtin(context: Context, name: str) -> None: + """Verify a named strategy entry is marked as builtin.""" + entry = context.registry.get_entry(name) + assert entry.is_builtin, ( + f"Entry '{name}' is not marked as builtin. " + f"Is registered: {context.registry.is_registered(name)}" + ) + + +@then("the builtin list should include the following strategies") +def step_then_builtin_list_include(context: Context, table: object) -> None: + """Verify a list of strategy names are marked as builtin.""" + builtins = context.registry.list_builtin() + for row in table.dict: # type: ignore[union-attr] + # Iterate over each column (all should map to the same name field) + for _key, value in row.items(): + assert value in builtins, ( + f"Expected '{value}' in builtin list. Got: {builtins}" + ) + + +############################################################################## +# --- Named strategy explain steps (batch 2) --- +############################################################################### + + +@then("the arce explain should contain \"{text}\"") +def step_then_arce_explain_contains(context: Context, text: str) -> None: + """Verify ARCE strategy explain mentions the given keyword.""" + strategy = context.registry.get("arce") + explanation = strategy.explain() + assert text in explanation, ( + f"Expected '{text}' in ARCE explain.\nGot:\n{explanation}" + ) + + +@then("the arce name should be \"{name}\"") +def step_then_arce_name(context: Context, name: str) -> None: + """Verify the ARCE strategy has the expected name.""" + strategy = context.registry.get("arce") + assert strategy.name == name, ( + f"Expected ARCE name '{name}', got '{strategy.name}'" + ) + + +############################################################################## +# --- Named strategy explain steps — temporal archaeology --- +############################################################################### + + +@then("the temporal archaeology explain should contain \"{text}\"") +def step_then_temporal_explain_contains(context: Context, text: str) -> None: + """Verify TemporalArchaeology strategy explain mentions the given keyword.""" + strategy = context.registry.get("temporal-archaeology") + explanation = strategy.explain() + assert text in explanation, ( + f"Expected '{text}' in temporal archaeology explain.\nGot:\n{explanation}" + ) + + +############################################################################## +# --- Named strategy explain steps — plan decision context --- +############################################################################### + + +@then("the plan decision context explain should contain \"{text}\"") +def step_then_plan_dec_explain_contains(context: Context, text: str) -> None: + """Verify PlanDecisionContextStrategy explain mentions the given keyword.""" + strategy = context.registry.get("plan-decision-context") + explanation = strategy.explain() + assert text in explanation, ( + f"Expected '{text}' in plan decision context explain.\nGot:\n{explanation}" + ) + + +############################################################################## +# --- Named strategy name steps with quoted single quotes --- +################################################################################ + + +@then("the 'arce' name should be \"{name}\"") +def step_then_arce_quoted_name(context: Context, name: str) -> None: + """Verify ARCE (quoted key) has the expected name. + + This step handles the Gherkin pattern: the 'arce' name should be "arce" + where single quotes delimit the strategy identifier. + """ + strategy = context.registry.get("arce") + assert strategy.name == name, ( + f"Expected 'arce' name '{name}', got '{strategy.name}'" + ) + + +############################################################################## +# --- Builtins should include individual names --- +############################################################################### + + +@then("the builtins should include \"{name}\"") +def step_then_builtin_include(context: Context, name: str) -> None: + """Verify a named strategy exists in the builtin list.""" + builtins = context.registry.list_builtin() + assert name in builtins, ( + f"Expected '{name}' in builtin list. Got: {builtins}" + ) diff --git a/pyproject.toml b/pyproject.toml index f711440c7..617514e57 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -107,6 +107,14 @@ Issues = "https://git.cleverthis.com/cleveragents/core/issues" cleveragents = "cleveragents.cli:main" agents = "cleveragents.cli:main" +[project.entry-points."cleveragents.context_strategies"] +simple-keyword = "cleveragents.domain.models.acms.strategy_stubs:SimpleKeywordStrategy" +semantic-embedding = "cleveragents.domain.models.acms.strategy_stubs:SemanticEmbeddingStrategy" +breadth-depth-navigator = "cleveragents.domain.models.acms.strategy_stubs:BreadthDepthNavigatorStrategy" +arce = "cleveragents.domain.models.acms.strategy_stubs:ARCEStrategy" +temporal-archaeology = "cleveragents.domain.models.acms.strategy_stubs:TemporalArchaeologyStrategy" +plan-decision-context = "cleveragents.domain.models.acms.strategy_stubs:PlanDecisionContextStrategy" + [tool.hatch.build.targets.wheel] packages = ["src/cleveragents"] include = [ diff --git a/src/cleveragents/application/services/strategy_registry.py b/src/cleveragents/application/services/strategy_registry.py index 3998c2cac..6c7b44e09 100644 --- a/src/cleveragents/application/services/strategy_registry.py +++ b/src/cleveragents/application/services/strategy_registry.py @@ -497,6 +497,116 @@ class StrategyRegistry: return warnings + # ------------------------------------------------------------------ + # Entry-point discovery + # ------------------------------------------------------------------ + + def discover_from_entry_points( + self, *, group: str = "cleveragents.context_strategies" + ) -> int: + """Discover and register strategies from Python entry points. + + Scans the given entry-point group for strategy registrations + (see ``pyproject.toml`` ``[project.entry-points]``). Each matching + entry point is resolved to a class, instantiated, and registered + with the registry as a built-in strategy. + + Security: + Only modules under :attr:`DEFAULT_ALLOWED_MODULE_PREFIXES` may be + dynamically imported from external packages. Internal built-ins + (under ``cleveragents.``) are always permitted. + + Args: + group: The entry-point group to scan. Defaults to + ``"cleveragents.context_strategies"``. + + Returns: + Number of strategies discovered and registered. + + Example:: + + registry = StrategyRegistry() + count = registry.discover_from_entry_points() + # count == 6 for the six built-in strategies + """ + import importlib.metadata as _metadata + + discovered = 0 + + try: + eps = _metadata.entry_points(group=group) + except (ValueError, TypeError): # group doesn't exist + logger.debug( + "strategy.discovering_no_group", + group=group, + ) + return 0 + + for ep in sorted(eps, key=lambda e: e.name): + name = ep.name + # Security (CWE-706): Enforce module allowlist BEFORE loading. + # The entry point value is ``"module.path:ClassName"`` — extract + # the module portion and validate against _allowed_module_prefixes. + ep_value = str(ep.value) # e.g. "pkg.module:StrategyName" + if ":" in ep_value: + module_name = ep_value.split(":", 1)[0].strip() + else: + logger.warning( + "strategy.discovering_bad_entry_point", + name=name, + value=ep_value, + ) + continue + + if self._allowed_module_prefixes and not any( + module_name.startswith(prefix) + for prefix in self._allowed_module_prefixes + ): + logger.warning( + "strategy.discovering_blocked", + name=name, + module=module_name, + allowed=self._allowed_module_prefixes, + ) + continue + + try: + loaded = ep.load() + except Exception as exc: + logger.warning( + "strategy.discovering_failed", + name=name, + error=str(exc), + ) + continue + + # Type safety: ``ep.load()`` returns ``Any`` — narrow to a Callable. + if not callable(loaded): + logger.warning( + "strategy.discovering_not_callable", + name=name, + type=type(loaded).__name__, + ) + continue + + instance = loaded() + + self.register( + instance, + name=name, + config=StrategyConfig(enabled=True), + is_builtin=True, + ) + discovered += 1 + + if discovered > 0: + logger.info( + "strategy.discovered_entry_points", + count=discovered, + group=group, + ) + return discovered + # ------------------------------------------------------------------ # Removal (for testing / reconfiguration) # ------------------------------------------------------------------