UAT: Invariant domain model missing frozen=True — model is mutable, contradicting spec requirement for immutability #3116

Closed
opened 2026-04-05 06:24:30 +00:00 by freemo · 5 comments
Owner

Metadata

  • Branch: fix/invariant-frozen-model-config
  • Commit Message: fix(invariant): add frozen=True to Invariant model config and redesign soft-delete pattern
  • Milestone: v3.2.0
  • Parent Epic: #394

Background

The spec (milestone v3.2.0, M3: Decisions + Validations + Invariants) requires the Invariant Pydantic v2 model to use frozen=True for immutability. The implementation instead uses validate_assignment=True, which allows mutation.

What was tested: Code-level analysis of src/cleveragents/domain/models/core/invariant.py

Expected behavior (from spec): The Invariant model should be declared with frozen=True in its model_config = ConfigDict(frozen=True, ...), making instances immutable (hashable, cannot be mutated after creation). This is a standard Pydantic v2 pattern for value objects in domain models.

Actual behavior: The model uses:

model_config = ConfigDict(
    str_strip_whitespace=True,
    validate_assignment=True,  # ← allows mutation
)

frozen=True is absent. Instances can be mutated after creation.

Code location: src/cleveragents/domain/models/core/invariant.py, lines ~100-107 (the Invariant.model_config)

Design conflict: The InvariantService.remove_invariant() method in src/cleveragents/application/services/invariant_service.py performs a direct mutation:

inv.active = False  # ← direct mutation of the model

If frozen=True were enforced, this line would raise a pydantic.ValidationError at runtime. The soft-delete pattern would need to be redesigned to create a new Invariant instance with active=False and replace it in the _invariants dict.

Also affected: InvariantViolation and InvariantEnforcementRecord models in the same file also use validate_assignment=True without frozen=True.

Steps to reproduce:

  1. Read src/cleveragents/domain/models/core/invariant.py
  2. Observe model_config = ConfigDict(str_strip_whitespace=True, validate_assignment=True) — no frozen=True
  3. Read src/cleveragents/application/services/invariant_service.py, remove_invariant() method
  4. Observe inv.active = False — direct mutation of a model that should be frozen

Impact:

  • Invariants can be mutated by any code holding a reference, violating the value-object contract
  • The soft-delete pattern (inv.active = False) is a side-effecting mutation that bypasses the service layer
  • Models are not hashable (frozen models are hashable in Pydantic v2)

Subtasks

  • Update Invariant.model_config to use frozen=True (remove validate_assignment=True)
  • Update InvariantViolation.model_config to use frozen=True
  • Update InvariantEnforcementRecord.model_config to use frozen=True
  • Redesign InvariantService.remove_invariant() to construct a new Invariant(active=False, ...) and replace the entry in _invariants dict instead of mutating in place
  • Update all other callers that mutate Invariant instances directly
  • Update BDD scenarios to cover immutability contract and the new soft-delete pattern
  • Verify models are hashable after frozen=True is applied

Definition of Done

  • Invariant.model_config updated to include frozen=True
  • InvariantViolation.model_config updated to include frozen=True
  • InvariantEnforcementRecord.model_config updated to include frozen=True
  • InvariantService.remove_invariant() redesigned to create a new Invariant with active=False instead of mutating in place
  • All tests pass with nox -e unit_tests
  • Type checking passes with nox -e typecheck
  • All nox stages pass
  • Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

## Metadata - **Branch**: `fix/invariant-frozen-model-config` - **Commit Message**: `fix(invariant): add frozen=True to Invariant model config and redesign soft-delete pattern` - **Milestone**: v3.2.0 - **Parent Epic**: #394 ## Background The spec (milestone v3.2.0, M3: Decisions + Validations + Invariants) requires the `Invariant` Pydantic v2 model to use `frozen=True` for immutability. The implementation instead uses `validate_assignment=True`, which allows mutation. **What was tested**: Code-level analysis of `src/cleveragents/domain/models/core/invariant.py` **Expected behavior (from spec)**: The `Invariant` model should be declared with `frozen=True` in its `model_config = ConfigDict(frozen=True, ...)`, making instances immutable (hashable, cannot be mutated after creation). This is a standard Pydantic v2 pattern for value objects in domain models. **Actual behavior**: The model uses: ```python model_config = ConfigDict( str_strip_whitespace=True, validate_assignment=True, # ← allows mutation ) ``` `frozen=True` is absent. Instances can be mutated after creation. **Code location**: `src/cleveragents/domain/models/core/invariant.py`, lines ~100-107 (the `Invariant.model_config`) **Design conflict**: The `InvariantService.remove_invariant()` method in `src/cleveragents/application/services/invariant_service.py` performs a direct mutation: ```python inv.active = False # ← direct mutation of the model ``` If `frozen=True` were enforced, this line would raise a `pydantic.ValidationError` at runtime. The soft-delete pattern would need to be redesigned to create a new `Invariant` instance with `active=False` and replace it in the `_invariants` dict. **Also affected**: `InvariantViolation` and `InvariantEnforcementRecord` models in the same file also use `validate_assignment=True` without `frozen=True`. **Steps to reproduce**: 1. Read `src/cleveragents/domain/models/core/invariant.py` 2. Observe `model_config = ConfigDict(str_strip_whitespace=True, validate_assignment=True)` — no `frozen=True` 3. Read `src/cleveragents/application/services/invariant_service.py`, `remove_invariant()` method 4. Observe `inv.active = False` — direct mutation of a model that should be frozen **Impact**: - Invariants can be mutated by any code holding a reference, violating the value-object contract - The soft-delete pattern (`inv.active = False`) is a side-effecting mutation that bypasses the service layer - Models are not hashable (frozen models are hashable in Pydantic v2) ## Subtasks - [ ] Update `Invariant.model_config` to use `frozen=True` (remove `validate_assignment=True`) - [ ] Update `InvariantViolation.model_config` to use `frozen=True` - [ ] Update `InvariantEnforcementRecord.model_config` to use `frozen=True` - [ ] Redesign `InvariantService.remove_invariant()` to construct a new `Invariant(active=False, ...)` and replace the entry in `_invariants` dict instead of mutating in place - [ ] Update all other callers that mutate `Invariant` instances directly - [ ] Update BDD scenarios to cover immutability contract and the new soft-delete pattern - [ ] Verify models are hashable after `frozen=True` is applied ## Definition of Done - [ ] `Invariant.model_config` updated to include `frozen=True` - [ ] `InvariantViolation.model_config` updated to include `frozen=True` - [ ] `InvariantEnforcementRecord.model_config` updated to include `frozen=True` - [ ] `InvariantService.remove_invariant()` redesigned to create a new `Invariant` with `active=False` instead of mutating in place - [ ] All tests pass with `nox -e unit_tests` - [ ] Type checking passes with `nox -e typecheck` - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester
freemo added this to the v3.2.0 milestone 2026-04-05 06:26:59 +00:00
Author
Owner

Issue triaged by project owner:

  • State: Verified
  • Priority: Critical (keeping existing — spec requires immutable domain models)
  • Milestone: v3.2.0 (already assigned, keeping)
  • MoSCoW: Must Have — the Invariant model must be immutable per the spec's value-object contract. The current mutable design allows side-effecting mutations that bypass the service layer.
  • Parent Epic: #394 (Decision Framework)

Note: This issue has a design conflict with InvariantService.remove_invariant() that performs direct mutation. The fix requires redesigning the soft-delete pattern to create new instances rather than mutating in place. This is the correct approach per DDD value-object patterns.


Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Critical (keeping existing — spec requires immutable domain models) - **Milestone**: v3.2.0 (already assigned, keeping) - **MoSCoW**: Must Have — the Invariant model must be immutable per the spec's value-object contract. The current mutable design allows side-effecting mutations that bypass the service layer. - **Parent Epic**: #394 (Decision Framework) **Note:** This issue has a design conflict with `InvariantService.remove_invariant()` that performs direct mutation. The fix requires redesigning the soft-delete pattern to create new instances rather than mutating in place. This is the correct approach per DDD value-object patterns. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Author
Owner

State transition attempt to In Progress.

Current labels: State/Verified, MoSCoW/Must have, Priority/Critical, Type/Bug

Precondition checks:

  • Not transitioning from State/Paused, so no blocker check required.
  • No Blocked label present, so no need to remove blocking links.

Actions planned:

  • Remove old state label: State/Verified
  • Add new state label: State/In Progress

Status: Label operations could not be executed via the current automation interface in this environment. The label changes should be performed on the Forgejo issue as described above. If you approve, I can retry this via an alternate method or perform these updates manually in Forgejo.

Notes:

  • Always ensure all State/* labels are cleared before adding the new one.
  • Do not close the issue on In Progress.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-state-updater

State transition attempt to In Progress. Current labels: State/Verified, MoSCoW/Must have, Priority/Critical, Type/Bug Precondition checks: - Not transitioning from State/Paused, so no blocker check required. - No Blocked label present, so no need to remove blocking links. Actions planned: - Remove old state label: State/Verified - Add new state label: State/In Progress Status: Label operations could not be executed via the current automation interface in this environment. The label changes should be performed on the Forgejo issue as described above. If you approve, I can retry this via an alternate method or perform these updates manually in Forgejo. Notes: - Always ensure all State/* labels are cleared before adding the new one. - Do not close the issue on In Progress. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-state-updater
Author
Owner

Label compliance fix applied:

  • Added missing labels: Type/Bug, Priority/Critical
  • Reason: Issue had only State/In Progress but was missing Type/* and Priority/* labels. Per CONTRIBUTING.md, all three are required. Inferred from issue title (UAT bug — invariant domain model missing frozen=True) and milestone v3.2.0.

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: ca-backlog-groomer

Label compliance fix applied: - Added missing labels: `Type/Bug`, `Priority/Critical` - Reason: Issue had only `State/In Progress` but was missing `Type/*` and `Priority/*` labels. Per CONTRIBUTING.md, all three are required. Inferred from issue title (UAT bug — invariant domain model missing frozen=True) and milestone v3.2.0. --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: ca-backlog-groomer
Author
Owner

Starting implementation on branch fix/invariant-frozen-model-config.

Changes being made:

  1. Invariant.model_config — replacing validate_assignment=True with frozen=True
  2. InvariantViolation.model_config — replacing validate_assignment=True with frozen=True
  3. InvariantEnforcementRecord.model_config — replacing validate_assignment=True with frozen=True
  4. InvariantSet.model_config — replacing validate_assignment=True with frozen=True
  5. InvariantService.remove_invariant() — redesigned to use model_copy(update={"active": False}) instead of direct mutation
  6. invariant_reconciliation_actor_steps.py — fixed step that mutated inv.non_overridable = True to use direct construction instead
  7. New BDD scenarios added to consolidated_domain_models.feature covering immutability contract and hashability

All 266 scenarios pass. Lint and typecheck pass.


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

Starting implementation on branch `fix/invariant-frozen-model-config`. **Changes being made:** 1. `Invariant.model_config` — replacing `validate_assignment=True` with `frozen=True` 2. `InvariantViolation.model_config` — replacing `validate_assignment=True` with `frozen=True` 3. `InvariantEnforcementRecord.model_config` — replacing `validate_assignment=True` with `frozen=True` 4. `InvariantSet.model_config` — replacing `validate_assignment=True` with `frozen=True` 5. `InvariantService.remove_invariant()` — redesigned to use `model_copy(update={"active": False})` instead of direct mutation 6. `invariant_reconciliation_actor_steps.py` — fixed step that mutated `inv.non_overridable = True` to use direct construction instead 7. New BDD scenarios added to `consolidated_domain_models.feature` covering immutability contract and hashability All 266 scenarios pass. Lint and typecheck pass. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
Author
Owner

PR #3342 created on branch fix/invariant-frozen-model-config. PR review and merge handled by continuous review stream.

Summary of changes:

  • Invariant, InvariantViolation, InvariantEnforcementRecord, InvariantSet models now use frozen=True
  • InvariantService.remove_invariant() redesigned to use model_copy(update={"active": False}) (copy-and-replace pattern)
  • Fixed invariant_reconciliation_actor_steps.py mutation of non_overridable field
  • 7 new BDD scenarios covering immutability contract and hashability

Quality gates: lint | typecheck | unit_tests (266 scenarios)


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

PR #3342 created on branch `fix/invariant-frozen-model-config`. PR review and merge handled by continuous review stream. **Summary of changes:** - `Invariant`, `InvariantViolation`, `InvariantEnforcementRecord`, `InvariantSet` models now use `frozen=True` - `InvariantService.remove_invariant()` redesigned to use `model_copy(update={"active": False})` (copy-and-replace pattern) - Fixed `invariant_reconciliation_actor_steps.py` mutation of `non_overridable` field - 7 new BDD scenarios covering immutability contract and hashability **Quality gates:** lint ✅ | typecheck ✅ | unit_tests (266 scenarios) ✅ --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core#3116
No description provided.