Commit Graph

3 Commits

Author SHA1 Message Date
freemo b122ec7ed5 fix(test-infra): remove redundant ${PYTHON} variable definitions from robot files
CI / lint (pull_request) Successful in 23s
CI / quality (pull_request) Successful in 51s
CI / typecheck (pull_request) Successful in 1m0s
CI / security (pull_request) Successful in 55s
CI / build (pull_request) Successful in 25s
CI / helm (pull_request) Successful in 32s
CI / push-validation (pull_request) Successful in 26s
CI / e2e_tests (pull_request) Successful in 3m38s
CI / integration_tests (pull_request) Successful in 6m42s
CI / unit_tests (pull_request) Successful in 8m19s
CI / docker (pull_request) Successful in 13s
CI / coverage (pull_request) Successful in 15m23s
CI / status-check (pull_request) Successful in 2s
Remove the local ${PYTHON}    python (and python3) variable definitions from
the *** Variables *** sections of all affected robot files. These local
definitions were overriding the pabot-injected venv Python path passed via
--variable PYTHON:/path/to/venv/python, causing tests to use the system
Python (which lacks required packages like structlog, sqlalchemy, etc.)
instead of the nox venv Python.

The correct ${PYTHON} value is already set by Setup Test Environment in
common.resource via sys.executable, and pabot passes it via --variable.
The local fallback definitions are redundant and harmful in parallel runs.

Audit found 56 robot files with the pattern (more than the 9 originally
identified in the issue). All occurrences have been removed.

ISSUES CLOSED: #1309
2026-04-14 14:50:55 +00:00
hurui200320 ee2024046f fix(plan): upsert action arguments during plan use to avoid UNIQUE constraint violation (#4197)
CI / lint (push) Successful in 37s
CI / quality (push) Successful in 53s
CI / typecheck (push) Successful in 57s
CI / security (push) Successful in 58s
CI / helm (push) Successful in 25s
CI / push-validation (push) Successful in 25s
CI / build (push) Successful in 30s
CI / integration_tests (push) Successful in 4m22s
CI / e2e_tests (push) Successful in 4m17s
CI / unit_tests (push) Successful in 5m10s
CI / docker (push) Successful in 1m30s
CI / coverage (push) Successful in 12m18s
CI / status-check (push) Successful in 1s
CI / benchmark-regression (push) Has been skipped
CI / benchmark-publish (push) Has been cancelled
## Summary

`agents plan use` crashed with `sqlite3.IntegrityError: UNIQUE constraint failed: action_arguments.action_name, action_arguments.name` when the action had arguments already registered via `action create`. The root cause was `ActionRepository.update()` using SQLAlchemy's relationship `.clear()` + `.append()` pattern, which deferred the DELETE and processed the INSERT first — triggering a UNIQUE constraint violation when the same `(action_name, name)` pair was being re-inserted.

## Approach

Replace the `.clear()` + `.append()` pattern with explicit bulk `sa_delete()` + `session.flush()` before re-inserting child rows for both `action_arguments` and `action_invariants`. After the flush, expire the relationship collections with `session.expire(row, ["arguments_rel", "invariants_rel"])` so SQLAlchemy reloads from the now-empty database state before appending replacements. This avoids stale identity map references and guarantees the DELETE is committed before any INSERT.

## Key Changes

### Bug fix (`src/cleveragents/infrastructure/database/repositories.py`)
- `ActionRepository.update()` now uses `sa_delete(ActionArgumentModel)` and `sa_delete(ActionInvariantModel)` with `synchronize_session=False`, followed by `session.flush()`, before re-inserting child rows.
- Targeted `session.expire(row, ["arguments_rel", "invariants_rel"])` replaces the removed `.clear()` calls to force collection reload.

### Schema parity (`src/cleveragents/infrastructure/database/models.py`)
- Added `UniqueConstraint("action_name", "position")` to `ActionInvariantModel`.
- Added `UniqueConstraint("action_name", "name")`, `CheckConstraint` for `arg_type`, and `CheckConstraint` for `requirement` to `ActionArgumentModel`.

### Alembic migration (`alembic/versions/a5_006_action_invariants_unique_constraint.py`)
- New migration adds all four constraints to both `action_invariants` and `action_arguments` tables.
- Includes deduplication guards and data normalization so the upgrade succeeds on existing databases with invalid or duplicate rows.
- Uses `batch_alter_table` for SQLite compatibility.

### Tests
- **Behave** (`features/plan_use_action_args_integrity.feature`): 6 scenarios covering the core bug path, zero-argument regression, multiple arguments, reusable action double-use, non-reusable action archival with invariants, and direct repository update.
- **Robot** (`robot/plan_use_action_args_integrity.robot`): Integration test mirroring the Behave scenarios via a helper script.
- **Shared factory** (`features/mocks/test_uow_factory.py`): Extracted `build_test_uow()` from both test suites into a single shared module to eliminate duplication (DRY).

### Minor
- Updated `src/cleveragents/domain/repositories/__init__.py` docstring from table format to bullet list (conflict resolution from rebase).

Closes #4174

Reviewed-on: #4197
Reviewed-by: HAL 9000 <HAL9000@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
2026-04-09 06:11:39 +00:00
freemo 0ad18d4306 feat(actor): align actor registry persistence 2026-02-20 08:48:33 -05:00