refactor(api): use sessionmaker in builtin tools manage service

[carlos4s]

Summary

• Replace with Session(db.engine) + session.commit() with sessionmaker(bind=db.engine).begin()
  in builtin_tools_manage_service (6 blocks)
• Remove explicit session.rollback() calls — .begin() handles rollback automatically
• Read-only session blocks and autoflush=False blocks left unchanged
• core/ops/ops_trace_manager.py skipped (all blocks read-only)
• Updated test mocks to match new sessionmaker pattern

Part of #24245

Test plan

• ruff check passes
• 30 builtin_tools_manage_service tests pass

[github-actions]

Pyrefly Diff

base → PR

--- /tmp/pyrefly_base.txt	2026-04-09 05:13:07.408975599 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-09 05:12:59.315028661 +0000
@@ -6719,7 +6719,7 @@
 ERROR Argument `dict[str, str]` is not assignable to parameter `node_config` with type `NodeConfigDict` in function `services.workflow_service.WorkflowService._build_human_input_node` [bad-argument-type]
     --> tests/unit_tests/services/test_workflow_service.py:2763:65
 ERROR Argument `Literal['api_key']` is not assignable to parameter `credential_type` with type `CredentialType` in function `services.tools.builtin_tools_manage_service.BuiltinToolManageService.list_builtin_provider_credentials_schema` [bad-argument-type]
-  --> tests/unit_tests/services/tools/test_builtin_tools_manage_service.py:84:89
+  --> tests/unit_tests/services/tools/test_builtin_tools_manage_service.py:91:89
 ERROR Object of class `Mapping` has no attribute `startswith` [missing-attribute]
    --> tests/unit_tests/services/tools/test_tools_transform_service.py:464:16
 ERROR Could not import `DatasetDocument` from `models.dataset` [missing-module-attribute]

[Copilot]

Pull request overview

Refactors BuiltinToolManageService write operations to use SQLAlchemy sessionmaker(...).begin() transaction blocks, simplifying commit/rollback handling and aligning unit test mocks with the new pattern.

Changes:

• Replaced several with Session(db.engine) + explicit commit()/rollback() blocks with with sessionmaker(bind=db.engine).begin() in builtin tools management service.
• Removed explicit session.commit() / session.rollback() calls where the transaction context manager now owns commit/rollback behavior.
• Updated unit tests to patch/mimic the sessionmaker().begin() context manager instead of Session.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
api/services/tools/builtin_tools_manage_service.py	Migrates multiple write-path DB interactions to sessionmaker(...).begin() and removes manual commit/rollback calls.
api/tests/unit_tests/services/tools/test_builtin_tools_manage_service.py	Updates mocks/patching to reflect the new sessionmaker transaction pattern and removes commit assertions.

Comments suppressed due to low confidence (1)

api/services/tools/builtin_tools_manage_service.py:397

• cache.delete() is now executed inside the DB transaction (the commit happens when the begin() context exits). This changes behavior vs the previous explicit session.commit() before cache invalidation: if cache deletion raises (e.g., Redis outage), the DB delete will be rolled back and the provider won’t be removed. Consider committing the DB delete before cache invalidation, or moving cache invalidation outside the transaction and handling cache errors separately so cache issues don’t block the core DB operation.

            provider_controller = ToolManager.get_builtin_provider(provider, tenant_id)
            _, cache = BuiltinToolManageService.create_tool_encrypter(
                tenant_id, db_provider, provider, provider_controller
            )
            cache.delete()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test name is now misleading: the service code no longer calls session.commit() explicitly (commit is handled by the begin() context manager). Rename the test to reflect the behavior being asserted (e.g., updating credentials / clearing cache) rather than committing.