From a5acc25de6742b2166b3f41c97bd895822277b94 Mon Sep 17 00:00:00 2001 From: Nan Gao Date: Thu, 20 Aug 2026 08:57:10 +0800 Subject: [PATCH] fix(mcp): exclude internal temp files from workspace changes (#4898) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): exclude internal temp files from workspace changes * fix(mcp): address review — shared tmp subdir constant, any-depth docs, nested test - Export MCP_TMP_SUBDIR from constants.py and import it in both stdio launch paths (tools.py, task_tool_caller.py) so the "/tmp" suffix is composed once. - Document that the .mcp exclusion matches by directory name at any depth (like .git/node_modules) in README.md and mcp/AGENTS.md — subagent work dirs below the workspace root get their own .mcp/tmp. - Pin the any-depth semantic in test_workspace_changes.py with a nested workspace/project/.mcp assertion. * docs(mcp): correct any-depth exclusion rationale; re-home tmp pinning comment The previous commit justified the any-depth `.mcp` exclusion with subagent work dirs sitting below the workspace root — a mechanism that doesn't exist: subagents share the parent's thread_id and both stdio launch paths resolve sandbox_work_dir(thread_id), so `.mcp/tmp` is always pinned at the workspace root. Reword mcp/AGENTS.md and the test comment to the real justification (consistency with the other reserved dir names, robustness against a server creating a relative `.mcp` from another cwd). Also move the orphaned "pinning the process temp dir" rationale from tools.py to constants.py next to MCP_TMP_SUBDIR, where both importers see it. --- README.md | 2 +- .../packages/harness/deerflow/constants.py | 16 ++++++++++++++ .../packages/harness/deerflow/mcp/AGENTS.md | 2 +- .../harness/deerflow/mcp/task_tool_caller.py | 5 ++--- .../packages/harness/deerflow/mcp/tools.py | 11 ++-------- .../deerflow/workspace_changes/scanner.py | 6 +++++- backend/tests/test_mcp_file_migration.py | 3 ++- backend/tests/test_mcp_session_pool.py | 15 +++++++------ backend/tests/test_workspace_changes.py | 21 +++++++++++++++++++ 9 files changed, 59 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 2e882ac2e..95213194d 100644 --- a/README.md +++ b/README.md @@ -1159,7 +1159,7 @@ The built-in `grep` tool searches either one text file or all matching text file Image bytes loaded for a vision-model call are transient: DeerFlow removes the hidden base64 message after the model consumes it so later checkpoints do not keep duplicating that payload. -After each run, DeerFlow records a workspace change summary for the run-owned `workspace` and `outputs` directories. The Web UI shows a compact "files changed" badge on the assistant turn; opening it reveals created, modified, and deleted files with text diffs when safe to display. Uploads are excluded because they are user inputs, not agent-generated changes. Large, binary, or sensitive-looking files are shown as metadata only. +After each run, DeerFlow records a workspace change summary for the run-owned `workspace` and `outputs` directories. The Web UI shows a compact "files changed" badge on the assistant turn; opening it reveals created, modified, and deleted files with text diffs when safe to display. Uploads are excluded because they are user inputs, not agent-generated changes, and stdio MCP temporary/debug files under the DeerFlow-owned `.mcp/` namespace are excluded because they are process-internal state (like `.git/` and `node_modules/`, any directory named `.mcp` is excluded at any depth). Large, binary, or sensitive-looking files are shown as metadata only. Files presented through `present_files` remain part of the thread's artifact state, and the Web UI restores the artifact panel and selected document after a page refresh. The currently selected formal artifact is refreshed once when the run finishes so edits become visible without a manual reload. Existing UTF-8 text artifacts under `/mnt/user-data/outputs` can also be edited and explicitly saved from the panel on Unix and Windows while the thread is idle; saves use content revisions to prevent overwriting agent changes. diff --git a/backend/packages/harness/deerflow/constants.py b/backend/packages/harness/deerflow/constants.py index 7ee937441..2730f6aaa 100644 --- a/backend/packages/harness/deerflow/constants.py +++ b/backend/packages/harness/deerflow/constants.py @@ -20,6 +20,22 @@ BROWSER_FRAMES_DIRNAME = ".browser-frames" # snapshot capture as an extra excluded dir name. TOOL_RESULTS_DIRNAME = ".tool-results" +# Hidden directory under a thread workspace owned by stdio MCP runtimes. The +# default subprocess temp directory lives at ``.mcp/tmp``; these files are +# process-internal state rather than workspace deliverables, so the +# workspace-changes scanner excludes the whole reserved namespace. MCP launch +# paths and the scanner share this name so writes and filtering cannot drift. +MCP_INTERNAL_DIRNAME = ".mcp" + +# Default subprocess temp subdirectory pinned into stdio MCP environments +# (``TMPDIR``/``TMP``/``TEMP``). Both stdio launch paths (persistent sessions +# and background task calls) import this instead of composing the suffix +# themselves. Pinning the process temp dir here (alongside its cwd) makes +# tools that write to ``os.tmpdir()`` / ``tempfile.gettempdir()`` land inside +# the mounted user-data tree, where their output is resolvable by the +# sandbox/artifact API — instead of on an unreachable host temp path. +MCP_TMP_SUBDIR = f"{MCP_INTERNAL_DIRNAME}/tmp" + # Default timeout (seconds) for MCP server bring-up: tool discovery (subprocess # spawn + initialize + tools/list) and persistent-session initialization. A hung # stdio server (e.g. npx blocked on a package download or a server that never diff --git a/backend/packages/harness/deerflow/mcp/AGENTS.md b/backend/packages/harness/deerflow/mcp/AGENTS.md index 87bdced79..3ada6576a 100644 --- a/backend/packages/harness/deerflow/mcp/AGENTS.md +++ b/backend/packages/harness/deerflow/mcp/AGENTS.md @@ -17,7 +17,7 @@ `tools/builtins/tool_search.py::get_mcp_routing_hints_prompt_section`, which references `tool_search` when a hinted MCP tool is currently deferred; do not add a parallel routing middleware for PR1-style preference hints. -- **Stdio file outputs**: Persistent stdio sessions are scoped by `user_id:thread_id`. For stdio transports only, DeerFlow pins the subprocess default `cwd` to the thread workspace and `TMPDIR`/`TMP`/`TEMP` to `workspace/.mcp/tmp/`, unless the operator explicitly configured `cwd` or temp env values. SSE/HTTP transports skip this filesystem prep entirely. +- **Stdio file outputs**: Persistent stdio sessions are scoped by `user_id:thread_id`. For stdio transports only, DeerFlow pins the subprocess default `cwd` to the thread workspace and `TMPDIR`/`TMP`/`TEMP` to `workspace/.mcp/tmp/`, unless the operator explicitly configured `cwd` or temp env values. `.mcp` is a DeerFlow-owned internal namespace: its temporary/debug files remain addressable when returned by a tool but are excluded from run workspace-change summaries — by directory name at any depth, consistent with the other reserved names in `EXCLUDED_DIR_NAMES` (`.git`, `node_modules`, …) and robust if a server ever creates a relative `.mcp` from a different cwd. Both launch paths pin it at the workspace root today. SSE/HTTP transports skip this filesystem prep entirely. - **Stdio path translation**: MCP-returned local file references are not copied. If a `ResourceLink` or conservative free-text path resolves to an existing file inside the thread's mounted user-data tree, it is translated deterministically to `/mnt/user-data/...`; paths outside that tree remain unchanged. - **Runtime updates**: Gateway API saves to extensions_config.json; the Gateway-embedded runtime detects changes via the resolved-path + content-signature check above, so multi-worker / stale-mtime deployments still pick up an added/removed MCP server without a restart (`PUT /api/mcp/config` keeps whole-payload validation, while `PATCH /api/mcp/config` changes only one server's `enabled` field, normalizes the same `type`/MCP-spec `transport` alias as the runtime config model, and validates the target only when enabling it; either endpoint's reset clears the cache only in its own worker). MCP, skill, and embedded-client writers share `atomic_write_extensions_config()`, which writes and fsyncs a same-directory temporary file before `os.replace()` and preserves an existing file's mode and symlink target; failed serialization or replacement leaves the prior config intact and cleans up the temporary file. - **Stdio launch policy at the HTTP boundary** (`routers/mcp.py::_validate_mcp_update_request`, shared by `PUT` and the enable branch of `PATCH`): a config file may express anything, but the API is untrusted input, so an API-registered stdio server must (a) name a bare executable from the allowlist — `_DEFAULT_MCP_STDIO_COMMAND_ALLOWLIST` = `{npx, uvx}`, extended by `DEER_FLOW_MCP_STDIO_COMMAND_ALLOWLIST`, with path separators, whitespace, and shell metacharacters rejected in `command`; (b) carry no `args` flag in `_ARBITRARY_EXEC_ARGS`; and (c) set no `env` name in `_CODE_INJECTING_ENV_VARS`. Checks (b) and (c) exist because the command check alone names a binary without constraining what that binary runs. The `env` denylist applies to **every** allowlisted command, and both denylists match `--flag=value` as well as `--flag value`. The `args` denylist's **scope depends on the command**, because where a launcher stops parsing its own flags is what decides whether a token is an exec flag at all: diff --git a/backend/packages/harness/deerflow/mcp/task_tool_caller.py b/backend/packages/harness/deerflow/mcp/task_tool_caller.py index c9581623a..8dadb0440 100644 --- a/backend/packages/harness/deerflow/mcp/task_tool_caller.py +++ b/backend/packages/harness/deerflow/mcp/task_tool_caller.py @@ -10,6 +10,7 @@ from typing import Any from deerflow.config.extensions_config import ExtensionsConfig from deerflow.config.paths import get_paths +from deerflow.constants import MCP_TMP_SUBDIR from deerflow.mcp.client import build_server_params from deerflow.mcp.interceptors import build_mcp_tool_interceptors from deerflow.mcp.oauth import OAuthTokenManager, build_oauth_tool_interceptor @@ -17,8 +18,6 @@ from deerflow.mcp.session_pool import get_session_pool logger = logging.getLogger(__name__) -_MCP_TASK_TMP_SUBDIR = ".mcp/tmp" - def mcp_task_session_scope_key(*, user_id: str, thread_id: str) -> str: """Keep background calls in the same per-user/per-thread session scope.""" @@ -34,7 +33,7 @@ def _prepare_stdio_connection( paths = get_paths() paths.ensure_thread_dirs(thread_id, user_id=user_id) work_dir = paths.sandbox_work_dir(thread_id, user_id=user_id) - tmp_dir = work_dir / _MCP_TASK_TMP_SUBDIR + tmp_dir = work_dir / MCP_TMP_SUBDIR tmp_dir.mkdir(parents=True, exist_ok=True) tmp_dir.chmod(0o700) diff --git a/backend/packages/harness/deerflow/mcp/tools.py b/backend/packages/harness/deerflow/mcp/tools.py index 6a62cda4c..9ce63caa2 100644 --- a/backend/packages/harness/deerflow/mcp/tools.py +++ b/backend/packages/harness/deerflow/mcp/tools.py @@ -16,7 +16,7 @@ from langgraph.config import get_config from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig, resolve_effective_mcp_routing from deerflow.config.paths import VIRTUAL_PATH_PREFIX, Paths, get_paths -from deerflow.constants import DEFAULT_MCP_SESSION_INIT_TIMEOUT +from deerflow.constants import DEFAULT_MCP_SESSION_INIT_TIMEOUT, MCP_TMP_SUBDIR from deerflow.mcp.client import build_servers_config from deerflow.mcp.interceptors import build_mcp_tool_interceptors from deerflow.mcp.oauth import build_oauth_tool_interceptor, get_initial_oauth_headers @@ -46,13 +46,6 @@ logger = logging.getLogger(__name__) # the load-time validation skill names get (skills/storage/skill_storage.py). _VALID_MCP_TOOL_NAME = re.compile(r"^[A-Za-z0-9_-]+$") -# Subdirectory under the thread's workspace used as the temp dir for stdio MCP -# subprocesses. Pinning the process temp dir here (alongside its cwd) makes -# tools that write to ``os.tmpdir()`` / ``tempfile.gettempdir()`` land inside -# the mounted user-data tree, where their output is resolvable by the -# sandbox/artifact API — instead of on an unreachable host temp path. -_MCP_TMP_SUBDIR = ".mcp/tmp" - # Matches local-file references embedded in free text returned by an MCP server. # Some servers (notably Playwright's ``browser_take_screenshot``) report saved # files only as text/markdown links rather than ``ResourceLink`` blocks. Those @@ -184,7 +177,7 @@ def _prepare_stdio_workspace(paths: Paths, *, thread_id: str, user_id: str) -> t """ paths.ensure_thread_dirs(thread_id, user_id=user_id) source_base_dir = paths.sandbox_work_dir(thread_id, user_id=user_id) - tmp_dir = source_base_dir / _MCP_TMP_SUBDIR + tmp_dir = source_base_dir / MCP_TMP_SUBDIR try: tmp_dir.mkdir(parents=True, exist_ok=True) tmp_dir.chmod(0o700) diff --git a/backend/packages/harness/deerflow/workspace_changes/scanner.py b/backend/packages/harness/deerflow/workspace_changes/scanner.py index 214e3b05f..50cb956f8 100644 --- a/backend/packages/harness/deerflow/workspace_changes/scanner.py +++ b/backend/packages/harness/deerflow/workspace_changes/scanner.py @@ -6,7 +6,7 @@ import os from codecs import BOM_UTF16_BE, BOM_UTF16_LE, getincrementaldecoder from pathlib import Path -from deerflow.constants import BROWSER_FRAMES_DIRNAME, TOOL_RESULTS_DIRNAME +from deerflow.constants import BROWSER_FRAMES_DIRNAME, MCP_INTERNAL_DIRNAME, TOOL_RESULTS_DIRNAME from .types import ( DiffUnavailableReason, @@ -21,6 +21,10 @@ EXCLUDED_DIR_NAMES = { ".hg", ".svn", ".cache", + # Stdio MCP subprocess temp/debug files live below this server-owned + # namespace. They remain addressable when a tool returns their path, but + # they are not user-authored workspace deliverables or workspace changes. + MCP_INTERNAL_DIRNAME, ".next", ".venv", # Transient per-step browser screenshots: live progress feedback surfaced in diff --git a/backend/tests/test_mcp_file_migration.py b/backend/tests/test_mcp_file_migration.py index c8216f804..9fe3f1c64 100644 --- a/backend/tests/test_mcp_file_migration.py +++ b/backend/tests/test_mcp_file_migration.py @@ -13,6 +13,7 @@ import pytest from mcp.types import CallToolResult, ResourceLink, TextContent from deerflow.config.paths import VIRTUAL_PATH_PREFIX, Paths +from deerflow.constants import MCP_TMP_SUBDIR from deerflow.mcp import tools as mcp_tools @@ -398,7 +399,7 @@ class TestPrepareStdioWorkspace: source_base_dir, tmp_dir, before = mcp_tools._prepare_stdio_workspace(paths, thread_id="t1", user_id="u1") assert source_base_dir == paths.sandbox_work_dir("t1", user_id="u1") - assert tmp_dir == source_base_dir / mcp_tools._MCP_TMP_SUBDIR + assert tmp_dir == source_base_dir / MCP_TMP_SUBDIR assert tmp_dir.is_dir() assert before == {existing: (existing.stat().st_mtime_ns, existing.stat().st_size)} diff --git a/backend/tests/test_mcp_session_pool.py b/backend/tests/test_mcp_session_pool.py index 892dfe923..01fa986dc 100644 --- a/backend/tests/test_mcp_session_pool.py +++ b/backend/tests/test_mcp_session_pool.py @@ -302,7 +302,8 @@ async def test_session_pool_tool_pins_cwd_and_temp_env(tmp_path): from pydantic import BaseModel, Field from deerflow.config.paths import Paths - from deerflow.mcp.tools import _MCP_TMP_SUBDIR, _make_session_pool_tool + from deerflow.constants import MCP_TMP_SUBDIR + from deerflow.mcp.tools import _make_session_pool_tool class Args(BaseModel): url: str = Field(..., description="url") @@ -336,7 +337,7 @@ async def test_session_pool_tool_pins_cwd_and_temp_env(tmp_path): session_connection = create_session.call_args.args[0] workspace = paths.sandbox_work_dir("thread-42", user_id="user-7") - tmp_dir = workspace / _MCP_TMP_SUBDIR + tmp_dir = workspace / MCP_TMP_SUBDIR assert session_connection["cwd"] == str(workspace) assert session_connection["env"]["KEEP"] == "1" @@ -354,7 +355,8 @@ async def test_session_pool_tool_does_not_override_explicit_tmpdir(tmp_path): from pydantic import BaseModel, Field from deerflow.config.paths import Paths - from deerflow.mcp.tools import _MCP_TMP_SUBDIR, _make_session_pool_tool + from deerflow.constants import MCP_TMP_SUBDIR + from deerflow.mcp.tools import _make_session_pool_tool class Args(BaseModel): url: str = Field(..., description="url") @@ -389,7 +391,7 @@ async def test_session_pool_tool_does_not_override_explicit_tmpdir(tmp_path): session_connection = create_session.call_args.args[0] # Operator-provided TMPDIR is preserved; TMP/TEMP still get our default. assert session_connection["env"]["TMPDIR"] == "/operator/tmp" - assert session_connection["env"]["TMP"].endswith(_MCP_TMP_SUBDIR) + assert session_connection["env"]["TMP"].endswith(MCP_TMP_SUBDIR) @pytest.mark.asyncio @@ -399,7 +401,8 @@ async def test_session_pool_tool_does_not_override_explicit_cwd(tmp_path): from pydantic import BaseModel, Field from deerflow.config.paths import Paths - from deerflow.mcp.tools import _MCP_TMP_SUBDIR, _make_session_pool_tool + from deerflow.constants import MCP_TMP_SUBDIR + from deerflow.mcp.tools import _make_session_pool_tool class Args(BaseModel): url: str = Field(..., description="url") @@ -434,7 +437,7 @@ async def test_session_pool_tool_does_not_override_explicit_cwd(tmp_path): session_connection = create_session.call_args.args[0] workspace = paths.sandbox_work_dir("thread-42", user_id="user-7") - tmp_dir = workspace / _MCP_TMP_SUBDIR + tmp_dir = workspace / MCP_TMP_SUBDIR assert session_connection["cwd"] == operator_cwd assert session_connection["env"]["TMPDIR"] == str(tmp_dir) diff --git a/backend/tests/test_workspace_changes.py b/backend/tests/test_workspace_changes.py index ff7a1889e..c46f20f87 100644 --- a/backend/tests/test_workspace_changes.py +++ b/backend/tests/test_workspace_changes.py @@ -247,6 +247,27 @@ def test_scan_workspace_roots_skips_excluded_directories(tmp_path): assert "/mnt/user-data/workspace/node_modules/ignored.js" not in snapshot.files +def test_scan_workspace_roots_skips_stdio_mcp_temp_files(tmp_path): + roots = _roots(tmp_path) + workspace = roots[0].host_path + (workspace / "report.md").write_text("keep", encoding="utf-8") + mcp_tmp = workspace / ".mcp" / "tmp" + mcp_tmp.mkdir(parents=True) + (mcp_tmp / "debug.json").write_text("internal", encoding="utf-8") + # `.mcp` is excluded by directory name at any depth, matching the other + # entries in EXCLUDED_DIR_NAMES and staying robust if a server ever + # creates a relative `.mcp` from a cwd below the workspace root. + nested_mcp = workspace / "project" / ".mcp" + nested_mcp.mkdir(parents=True) + (nested_mcp / "nested.json").write_text("internal", encoding="utf-8") + + snapshot = scan_workspace_roots(roots) + + assert "/mnt/user-data/workspace/report.md" in snapshot.files + assert "/mnt/user-data/workspace/.mcp/tmp/debug.json" not in snapshot.files + assert "/mnt/user-data/workspace/project/.mcp/nested.json" not in snapshot.files + + def test_scan_workspace_roots_skips_browser_frames(tmp_path): roots = _roots(tmp_path) outputs = roots[1].host_path