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

Merged
freemo merged 1 commits from fix/tool-add-not-registering into master 2026-03-08 22:17:09 +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

  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)
## 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)
freemo added this to the v3.2.0 milestone 2026-03-08 03:15:35 +00:00
freemo added the
Type
Bug
label 2026-03-08 03:15:35 +00:00
freemo added a new dependency 2026-03-08 22:11:30 +00:00
freemo added a new dependency 2026-03-08 22:11:40 +00:00
freemo force-pushed fix/tool-add-not-registering from 7e9157dd06 to d3cc7d30d7 2026-03-08 22:11:51 +00:00 Compare
freemo scheduled this pull request to auto merge when all checks succeed 2026-03-08 22:12:01 +00:00
freemo merged commit d3cc7d30d7 into master 2026-03-08 22:17:09 +00:00
freemo deleted branch fix/tool-add-not-registering 2026-03-08 22:17:09 +00:00
Sign in to join this conversation.
No Reviewers
No Label
Type
Bug
1 Participants
Notifications
Due Date
No due date set.
Reference: cleveragents/cleveragents-core#641