* Fixes#5060: P1 snapshot config before loading, P2 RLock for sync path
P1: Config changes during initialization can permanently cache stale tools.
- Snapshot _config_path and _config_signature BEFORE await get_mcp_tools()
- Compare AFTER get_mcp_tools() completes using _current_config_state()
- If config changed during loading, discard stale result and retry
- Prevents publishing old tools with new signature, which would make
_is_cache_stale() permanently return False
P2: Module-level asyncio.Lock still fails across event loops after real contention.
- _init_lock = threading.RLock() for sync path (reentrant, prevents races)
- _async_init_lock = asyncio.Lock() for async init serialization
- reset_mcp_tools_cache() now acquires _init_lock for serialization
Also fixes test P3: removed duplicated test bodies that leaked state between tests.
* fix(mcp): make cache initialization cross-loop safe
- Replace the module-level asyncio.Lock with thread-safe generation claiming
- Snapshot config state before/after MCP loading and discard stale results
- Keep reset state changes short and non-blocking for async endpoints
- Add regression coverage for contended cross-loop init, config rewrites during load, and reset while init is in flight
* fix: release MCP init claim on cancellation
Release the in-flight generation claim from a cancellation-safe finally block so cancelling the task that owns initialization does not strand future callers. Add regression coverage for cancelling the owner and then reinitializing successfully.
* fix(mcp): retire session pool before cache reset release
Prevent a concurrent MCP cache initializer from publishing tool wrappers
bound to the session-pool singleton that reset_mcp_tools_cache() is
already retiring. Add regression coverage for that interleaving.
* fix(mcp): retire session pool on stale cache invalidation
* fix(mcp): retire pool on init discard
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(mcp): keep session owner teardown safe across cancellation paths
* fix(mcp): gate pooled-session publication on the commit inside the owner task
The owner resolved `ready` as soon as initialize() finished, but the
session only became pool property when the creator promoted it into
_entries in Phase 4. A concurrent get_session() could join the in-flight
creation and receive that session from Phase 2b while the creator was
still parked in the Phase-2 eviction teardown; cancelling the creator
then ran the Phase-2 unwind, which unconditionally shut the owner down —
closing the session underneath the joiner (#5008 review).
Move the commit into the owner task: initialize() success now pops the
in-flight record, registers the session in _entries, and resolves ready
with the session in one atomic critical section, so 'ready resolved with
a result' is exactly 'session registered and pool-owned'. Joiners can
therefore only ever receive a committed session, and both creator unwind
paths (Phase 2 and Phase 3) skip teardown when the creation already
committed, leaving the pooled session to LRU eviction / close_*. When the
record was removed before the commit (close_* or creator unwind), the
owner aborts and ready carries the same cancellation the old Phase-4
not-still-ours path raised, so joiners fail with the creation's outcome
instead of hanging or holding an unmanaged session.
test_cancelled_creator_does_not_close_session_held_by_joiner reproduces
the review's scenario deterministically (MAX_SESSIONS=1, hung LRU victim,
gated initialize, second caller receives the session, creator cancelled):
red on the previous commit, green now.
test_joiner_follows_creation_outcome_when_creator_is_cancelled pins the
joiner outcome-gating semantics as a drift guard.
* feat(mcp): add durable task runtime foundation
* fix(chart): sync embedded config version
* fix(mcp): isolate task polls during shutdown
* feat(mcp): track consecutive poll errors on mcp_tasks
poll_attempt_count grows on every claim (successful polls included), so it
cannot drive a failure backoff without misjudging normal long tasks. Add
consecutive_poll_error_count: incremented when a claim is released after a
poll error, reset to zero by any applied snapshot. The backoff/terminal
policy that consumes it lands with the first concrete driver.
* fix(mcp): harden durable task lifecycle
* feat(mcp): add ordinary durable task driver
* test(mcp): address durable task review feedback
* fix(mcp): preserve submit tool descriptions
* fix(mcp): bound remote task calls
* fix(mcp): bound persisted task payloads
* fix(mcp): preserve task tool error details
* fix(mcp): enforce durable task boundaries
* test(mcp): cover task config snapshot lifecycle
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(mcp): synchronize session pool singleton lifecycle
Parity follow-up to #3778 (skill storage) and #3730 (sandbox provider).
get_session_pool() already serialised creation with _pool_lock, but its
fast-path check and final return read the global separately, and
reset_session_pool() cleared it with no lock. A reset_mcp_tools_cache()
(reachable via the /api/mcp/cache/reset admin endpoint) racing a
concurrent get could null the singleton between the None-check and the
return, handing the caller None despite the -> MCPSessionPool annotation.
Build and return the pool inside _pool_lock with a double check, and
clear it under the same lock in reset_session_pool(). The critical
section is tiny and never awaits, so holding the threading.Lock is safe
from both the async and sync/worker-thread paths. No behavior change for
single-threaded callers.
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* Apply suggestions from code review
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(mcp): close stdio sessions on their owning loop to avoid cross-task cancel-scope error (#3379)
Adopt an owner-task lifecycle for pooled MCP ClientSessions so each
session is entered, initialized, and exited within a single asyncio task
on its owning event loop. This eliminates the anyio "Attempted to exit
cancel scope in a different task than it was entered in" RuntimeError
that surfaced when stdio MCP tools were used via the sync tool wrapper
(which spins up and tears down event loops across tasks).
Also harden the pool lifecycle:
- track in-flight session creation per (server, scope) to dedupe
concurrent get_session() calls for the same key
- make close_scope/close_server/close_all/close_all_sync cover both
established entries and in-flight creations so sessions cannot be
resurrected or leaked after close
- handle cross-loop preemption of an in-flight creation by cancelling
the stale owner task instead of only signalling it
- define close_all_sync() semantics for a running loop on the current
thread (signal-only, async completion) and route reset_mcp_tools_cache
through a deterministic async close in that case
* fix(mcp): avoid reset deadlock on running loop cache reset
* fix(mcp): address session pool review feedback
* fix(mcp): persist MCP sessions across tool calls for stateful servers
MCP tools loaded via langchain-mcp-adapters created a new session on
every call, causing stateful servers like Playwright to lose browser
state (pages, forms) between consecutive tool invocations within the
same thread.
Add MCPSessionPool that maintains persistent sessions scoped by
(server_name, thread_id). Tool calls within the same thread now reuse
the same MCP session, preserving server-side state. Sessions are evicted
in LRU order (max 256) and cleaned up on cache invalidation.
Fixes#3054
* fix(sandbox): add group/other read permissions to uploaded files for Docker sandbox (#3127)
When using AIO sandbox with LocalContainerBackend, uploaded files are
created with 0o600 (owner-only) permissions by the gateway process
running as root. The sandbox process inside the Docker container runs
as a non-root user and cannot read these bind-mounted files, causing
a "Permission denied" error on read_file.
Add `needs_upload_permission_adjustment` attribute to SandboxProvider
(default True) to indicate that uploaded files need chmod adjustment.
LocalSandboxProvider opts out (same user). A new `_make_file_sandbox_readable`
function adds S_IRGRP | S_IROTH bits after files are written, changing
permissions from 0o600 to 0o644 so the sandbox can read the uploads.
* fix(mcp): address review comments on session pool and tools
- _extract_thread_id: return "default" instead of stringifying None
when get_config() returns no thread_id
- call_with_persistent_session: fix **arguments annotation from
dict[str,Any] to Any
- Replace private _convert_call_tool_result import with a local
implementation that handles all MCP content block types
- _make_session_pool_tool: accept tool_interceptors and apply the
configured interceptor chain on every call (preserving OAuth and
custom interceptors)
- MCPSessionPool: replace asyncio.Lock with threading.Lock; restructure
get/close methods to never await while holding the lock; add
close_all_sync() that closes sessions on their owning event loops
- reset_mcp_tools_cache: use pool.close_all_sync() instead of
asyncio.run-in-thread to close sessions deterministically
- test: add test_session_pool_tool_sync_wrapper_path_is_safe covering
tool invocation via the sync wrapper (tool.func) path
Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/9e7f9e7f-1d2b-464a-b3b7-7f1649b74122
Co-authored-by: WillemJiang <219644+WillemJiang@users.noreply.github.com>
* fix(mcp): extract SESSION_CLOSE_TIMEOUT to class constant
Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/9e7f9e7f-1d2b-464a-b3b7-7f1649b74122
Co-authored-by: WillemJiang <219644+WillemJiang@users.noreply.github.com>
* Potential fix for pull request finding 'Empty except'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
