cleveragents/cleveragents-core

Fork 3

UAT: NamespacedProjectRepository, ProjectResourceLinkRepository, ToolRegistryRepository, and ValidationAttachmentRepository use auto-commit — violates ADR-019 Unit of Work constraint #1766

New issue

Open

opened 2026-04-02 23:45:56 +00:00 by freemo · 2 comments

freemo commented

2026-04-02 23:45:56 +00:00

Owner

Metadata

Branch: fix/repo-auto-commit-violation
Commit Message: fix(infra): replace auto-commit with flush in repository implementations
Milestone: v3.7.0
Parent Epic: N/A — no suitable Epic exists; linked to v3.7.0 milestone per triage guidance

Description

ADR-019 (Storage and Persistence) explicitly states: "Repository implementations must use the Unit of Work pattern — no auto-commit, no implicit transactions." However, several repository implementations call session.commit() directly within their methods, bypassing the Unit of Work pattern.

What was tested: Code-level analysis of src/cleveragents/infrastructure/database/repositories.py for auto-commit usage.

Expected Behavior (from spec)

Per ADR-019:

"Repository implementations must use the Unit of Work pattern — no auto-commit, no implicit transactions."

All mutating repository methods should only call session.flush() (to send SQL to the DB without committing), leaving commit/rollback responsibility to the caller (UnitOfWork or explicit transaction manager).

Actual Behavior

The following repository classes call session.commit() directly within their mutating methods:

NamespacedProjectRepository (lines ~2912, ~3062, ~3098): create(), update(), delete() all call session.commit() directly.
ProjectResourceLinkRepository (lines ~3178, ~3262): create_link(), remove_link() call session.commit() directly.
ToolRegistryRepository (lines ~3375, ~3508, ~3548): Multiple methods call session.commit() directly.
ValidationAttachmentRepository (lines ~3840, ~3889): Multiple methods call session.commit() directly.

Example from NamespacedProjectRepository.create():

def create(self, project: Any) -> Any:
    session = self._session()
    try:
        db_model = NamespacedProjectModel.from_domain(project)
        session.add(db_model)
        session.commit()  # VIOLATION: should be session.flush() only
        return project
    ...
    finally:
        session.close()

Impact

Operations across multiple repositories cannot be made atomic — if NamespacedProjectRepository.create() succeeds but a subsequent operation fails, the project creation cannot be rolled back.
The Unit of Work pattern is broken for these entities — callers cannot wrap multiple operations in a single transaction.
These repositories are not usable within a UnitOfWorkContext transaction (they commit immediately, bypassing the outer transaction).

Code location: src/cleveragents/infrastructure/database/repositories.py

Steps to Reproduce

grep -n "session.commit()" src/cleveragents/infrastructure/database/repositories.py
# Shows commits at lines ~2912, ~3062, ~3098, ~3178, ~3262, ~3375, ~3508, ~3548, ~3840, ~3889

Subtasks

Audit all session.commit() calls in repositories.py and identify every violation
Replace session.commit() with session.flush() in NamespacedProjectRepository.create(), update(), delete()
Replace session.commit() with session.flush() in ProjectResourceLinkRepository.create_link(), remove_link()
Replace session.commit() with session.flush() in all affected ToolRegistryRepository methods
Replace session.commit() with session.flush() in all affected ValidationAttachmentRepository methods
Update or add Behave unit tests to verify no session.commit() is called within repository methods
Verify all nox stages pass after changes
Confirm coverage ≥ 97%

Definition of Done

No session.commit() calls remain in any repository implementation in repositories.py
All mutating repository methods use session.flush() only, deferring commit/rollback to the Unit of Work caller
Behave unit tests cover the flush-only behaviour for all four affected repository classes
nox -e typecheck passes with zero Pyright errors
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/repo-auto-commit-violation` - **Commit Message**: `fix(infra): replace auto-commit with flush in repository implementations` - **Milestone**: v3.7.0 - **Parent Epic**: N/A — no suitable Epic exists; linked to v3.7.0 milestone per triage guidance ## Description ADR-019 (Storage and Persistence) explicitly states: "Repository implementations must use the Unit of Work pattern — no auto-commit, no implicit transactions." However, several repository implementations call `session.commit()` directly within their methods, bypassing the Unit of Work pattern. **What was tested**: Code-level analysis of `src/cleveragents/infrastructure/database/repositories.py` for auto-commit usage. ### Expected Behavior (from spec) Per ADR-019: > "Repository implementations must use the Unit of Work pattern — no auto-commit, no implicit transactions." All mutating repository methods should only call `session.flush()` (to send SQL to the DB without committing), leaving commit/rollback responsibility to the caller (UnitOfWork or explicit transaction manager). ### Actual Behavior The following repository classes call `session.commit()` directly within their mutating methods: 1. **`NamespacedProjectRepository`** (lines ~2912, ~3062, ~3098): `create()`, `update()`, `delete()` all call `session.commit()` directly. 2. **`ProjectResourceLinkRepository`** (lines ~3178, ~3262): `create_link()`, `remove_link()` call `session.commit()` directly. 3. **`ToolRegistryRepository`** (lines ~3375, ~3508, ~3548): Multiple methods call `session.commit()` directly. 4. **`ValidationAttachmentRepository`** (lines ~3840, ~3889): Multiple methods call `session.commit()` directly. Example from `NamespacedProjectRepository.create()`: ```python def create(self, project: Any) -> Any: session = self._session() try: db_model = NamespacedProjectModel.from_domain(project) session.add(db_model) session.commit() # VIOLATION: should be session.flush() only return project ... finally: session.close() ``` ### Impact 1. Operations across multiple repositories cannot be made atomic — if `NamespacedProjectRepository.create()` succeeds but a subsequent operation fails, the project creation cannot be rolled back. 2. The Unit of Work pattern is broken for these entities — callers cannot wrap multiple operations in a single transaction. 3. These repositories are not usable within a `UnitOfWorkContext` transaction (they commit immediately, bypassing the outer transaction). **Code location**: `src/cleveragents/infrastructure/database/repositories.py` ### Steps to Reproduce ```bash grep -n "session.commit()" src/cleveragents/infrastructure/database/repositories.py # Shows commits at lines ~2912, ~3062, ~3098, ~3178, ~3262, ~3375, ~3508, ~3548, ~3840, ~3889 ``` ## Subtasks - [ ] Audit all `session.commit()` calls in `repositories.py` and identify every violation - [ ] Replace `session.commit()` with `session.flush()` in `NamespacedProjectRepository.create()`, `update()`, `delete()` - [ ] Replace `session.commit()` with `session.flush()` in `ProjectResourceLinkRepository.create_link()`, `remove_link()` - [ ] Replace `session.commit()` with `session.flush()` in all affected `ToolRegistryRepository` methods - [ ] Replace `session.commit()` with `session.flush()` in all affected `ValidationAttachmentRepository` methods - [ ] Update or add Behave unit tests to verify no `session.commit()` is called within repository methods - [ ] Verify all nox stages pass after changes - [ ] Confirm coverage ≥ 97% ## Definition of Done - [ ] No `session.commit()` calls remain in any repository implementation in `repositories.py` - [ ] All mutating repository methods use `session.flush()` only, deferring commit/rollback to the Unit of Work caller - [ ] Behave unit tests cover the flush-only behaviour for all four affected repository classes - [ ] `nox -e typecheck` passes with zero Pyright errors - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added this to the v3.7.0 milestone

2026-04-02 23:46:14 +00:00

freemo commented

2026-04-02 23:47:45 +00:00

Author

Owner

⚠️ Orphan Issue — Manual Linking Required

This issue was created without a parent Epic because no suitable infrastructure/database/repository Epic currently exists in the repository. A search of all open and closed Epics (Type/Epic label) returned only one result: #1678 (CI Execution Time Optimization), which is unrelated to this ADR-019 violation.

Action required by project owner: Please either:

Create a new Epic covering ADR-019 / Unit of Work / Repository compliance and link this issue as a child (this issue should block that Epic), or
Manually link this issue to an existing appropriate Epic using Forgejo's dependency system.

Per CONTRIBUTING.md: "Orphan issues are NOT permitted. Every issue must be linked to a parent Epic."

This issue is assigned to milestone v3.7.0 as instructed by the UAT tester.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

⚠️ **Orphan Issue — Manual Linking Required** This issue was created without a parent Epic because no suitable infrastructure/database/repository Epic currently exists in the repository. A search of all open and closed Epics (`Type/Epic` label) returned only one result: **#1678** (CI Execution Time Optimization), which is unrelated to this ADR-019 violation. **Action required by project owner**: Please either: 1. Create a new Epic covering ADR-019 / Unit of Work / Repository compliance and link this issue as a child (this issue should **block** that Epic), or 2. Manually link this issue to an existing appropriate Epic using Forgejo's dependency system. Per CONTRIBUTING.md: *"Orphan issues are NOT permitted. Every issue must be linked to a parent Epic."* This issue is assigned to milestone **v3.7.0** as instructed by the UAT tester. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo referenced this issue

2026-04-02 23:48:06 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 451 — v3.7.0 #1778

freemo commented

2026-04-02 23:51:10 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
MoSCoW: MoSCoW/Should Have — bug or spec compliance issue.

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

Issue triaged by project owner: - **State**: Verified - **MoSCoW**: MoSCoW/Should Have — bug or spec compliance issue. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added the

labels

2026-04-02 23:52:11 +00:00

freemo referenced this issue

2026-04-02 23:56:42 +00:00

[Automated] Backlog Groomer Session State — v3.7.0 #1463