1 Commits

Author SHA1 Message Date
freemo d3cc7d30d7 fix(tool): persist tool registration to database after add
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 19s
CI / security (pull_request) Successful in 34s
CI / typecheck (pull_request) Successful in 36s
CI / unit_tests (pull_request) Successful in 3m45s
CI / docker (pull_request) Successful in 40s
CI / integration_tests (pull_request) Successful in 4m34s
CI / coverage (pull_request) Successful in 4m39s
CI / lint (push) Successful in 12s
CI / build (push) Successful in 15s
CI / quality (push) Successful in 17s
CI / security (push) Successful in 33s
CI / typecheck (push) Successful in 34s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Successful in 2m21s
CI / integration_tests (push) Successful in 3m9s
CI / docker (push) Successful in 1m0s
CI / coverage (push) Successful in 4m37s
CI / benchmark-publish (push) Has been cancelled
CI / benchmark-regression (pull_request) Successful in 30m5s
ToolRegistryRepository.create(), .update(), and .delete() called
session.flush() but never session.commit().  The CLI factory creates a
raw sessionmaker without a UnitOfWork wrapper, so the transaction was
never committed and SQLAlchemy performed an implicit rollback when the
session was garbage-collected.

The same bug existed in ValidationAttachmentRepository.attach() and
.detach().

Changes:
- Add session.commit() after session.flush() in all five mutating
  methods across ToolRegistryRepository and
  ValidationAttachmentRepository.
- Add finally: session.close() to guarantee session cleanup regardless
  of success or failure.
- Update class docstrings to reflect the new commit-on-write semantics.
- Add Behave BDD feature (tool_add_persist.feature) with scenarios for
  single-tool round-trip, multi-tool persistence, and duplicate
  rejection, using file-based SQLite to reproduce the cross-session
  issue.
- Add Robot Framework integration test (tool_add_persist.robot) with
  add-then-list and fresh-list-empty scenarios.
- Add ASV benchmark (tool_add_persist_bench.py) with
  track_list_after_add_count metric.

Key decisions:
- File-based SQLite (not in-memory) is used in tests because the bug
  only manifests when the session/engine is fully disposed between add
  and list, simulating separate CLI invocations.
- Step patterns are prefixed with "tool-persist" to avoid AmbiguousStep
  collisions with existing tool_registry_steps.py.
- The commit-in-repository approach was chosen over adding a UnitOfWork
  to the CLI factory because the CLI commands are simple CRUD operations
  that should auto-persist without requiring callers to remember to
  commit.

ISSUES CLOSED: #621
2026-03-08 22:11:49 +00:00