Files
cleveragents-core/features/repositories_coverage_boost.feature
T
freemo 06130212ed
CI / build (push) Successful in 17s
CI / security (push) Successful in 1m7s
CI / lint (push) Successful in 3m18s
CI / quality (push) Successful in 3m40s
CI / typecheck (push) Successful in 3m48s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Has started running
CI / docker (push) Blocked by required conditions
CI / status-check (push) Blocked by required conditions
CI / integration_tests (push) Has started running
CI / e2e_tests (push) Has started running
CI / coverage (push) Has started running
CI / benchmark-publish (push) Has started running
test(e2e): workflow example 5 — database schema migration with safety nets (review profile) (#816)
## 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: #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>
2026-03-25 13:05:04 +00:00

149 lines
7.6 KiB
Gherkin
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
@phase1 @domain @repository @coverage_boost
Feature: Repository coverage boost for actions, plans, and resources
As a developer ensuring high test coverage
I want to exercise uncovered repository branches
So that the persistence layer is thoroughly tested
Background:
Given a fresh in-memory database with full lifecycle schema
And an action repository using the session factory
And a lifecycle plan repository using the session factory
# ---------------------------------------------------------------------------
# ActionRepository.update non-existent action raises DatabaseError
# ---------------------------------------------------------------------------
@action_update @error_handling
Scenario: Updating a non-existent action raises DatabaseError with "not found for update"
Given a valid action object named "local/ghost-action"
When the action is updated without being persisted first
Then a DatabaseError mentioning "not found for update" should be raised
# ---------------------------------------------------------------------------
# ActionRepository.update with arguments (including default_value) and invariants
# ---------------------------------------------------------------------------
@action_update
Scenario: Updating an action replaces its child arguments and invariants
Given a valid action object named "local/updatable-with-children"
And the action has been persisted in the database
When the action arguments are replaced with new arguments including defaults
And the action invariants are set to new invariant texts
And the action is updated via the repository
Then the update should succeed without error
And retrieving the action should show the new arguments
And retrieving the action should show the new invariants
# ---------------------------------------------------------------------------
# LifecyclePlanRepository.get_by_name
# ---------------------------------------------------------------------------
@plan_read
Scenario: A lifecycle plan is retrieved by its namespaced name
Given a valid action object named "local/plan-action-lookup"
And the action has been persisted in the database
And a lifecycle plan domain object linked to "local/plan-action-lookup"
And the lifecycle plan has been persisted in the database
When the lifecycle plan is looked up by namespaced name
Then the returned plan should match the original plan identity
@plan_read
Scenario: Looking up a non-existent plan by name returns nothing
When a lifecycle plan is looked up by name "local/nonexistent-plan"
Then no lifecycle plan should be returned
# ---------------------------------------------------------------------------
# LifecyclePlanRepository.update PlanNotFoundError
# ---------------------------------------------------------------------------
@plan_update @error_handling
Scenario: Updating a non-existent lifecycle plan raises PlanNotFoundError
Given a valid action object named "local/plan-action-missing"
And the action has been persisted in the database
And a lifecycle plan domain object linked to "local/plan-action-missing"
When the lifecycle plan is updated without being persisted first
Then a PlanNotFoundError should be raised for the repository boost
# ---------------------------------------------------------------------------
# LifecyclePlanRepository.update with project_links, arguments, invariants
# ---------------------------------------------------------------------------
@plan_update
Scenario: Updating a plan replaces project links, arguments, and invariants
Given a valid action object named "local/plan-action-update"
And the action has been persisted in the database
And a lifecycle plan domain object linked to "local/plan-action-update"
And the lifecycle plan has been persisted in the database
When the plan is updated with project links, arguments, and invariants
Then the plan update should succeed without error
And retrieving the plan should show the new project links
And retrieving the plan should show the new plan arguments
And retrieving the plan should show the new plan invariants
@plan_update
Scenario: Updating a plan argument with the same name does not hit UNIQUE constraints
Given a valid action object named "local/plan-action-arg-regression"
And the action has been persisted in the database
And a lifecycle plan domain object linked to "local/plan-action-arg-regression"
And the lifecycle plan has argument "x" set to "v1"
And the lifecycle plan has been persisted in the database
When the lifecycle plan argument "x" is updated to "v2"
Then the plan update should succeed without error
And retrieving the plan should show argument "x" value "v2"
# ---------------------------------------------------------------------------
# LifecyclePlanRepository.list_plans filtered by phase
# ---------------------------------------------------------------------------
@plan_list
Scenario: Listing plans filtered by phase returns only matching plans
Given a valid action object named "local/plan-action-phase"
And the action has been persisted in the database
And lifecycle plans in different phases linked to "local/plan-action-phase"
When plans are listed filtered by phase "strategize"
Then only the strategize-phase plans should be returned
# ---------------------------------------------------------------------------
# ResourceTypeRepository create, get, list_types
# ---------------------------------------------------------------------------
@resource_type
Scenario: A resource type is created and retrieved by name
Given a resource type repository using the session factory
And a valid resource type domain object named "test/my-git-checkout"
When the resource type is created through the repository
Then the resource type should be retrievable by name "test/my-git-checkout"
@resource_type @error_handling
Scenario: Creating a duplicate resource type raises DuplicateResourceTypeError
Given a resource type repository using the session factory
And a valid resource type domain object named "test/dup-type"
And the resource type has been persisted in the database
When the same resource type is created again
Then a DuplicateResourceTypeError should be raised
# ---------------------------------------------------------------------------
# ResourceRepository create, get, get_by_name, list_resources by type
# ---------------------------------------------------------------------------
@resource
Scenario: A resource is created and retrieved by ID and name
Given a resource type repository using the session factory
And a resource repository using the session factory
And a valid resource type domain object named "test/res-type-a"
And the resource type has been persisted in the database
And a valid resource domain object of type "test/res-type-a"
When the resource is created through the repository
Then the resource should be retrievable by its ID
And the resource should be retrievable by its namespaced name
@resource
Scenario: Resources are listed by type name
Given a resource type repository using the session factory
And a resource repository using the session factory
And a valid resource type domain object named "test/list-type"
And the resource type has been persisted in the database
And multiple resources of type "test/list-type" have been created
When resources are listed by type "test/list-type"
Then all resources of that type should be returned