mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-10 13:00:38 +00:00
17 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
spud
|
eadc1e230e
|
feat(mcp): reuse unchanged per-server tool discovery results (#6795)
* feat(mcp): add per-server discovery cache scaffolding Introduce the per-server tool cache entry, canonical parsed-config snapshots on the applied revision, and the shared _binding_matches reuse predicate. Identity reuse is object-identity based on the pool's current binding (no value equality); snapshots and ownership handles stay out of repr. No selection, eviction or publication is wired yet. * feat(mcp): selectively evict stale per-server tool cache entries Evict per-server discovery results during selective reconciliation, after the new binding epoch is installed, so unchanged servers keep their exact entry objects while changed, removed or re-bound servers (including lifecycle-token rotations) are dropped. Delegate the binding verdict to _binding_matches so reuse and eviction cannot diverge. Clear _server_tool_cache on every full reset, and count retained entries in the reconciliation state predicates so a lingering entry can never let a later config change be silently skipped. * feat(mcp): reuse unchanged per-server results during initialization Initialize MCP tools from per-server discovery instead of a single full pass: select reusable entries under the claim lock, discover only the missing servers with get_mcp_tools_by_server, then publish atomically after re-validating every candidate binding against the pool's current binding via _binding_matches. Failed servers stay absent (never cached as empty), and superseded or partially bound candidates are discarded whole. Test fixtures now drive the grouped entry point. * fix(mcp): close T4 review gaps in per-server cache publication Address four findings from the adversarial review: treat retained entries as unverifiable when the applied baseline is None so an interceptor change still forces a full reset; count retained entries in the claim-time shared-reset predicate; refuse to publish a false-success empty cache when every enabled server failed discovery; and hoist the lazy imports inside the initializer's try/finally so an import failure releases the initialization claim. * fix(mcp): sanitize cache failure logging and document per-server cache The outer lazy-initialization handler now logs only the exception type, never the raw message or traceback, so a discovery error cannot print a resolved $VAR credential. The committed-writer planner mirrors the reader's no-baseline guard so retained entries are never reused under an unknown interceptor chain. Document the per-server tool cache (reuse, invalidation, failure handling, remote tool changes) in MCP_SERVER.md and mcp/AGENTS.md, and drop phase-staging wording from the new code and tests. * docs(mcp): keep the agent-guidance chain under its hard limit F2's cache notes pushed the effective AGENTS.md instruction chain 905 bytes past its 98304-byte hard limit, failing the agent-guidance CI job. Compress the selective-reconciliation bullet to its invariants (the full cache behaviour lives in docs/MCP_SERVER.md), and reword the total-failure comment so it no longer implies that build-invalid connections were attempted. * fix(mcp): fence cached tools by interceptor chain --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
spud
|
75ce6f4920
|
fix(mcp): preserve server binding history across gateway workers (#6617)
* fix(mcp): preserve server binding history across gateway workers A peer Gateway worker could keep a stale pooled session after another worker deleted and identically re-added a stdio server, because the final effective MCP slice matched the revision it had already applied. Record a per-server lifecycle generation in the shared extensions config and consume it in the reader, discovery claim, and committed handoff so an observed generation change force-rebinds only the affected server. Legacy configs derive a stable in-memory token; deletions leave permanent tombstones. * fix(mcp): fail closed on an invalid mcpLifecycle instead of retrying An unverifiable mcpLifecycle made initialize_mcp_tools() unpublishable, so get_cached_mcp_tools() retried discovery forever and could keep respawning stdio servers. Abort discovery on an invalid ledger after conservatively retiring local MCP state; a fixed config recovers on the next call. Also documents the rolling-upgrade prerequisite: cross-worker ABA detection requires configuration writes to go through a lifecycle-aware writer. * refactor(mcp): share stdio fingerprinting across lifecycle paths Reuse lifecycle._stdio_fingerprints from the applied-revision builder so the connection-identity rule has a single implementation, and move the unusable-server diagnostic into the shared helper. Add a regression proving the Gateway Skills writer preserves mcpLifecycle byte-for-value, and document that DeerFlowClient.update_mcp_config can raise RuntimeError after the config was committed. * docs(mcp): condense lifecycle guidance under the AGENTS chain limit The effective mcp/AGENTS.md chain was 15 bytes under the 98,304-byte CI hard limit, so the standalone cross-worker lifecycle bullet pushed it over. Fold the mcpLifecycle and rolling-upgrade facts into the existing selective-reconciliation bullet and condense it, keeping the chain under the limit while preserving the contract. * docs: trim inherited agent guidance below CI size limit --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
spud
|
cd15687284
|
fix(mcp): reconcile committed Gateway config mutations (#6567)
* fix(mcp): reconcile committed Gateway config mutations Install selective MCP binding fences inside the Gateway config commit boundary and finish teardown after releasing config locks. Reject frozen durable-task server changes before persistence. Preserve drained cancellation semantics and cover delete/re-add, durable-only bindings, multi-server diffs, shared writers, and post-write reconciliation failure. * test(mcp): pin committed handoff boundaries Prove the fallback fence is installed before releasing the config lock, a committed writer cannot swallow an earlier shared reset, delete/re-add advances the applied baseline and cache generation, and retries after a post-write 500 have explicit conflict or missing semantics. * fix(mcp): fence committed config after reload failure Treat a post-write reload failure as a committed handoff failure so local MCP state is detached before releasing the config lock and teardown remains outside it. * fix(mcp): always finish committed pool teardown Run finish_mcp_reconciliation from a post-lock try/finally so response or logging failures cannot strand a retired full-reset pool. * test(mcp): fence cold-start binding race Pin the in-flight discovery path that keeps the committed fast path from admitting a late old-fingerprint binding. * docs(mcp): document committed teardown guarantee * fix(mcp): sanitize committed response errors Keep post-write response conversion failures behind a fixed HTTP detail while preserving the retired-pool teardown guarantee. * fix(mcp): avoid duplicate committed reconciliation error logs --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
spud
|
db1e0e10b4
|
fix(mcp): selectively reconcile changed deployment servers (#6487)
* fix(mcp): selectively reconcile changed deployment servers * fix(mcp): validate the frozen task snapshot before installing epochs Durable background calls resolve their connection from the configuration frozen at Gateway startup, so installing a newer revision's binding epoch fenced every status/cancel call with `StaleMCPBindingError` — even though hot reload of that server is correctly rejected. The rejection only landed in `get_mcp_tools()`, which runs after the cache had already installed the epoch. Both decisions that install an epoch now consult the frozen snapshot first. The reconciliation planner falls back to the conservative full reset, where callers that still run against the startup configuration rebind in the replacement pool, and never installs the rejected epoch. A discovery claim raises the same `McpTaskConfigurationError` that `get_mcp_tools()` would, before touching the pool and without recording the revision as applied. Unrelated non-task servers still reconcile selectively. Adds the three requested regressions — a rejected hot change keeps background task callers working, a rejected revision installs neither epoch nor tombstone (changed connection and disabled server), and an unrelated server change stays selective — plus mutation proofs for both new gates. * fix(mcp): always tear down a pool the claim path retired `reset_session_pool()` only fences and unlinks the singleton; the owners are signalled by `close_all_sync()`. The claim ran that teardown *after* its critical section, so a claim that retired the pool for a new shared-reset generation and then raised — a revision the frozen durable-task snapshot rejects is the reachable case — left the retired pool's sessions running without ever receiving a close signal. Wrap the claim in `try`/`finally` so the retired pool is torn down on every path, still outside every lock and after the lock is released. Adds a regression covering the shared-reset-then-rejected-revision sequence: the error propagates, the retired pool's owner observes exactly one `__aexit__`, the replacement pool holds no rejected epoch, and restoring the startup configuration initializes normally again. * fix(mcp): deliver retired-pool cleanup before the cache reader waits A synchronous `get_cached_mcp_tools()` reader can apply a full reconciliation while a discovery is in flight — an interceptor change and a new shared-reset generation both qualify, because the applied baseline exists before any cache is published. `reset_session_pool()` only fences and unlinks the pool, and the reader then entered its wait/retry branch from inside the condition, skipping the `close_all_sync()` that signals the retired pool's owners. The replacement cache published normally while the discarded pool kept a live owner and stdio subprocess that no later reset could reach. Hoist the observed waiting generation out of the critical section and deliver the retirement before every wait/retry path, then re-acquire the condition and wait on that exact generation so the predicate cannot miss a wakeup. Cleanup stays outside `_init_condition`, and the early `return _mcp_tools_cache` cannot skip a retirement because applying a plan always clears the published cache. Adds both triggers as a parametrised regression asserting the retired pool's owner observes exactly one `__aexit__` and the registry drains — not merely that the pool is fenced — plus a mutation proof for the wait-path cleanup. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Nan Gao
|
4bab280250
|
fix(skills): share skill cache invalidation across Gateway replicas (#6495)
* fix(skills): share skill cache invalidation across Gateway replicas SkillStorage.load_skills() rescans disk and re-reads extensions_config.json on every call, but the prompt layer in deerflow/agents/lead_agent/prompt.py caches the enabled-skills list, the per-(app_config, user_id) entries and the rendered <skill_system> section per process, and every skill mutation in routers/skills.py only refreshed the process that handled it. With several uvicorn workers or several Pods sharing one home volume, a skill installed, edited, deleted, rolled back or toggled through replica A kept being offered as the old skill set by replica B until B restarted, and POST /api/skills/reload had to be sent to every Pod separately. Reuse the MCP cache-reset mechanism. A new generic helper, deerflow.config.shared_reset_marker, owns a hidden JSON marker beside the resolved extensions config (.<config name>.<suffix>.json): publish() replaces it atomically with a random generation under extensions_config_write_lock + extensions_config_file_lock, chaining previous_generation and an optional user_id; SharedResetMarkerTracker polls it at most once per second (monotonic clock, never blocking behind a concurrent poller), adopts the current state silently on its first poll, and classifies a change as user-scoped only when exactly one publication carrying that user happened since the last poll. deerflow/mcp/cache.py now builds its .<config>.mcp-cache-reset.json marker through the same helper with no behavior change. prompt.py publishes .<config>.skills-cache-reset.json via publish_skills_cache_reset(user_id=...) and checks the marker on every cache lookup (global cache, per-config cache, prompt-section LRU, user scoped entries), calling _invalidate_enabled_skills_cache() or invalidate_user_skill_cache(). The publishing process remembers its own generation so it does not retire the caches it just rebuilt. Every mutation tail in routers/skills.py (install/upload, edit, delete, rollback, enable/disable, reload) refreshes locally and then publishes the marker off the event loop; /skills/reload answers scope=shared_config, or scope=process when no extensions config path resolves, and a marker publication failure is a 500 so a 200 never means only one process was refreshed. Docs: README, backend/docs/API.md, skills/config/mcp/gateway AGENTS.md, and frontend skills.mdx (en/zh; also fixes the reference to a non-existent skills/loader.py) describe the shared invalidation and drop the per-Pod guidance. CHANGELOG entry under Unreleased / Fixed. * docs(changelog): link the skills cache reset entry to #6495 * fix(skills): drain the reload tail and keep marker failures out of 404 Two review follow-ups on the shared skills cache reset (#6495). POST /skills/reload awaited the local refresh and the marker publication unshielded. CancelledError is a BaseException, so a caller that disconnected between the two slipped past the `except Exception -> 500` mapping, surfaced no error, and left every peer replica serving the old skill set -- the one outcome the operator hook for external mount writes exists to prevent. The refresh and the publication now run as one `await_drained(_drain_skill_mutation("reload", ...))` tail, the same discipline the install/edit/delete/rollback/toggle tails already use; the exception -> 500 mapping and the scope response are unchanged. Publication errors could surface as 404. resolve_shared_config_path() only swallows the resolution-time FileNotFoundError; a config directory that vanishes between resolution and the lock file / atomic replace still raised FileNotFoundError from the marker write, and the install, edit, delete and rollback handlers mapped that to HTTP 404 with the raw OS error text for a skill that had in fact been applied. publish_skills_cache_reset() now re-raises OSError (and a misconfigured project root's ValueError) as SkillCacheResetPublishError, a RuntimeError that no handler maps to 404/400, so every handler reports it through its generic 500 branch; the message names only the exception type, never a server path. The install handler gains an explicit clause that pins that contract beside its 404 mapping. Tests: cancelling reload after the refresh still publishes the marker; a FileNotFoundError from the marker write yields 500 (not 404) for install, edit, delete and rollback, 500 for the public toggle and the generic reload error; the wrapper type is not an OSError. |
||
|
spud
|
b58aea3101
|
fix(mcp): fence superseded stdio server bindings (#6435)
* test(mcp): pin stdio binding fencing * fix(mcp): fence superseded stdio server bindings * docs(mcp): document stdio binding fence * fix(mcp): address binding fencing review * fix(mcp): bind discovery to cache-owned pool |
||
|
PeaceMaker-best
|
90d466c917
|
feat(mcp): propagate cache resets across workers (#6126)
* feat(mcp): propagate cache resets across workers Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> * docs: keep MCP reset guidance within instruction budgets --------- Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hyeonsang Cho
|
174c6f7fd3
|
fix(mcp): run lazy MCP discovery once when initialization itself raises (#5836)
* fix(mcp): run lazy MCP discovery once when initialization itself raises get_cached_mcp_tools() wrapped the whole lazy-initialization call in the `except RuntimeError` that was only meant to catch asyncio.get_event_loop() raising when no loop is set. run_until_complete() and Future.result() re-raise the coroutine's own exception, so a RuntimeError from discovery — e.g. the deliberately re-raised McpTaskConfigurationError — took the same fallback and ran asyncio.run(initialize_mcp_tools()) a second time, respawning every stdio server and re-fetching OAuth tokens before giving up. Inside a running loop that fallback fails with "asyncio.run() cannot be called from a running event loop", so the logged traceback hid the real cause. Resolve the loop first, in its own try/except, and drive initialization exactly once: asyncio.run when no loop is set or the current loop is closed, a worker thread when the loop is running, run_until_complete otherwise. Any failure is logged once and returns [] as before. Regression tests pin one discovery call and the real logged cause with and without a running loop, and keep both fallbacks (no loop set, closed loop) covered. * test(mcp): surface worker-thread failures in the no-loop fallback test Thread.join() does not propagate exceptions, so a regression of the asyncio.run fallback failed this test with a bare KeyError: 'result' instead of the real traceback. Capture sys.exc_info() in the worker and re-raise it on the main thread before the assertions. |
||
|
spud
|
8204dcbb20
|
fix(mcp): invalidate cache only for effective MCP config changes (#5750)
* fix(mcp): avoid cache resets for unrelated config changes extensions_config.json carries MCP servers, skills and middleware settings, but the MCP cache treated any whole-file signature change as MCP state change, so toggling a skill retired every cached tool and closed all pooled sessions. Record a snapshot of the effective MCP slice (enabled servers' parsed config plus mcpInterceptors) and retire MCP state only when that slice changes. Hand the exact config revision to tool discovery so a change during discovery cannot publish tools built from a superseded revision, and add a non-initializing refresh entry point so disabling the last enabled server still converges the cache and pool on the next tool assembly. * docs(mcp): align cache guide and restore error diagnostics Review feedback on #5750 flagged three non-blocking issues: the MCP module guide still described the pre-semantic-snapshot invalidation contract; the tools.py sanitization covered every exception in the MCP block, discarding diagnostics for failures that cannot carry resolved credentials; and _is_cache_stale()'s docstring presented it as a pure predicate despite adopting a new file signature. Narrow the type-only logging to ExtensionsConfig.from_file() and restore logger.exception() for tool caching, pool cleanup and plugin filtering, with a regression test asserting the message and traceback survive. Document the effective-MCP-slice comparison, the stable config handoff to discovery, refresh_mcp_cache_if_active() and the _initialized_without_config handling in the module guide, and note the signature-adoption side effect in the docstring. * fix(mcp): unify config normalization and verify signatures Review follow-up on #5750. - Normalize mcpInterceptors (bare string == single-element list; missing key == empty list) and the type/transport alias through one shared module used by the tools cache, the interceptor builder and the durable-task config snapshot, so equivalent spellings no longer retire tools/sessions or raise McpTaskConfigurationError. - Treat a resolved-path switch as a signal to re-read the effective MCP slice instead of an invalidation reason: an identical slice adopts the new path and signature and keeps pooled sessions. - Refuse to publish or adopt a signature whose sha256 digest is missing ((mtime, size, None) from a file that could not be read). - Sync the English and Chinese MCP user docs with the effective-config comparison, and document that the Gateway admin reset stays unconditional. * docs(mcp): note the unverifiable-signature fail-closed branch A present config file whose content cannot be hashed is treated as stale, unlike a deleted file which keeps the last-known-good cache. Record that asymmetry in the module guide. * docs(mcp): scope the no-hash rationale to never-initialized processes The cost-free early return in refresh_mcp_cache_if_active() only covers a process that never published an MCP cache. A process that did publish one (including an empty cache from the zero-server or LangGraph Studio lazy-init path) still pays one stat+sha256 per tool assembly. Correct the rationale in the docstring and the matching comment in get_available_tools(). |
||
|
goloisme
|
683d146a30
|
fix(mcp): MCP cache re-initialization broken by cross-loop asyncio.Lock (#5062)
* 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> |
||
|
Daoyuan Li
|
3ed2e1f1d9
|
fix(config): close out #4124 review follow-ups (shared signature helper, resolve_config_path None contract) (#4275)
* fix(config): close out #4124 review follow-ups Extracts the (mtime, size, sha256) content-signature helper that was duplicated between config/app_config.py and mcp/cache.py into a new config/file_signature.py, and fixes ExtensionsConfig.resolve_config_path() to return None instead of raising FileNotFoundError when an explicit config_path argument or DEER_FLOW_EXTENSIONS_CONFIG_PATH points at a file that has since been deleted -- the exact resolution mode Docker dev/prod uses per AGENTS.md, so the MCP tools-cache staleness check could raise instead of degrading to "not stale". Both were flagged by willem-bd in review on #4124 and explicitly deferred there ("flagging for visibility", "leaving the extraction as the follow-up you suggested"). * fix(config): keep explicit extensions-config paths fail-loud resolve_config_path() previously turned every missing-file case into a clean None, including an explicit config_path argument or DEER_FLOW_EXTENSIONS_CONFIG_PATH (the exact mode Docker dev/prod uses). That silently downgrades a bad Docker mount, typo, or deleted production config to "no extensions" instead of surfacing the misconfiguration, per fancyboi999's review [P1] and willem-bd's follow-up notes on this PR. Restores FileNotFoundError for the two explicit modes (config_path argument, DEER_FLOW_EXTENSIONS_CONFIG_PATH); only the fallback search mode (no explicit path/env var, nothing found in the usual locations) still returns None, since that is the legitimate "extensions were never configured" case. The one caller that needs the old fail-soft behavior -- the MCP tools-cache staleness check, which re-resolves the path on every get_cached_mcp_tools() call -- gets a narrow, local catch instead (deerflow.mcp.cache._resolve_config_path) so a config file going missing mid-run still degrades the cache to "not stale" rather than crashing a hot per-request path. Also hoists the double os.getenv() read in the env-var branch into a local, per willem-bd's nit. Adds resolver-level tests for both explicit-path and env-var raises, a dedicated search-mode None regression test, and updates the existing MCP-cache docstrings to describe the corrected split. |
||
|
Daoyuan Li
|
cdefd4a8aa
|
fix(mcp): invalidate tools cache on config content + path, not just newer mtime (#4124)
* fix(mcp): invalidate tools cache on config content + path, not just newer mtime The MCP tools cache invalidated only on a strict extensions-config mtime `>` comparison and tracked no resolved config path, so `_is_cache_stale()` missed: - content changes with an unchanged mtime (same-second edits; object-store / network mounts that do not bump mtime); - content changes with a backward mtime (git checkout, cp -p / backup restore, tar / rsync preserving timestamps); - a resolved-path switch to a different config file with mtime <= the recorded value (structurally invisible — no path was tracked at all). On multi-worker (uvicorn/gunicorn) or stale-mtime deployments this leaves the LangGraph-embedded runtime and every non-writer worker serving stale MCP tools after `PUT /api/mcp/config`, breaking the module's documented promise that changes made through the Gateway API are reflected in the embedded runtime. Record the resolved config path and a `(mtime, size, sha256)` content signature at initialization and invalidate when the path OR the signature differs (`!=`), mirroring `config/app_config.py::get_app_config()` so the two runtime-editable config files share one content-based staleness signal. The per-call stat was already paid, and the small-file sha256 matches the cost app_config pays per request. The "config missing / not yet initialized" no-op behavior and the cache reset endpoint are preserved. Adds backend/tests/test_mcp_cache.py covering all three failure modes plus unchanged-file and forward-edit sanity cases. Also folds in an incidental backend/AGENTS.md doc-sync: the restart-required field list was missing `scheduler` and `run_ownership`, both present in reload_boundary.py::STARTUP_ONLY_FIELDS. * test(mcp): pin cache staleness contracts raised in review Two review observations on the content-signature cache fix, both raised as non-blocking design questions rather than bugs: - Whether relying on mtime+size alone (skipping the sha256) could ever be "optimized" back in, reopening the narrow same-second / identical-length swap gap the signature was built to close. - Whether the extensions config being deleted entirely after a successful init leaves the cache in a defined state, since current_signature flips to None and _is_cache_stale() returns False. Neither is a behavior change: both were already the intended contract, preserved verbatim from the pre-fix mtime-only code (which also returned False once the file could no longer be stat-ed). Record the reasoning inline and add regression tests that pin each contract so a future change cannot alter either silently: - test_same_mtime_same_size_swap_is_stale: a same-length server-name swap that leaves mtime AND size unchanged (the precise scenario from review, sharper than the existing same-mtime test, which also changes size) is still caught only because the sha256 is computed unconditionally. - test_config_deleted_after_init_is_not_stale: deleting the config file after init keeps the cache serving its last-known-good MCP tools instead of invalidating into an unconfigured state. Both new tests were confirmed to fail against a deliberately reintroduced version of the regression they guard (hash short-circuit / removed None-guard), then confirmed to pass against the real code. |
||
|
Ryker_Feng
|
d8b728f7cb
|
fix(mcp): close stdio sessions on their owning loop to avoid cross-task cancel-scope error (#3379) (#3392)
* 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 |
||
|
Eilen Shin
|
872079b894
|
docs: clean standalone LangGraph server remnants (#3301) | ||
|
Willem Jiang
|
c881d95898
|
fix(mcp): persist MCP sessions across tool calls for stateful servers (#3089)
* 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> |
||
|
imhaoran
|
24fe5fbd8c
|
fix(mcp): prevent RuntimeError from escaping except block in get_cach… (#2252)
* fix(mcp): prevent RuntimeError from escaping except block in get_cached_mcp_tools When `asyncio.get_event_loop()` raises RuntimeError and the fallback `asyncio.run()` also fails, the exception escapes unhandled because Python does not route exceptions raised inside an `except` block to sibling `except` clauses. Wrap the fallback call in its own try/except so failures are logged and the function returns [] as intended. * fix: use logger.exception to preserve stack traces on MCP init failure |
||
|
DanielWalnut
|
76803b826f
|
refactor: split backend into harness (deerflow.*) and app (app.*) (#1131)
* refactor: extract shared utils to break harness→app cross-layer imports Move _validate_skill_frontmatter to src/skills/validation.py and CONVERTIBLE_EXTENSIONS + convert_file_to_markdown to src/utils/file_conversion.py. This eliminates the two reverse dependencies from client.py (harness layer) into gateway/routers/ (app layer), preparing for the harness/app package split. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: split backend/src into harness (deerflow.*) and app (app.*) Physically split the monolithic backend/src/ package into two layers: - **Harness** (`packages/harness/deerflow/`): publishable agent framework package with import prefix `deerflow.*`. Contains agents, sandbox, tools, models, MCP, skills, config, and all core infrastructure. - **App** (`app/`): unpublished application code with import prefix `app.*`. Contains gateway (FastAPI REST API) and channels (IM integrations). Key changes: - Move 13 harness modules to packages/harness/deerflow/ via git mv - Move gateway + channels to app/ via git mv - Rename all imports: src.* → deerflow.* (harness) / app.* (app layer) - Set up uv workspace with deerflow-harness as workspace member - Update langgraph.json, config.example.yaml, all scripts, Docker files - Add build-system (hatchling) to harness pyproject.toml - Add PYTHONPATH=. to gateway startup commands for app.* resolution - Update ruff.toml with known-first-party for import sorting - Update all documentation to reflect new directory structure Boundary rule enforced: harness code never imports from app. All 429 tests pass. Lint clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: add harness→app boundary check test and update docs Add test_harness_boundary.py that scans all Python files in packages/harness/deerflow/ and fails if any `from app.*` or `import app.*` statement is found. This enforces the architectural rule that the harness layer never depends on the app layer. Update CLAUDE.md to document the harness/app split architecture, import conventions, and the boundary enforcement test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add config versioning with auto-upgrade on startup When config.example.yaml schema changes, developers' local config.yaml files can silently become outdated. This adds a config_version field and auto-upgrade mechanism so breaking changes (like src.* → deerflow.* renames) are applied automatically before services start. - Add config_version: 1 to config.example.yaml - Add startup version check warning in AppConfig.from_file() - Add scripts/config-upgrade.sh with migration registry for value replacements - Add `make config-upgrade` target - Auto-run config-upgrade in serve.sh and start-daemon.sh before starting services - Add config error hints in service failure messages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix comments * fix: update src.* import in test_sandbox_tools_security to deerflow.* Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: handle empty config and search parent dirs for config.example.yaml Address Copilot review comments on PR #1131: - Guard against yaml.safe_load() returning None for empty config files - Search parent directories for config.example.yaml instead of only looking next to config.yaml, fixing detection in common setups Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: correct skills root path depth and config_version type coercion - loader.py: fix get_skills_root_path() to use 5 parent levels (was 3) after harness split, file lives at packages/harness/deerflow/skills/ so parent×3 resolved to backend/packages/harness/ instead of backend/ - app_config.py: coerce config_version to int() before comparison in _check_config_version() to prevent TypeError when YAML stores value as string (e.g. config_version: "1") - tests: add regression tests for both fixes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: update test imports from src.* to deerflow.*/app.* after harness refactor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |