- Move psycopg2-binary to optional [server] extra with lazy import
- Replace silent fallback PostgreSQL URL with ValueError
- Add TODO for wiring resolve_database_url into engine paths
- Add pool-parameter source comment in UnitOfWork
- Add read-only properties for pool params in UnitOfWork
- Add UUID/INTERVAL/ARRAY/JSONB to portable type allowlist
- Add development-only warning to Docker Compose credentials
- Update BDD scenario for new ValueError behavior
Add PostgreSQL support as the server-mode storage backend alongside
existing SQLite for local mode. Verify all ORM models are dialect-
agnostic, configure connection pooling for multi-user access, add
Docker Compose for local PG development, and wire database URL
selection based on deployment mode.
Changes:
- Add psycopg2-binary dependency to pyproject.toml
- Add server_mode, db_pool_size, db_max_overflow, db_pool_recycle
settings to Settings with environment variable support
- Add resolve_database_url() and is_postgresql() to Settings for
mode-aware database URL resolution
- Configure UnitOfWork engine creation with pool_size, max_overflow,
pool_recycle, and pool_pre_ping for PostgreSQL connections
- Update MigrationRunner to handle both SQLite and PostgreSQL backends
- Add compare_type=True to Alembic env.py for dialect-aware migrations
- Add docker-compose.yml with PostgreSQL 16-alpine for local development
- Add Behave BDD feature (14 scenarios) covering settings, pool config,
engine creation, ORM dialect compatibility, and migration runner
- Add Robot Framework integration tests (12 test cases) for the
abstraction layer with requires_postgresql tag for live PG tests
ISSUES CLOSED: #878
Define TextBackend, VectorBackend, and GraphBackend protocols with
corresponding result dataclasses. Implement in-memory stub backends
for pipeline integration testing. Register backends in DI container
with configurable provider selection.
ISSUES CLOSED: #498
Root cause: _build_skill_service in container.py created a SkillRepository
pointing at the database but did not ensure the skills/skill_items tables
existed. When the tables were missing, SkillRepository.list_all() and
create() failed silently (caught by SkillService._load_from_db and
_persist_skill exception handlers), causing the service to operate in
in-memory-only mode. Skills added in one CLI process were lost when a
new process created a fresh SkillService.
Additionally, SkillRepository lacked auto_commit support. Each call to
session_factory() returned a new session, so the flush in create/update/
delete operated on a different session than the commit in
SkillService._commit(), meaning data was never actually persisted even
when the tables existed.
Fix:
1. Add targeted table creation in _build_skill_service (following the
pattern in _build_session_service) — checks for missing skills and
skill_items tables and creates them via Base.metadata.create_all.
2. Add auto_commit parameter to SkillRepository (following the pattern
in SessionRepository) so each mutating method commits and closes its
own session.
3. Pass auto_commit=True from the container builder.
4. Remove @tdd_expected_fail from TDD test (leaving @tdd_bug and
@tdd_bug_980 as permanent regression guards).
ISSUES CLOSED: #980
Write cross-process Behave and Robot tests capturing skill add
persistence regression. Tests use subprocess invocations to verify
skills persist across CLI process boundaries.
The existing persistence tests (skill_add_persist.feature) verify
round-trip within the same Python process by creating two SkillService
instances sharing the same in-memory database. This approach cannot
detect the cross-process regression where _build_skill_service falls
back to in-memory storage because the skills table does not exist in
the database created by agents init.
Behave: features/tdd_skill_add_regression.feature
Robot: robot/tdd_skill_add_regression.robot
Tags: @tdd_bug, @tdd_bug_980, @tdd_expected_fail
ISSUES CLOSED: #981
Extend the ResourceHandler protocol with six content operations (read,
write, delete, list_children, diff, discover_children) and four frozen
dataclass result types (Content, WriteResult, DeleteResult, DiffResult).
Handler implementations:
- GitCheckoutHandler: read via git show (binary-safe), write/delete via
filesystem ops, list via git ls-tree, diff via git diff --no-index
with locale-safe shortstat parsing, discover via git ls-tree -d
- FsDirectoryHandler: full CRUD via pathlib/os/difflib/shutil
- DevcontainerHandler: read/write/discover via devcontainer exec
- CloudResourceHandler: NotImplementedError stubs for protocol compliance
- DatabaseResourceHandler: inherits base NotImplementedError stubs
Security:
- Path traversal guard (_safe_resolve) on all read/write/delete ops
using os.sep-suffixed startswith check to prevent prefix collisions
- Empty-path deletion rejected with PermissionError
Tests:
- 22 Behave scenarios (115 steps): CRUD for FsDirectory and GitCheckout,
path traversal rejection (3 scenarios), NotImplementedError defaults
- 2 Robot integration tests: read -> write -> diff cycle on real temp
directories and git repos
ISSUES CLOSED: #827
Add 4 LSP-related built-in resource types to the resource registry:
- executable: system binary/interpreter/LSP server binary with
auto-discovery from container-exec-env and fs-directory (lazy)
- lsp-server: LSP server definition with command, language-ids,
transport, args, port, initialization-options config;
children: lsp-workspace
- lsp-workspace: workspace root tracked by LSP server, auto-discovered
from lsp-server; children: lsp-document; not user-addable
- lsp-document: text document tracked by LSP server, auto-discovered
from lsp-workspace; read+write capabilities; not user-addable
Type definitions extracted to _resource_registry_lsp.py for consistency
with existing type modules. Parent/child hierarchy: lsp-server ->
lsp-workspace -> lsp-document. YAML configs with ADR references
(ADR-039, ADR-040). All 7 lsp-server CLI args per ADR-040.
Behave tests (21 scenarios): YAML loading, user-addable flags,
capabilities, parent/child hierarchy, auto-discovery for all 3
discoverable types, BUILTIN_NAMES, DB bootstrap roundtrip, negative
tests for manual registration rejection.
Robot tests (6 tests): import, BUILTIN_NAMES, DB roundtrip, hierarchy,
auto-discovery, user-addable guard.
ISSUES CLOSED: #832
Add Behave and Robot Framework regression tests for bug #647, where
plan tree, plan explain, and plan correct CLI commands crashed with
AttributeError when resolving DecisionService from the DI container.
Tests use a real DI container with seeded decisions (not MagicMock)
to catch the exact class of bug that existing M3 tests missed.
Assertions verify successful execution and command-specific output
content. Includes Settings.reset() classmethod for robust singleton
cleanup in test teardown.
Review feedback addressed (hurui200320 Round 5):
- Fixed Behave step engine leak by capturing UoW in cleanup closure
- Removed dead @then decorator; renamed to private _assert_command_succeeded
- Strengthened plan correct assertions with revert/dry-run content checks
- Updated misleading get_container() comment to reflect singleton warming
- Added test-only warning to Settings.reset() docstring
- Added type annotations to 4 settings step functions
- Fixed CONTRIBUTORS.md alphabetical ordering and removed duplicate entry
- Replaced glob.glob with pathlib suffix iteration in Robot helper
- Fixed feature description line break for readability
- Removed redundant TYPE_CHECKING import for Decision
ISSUES CLOSED: #648
Aligned the plan lifecycle model with the specification:
1. ERRORED is now treated as terminal in is_terminal property,
matching the spec table where errored is marked "Terminal? Yes"
for all processing phases.
2. Added per-phase state validation via model_validator: APPLIED
and CONSTRAINED are only valid in APPLY phase; COMPLETE is only
valid in STRATEGIZE or EXECUTE phases. Invalid combinations
now raise ValueError at construction time.
3. Updated ProcessingState.COMPLETE docstring to clarify phase-level
terminality semantics.
4. Fixed assignment ordering in execute_plan() to set
processing_state before phase, consistent with the state-first
pattern used in apply_plan() and _perform_reversion().
5. Added defensive coercion in LifecyclePlanModel.to_domain() to
handle legacy DB rows with invalid phase/state combinations
(e.g. APPLY/COMPLETE -> APPLY/APPLIED) with warning-level
logging for observability.
6. Updated module docstrings: ERRORED description now reflects
terminal semantics, terminal outcomes location clarified for
all phases, can_revert_to docstring notes ERRORED/CONSTRAINED
are terminal but revertable, is_terminal docstring explains
the distinction between terminal and permanently irrecoverable
and documents why COMPLETE is not plan-terminal despite the
spec marking it "Terminal? Yes" (phase-level vs plan-level).
7. Updated PlanResumeService.validate_eligibility() docstring to
reflect that ERRORED is now terminal but still eligible for
resume.
8. Added CHANGELOG entry.
ISSUES CLOSED: #918
Replaced shell=True with shell=False and shlex.split() for command
tokenization in cli_coverage_steps.py, consistent with the pattern
already used in cli_plan_context_commands_steps.py. Audited all step
files for additional shell=True usages.
ISSUES CLOSED: #734
Enhanced the CorrectionService to properly isolate correction scope:
1. analyze_impact() now populates excluded_decisions by computing
the set difference between all plan decisions and the affected
subtree, ensuring root and sibling decisions are tracked.
2. Added rollback_tier computation that counts parent hops from
the target decision to the root, enabling depth-aware rollback
strategies (tier 0 = root targeted, tier N = N levels deep).
3. Enhanced dry-run report with excluded decisions list, rollback
tier, and tier-0 warning when entire tree is affected.
4. Added subtree isolation validation confirming root exclusion
and sibling non-contamination for non-root corrections.
5. Fixed status state-machine regression in execute_revert() where
analyze_impact() overwrote status back to ANALYZING from EXECUTING.
Guard now only transitions to ANALYZING when status is PENDING.
execute_revert() now transitions through ANALYZING before EXECUTING
for correct lifecycle ordering.
6. Fixed validate_subtree_isolation() to use structural-only BFS for
the sibling invariant check, so that influence-DAG-caused sibling
reachability is not misreported as an isolation violation (per spec
§ Affected Subtree Computation).
7. Fixed false-positive cycle-detection warnings from convergent
(diamond) topologies by replacing per-node seen_this_round with a
global enqueued set in BFS, preventing duplicate queue insertions
from different parents.
8. Added dry_run enforcement guard in _assert_executable() to prevent
execution of dry-run-only corrections per spec (§ plan correct
--dry-run: "Show impact without executing").
9. Fixed generate_dry_run_report() to preserve request status so that
generating a preview does not advance the correction lifecycle
(dry-run is non-mutating per spec). Status restoration now uses
try/finally to guarantee recovery even when analyze_impact() raises
after transitioning the status.
10. Improved cycle-detection log message accuracy to cover both
structural tree and influence DAG sources.
11. Extracted cost/time estimation constants (_COST_PER_DECISION,
_RECOMPUTE_SECONDS_PER_DECISION) from magic numbers.
12. Added terminal-state guard in analyze_impact() to reject
re-analysis after execution (APPLIED/FAILED/CANCELLED/REJECTED),
preventing audit-data corruption. Promoted terminal-status set
to a module-level _TERMINAL_STATUSES frozenset constant.
13. Added mode validation in execute_revert()/execute_append() to
prevent mode-mismatched execution (e.g. calling execute_revert
on an APPEND correction).
14. Fixed _collect_all_decisions() to always include the target
decision in the universe, preventing broken partition invariant
for isolated single-node plans.
15. Fixed tier-0 dry-run warning to only trigger when the target
is genuinely in the structural tree (avoiding false warnings for
nodes not present in the tree).
16. Removed duplicate as_cli_dict regression scenarios in
resource_type_deferred_physical.feature.
ISSUES CLOSED: #845
The _notify_facade and _facade_dispatch functions call get_container()
during lazy facade construction. This can trigger structlog output that
corrupts CLI stdout captured by CliRunner in tests.
Wrap facade construction in redirect_stdout/redirect_stderr to
suppress any side-effect output. Also reset the facade singleton
in after_scenario for test isolation.
Click/Typer CliRunner.Result.stderr is a property that raises
ValueError when stderr was not separately captured (mix_stderr=True
is the default). Wrap all result.stderr accesses in try/except
to handle this gracefully.
The A2A facade now exposes 42 operations (31 extension + 11 legacy)
after the spec-aligned _cleveragents/ extension methods were added.
Update the BDD assertion and docs to match the actual count.
Wire CLI session and plan lifecycle commands through the A2A local
facade, establishing the A2A protocol data flow:
CLI -> A2aLocalFacade.dispatch() -> Service -> Domain.
Key changes:
- Added cli_bootstrap.py module providing get_facade() which lazily
constructs a process-wide A2aLocalFacade instance wired to the DI
container (plan_lifecycle_service, session_service,
resource_registry_service, tool_registry). Service wiring is
best-effort via contextlib.suppress.
- Session CLI create command now notifies the A2A facade after session
creation for protocol bookkeeping and telemetry.
- Plan CLI commands (use, execute, lifecycle-apply) now notify the A2A
facade via _notify_facade() helper after operations complete. The
notification is best-effort (exceptions are suppressed) to avoid
breaking CLI functionality if the facade is not available.
- Added Behave feature (a2a_cli_facade_integration.feature) with 8
scenarios covering: facade bootstrap wiring, all 11 operations
supported, session/plan dispatch through facade, and best-effort
error suppression.
The facade notification pattern preserves backward compatibility:
CLI commands still perform the primary work via direct service calls,
then notify the facade for A2A protocol compliance. This allows
incremental migration toward full facade-first routing.
ISSUES CLOSED: #852
Add Server-Sent Events (SSE) streaming infrastructure to the A2A
event system, enabling real-time delivery of task status updates
and artifact notifications.
Key changes:
- Defined SSE event type constants: TASK_STATUS_UPDATE
(TaskStatusUpdateEvent) and TASK_ARTIFACT_UPDATE
(TaskArtifactUpdateEvent) per the A2A protocol specification.
- Added SseEventFormatter class that converts A2aEvent instances
to text/event-stream format with event, id, and data fields.
Includes keepalive formatting for long-lived connections.
- Added EventBusBridge class that subscribes to the internal
EventBus (ReactiveEventBus) and translates DomainEvent instances
into A2aEvent instances published to the A2aEventQueue. Maps
plan lifecycle events (PLAN_CREATED, PLAN_PHASE_CHANGED, etc.)
to TaskStatusUpdateEvent and checkpoint events to
TaskArtifactUpdateEvent.
- Bridge handles closed queue gracefully via contextlib.suppress.
- Added 8 Behave scenarios covering SSE formatting, event type
constants, EventBusBridge translation for both status and
artifact events, closed queue handling, and JSON payload
validation.
ISSUES CLOSED: #875
## Summary
- **`plan correct` now accepts a plan_id** as its positional argument (in addition to decision_id). When a plan_id is given, the root decision is automatically selected as the correction target.
- The positional parameter is renamed from `decision_id` to `identifier` with updated help text reflecting dual use.
- Backward compatibility is fully preserved: decision_id inputs continue to work exactly as before.
## How it works
1. Try `container.plan_lifecycle_service().get_plan(identifier)` to check if the identifier is a plan_id
2. If it resolves to a real `Plan` object, use it as `resolved_plan_id` and auto-select the root decision (`parent_decision_id is None`)
3. If lookup fails (`ResourceNotFoundError`) or the result is not a `Plan` instance, fall back to treating the identifier as a decision_id (original behavior)
## Verification
- `nox -s lint` — All checks passed
- `nox -s typecheck` — 0 errors, 1 pre-existing warning
- `nox -s unit_tests` (correction features) — 150 scenarios passed, 683 steps passed, 0 failures
ISSUES CLOSED: #969
Reviewed-on: cleveragents/cleveragents-core#1055
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
## Summary
TDD expected-fail tests proving bug #932 exists: the `plan apply` (`lifecycle-apply`) command does not accept the `--yes`/`-y` flag required by the specification. The flag should skip the confirmation prompt before applying plan changes.
### Tests Added
**Behave scenarios** (`features/tdd_plan_apply_yes_flag.feature`):
- `lifecycle-apply --yes` should be accepted (tests `--yes` long flag)
- `lifecycle-apply -y` should be accepted (tests `-y` short flag)
Tags: `@tdd_expected_fail @tdd_bug @tdd_bug_932`
**Robot Framework tests** (`robot/tdd_plan_apply_yes_flag.robot`):
- `check-yes-long` — invokes CLI with `--yes`, asserts no "No such option" error
- `check-yes-short` — invokes CLI with `-y`, asserts no "No such option" error
### How the Bug Is Proven
The `lifecycle_apply_plan` function (`cleveragents.cli.commands.plan`) defines only `plan_id` and `--format` parameters — no `--yes`/`-y`. When tests invoke `lifecycle-apply --yes`, Typer/Click rejects it with `"No such option: --yes"` and exit code 2. The assertion that this error is absent **fails**, confirming bug #932. The `@tdd_expected_fail` tag inverts this to a pass.
### Quality Gates
| Session | Result |
|---|---|
| `nox -s lint` | PASS |
| `nox -s typecheck` | PASS (0 errors) |
| `nox -s unit_tests` | PASS (10,808 scenarios) |
| `nox -s integration_tests` | PASS (1,508 tests) |
| `nox -s coverage_report` | 98% (>= 97%) |
Closes#950
Reviewed-on: cleveragents/cleveragents-core#958
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
## Summary
Add a `code_review.yaml` actor example that uses Claude Opus 4 with file and git tools to perform automated code reviews against the project's review playbook.
### Changes
- **New file**: `examples/actors/code_review.yaml` — LLM actor configured with `files/read_file`, `files/list_directory`, and `builtin/git-*` tools, with a system prompt that reads `docs/development/review-playbook.md` and diffs against `master`
- **Updated**: `features/actor_examples.feature` — bumped example count from 7 to 8 and added `code_review.yaml` to the file listing assertion
### Verification
- `nox -e lint` — passed
- `nox -e typecheck` — 0 errors
- `nox -e format -- --check` — all files formatted
- `nox -s unit_tests -- features/actor_examples.feature` — 25 scenarios passed
### Rebase Notes
- Rebased onto current master (`4d3499dc`)
- Squashed 2 commits into 1 (eliminated 3 merge commits)
- Resolved conflict in `actor_examples.feature`: master added `strategy_with_subplan.yaml` (count 7), our branch adds `code_review.yaml` (count now 8)
Reviewed-on: cleveragents/cleveragents-core#458
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
## Summary
Fixes `plan explain` to accept both decision IDs and plan IDs as the positional argument, matching the M3 acceptance test usage pattern `plan explain <plan_id>`.
### Problem
`explain_decision_cmd` only accepted a decision ULID. When the M3 acceptance test passed a plan ID, `svc.get_decision(plan_id)` returned `None`, causing "Decision not found" error with exit code 1.
### Fix
1. Renamed parameter `decision_id` → `identifier`
2. Tries `svc.get_decision(identifier)` first (backward compat)
3. Falls back to `svc.list_decisions(identifier)` treating it as a plan_id, explaining the root decision
4. Clear error if neither resolves
### Quality Gates
| Session | Result |
|---------|--------|
| `nox -s lint` | PASS |
| `nox -s typecheck` | PASS (0 errors) |
| `nox -s unit_tests` (explain features) | 46/46 PASS |
Closes#968
Reviewed-on: cleveragents/cleveragents-core#1057
Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
Consolidate the execute_plan CLI handler to eliminate a redundant
service.get_plan() call (the separate read-only pre-check now
reuses the same current_plan reference used for phase detection),
update the command-table description from the stale 'Transition to
Execute phase' to 'Run phase-aware plan execution', and replace
the static post-execution hint with a state-aware message that
distinguishes execute/complete (ready for apply) from other states
(continue executing).
Update corresponding Behave test mocks to match the reduced
get_plan call sequence and the updated output panel title.
ISSUES CLOSED: #967
## Summary
Add the missing `--skill` repeatable flag to `actor run` and `actor-run` CLI commands, aligning the implementation with the specification (CLI Synopsis line 277). The flag enables ad-hoc skill injection at runtime without modifying YAML configuration.
Closes#887
## Changes
### DI Container
- **`container.py`**: Added `_build_skill_service()` factory and `skill_service` Singleton provider, following the established `_build_*` pattern. Falls back to in-memory `SkillService()` when the database is unavailable. Exception handling narrowed to `(ImportError, OperationalError, DatabaseError, OSError)` with `exc_info=True` for traceability.
### CLI Layer
- **`actor.py`**: Added `--skill` Typer option (`list[str] | None`, repeatable, `metavar="NAME"`). Help text notes that skills only augment tool-bearing agents. Wrapped constructor in the existing `try/except` block so `CleverAgentsException` from skill resolution is properly caught.
- **`actor_run.py`**: Same `--skill` option with `metavar="NAME"`. Exception handler catches `CleverAgentsException` (matching master — not broadened to `CleverAgentsError`).
- **`skill.py`**: Removed module-level `_service` cache. `_get_skill_service()` now always delegates to `get_container().skill_service()` so that `reset_container()` correctly invalidates the cached instance. `_reset_skill_service()` now overrides the container's provider via `providers.Object()`. Removed dead `validate_skill_names()` function.
### Runtime Layer
- **`application.py`** (438 lines, down from 625): `ReactiveCleverAgentsApp` gains `skill_names` parameter with automatic deduplication via `dict.fromkeys`. `_resolve_skills()` obtains `SkillService` from the DI container (no CLI layer import). Separate `except KeyError` and `except ValueError` produce distinct error messages (`"not found in registry"` vs `"resolution failed: {exc}"`). Skill tools are only injected into agents that already have tools (`if self._resolved_skill_tools and tools:`), preventing LLM agents from being converted to pass-through `SimpleToolAgent` instances. When skill tools are skipped for tool-less agents, `logger.debug` emits a diagnostic message. `_sanitize_skill_name()` validates skill name format with tightened regex: `^[\w.-]{1,127}/[\w.-]{1,127}$` with `re.ASCII` flag. Zero-tool skill warning now uses `logger.warning` (not `print(stderr)`), ensuring structured log output and proper log-level filtering.
- **`graph_executor.py`** (334 lines): Extracted graph execution logic. Type annotations improved.
### Tests
- 24+ Behave scenarios across feature files covering: single/multiple/unknown skill flags, skill+context combined, duplicate deduplication, skill resolution, ValueError path, zero-tool resolution, error handling, tool merging, default behavior, overrides, LLM agent guard, `_sanitize_skill_name` edge cases (empty string, too-long name, ANSI escape codes, disallowed characters), `_build_skill_service` happy+fallback paths, `_get_skill_service` container delegation.
- CLI "unknown skill" tests for **both** `actor.py` and `actor_run.py` exercise the real error chain (mock only `get_container()`, not the entire `ReactiveCleverAgentsApp`), testing `_resolve_skills()` → `CleverAgentsException` → `except CleverAgentsException` → exit code 2 end-to-end.
- Combined skill+context tests assert `ContextManager` was instantiated and `exists()` was called in dedicated **Then** steps.
- `@coverage` tags added to all new scenarios.
- **Robot Framework smoke tests** added (`robot/skill_actor_run.robot` + `robot/helper_skill_actor_run.py`): unknown-skill error path and valid-skill acceptance path.
### Changelog
- Added entry under `## Unreleased` in `CHANGELOG.md`.
## Review Fixes Applied (Brent Edwards, Rounds 1 & 2)
| # | Finding | Resolution |
|---|---------|------------|
| **P1-1** | `print(stderr)` for zero-tool skill warning | **Fixed** — replaced with `logger.warning("Skill '%s' resolved to zero tools", name)`, removed unused `import sys` |
| **P2-2** | Skill tools silently skipped for tool-less agents | **Fixed** — added `logger.debug` when skipped; updated `--skill` help text to note "only augments tool-bearing agents" |
| **P2-3** | `container.py` at 739 lines | **Acknowledged** — pre-existing growth (+59 lines for `_build_skill_service`); extracting factories is a separate refactoring task |
| **P2-4↑** | `CleverAgentsException` → `CleverAgentsError` broadens catch scope | **Fixed** — reverted `actor_run.py` to `except CleverAgentsException` matching master |
| **P3-5** | No Robot Framework smoke test for `--skill` | **Fixed** — added `skill_actor_run.robot` with 2 test cases (unknown-skill error, valid-skill acceptance) |
| **P3-6** | `GraphExecutor._follow_chained_edges` static-calling-static | **Acknowledged** — cosmetic pattern that doesn't affect correctness; can address in a follow-up |
## Known Limitations / Deferred Items
| Item | Reason |
|------|--------|
| `actor.py` at 679 lines (500-line guideline) | Pre-existing (670 on master), +9 lines for `--skill`. Refactoring the shared `_execute()` closure is a separate task. |
| `container.py` at 739 lines (500-line guideline) | Was 680 lines on master, +59 lines for `_build_skill_service()` and `skill_service` provider. Refactoring into sub-modules is a separate task. |
| Code duplication between `actor.py` and `actor_run.py` `run()` | ~47 lines identical code. Coupled with the line-count issue above — both require extracting shared execution logic into a helper module. |
| `SimpleToolAgent` only executes `tools[0]` | Deferred to #974. Pre-existing architectural limitation, not introduced by this PR. |
| `GraphExecutor._follow_chained_edges` static-calling-static pattern | Cosmetic, doesn't affect behavior. |
## Quality Gates
- `nox -s lint`: ✅ PASS
- `nox -s typecheck`: ✅ PASS (0 errors)
- `nox -s unit_tests`: ✅ PASS (11,130 scenarios, 0 failures)
- `nox -s integration_tests`: ✅ PASS (1,559 tests, 0 failures)
- `nox -s coverage_report`: ✅ 97% (meets threshold)
- Branch rebased onto latest `master` (`ab1fd19b`)
Reviewed-on: cleveragents/cleveragents-core#971
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
freemo
hamza.khyari
aditya
Luis Mendes
brent.edwards
salcedo
hurui200320