fix: fix the wrong cache for second call of agent tools#195
Merged
Conversation
Contributor
审阅者指南(在小型 PR 上折叠)审阅者指南调整 Celery worker/工具的数据库会话处理方式,使引擎按(进程、事件循环)进行缓存,而不是仅按进程缓存,从而防止在不同事件循环之间重用连接,并修复后续工具调用中的错误行为。 Celery worker 任务数据库会话创建与缓存查找的时序图sequenceDiagram
actor CeleryWorker
participant EventLoop
participant get_task_db_session
participant _worker_engines
participant Database
CeleryWorker->>EventLoop: Run async task
EventLoop->>get_task_db_session: async with get_task_db_session()
get_task_db_session->>get_task_db_session: pid = os.getpid()
get_task_db_session->>EventLoop: get_running_loop()
EventLoop-->>get_task_db_session: loop instance
get_task_db_session->>get_task_db_session: loop_id = id(loop)
get_task_db_session->>get_task_db_session: cache_key = (pid, loop_id)
alt cache miss for cache_key
get_task_db_session->>_worker_engines: find old_keys for pid and loop_id != current
_worker_engines-->>get_task_db_session: old_keys list
get_task_db_session->>_worker_engines: delete old_keys
get_task_db_session->>Database: create_async_engine(ASYNC_DATABASE_URL)
Database-->>get_task_db_session: engine
get_task_db_session->>_worker_engines: store async_sessionmaker with cache_key
else cache hit for cache_key
get_task_db_session->>_worker_engines: reuse async_sessionmaker
end
get_task_db_session->>Database: open AsyncSession via async_sessionmaker
Database-->>get_task_db_session: AsyncSession
get_task_db_session-->>EventLoop: yield AsyncSession
EventLoop-->>CeleryWorker: session usable in task
数据库连接模块缓存结构的类图classDiagram
class ConnectionModule {
ASYNC_DATABASE_URL: str
_worker_engines: dict[tuple[int,int], async_sessionmaker]
+get_task_db_session() AsyncContextManager[AsyncSession]
+get_session() AsyncGenerator[AsyncSession, None]
}
class AsyncEngine {
+dispose() None
}
class AsyncSessionMaker {
+__call__() AsyncSession
}
class AsyncSession {
+execute(statement) Any
+commit() None
+close() None
}
ConnectionModule "*" o-- AsyncEngine : creates
ConnectionModule "*" o-- AsyncSessionMaker : caches per(pid,loop)
AsyncSessionMaker "*" --> AsyncSession : produces
文件级变更
技巧与命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts Celery worker/tool database session handling so engines are cached per (process, event loop) instead of per process, preventing reuse of connections across different event loops and fixing incorrect behavior on subsequent tool calls. Sequence diagram for Celery worker task DB session creation and cache lookupsequenceDiagram
actor CeleryWorker
participant EventLoop
participant get_task_db_session
participant _worker_engines
participant Database
CeleryWorker->>EventLoop: Run async task
EventLoop->>get_task_db_session: async with get_task_db_session()
get_task_db_session->>get_task_db_session: pid = os.getpid()
get_task_db_session->>EventLoop: get_running_loop()
EventLoop-->>get_task_db_session: loop instance
get_task_db_session->>get_task_db_session: loop_id = id(loop)
get_task_db_session->>get_task_db_session: cache_key = (pid, loop_id)
alt cache miss for cache_key
get_task_db_session->>_worker_engines: find old_keys for pid and loop_id != current
_worker_engines-->>get_task_db_session: old_keys list
get_task_db_session->>_worker_engines: delete old_keys
get_task_db_session->>Database: create_async_engine(ASYNC_DATABASE_URL)
Database-->>get_task_db_session: engine
get_task_db_session->>_worker_engines: store async_sessionmaker with cache_key
else cache hit for cache_key
get_task_db_session->>_worker_engines: reuse async_sessionmaker
end
get_task_db_session->>Database: open AsyncSession via async_sessionmaker
Database-->>get_task_db_session: AsyncSession
get_task_db_session-->>EventLoop: yield AsyncSession
EventLoop-->>CeleryWorker: session usable in task
Class diagram for database connection module cache structuresclassDiagram
class ConnectionModule {
ASYNC_DATABASE_URL: str
_worker_engines: dict[tuple[int,int], async_sessionmaker]
+get_task_db_session() AsyncContextManager[AsyncSession]
+get_session() AsyncGenerator[AsyncSession, None]
}
class AsyncEngine {
+dispose() None
}
class AsyncSessionMaker {
+__call__() AsyncSession
}
class AsyncSession {
+execute(statement) Any
+commit() None
+close() None
}
ConnectionModule "*" o-- AsyncEngine : creates
ConnectionModule "*" o-- AsyncSessionMaker : caches per(pid,loop)
AsyncSessionMaker "*" --> AsyncSession : produces
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些总体反馈:
- 在清理旧事件循环所对应的 engine 时,你目前只是从
_worker_engines中删除条目,却没有释放底层的 engine,这可能会导致连接泄漏;建议在移除之前调用await task_engine.dispose()(或以其他方式关闭)。 - 使用
id(asyncio.get_running_loop())作为循环标识在实践中是可行的,但依赖于 CPython 的对象标识语义;如果你需要在时间维度上更强的保证,可以考虑使用单调递增的循环 token 或显式的循环名称来代替id()。 - 对全局
_worker_engines字典的访问目前是无同步保护的;如果 Celery 未来被配置为在单个进程中使用线程或多个事件循环,你可能需要用锁来保护对该字典的修改,以避免多个任务并发初始化或清理 engine 时产生竞争条件。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- 在清理旧事件循环所对应的 engine 时,你目前只是从 `_worker_engines` 中删除条目,却没有释放底层的 engine,这可能会导致连接泄漏;建议在移除之前调用 `await task_engine.dispose()`(或以其他方式关闭)。
- 使用 `id(asyncio.get_running_loop())` 作为循环标识在实践中是可行的,但依赖于 CPython 的对象标识语义;如果你需要在时间维度上更强的保证,可以考虑使用单调递增的循环 token 或显式的循环名称来代替 `id()`。
- 对全局 `_worker_engines` 字典的访问目前是无同步保护的;如果 Celery 未来被配置为在单个进程中使用线程或多个事件循环,你可能需要用锁来保护对该字典的修改,以避免多个任务并发初始化或清理 engine 时产生竞争条件。帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- When cleaning up engines for old event loops you currently just delete the entry from
_worker_engineswithout disposing the underlying engine, which can leak connections; consider callingawait task_engine.dispose()(or otherwise closing) before removing them. - Using
id(asyncio.get_running_loop())as the loop identifier works in practice but relies on CPython object identity semantics; if you need stronger guarantees over time, consider using a monotonically increasing loop token or explicit loop name instead ofid(). - Access to the global
_worker_enginesdict is unsynchronized; if Celery is ever configured to use threads or multiple event loops in a single process, you may want to guard modifications with a lock to avoid race conditions when multiple tasks initialize or clean up engines concurrently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When cleaning up engines for old event loops you currently just delete the entry from `_worker_engines` without disposing the underlying engine, which can leak connections; consider calling `await task_engine.dispose()` (or otherwise closing) before removing them.
- Using `id(asyncio.get_running_loop())` as the loop identifier works in practice but relies on CPython object identity semantics; if you need stronger guarantees over time, consider using a monotonically increasing loop token or explicit loop name instead of `id()`.
- Access to the global `_worker_engines` dict is unsynchronized; if Celery is ever configured to use threads or multiple event loops in a single process, you may want to guard modifications with a lock to avoid race conditions when multiple tasks initialize or clean up engines concurrently.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Mile-Away
added a commit
that referenced
this pull request
Jan 22, 2026
* Feature/literature mcp (#192) * feat: literature-MCP 完整功能 * refactor: improve boolean parsing and logging in literature search functions * feat: enhance literature search functionality with improved query validation and detailed results formatting * refactor: rename oa_url to access_url in LiteratureWork model and related tests * feat: remove test-build workflow and update README for development setup * feat: tool cost system and PPTX image handling fixes (#193) * fix: prompt, factory * feat: enhanced ppt generation with image slides mode - Add image_slides mode for PPTX with full-bleed AI-generated images - Add ImageBlock.image_id field for referencing generated images - Add ImageSlideSpec for image-only slides - Add ImageFetcher service for fetching images from various sources - Reorganize knowledge module from single file to module structure - Move document utilities from app/mcp/ to app/tools/utils/documents/ - Resolve image_ids to storage URLs in async layer (operations.py) - Fix type errors and move tests to proper location Co-Authored-By: Claude <noreply@anthropic.com> * feat: implement the tool cost --------- Co-authored-by: Claude <noreply@anthropic.com> * fix: fix the first time calling knowledge tool error (#194) * fix: fix the wrong cache for second call of agent tools (#195) * feat: several improvements (#196) * fix: jump to latest topic when click agent * feat: allow more than one image for generate image * feat: allow user directly edit mcp in the chat-toolbar * feat: improve the frontend perf * feat: multiple UI improvements and fixes (#198) * fix: jump to latest topic when click agent * feat: allow more than one image for generate image * feat: allow user directly edit mcp in the chat-toolbar * feat: improve the frontend perf * fix: restore previous active topic when clicking agent Instead of always jumping to the latest topic, now tracks and restores the previously active topic for each agent when switching between them. Co-Authored-By: Claude <noreply@anthropic.com> * feat: add context menu to FocusedView agents and download button to lightbox - Add right-click context menu (edit/delete) to compact AgentListItem variant - Render context menu via portal to escape overflow:hidden containers - Add edit/delete handlers to FocusedView with AgentSettingsModal and ConfirmationModal - Add download button to image lightbox with smart filename detection Co-Authored-By: Claude <noreply@anthropic.com> * feat: add web_fetch tool bundled with web_search - Add web_fetch tool using Trafilatura for content extraction - Bundle web_fetch with web_search in frontend toolConfig - Group WEB_SEARCH_TOOLS for unified toggle behavior - Only load web_fetch when web_search is available (SearXNG enabled) - Update tool capabilities mapping for web_fetch Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Meng Junxing <junxingmeng@gmail.com> Co-authored-by: Harvey <q-query@outlook.com> Co-authored-by: Claude <noreply@anthropic.com>
Mile-Away
added a commit
that referenced
this pull request
Jan 25, 2026
* Feature/literature mcp (#192) * feat: literature-MCP 完整功能 * refactor: improve boolean parsing and logging in literature search functions * feat: enhance literature search functionality with improved query validation and detailed results formatting * refactor: rename oa_url to access_url in LiteratureWork model and related tests * feat: remove test-build workflow and update README for development setup * feat: tool cost system and PPTX image handling fixes (#193) * fix: prompt, factory * feat: enhanced ppt generation with image slides mode - Add image_slides mode for PPTX with full-bleed AI-generated images - Add ImageBlock.image_id field for referencing generated images - Add ImageSlideSpec for image-only slides - Add ImageFetcher service for fetching images from various sources - Reorganize knowledge module from single file to module structure - Move document utilities from app/mcp/ to app/tools/utils/documents/ - Resolve image_ids to storage URLs in async layer (operations.py) - Fix type errors and move tests to proper location Co-Authored-By: Claude <noreply@anthropic.com> * feat: implement the tool cost --------- Co-authored-by: Claude <noreply@anthropic.com> * fix: fix the first time calling knowledge tool error (#194) * fix: fix the wrong cache for second call of agent tools (#195) * feat: several improvements (#196) * fix: jump to latest topic when click agent * feat: allow more than one image for generate image * feat: allow user directly edit mcp in the chat-toolbar * feat: improve the frontend perf * feat: multiple UI improvements and fixes (#198) * fix: jump to latest topic when click agent * feat: allow more than one image for generate image * feat: allow user directly edit mcp in the chat-toolbar * feat: improve the frontend perf * fix: restore previous active topic when clicking agent Instead of always jumping to the latest topic, now tracks and restores the previously active topic for each agent when switching between them. Co-Authored-By: Claude <noreply@anthropic.com> * feat: add context menu to FocusedView agents and download button to lightbox - Add right-click context menu (edit/delete) to compact AgentListItem variant - Render context menu via portal to escape overflow:hidden containers - Add edit/delete handlers to FocusedView with AgentSettingsModal and ConfirmationModal - Add download button to image lightbox with smart filename detection Co-Authored-By: Claude <noreply@anthropic.com> * feat: add web_fetch tool bundled with web_search - Add web_fetch tool using Trafilatura for content extraction - Bundle web_fetch with web_search in frontend toolConfig - Group WEB_SEARCH_TOOLS for unified toggle behavior - Only load web_fetch when web_search is available (SearXNG enabled) - Update tool capabilities mapping for web_fetch Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * feat: fix the fork issue and implement the locked fork --------- Co-authored-by: Meng Junxing <junxingmeng@gmail.com> Co-authored-by: Harvey <q-query@outlook.com> Co-authored-by: Claude <noreply@anthropic.com>
Mile-Away
added a commit
that referenced
this pull request
Jan 26, 2026
* Feature/better agent community (#200) * Feature/literature mcp (#192) * feat: literature-MCP 完整功能 * refactor: improve boolean parsing and logging in literature search functions * feat: enhance literature search functionality with improved query validation and detailed results formatting * refactor: rename oa_url to access_url in LiteratureWork model and related tests * feat: remove test-build workflow and update README for development setup * feat: tool cost system and PPTX image handling fixes (#193) * fix: prompt, factory * feat: enhanced ppt generation with image slides mode - Add image_slides mode for PPTX with full-bleed AI-generated images - Add ImageBlock.image_id field for referencing generated images - Add ImageSlideSpec for image-only slides - Add ImageFetcher service for fetching images from various sources - Reorganize knowledge module from single file to module structure - Move document utilities from app/mcp/ to app/tools/utils/documents/ - Resolve image_ids to storage URLs in async layer (operations.py) - Fix type errors and move tests to proper location Co-Authored-By: Claude <noreply@anthropic.com> * feat: implement the tool cost --------- Co-authored-by: Claude <noreply@anthropic.com> * fix: fix the first time calling knowledge tool error (#194) * fix: fix the wrong cache for second call of agent tools (#195) * feat: several improvements (#196) * fix: jump to latest topic when click agent * feat: allow more than one image for generate image * feat: allow user directly edit mcp in the chat-toolbar * feat: improve the frontend perf * feat: multiple UI improvements and fixes (#198) * fix: jump to latest topic when click agent * feat: allow more than one image for generate image * feat: allow user directly edit mcp in the chat-toolbar * feat: improve the frontend perf * fix: restore previous active topic when clicking agent Instead of always jumping to the latest topic, now tracks and restores the previously active topic for each agent when switching between them. Co-Authored-By: Claude <noreply@anthropic.com> * feat: add context menu to FocusedView agents and download button to lightbox - Add right-click context menu (edit/delete) to compact AgentListItem variant - Render context menu via portal to escape overflow:hidden containers - Add edit/delete handlers to FocusedView with AgentSettingsModal and ConfirmationModal - Add download button to image lightbox with smart filename detection Co-Authored-By: Claude <noreply@anthropic.com> * feat: add web_fetch tool bundled with web_search - Add web_fetch tool using Trafilatura for content extraction - Bundle web_fetch with web_search in frontend toolConfig - Group WEB_SEARCH_TOOLS for unified toggle behavior - Only load web_fetch when web_search is available (SearXNG enabled) - Update tool capabilities mapping for web_fetch Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * feat: fix the fork issue and implement the locked fork --------- Co-authored-by: Meng Junxing <junxingmeng@gmail.com> Co-authored-by: Harvey <q-query@outlook.com> Co-authored-by: Claude <noreply@anthropic.com> * fix: prevent forked agents from being republished to marketplace (#201) * fix: prevent forked agents from being republished to marketplace Forked agents were able to be republished, which could expose the original agent's configuration. This fix adds validation at both API and UI levels: - Backend: Add validation in publish endpoint to reject agents with original_source_id set (HTTP 400) - Frontend: Hide publish button for forked agents in AgentSettingsModal and WorkflowEditor - Types: Add original_source_id and source_version fields to Agent interface Co-Authored-By: Claude <noreply@anthropic.com> * refactor: address code review feedback for fork detection - Extract `isForked` helper variable to avoid duplication - Use explicit nullish check (`!= null`) to match backend `is not None` semantic - Replace implicit empty div spacer with dynamic justify-* class in WorkflowEditor Co-Authored-By: Claude <noreply@anthropic.com> * feat: add justfile for better command * feat: improve AGENTS.md and fix backend fix --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: xinquiry(SII) <100398322+xinquiry@users.noreply.github.com> Co-authored-by: Meng Junxing <junxingmeng@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更内容
简要描述本次 PR 的主要变更内容。
相关 Issue
请关联相关 Issue(如有):#编号
检查清单
默认已勾选,如不满足,请检查。
其他说明
如有特殊说明或注意事项,请补充。
Summary by Sourcery
增强:
Original summary in English
Summary by Sourcery
Enhancements:
Original summary in English
Summary by Sourcery
增强:
Original summary in English
Summary by Sourcery
Enhancements: