Implement the missing runtime logic for the ACMS context tier service.
Previously only data models and manual promote()/demote()/evict_lru()
methods existed. This commit adds:
- Auto-promotion on access: get() now promotes fragments one tier up
when access_count reaches the configurable promotion_threshold
(default: 5 accesses). The access counter resets after each
successful promotion so fragments must accumulate fresh accesses
before the next tier transition.
- Staleness enforcement: new enforce_staleness() method demotes hot
fragments older than hot_ttl (default: 24h) to warm, and warm
fragments older than warm_ttl to cold. A snapshot of existing
warm-tier IDs prevents double-demotion in a single pass.
- Budget enforcement on store and promote: store() and promote()
now enforce TierBudget.max_tokens_hot by evicting LRU hot-tier
fragments until the token budget is met. The eviction loop uses
incremental token tracking to avoid recomputing the sum.
- Event emission: Added TIER_PROMOTED, TIER_DEMOTED, TIER_EVICTED
event types to EventType enum. All tier transitions emit
DomainEvent instances through the optional EventBus.
- Configuration: Added context_tier_promotion_threshold,
context_tier_hot_ttl_hours, context_tier_warm_ttl_hours settings.
The warm TTL setting also accepts the spec-defined env var
CLEVERAGENTS_CTX_WARM_HOURS as an alias.
- DI wiring: container.py now injects event_bus into
context_tier_service.
Review fixes applied (code review on PR #1150):
- C1: Reset access_count to 0 after each auto-promotion to prevent
chain promotion that bypassed the warm tier.
- C2: Call _enforce_hot_budget() inside promote() warm-to-hot path
so auto-promoted fragments respect the token budget.
- H1: Corrected _enforce_hot_budget() docstring: actual complexity
is O(n + n*k) not O(n), since min() scans remaining entries on
each eviction.
- M1: Added CLEVERAGENTS_CTX_WARM_HOURS as an additional env var
alias for context_tier_warm_ttl_hours per specification line 30555.
Review fixes applied (second code review on PR #1150):
- B-CRIT-1: Fixed data loss in promote() warm-to-hot: emit
TIER_PROMOTED before _enforce_hot_budget(), and if the promoted
fragment is evicted by budget, restore it to the warm tier instead
of silently losing it.
- B-HIGH-1: Fixed self-eviction on store(): fragments whose
token_count exceeds the entire hot-tier budget are now redirected
to the warm tier with a warning log.
- B-MED-1: Wrapped _emit_tier_event() in try/except so a failing
event bus does not break tier operations (best-effort emission).
- B-MED-2: Fixed event ordering so TIER_PROMOTED fires before any
budget-triggered TIER_EVICTED events.
- D-LOW-1: Fixed type hint in Robot helper (dict[str, Callable]).
- D-LOW-2: Added __all__ export to context_tiers.py.
- S-LOW-1: Added thread-safety docstring note to ContextTierService.
Review fixes applied (third code review on PR #1150):
- B-MED-1: Added TIER_DEMOTED event emission for oversized fragment
redirect in store(), closing the observability gap where the only
tier transition without event emission was the hot-to-warm redirect
for fragments exceeding the entire hot-tier budget.
- S-LOW-1: Added CLEVERAGENTS_CTX_HOT_HOURS as an additional env var
alias for context_tier_hot_ttl_hours, for consistency with the
warm-tier alias CLEVERAGENTS_CTX_WARM_HOURS.
- S-LOW-2: Added docstring note to enforce_staleness() reconciling
the hot-tier TTL with the specification statement that hot-tier
retention is "Until resource removed" (TTL controls tier placement,
not data retention).
Review fixes applied (fourth code review on PR #1150):
- B-HIGH-1: Reset access_count to 0 on demotion so that demoted
fragments must accumulate fresh accesses before re-promotion.
Without this reset, a previously popular fragment whose
access_count already exceeded the promotion threshold would be
re-promoted on the very next get() call, making staleness
enforcement ineffective.
Review fixes applied (freemo APPROVED review on PR #1150):
- #1: Removed all # type: ignore annotations from test files.
Fixed _EventCollector, _FailingBus, and _NullBus subscribe()
signatures to use Callable[[DomainEvent], None] matching the
EventBus protocol. Replaced dict-spread TieredFragment construction
with explicit keyword arguments and post-construction assignment.
- #2: Extracted runtime policy logic (enforce_staleness,
_maybe_auto_promote, _re_fetch_after_promotion, _enforce_hot_budget,
_emit_tier_event) into TierRuntimeMixin in tier_runtime.py to
reduce context_tiers.py toward the 500-line guideline.
- #3: Added fragment_id non-empty validation guard to promote() and
demote() per CONTRIBUTING.md argument validation policy.
- #8: Renamed _resolve to _re_fetch_after_promotion for clarity.
Removed @tdd_expected_fail from TDD tests (Behave + Robot) as the
bug is now fixed. All 3 TDD scenarios pass normally.
Tests: 27 Behave scenarios (24 feature + 3 TDD), 4 Robot integration
tests, 4 ASV benchmark suites.
ISSUES CLOSED: #821
## Summary
E2E test for Workflow Example 5 — database schema migration with safety nets using the **review** automation profile. Exercises the full spec-aligned workflow:
- **Custom resource type registration** via `resource type add --config` (postgres-db type with `transaction_rollback` sandbox strategy, `--host`, `--port`, `--database`, `--schema` CLI args with flat `type`/`default` fields per `ResourceTypeArgument` schema)
- **Custom resource instantiation** — attempts `resource add` with the custom type to exercise mixed resource types, followed by `project link-resource` to link DB resource to the project
- **Custom skill creation** with spec-aligned database tools: `local/query_db` (read-only), `local/execute_migration` (writes, checkpointable), `local/backfill_column` (writes, checkpointable) — registered via `skill add --config`, with namespaced tool reference names per `SkillToolRefSchema` validation
- **Action creation** with `automation_profile: review`, `reusable: true`, `state: available`, spec invariants, and typed `arguments` section (`table_name`, `column_name`, `column_type`, `backfill_source` — all required per spec, using `arguments` field per `ActionConfigSchema`)
- **Plan use** with `--arg` flags exercising parameterized action invocation including `backfill_source=audit_log`, plus **explicit `--automation-profile review`** flag (action-to-plan profile propagation is not yet wired in `PlanLifecycleService.use_action`)
- **Phased child plan verification** via `plan tree --format json` with `decision_count >= 2` hard assertion on framework decisions plus WARN tiers for LLM decomposition quality (`< 3`, `< 5`)
- **Plan phase assertion** — hard assertion that phase is populated after execute
- **Checkpoint-based rollback** with hard assertions: `rc=0` on rollback success, `rc!=0` on fake checkpoint, None guard for JSON null checkpoint IDs, re-execute with Traceback/INTERNAL checks on success and explanatory comment on failure path
- **Plan diff** with hard `rc=0` assertion and content-signal verification
- **Migration content verification** — baseline SHA saved before apply, diff against baseline (not `HEAD~1`), WARN-level check on migration keywords (`last_login`, `schema`, `migration`, `column`, `alter`) — flexible per LLM non-determinism
- **Commit count** assertion `>= 2` (fixture baseline: Create Temp Git Repo + DB fixture commit), WARN if no additional commits from lifecycle-apply
- **Backfill evidence** WARN-level check in plan tree/execution output (`backfill`, `batch`, `populate`, `last_login`) with explanatory comment noting tree covers decomposition plan
- **Combined AC #6 gate** — if *both* migration content *and* backfill evidence are absent, explicit WARN visibility for CI debugging
- **Terminal state assertion** after `lifecycle-apply` — `plan status` call verifies phase/processing_state reflects terminal or apply-progress outcome
- **Automation profile fallback verification** — if `plan use` output omits `automation_profile`, falls back to `plan status` for secondary verification (hard assertion always runs)
- **Traceback and INTERNAL checks** on all CLI commands (resource add, project create, resource type add, skill add, action create, plan use, strategize, execute, plan tree, plan status, plan diff, plan rollback, re-execute after rollback, lifecycle-apply) including custom resource error paths
- **Dynamic actor selection** — detects available API keys (Anthropic/OpenAI) at suite setup
- **Skip If No LLM Keys** guard for graceful CI degradation
- **Test-level teardown** with diagnostic logging for both plan status and plan tree on failure
- **30-minute timeout** covering worst-case rollback+re-execute path
- **Force Tags** for consistency with `m6_acceptance.robot`
- **Timeout parameters** (`timeout=60s on_timeout=kill`) on all local `Run Process` git commands
- **Sequential section numbering** (1 through 15) for readability
Closes#751
ISSUES CLOSED: #751
## Approach
Follows the patterns established by `m6_acceptance.robot` and `m2_acceptance.robot`:
- `WF05 Suite Setup` initialises the workspace, generates a unique run suffix, and detects available LLM API keys
- `Safe Parse Json Field` from `common_e2e.resource` for JSON field extraction with None guards for JSON null values
- All CLI commands use `--format json` for predictable, parseable output
- `expected_rc=None` with explicit `Should Be Equal As Integers` for detailed failure messages
- Hard assertions on infrastructure/framework behavior (CLI commands, phase transitions, tool registration)
- WARN-level assertions on LLM-dependent output (decision decomposition, migration content, backfill evidence, commit count) — per ticket requirement "output validation is flexible"
- Traceback and INTERNAL checks on all CLI commands following `m2_acceptance.robot` pattern
- Baseline SHA approach for post-apply diff verification eliminates false positives from fixture commits
## Bug Fix: LifecyclePlanRepository.update() UNIQUE Constraint Violation
**Root cause**: `LifecyclePlanRepository.update()` called `clear()` on child relationship collections (project_links, arguments, invariants) followed by `append()` with new items, but only flushed at the end. SQLAlchemy's default operation ordering can emit INSERTs before DELETEs within the same flush, causing `UNIQUE constraint failed: plan_arguments.plan_id, plan_arguments.name` when plans have arguments.
**Fix**: Group all three `clear()` calls together and flush them before appending new rows. This ensures the DELETEs are committed before any INSERTs, preventing the UNIQUE constraint violation.
**Impact**: This was a latent bug affecting ALL plans with arguments when `update()` is called. Previously undetected because existing E2E tests (M1, M2, M5, M6) create plans without `--arg` flags.
## Review Fixes (addressing medium findings from @CoreRasurae review)
| # | Finding | Fix |
|---|---------|-----|
| **BUG-1** | No regression test for UNIQUE constraint fix | Added targeted BDD scenario in `repositories_coverage_boost.feature` — creates plan with argument `x=v1`, updates to `x=v2`, asserts no `IntegrityError` |
| **TEST-1** | AC #4 weakened — fragile string counting | Replaced raw `count('"decision_id"')` with proper JSON parsing via `json.loads()`, recursive tree walking for decision counting, structural `children_key_count` and `child_link_count` verification |
| **TEST-2** | AC #5 conditionally tested | Added explicit WARN log when no checkpoint_id is present ("AC #5 visibility"); fake checkpoint test now runs unconditionally (moved outside IF/ELSE) with Traceback/INTERNAL checks |
| **TEST-3** | No terminal state assertion after lifecycle-apply | Added `plan status` call after apply with phase/processing_state extraction; hard assertion on terminal state or apply-phase progress |
| **TEST-4** | AC #6 migration/backfill WARN-only | Added combined gate (`has_ac6_evidence`): if *both* migration and backfill evidence are absent, explicit WARN for CI visibility. WARN-only is intentional per ticket AC "output validation is flexible" |
| **TEST-5** | Automation profile silently skipped | Added fallback to `plan status --format json` when `plan use` output omits `automation_profile`; hard assertion (`Should Be Equal As Strings review`) now always executes |
| **TEST-8** | Missing Traceback/INTERNAL on custom resource error paths | Added Traceback/INTERNAL checks inside both `resource add` and `project link-resource` ELSE branches with `NoSuchOption` guard |
## Quality Gates
- `nox -e lint` ✅
- `nox -e typecheck` ✅ (0 errors)
- `nox -e unit_tests` ✅ (471 features, 12,422 scenarios, 0 failures)
- `nox -e integration_tests` ✅ (1,727 tests, 0 failures)
- `nox -e e2e_tests` ✅ (42 tests, 42 passed, 0 failed)
- `nox -e coverage_report` ✅ (98%, meets threshold)
## Manual Verification
### Prerequisites
- `OPENAI_API_KEY` or `ANTHROPIC_API_KEY` environment variable set
### Commands
```bash
nox -e e2e_tests
# Or run just this suite:
python -m robot --outputdir build/reports/robot --include E2E robot/e2e/wf05_db_migration.robot
```
Reviewed-on: cleveragents/cleveragents-core#816
Reviewed-by: Luis Mendes <luis.mendes@cleverthis.com>
Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
Verified the context assembly CLI commands (context list, add, show, clear)
are fully functional and integrated with ContextService and the DI container.
Both Robot E2E acceptance tests (ACMS Scoped Context Output Per Phase and
Context Policy Clear And Inheritance Fallback) pass, confirming phase-scoped
view resolution, size limit narrowing, inheritance fallback on clear, and
persistence round-trip correctness.
Added 5 new Behave scenarios to m5_acms_smoke.feature covering the required
unit test patterns: multiple resources management (list, add, show with
multiple files), clear-then-re-add flow, and clear on empty project. All
12,235 unit test scenarios pass. Coverage is 98.38% (threshold >97%).
ISSUES CLOSED: #848
Make require_confirmation configurable on UnitOfWork so that when
the --yes flag is passed to agents init, the migration runner skips
the confirmation prompt entirely rather than calling a prompt that
always returns True. The previous approach (injecting a prompt
callback that auto-approves) was a workaround that left the
require_confirmation=True hardcoded in _ensure_database_initialized,
meaning the prompt code path was still exercised unnecessarily.
Changes:
- Add require_confirmation parameter to UnitOfWork constructor
(default True for backward compatibility)
- Pass self._require_confirmation to MigrationRunner.init_or_upgrade
instead of hardcoded True
- Update init_command to pass require_confirmation=False when --yes
is set, with the prompt callback retained as belt-and-suspenders
The TDD bug-capture tests from #842 (features/tdd_init_yes_no_input.feature,
robot/tdd_init_yes_no_input.robot) now run as normal regression tests
with @tdd_expected_fail already removed.
All nox quality gates pass:
- lint, typecheck: clean
- unit_tests: 12230 scenarios passed
- integration_tests: all passed
- coverage_report: 98.38% (threshold >97%)
ISSUES CLOSED: #783
## Summary
This PR adds a Robot Framework E2E test suite (`robot/e2e/tdd_acms_behavioral_validation.robot`) that proves bug #1028 exists — the ACMS indexing pipeline is not wired into the CLI, so `ContextTierService` starts empty on every invocation.
### Changes
- **New file**: `robot/e2e/tdd_acms_behavioral_validation.robot` — 4 E2E test cases tagged `tdd_expected_fail`, `tdd_bug`, `tdd_bug_1028`, `E2E`
- **Modified**: `robot/e2e/common_e2e.resource` — Extracted shared keywords (`Run CLI`, `Extract JSON From Stdout`, `Link Resource To Project`, `Create Synthetic Codebase`) from both `m5_acceptance.robot` and `tdd_acms_behavioral_validation.robot` to eliminate ~97 lines of duplication. `Create Synthetic Codebase` is parameterized with `project_label`.
- **Modified**: `robot/e2e/m5_acceptance.robot` — Removed duplicated keywords now provided by `common_e2e.resource`.
- **CHANGELOG.md**: Added entry under `## Unreleased` documenting the TDD tests for #1029.
### Test Cases
- **Test 1**: Context Simulate Returns Non-Empty Tier Data — asserts `fragment_count > 0` (fails, proving bug)
- **Test 2**: Context Inspect Shows Indexed Resources — asserts tier metrics total > 0 (fails, proving bug)
- **Test 3**: Budget Enforcement Excludes Oversized Files — asserts `fragment_count > 0` with `max_file_size` policy (fails, proving bug). Includes TODO comment for post-fix exclusion assertion.
- **Test 4**: Large Project Indexes Without Timeout — generates 10K+ files, asserts `fragment_count > 0` (fails, proving bug). Includes explicit developer responsibility note about git-tracking of generated files.
All tests pass CI through result inversion by the `tdd_expected_fail_listener.py` — failing assertions (bug confirmed) are inverted to PASS.
### Documentation & Robustness Improvements
- Suite-level documentation expanded with **known limitation** section explaining `tdd_expected_fail` result inversion scope.
- Suite setup error messages include `(rc=${var.rc}). Check DEBUG logs above.` for debugging consistency with `m5_acceptance.robot`.
- Redundant exit code assertions after `Run CLI` calls removed (Run CLI already validates rc internally).
- `Run CLI` keyword documentation includes API key security notes.
- Budget enforcement test (Test 3) includes `TODO(bugfix/...)` comment for the bug-fix developer.
- Large project test (Test 4) includes explicit NOTE assigning responsibility to the bug-fix developer to evaluate filesystem vs. git-tracked content indexing.
### Review Fix Round
Addressed all findings from Luis's review (review #2691):
- **C1 (CRITICAL)**: Restored the #845 `CorrectionService` changelog entry (50 lines) accidentally deleted during merge conflict resolution.
- **L1 (LOW)**: Extracted ~97 lines of duplicated keywords into `common_e2e.resource` (parameterized `Create Synthetic Codebase`, shared `Run CLI`, `Extract JSON From Stdout`, `Link Resource To Project`).
- **M1 (MEDIUM)**: Strengthened NOTE comment about 10K files not being git-committed — explicit developer MUST responsibility.
- **L3 (LOW)**: Made suite setup error messages verbose with `(rc=...). Check DEBUG logs above.`
- **L2 (LOW)**: Removed redundant exit code assertions after `Run CLI` calls.
- **M2 (MEDIUM)**: Acknowledged — partial assertion is acceptable for TDD capture phase (TODO documents the gap).
- **I1 (INFO)**: Acknowledged — `tdd_expected_fail` masking is documented and mitigated.
### Motivation
Per the Bug Fix Workflow in CONTRIBUTING.md, this TDD issue (#1029) is the prerequisite for bug fix#1028. The tests capture the buggy behavior so that when the fix is implemented, removing the `tdd_expected_fail` tag will cause the tests to pass normally.
### Quality Gates
| Gate | Result |
|------|--------|
| lint | PASS |
| typecheck | PASS |
| unit_tests | PASS |
| integration_tests | PASS |
| e2e_tests | PASS (41 tests, 4 TDD) |
| coverage_report | PASS (>=97%) |
Closes#1029
Reviewed-on: cleveragents/cleveragents-core#1124
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
Add Behave BDD scenarios (tagged @tdd_bug @tdd_bug_1024 @tdd_expected_fail)
that verify the default database_url resolves inside CLEVERAGENTS_HOME rather
than the current working directory. Two scenarios exercise both the container
get_database_url() helper and the Settings model default.
Add Robot Framework integration tests with a helper script exercising the same
resolution paths via subprocess, verifying database URL resolution and CLI
database file placement relative to CLEVERAGENTS_HOME.
The tests currently fail as expected because the bug in #1024 is still present:
the relative SQLite path sqlite:///cleveragents.db resolves against CWD. The
@tdd_expected_fail tag inverts the result so CI passes.
ISSUES CLOSED: #1034
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>
Centralize CLI output through shared renderers module with get_console()
and get_err_console() accessors. Introduce ColumnSpec-based table rendering
for plan list, resource list, and actor list commands. Add _FORMAT_HELP
constant and fmt= parameter threading for consistent format handling.
Includes Behave BDD tests for format output validation, Robot Framework
integration tests for CLI formatting consistency, and CHANGELOG entry.
ISSUES CLOSED: #210
## 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>
brent.edwards
Luis Mendes
freemo
aditya
hurui200320
hamza.khyari
salcedo