fix(tool): persist tool registration to database after add #641

Merged

freemo merged 1 commit from fix/tool-add-not-registering into master

2026-03-08 22:17:09 +00:00

freemo commented

2026-03-08 03:15:35 +00:00

Owner

Summary

Fixes the bug where agents tool add followed by agents tool list shows "No tools found" because the tool registration was never committed to the database.

Closes #621
Closes #638

Root Cause

ToolRegistryRepository.create() in src/cleveragents/infrastructure/database/repositories.py called session.flush() but never session.commit(). The _get_tool_registry_service() factory in the CLI creates a raw sessionmaker without a UnitOfWork wrapper, so nobody ever committed the transaction. When the session was garbage-collected, SQLAlchemy performed an implicit rollback.

The same bug existed in:

ToolRegistryRepository.update()
ToolRegistryRepository.delete()
ValidationAttachmentRepository.attach()
ValidationAttachmentRepository.detach()

Changes

Bug Fix (repositories.py)

Added session.commit() after session.flush() in all five mutating methods
Added finally: session.close() to guarantee session cleanup regardless of success or failure
Updated class docstrings to reflect the new commit-on-write semantics

Behave BDD Tests (features/tool_add_persist.feature)

Scenario: Tool add followed by tool list shows the added tool — single-tool round-trip with file-based SQLite
Scenario: Multiple tools added and listed — multi-tool persistence verification
Scenario: Tool add with duplicate name produces error — duplicate rejection
Step patterns prefixed with tool-persist to avoid AmbiguousStep collisions

Robot Framework Tests (robot/tool_add_persist.robot)

Tool Add Then List Shows Tool — integration test via helper script
Tool List On Fresh Init Shows No Tools — empty database baseline

ASV Benchmark (benchmarks/tool_add_persist_bench.py)

track_list_after_add_count — returns count of tools after add (should be 1)

Key Design Decisions

File-based SQLite (not in-memory) in tests because the bug only manifests when the session/engine is fully disposed between add and list, simulating separate CLI invocations.
Commit-in-repository approach chosen over adding a UnitOfWork to the CLI factory because CLI commands are simple CRUD operations that should auto-persist without requiring callers to remember to commit.

Quality Gates

nox -s lint — PASSED
nox -s typecheck — PASSED (0 errors, 0 warnings)
nox -s unit_tests — PASSED (9109 scenarios, 0 failures)
nox -s integration_tests — Our new tests PASSED; pre-existing failures in cli_plan_context_commands are unrelated
nox -s coverage_report — 97% (meets threshold)

## Summary Fixes the bug where `agents tool add` followed by `agents tool list` shows "No tools found" because the tool registration was never committed to the database. Closes #621 Closes #638 ## Root Cause `ToolRegistryRepository.create()` in `src/cleveragents/infrastructure/database/repositories.py` called `session.flush()` but **never** `session.commit()`. The `_get_tool_registry_service()` factory in the CLI creates a raw `sessionmaker` without a `UnitOfWork` wrapper, so nobody ever committed the transaction. When the session was garbage-collected, SQLAlchemy performed an implicit rollback. The same bug existed in: - `ToolRegistryRepository.update()` - `ToolRegistryRepository.delete()` - `ValidationAttachmentRepository.attach()` - `ValidationAttachmentRepository.detach()` ## Changes ### Bug Fix (repositories.py) - Added `session.commit()` after `session.flush()` in all five mutating methods - Added `finally: session.close()` to guarantee session cleanup regardless of success or failure - Updated class docstrings to reflect the new commit-on-write semantics ### Behave BDD Tests (features/tool_add_persist.feature) - **Scenario: Tool add followed by tool list shows the added tool** — single-tool round-trip with file-based SQLite - **Scenario: Multiple tools added and listed** — multi-tool persistence verification - **Scenario: Tool add with duplicate name produces error** — duplicate rejection - Step patterns prefixed with `tool-persist` to avoid AmbiguousStep collisions ### Robot Framework Tests (robot/tool_add_persist.robot) - **Tool Add Then List Shows Tool** — integration test via helper script - **Tool List On Fresh Init Shows No Tools** — empty database baseline ### ASV Benchmark (benchmarks/tool_add_persist_bench.py) - `track_list_after_add_count` — returns count of tools after add (should be 1) ## Key Design Decisions 1. **File-based SQLite** (not in-memory) in tests because the bug only manifests when the session/engine is fully disposed between add and list, simulating separate CLI invocations. 2. **Commit-in-repository** approach chosen over adding a UnitOfWork to the CLI factory because CLI commands are simple CRUD operations that should auto-persist without requiring callers to remember to commit. ## Quality Gates - `nox -s lint` — PASSED - `nox -s typecheck` — PASSED (0 errors, 0 warnings) - `nox -s unit_tests` — PASSED (9109 scenarios, 0 failures) - `nox -s integration_tests` — Our new tests PASSED; pre-existing failures in cli_plan_context_commands are unrelated - `nox -s coverage_report` — 97% (meets threshold)

Rows
Columns