freemo/cleveragents-core
0
0
Fork 0
forked from cleveragents/cleveragents-core
Code Pull Requests Activity
1,524 Commits 352 Branches 3 Tags
fix/a2a-python-sdk-dependency
Commit Graph

6 Commits

Author SHA1 Message Date
brent.edwards 998aaf25f8 fix(infra): ensure E2E suite setup initializes database before CLI commands 
Centralize E2E initialization in common suite setup so DB-dependent CLI commands no longer rely on per-suite or per-test init workarounds. This also sanitizes suite-home names to prevent invalid path-derived initialization failures in isolated Robot runs.

ISSUES CLOSED: #1023
 2026-04-03 01:43:27 +00:00
aditya 2aa2bb6a43 fix(e2e): disable git commit signing in temp test repos 2026-03-25 12:05:40 +00:00
hurui200320 83c22b8391 test(e2e): TDD behavioral test proving ACMS indexing pipeline is not wired into CLI (bug #1028) (#1124) 
## 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>
 2026-03-25 11:13:37 +00:00
hurui200320 cbf8bcc993 test(e2e): E2E acceptance criteria for M5 (v3.4.0) — ACMS v1 and context scaling (#811) 
## Summary

Add `robot/e2e/m5_acceptance.robot` with **21 zero-mock E2E test cases** (in addition to existing M5 test suite) covering all M5 (v3.4.0) acceptance criteria:

1. **Context Assembly** — add/list/show/clear files in the context pipeline
2. **Context Scaling** — 10,000+ file project setup with simulate plumbing *(structural)*
3. **Context Policy Configuration** — per-view include/exclude paths, file-size limits
4. **Budget Enforcement** — max_file_size / max_total_size constraint storage *(structural)*
5. **Context Analysis** — ACMS pipeline inspect (tier schema) and simulate (JSON schema) *(structural)*
6. **Plan Execution** — real LLM calls via `openai/gpt-4o-mini` (`plan use` + `plan resume`)

### Structural vs. Behavioural Scope

Tests in sections 1b–4 that use `project context simulate` or `inspect` are **structural / plumbing validations** — they verify CLI execution, JSON serialization, and stored configuration but do **not** exercise actual ACMS indexing or budget enforcement because the `ContextTierService` is an in-memory singleton that starts empty per CLI process. Each affected test has a `[Documentation]` note explaining this limitation. Behavioural ACMS validation is deferred until the full indexing pipeline is wired.

### Production Bug Fixes

| Fix | File | Description |
|-----|------|-------------|
| `session.flush()` → `session.commit()` | `project_context.py` | Policy changes silently lost on `session.close()` |
| `contextlib.suppress` rollback wrapper | `project_context.py` | Prevents rollback failure from masking original commit exception |
| Add `session_factory` DI provider | `container.py` | `project context` commands hit `AttributeError` |
| `providers.Factory` → `providers.Singleton` | `container.py` | Avoid creating duplicate engines per call |
| Add Gemini API key pattern | `redaction.py` | `AIzaSy...` keys now redacted in logs |

### Review Feedback Addressed (Tenth Pass — @CoreRasurae Review #2410)

| # | Severity | Finding | Fix |
|---|----------|---------|-----|
| P3-1 | Medium | "Clear Context" test tautological — never asserts files were present before clearing | Added `Should Contain ${list_before.stdout} config.py` precondition check after `context-load` and before `clear` |
| P3-2 | Medium | Policy/budget verification uses substring matching (`Should Contain 262144`) | Replaced with `Extract JSON From Stdout` + `$rv.get('max_file_size') == 262144` parsed JSON assertions using `resolved_view` dict access |
| P3-3 | Medium | Plan resume doesn't verify `phase` value, only existence | Added `Should Not Be Equal As Strings ${phase} queued` assertion to verify plan transitioned from queued |
| P3-4 | Medium | Plan JSON extraction inconsistency (`rindex` vs `Extract JSON From Stdout`) | Replaced fragile `rindex`-based extraction with `Extract JSON From Stdout` keyword for consistency |
| P3-6 | Medium | Context show summary weak content assertions | Added `Should Not Contain` guards against traceback/error output to reject false positives |
| P3-8 | Medium | `_SafeSession` singleton may accumulate dirty state after rollback | Changed `_SafeSession.close()` from pure no-op to `real.rollback()` to reset session state between calls |
| P3-14 | Medium | No test for `_save_policy_json` rollback path | Added BDD scenario "Save policy rollback re-raises after commit failure" with monkey-patched commit |
| P3-15 | Medium | No test for `_save_policy_json` on nonexistent project | Added BDD scenario "Save policy on nonexistent project row updates zero rows" verifying silent 0-row behavior |
| P4-1 | Low | Plan resume TRY/EXCEPT swallows assertion details | Moved field assertions outside TRY block; TRY only guards JSON extraction |
| P4-2 | Low | `Safe Parse Json Field` logs stale error context | Fixed to track and report both Strategy 1 and Strategy 2 error contexts separately |
| P4-4 | Low | SQLite WAL/SHM files not cleaned in regression test | Added cleanup loop for `-wal` and `-shm` suffixes alongside `.db` file |

### Deferred Items (Out of Scope)

| ID | Severity | Reason |
|----|----------|--------|
| P2-1 | High | `execution_environment` silently dropped on subsequent `context set` — pre-existing production code bug in `_write_policy()`, not introduced by this PR |
| P2-2 | High | Unhandled `ValidationError` on corrupt policy blob — pre-existing `_read_policy()` code, not changed by this PR |
| P2-3 | High | Silent no-op UPDATE when `ns_projects` row missing — pre-existing `_save_policy_json` logic; this PR only changed error handling |
| P3-5 | Medium | Structural tests cannot detect regressions — already honestly documented in every affected test's `[Documentation]` block |
| P3-7 | Medium | View inheritance/override behavior not tested — nice-to-have, not in ticket acceptance criteria |
| P3-9 | Medium | `context_set` double-writes when `execution_environment` set — pre-existing production logic |
| P3-10 | Medium | `budget_tokens=0` silently replaced by default (falsy `or`) — pre-existing production code bug |
| P3-11 | Medium | `context set` replaces entire view instead of merging — pre-existing design choice |
| P3-12 | Medium | GEMINI_API_KEY propagated but potentially unused — security-first: propagating for redaction testing |
| P3-13 | Medium | `reset_container()` doesn't dispose Singleton resources — pre-existing container lifecycle issue |
| M5 | Medium | `_build_session_factory` engine never disposed — production code architecture, out of scope for testing ticket |
| M6 | Medium | Missing `check_same_thread`/`isolation_level` — production code architecture, out of scope for testing ticket |
| L1 | Low | `plan resume` not in spec CLI synopsis — informational |
| L2 | Low | Context summary assertions depend on exact CLI wording — acceptable stability risk |
| L3 | Low | Gemini regex minimum length slightly loose — acceptable security-first trade-off |
| L4 | Low | Missing Google OAuth2 credential patterns — out of scope for this PR |
| P4-3 | Low | `Run CLI` keyword duplicated — different purpose (uses `${WS}` as default cwd), not a true duplicate |
| P4-5–P4-9 | Low | Various additional E2E coverage gaps — nice-to-have, not in ticket acceptance criteria |

### Quality Gates

| Gate | Result |
|------|--------|
| lint | PASS |
| typecheck | PASS (0 errors) |
| unit_tests | **393/393** features, 11,210 scenarios |
| integration_tests | **1,576/1,576** |
| e2e_tests | **37/37** (21 M5 + 12 M6 + 2 smoke + 2 M1) |
| coverage_report | **97%** (threshold: 97%) |

### Files Changed

| File | Change |
|------|--------|
| `robot/e2e/m5_acceptance.robot` | **NEW** — 21 E2E test cases with honest structural documentation, parsed JSON assertions, prerequisite skip guards on all sections, safe assertion messages |
| `robot/e2e/common_e2e.resource` | `on_timeout=kill` + return code checks + safe key evaluation via `os.environ.get` + fixed stale error logging in `Safe Parse Json Field` |
| `robot/e2e/m1_acceptance.robot` | `on_timeout=kill` on git log |
| `robot/e2e/m2_acceptance.robot` | `on_timeout=kill` + return code checks + safe assertion messages (no stderr embedding) |
| `src/cleveragents/application/container.py` | Add `_build_session_factory` + `session_factory` Singleton |
| `src/cleveragents/cli/commands/project_context.py` | `flush()` → `commit()` + `contextlib.suppress` rollback |
| `src/cleveragents/shared/redaction.py` | Add Gemini API key pattern |
| `noxfile.py` | Propagate `GEMINI_API_KEY` in e2e_tests |
| `CHANGELOG.md` | 4 entries for #745 |
| `features/application_container_coverage_boost.feature` | Updated title + 3 scenarios |
| `features/steps/application_container_coverage_boost_steps.py` | Step defs for `_build_session_factory` |
| `features/consolidated_security.feature` | 2 Gemini API key redaction scenarios |
| `features/project_context_cli_coverage_boost.feature` | `flush()→commit()` regression test + rollback path + nonexistent project tests |
| `features/steps/project_context_cli_coverage_boost_steps.py` | Separate engines for regression test + `try/finally` cleanup + `_SafeSession.close()` state reset + rollback/nonexistent test steps + WAL/SHM cleanup |

Closes #745

ISSUES CLOSED: #745

Reviewed-on: cleveragents/cleveragents-core#811
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>
 2026-03-19 06:53:06 +00:00
Luis Mendes cb583021df test(e2e): E2E acceptance criteria for M6 (v3.5.0) — autonomy hardening 
Implemented Robot Framework E2E test suite for M6 autonomy hardening
acceptance criteria.  Tests exercise the real CleverAgents CLI with zero
mocking, covering session lifecycle, automation profiles, project setup,
plan lifecycle via A2A facade, guard enforcement, and a full autonomy
acceptance flow.  LLM-dependent tests use Skip If No LLM Keys for
graceful degradation when API keys are unavailable.

Hardened shared E2E keywords (common_e2e.resource):
- Safe JSON parsing with rfind-bounded extraction and error wrapping (C1, M1)
- Multi-object fallback: last-line reverse scan for multi-JSON output (M5)
- Moved Safe Parse Json Field to common_e2e.resource for reuse (L1)
- Migrated deprecated Run Keyword If to IF/ELSE blocks (M4)
- Added cwd parameter to Run CleverAgents Command (M6)
- API key protection via inline evaluation instead of RF variables (S1)
- Git return-code assertions in Create Temp Git Repo (L1)
- Removed unused Collections library import (L2)
- Warning log on directory removal failure instead of silent ignore (L6)

Hardened m6_acceptance.robot:
- Force Tags E2E instead of per-test [Tags] (L3)
- Per-test [Teardown] for resource cleanup, including Init test (M2, M9, L5)
- Collision-safe uuid4 hex suffix instead of randint (L7)
- Initialized session_id to EMPTY before test body for safe teardown (L2)
- ELSE branches with WARN log on conditional assertions (M3, M4, M6)
- Guard enforcement checks specific automation-profile fields (M5)
- Strengthened ci assertions with case-sensitive matching and JSON
  field parsing for config get verification (M9)
- Eliminated Python code injection via string interpolation (H2)
- Added assertions to Full Flow Apply Step keyword (H4)
- Removed redundant config set in Guard test (M10)
- Accurate CHANGELOG entry describing actual test scope (C2)

Post-review fixes applied:
- Verify all 8 built-in profiles (manual, review, supervised, cautious,
  trusted, auto, ci, full-auto) instead of 4 (M1)
- Session delete confirms removal via re-list (M2)
- Full Flow Apply Step verifies plan phase transition (M3)
- Plan execute output asserts plan_id presence + phase parsing (M4)
- Safe Parse Json Field gains two-strategy approach: outer-bracket
  extraction then last-line reverse scan fallback (M5)
- Removed redundant automation profile reset in Config test (L1)
- JSON-quoted assertions ("ci", "auto") prevent false-positive
  substring matches on short profile names (L3)

New E2E tests covering remaining acceptance criteria:
- Guard enforcement with custom profile: registers a profile with
  explicit denylist, budget cap, and tool-call limits, then verifies
  all guard fields via automation-profile show (AC-4 / H1)
- Profile precedence resolution: sets global profile to "review",
  creates plan with --automation-profile trusted, asserts plan output
  shows "trusted" not "review" (AC-5 / C2)
- Event queue via plan lifecycle transitions: creates plan, captures
  initial state, executes, verifies state transition proving domain
  event bus delivered and processed events (AC-3 / C1)
- Hierarchical decomposition via plan tree: creates plan with full-auto
  profile, executes, runs plan tree --format json, verifies decision
  nodes and children structure (AC-6 / C3)

Robot Framework uses dots as hierarchy separators in suite names
(e.g. "E2E.M6 Acceptance").  The E2E Suite Setup keyword replaced
spaces with underscores but preserved dots, producing directory names
like "E2E.M6_Acceptance".  The CLI init command derives the project
name from Path.cwd().name, and Project.validate_name() rejects dots
("Name must be alphanumeric with hyphens, underscores, or spaces").

Added a second Replace String call to convert dots to underscores so
the sanitized suite name passes Project name validation.

Also increased subprocess timeouts in m3_e2e_verification.robot (60s->120s)
and m4_e2e_verification.robot (30s->120s) to prevent flaky CI failures from
Python startup overhead.

The plan lifecycle E2E tests call "plan use local/code-review" but
actions are user-defined entities that must be explicitly registered
via "action create --config <yaml>" before use.  Without the action,
plan use failed with "Action 'local/code-review' not found" and all
LLM-dependent tests skipped.

Added action registration to M6 Suite Setup that dynamically selects
the actor matching the available API key (anthropic/claude-sonnet-4
when ANTHROPIC_API_KEY is set, openai/gpt-4o otherwise) and creates
the action YAML inline.

ISSUES CLOSED: #746
 2026-03-17 10:13:23 +00:00
freemo d4a1a0d87b test(e2e): set up E2E test infrastructure — nox session, CI job, Robot Framework @E2E tag 
Added dedicated E2E test infrastructure completely separate from the existing
integration test suite. E2E tests use zero mocking — they exercise the real
CleverAgents CLI with real LLM API keys.

Key changes:
- New e2e_tests nox session running Robot Framework with --include E2E tag
  filter against robot/e2e/ directory. Uses sequential robot (not pabot)
  since E2E tests hit real API endpoints with rate limits. Propagates
  ANTHROPIC_API_KEY, OPENAI_API_KEY, and GOOGLE_API_KEY from environment.
  Output goes to build/reports/robot-e2e/ to avoid artifact collisions.
- Existing integration_tests session now excludes E2E-tagged tests via
  --exclude E2E on the pabot invocation.
- New robot/e2e/common_e2e.resource provides shared E2E keywords: suite
  setup/teardown with per-suite isolation (no mock AI), graceful skip when
  LLM API keys are absent, CLI runner keyword, flexible output assertions,
  and temporary git repo fixture creation.
- Minimal smoke test (robot/e2e/smoke_test.robot) validates the harness by
  running agents --version and agents --help. Does not require LLM keys.
- Dedicated e2e_tests CI job in .forgejo/workflows/ci.yml injects LLM API
  keys from Forgejo secrets. Runs independently (no needs dependencies)
  and does not block regular CI.
- e2e_tests is deliberately NOT in the default nox sessions list since it
  requires real API keys not present in all environments.

ISSUES CLOSED: #740
 2026-03-12 21:11:37 +00:00