mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-16 09:38:41 +00:00
* fix: bound MCP server bring-up timeouts and exclude externalized tool outputs from delivery verification Two related robustness fixes: 1. MCP server bring-up was unbounded. tool_call_timeout only covered session.call_tool(); tool discovery (subprocess spawn + initialize + tools/list) and persistent stdio session initialization could hang forever, blocking agent construction (and on the Gateway event loop, the whole process). Add a per-server session_init_timeout (default DEFAULT_MCP_SESSION_INIT_TIMEOUT = 60s, null disables) that bounds both discovery and pooled-session initialization. The session pool's existing cancellation handling tears down a session stuck mid-creation in its own task. 2. ToolOutputBudgetMiddleware externalizes oversized tool outputs into outputs/.tool-results/ (configurable tool_output.storage_subdir). The workspace-change scanner and run delivery verification counted those files as produced artifacts, so any run that externalized a tool output without also presenting a real artifact failed with "Artifact delivery incomplete". Exclude TOOL_RESULTS_DIRNAME via a shared constant (mirroring BROWSER_FRAMES_DIRNAME) and thread the configured storage_subdir through snapshot capture so both workspace-changes events and delivery verification stay clean. * review: enforce single-segment tool_output.storage_subdir; document discovery-timeout cleanup Address review feedback: 1. A custom tool_output.storage_subdir with a path separator (e.g. cache/tool-results) silently no-oped the workspace-scanner exclusion: os.walk yields one-segment dirnames, so a nested value never matched and its files were counted as produced artifacts again. ToolOutputConfig now validates storage_subdir as a single directory name (rejects separators, .., absolute, empty) with tests, so the exclusion is always sound. 2. The discovery-timeout path now documents why cancellation is safe, mirroring the session-init note: discovery runs inside the adapter's nested async context managers, and stdio_client's finally terminates the process tree (SIGTERM->SIGKILL on POSIX, process-tree on Windows), so a timed-out npx subprocess and its children are reaped rather than accumulating. * review: log session-init timeouts and align API response model default with runtime config Address second-round review feedback: 1. A session-init timeout raised TimeoutError without any log, unlike the discovery timeout which logs a WARNING. Wrap the bounded get_session in a try/except that logs the timeout (server name + seconds) and re-raises, so operators can diagnose tool-call failures caused by hung MCP sessions. 2. McpServerConfigResponse.session_init_timeout defaulted to None while McpServerConfig defaults to 60s: a server created via PUT /api/mcp/config without the field was persisted with null (no timeout) while the same server created in the config file got 60s. Align the response-model default to DEFAULT_MCP_SESSION_INIT_TIMEOUT so API-created and file-created servers behave the same; an explicit null still opts out. * review: narrow the discovery-timeout handler to the bounded wait_for path The except TimeoutError clause covered both the bounded wait_for branch and the bare discovery branch. With session_init_timeout opted out (None), a TimeoutError raised by discovery itself would hit the %.1f format with None: logging raises TypeError internally, the WARNING is silently dropped, and a --- Logging error --- traceback goes to stderr. Narrow the handler to wrap only the wait_for call, where the branch condition guarantees the timeout value is not None. A discovery-internal TimeoutError on the opted-out path now falls through to the generic failure handler and is reported as 'tool discovery failed' with exc_info. Covered by a regression test that asserts the skip is reported without any broken format.
346 lines
10 KiB
Python
346 lines
10 KiB
Python
from __future__ import annotations
|
|
|
|
import fnmatch
|
|
import hashlib
|
|
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 .types import (
|
|
DiffUnavailableReason,
|
|
FileSnapshot,
|
|
WorkspaceChangeLimits,
|
|
WorkspaceRoot,
|
|
WorkspaceSnapshot,
|
|
)
|
|
|
|
EXCLUDED_DIR_NAMES = {
|
|
".git",
|
|
".hg",
|
|
".svn",
|
|
".cache",
|
|
".next",
|
|
".venv",
|
|
# Transient per-step browser screenshots: live progress feedback surfaced in
|
|
# the browser panel + inline thumbnails, not workspace deliverables. Shared
|
|
# constant with the browser tools so the name cannot drift out of sync.
|
|
BROWSER_FRAMES_DIRNAME,
|
|
# Externalized oversized tool outputs (the tool-output budget middleware's
|
|
# default storage_subdir): process feedback the model reads back via
|
|
# read_file, not workspace deliverables — same intent as the browser frames
|
|
# exclusion above. Without this, a run that externalizes any tool output
|
|
# would trip run delivery verification (produced output never presented)
|
|
# and fail as an error. Custom storage_subdir values are passed through
|
|
# ``extra_excluded_dir_names`` instead.
|
|
TOOL_RESULTS_DIRNAME,
|
|
"__pycache__",
|
|
"build",
|
|
"dist",
|
|
"node_modules",
|
|
}
|
|
|
|
BINARY_EXTENSIONS = {
|
|
".7z",
|
|
".avif",
|
|
".bmp",
|
|
".class",
|
|
".db",
|
|
".dll",
|
|
".dmg",
|
|
".doc",
|
|
".docx",
|
|
".exe",
|
|
".gif",
|
|
".gz",
|
|
".ico",
|
|
".jar",
|
|
".jpeg",
|
|
".jpg",
|
|
".mov",
|
|
".mp3",
|
|
".mp4",
|
|
".o",
|
|
".pdf",
|
|
".png",
|
|
".pyc",
|
|
".so",
|
|
".tar",
|
|
".webp",
|
|
".xls",
|
|
".xlsx",
|
|
".zip",
|
|
}
|
|
|
|
SENSITIVE_PATH_PATTERNS = (
|
|
".env",
|
|
".env.*",
|
|
"*api_key*",
|
|
"*apikey*",
|
|
"*.key",
|
|
"*.pem",
|
|
"*credential*",
|
|
"*password*",
|
|
"*private_key*",
|
|
"*secret*",
|
|
"*token*",
|
|
)
|
|
|
|
SAMPLE_BYTES = 4096
|
|
_UTF16_BOMS = (BOM_UTF16_LE, BOM_UTF16_BE)
|
|
|
|
|
|
def is_sensitive_workspace_path(path: str) -> bool:
|
|
normalized = path.lower()
|
|
parts = [part.lower() for part in Path(path).parts]
|
|
basename = parts[-1] if parts else normalized
|
|
for pattern in SENSITIVE_PATH_PATTERNS:
|
|
if fnmatch.fnmatch(basename, pattern) or fnmatch.fnmatch(normalized, pattern):
|
|
return True
|
|
if any(fnmatch.fnmatch(part, pattern) for part in parts):
|
|
return True
|
|
return False
|
|
|
|
|
|
def scan_workspace_roots(
|
|
roots: list[WorkspaceRoot],
|
|
*,
|
|
limits: WorkspaceChangeLimits | None = None,
|
|
include_text: bool = True,
|
|
text_paths: set[str] | None = None,
|
|
text_cache_dir: Path | None = None,
|
|
extra_excluded_dir_names: frozenset[str] | None = None,
|
|
) -> WorkspaceSnapshot:
|
|
resolved_limits = limits or WorkspaceChangeLimits()
|
|
cache_dir = Path(text_cache_dir) if text_cache_dir is not None else None
|
|
if cache_dir is not None:
|
|
cache_dir.mkdir(parents=True, exist_ok=True)
|
|
# Operator-customized tool_output.storage_subdir values arrive here; the
|
|
# default name is already part of EXCLUDED_DIR_NAMES, so merging is safe.
|
|
# Only single-segment directory names are meaningful: os.walk yields
|
|
# one-segment dirnames, so a nested value like "cache/tool-results" would
|
|
# never match. ToolOutputConfig enforces the single-segment contract, so a
|
|
# multi-segment value is a caller error, not a silent no-op.
|
|
excluded_dir_names = EXCLUDED_DIR_NAMES | extra_excluded_dir_names if extra_excluded_dir_names else EXCLUDED_DIR_NAMES
|
|
files: dict[str, FileSnapshot] = {}
|
|
scanned = 0
|
|
truncated = False
|
|
|
|
for root in roots:
|
|
if not root.host_path.exists():
|
|
continue
|
|
|
|
for dirpath, dirnames, filenames in os.walk(root.host_path, followlinks=False):
|
|
dirnames[:] = [dirname for dirname in dirnames if dirname not in excluded_dir_names and not (Path(dirpath) / dirname).is_symlink()]
|
|
for filename in sorted(filenames):
|
|
if scanned >= resolved_limits.max_scanned_files:
|
|
truncated = True
|
|
return WorkspaceSnapshot(
|
|
files=files,
|
|
truncated=truncated,
|
|
text_cache_dir=str(cache_dir) if cache_dir is not None else None,
|
|
)
|
|
|
|
host_file = Path(dirpath) / filename
|
|
if host_file.is_symlink():
|
|
# A symlink must never be followed for stat/content purposes: its
|
|
# target can point anywhere on the host (including outside the
|
|
# scanned root), so it is recorded as a metadata-only stub -
|
|
# mirroring how binary/large/sensitive-looking files are handled
|
|
# below - instead of being silently omitted from the snapshot.
|
|
symlink_snapshot = _snapshot_symlink(root, host_file)
|
|
if symlink_snapshot is not None:
|
|
files[symlink_snapshot.path] = symlink_snapshot
|
|
scanned += 1
|
|
continue
|
|
if not host_file.is_file():
|
|
continue
|
|
|
|
snapshot = _snapshot_file(
|
|
root,
|
|
host_file,
|
|
limits=resolved_limits,
|
|
include_text=include_text,
|
|
text_paths=text_paths,
|
|
text_cache_dir=cache_dir,
|
|
)
|
|
if snapshot is not None:
|
|
files[snapshot.path] = snapshot
|
|
scanned += 1
|
|
|
|
return WorkspaceSnapshot(
|
|
files=files,
|
|
truncated=truncated,
|
|
text_cache_dir=str(cache_dir) if cache_dir is not None else None,
|
|
)
|
|
|
|
|
|
def _snapshot_file(
|
|
root: WorkspaceRoot,
|
|
host_file: Path,
|
|
*,
|
|
limits: WorkspaceChangeLimits,
|
|
include_text: bool,
|
|
text_paths: set[str] | None,
|
|
text_cache_dir: Path | None,
|
|
) -> FileSnapshot | None:
|
|
try:
|
|
stat = host_file.stat()
|
|
size = stat.st_size
|
|
mtime_ns = stat.st_mtime_ns
|
|
relative = host_file.relative_to(root.host_path).as_posix()
|
|
virtual_path = f"{root.virtual_prefix}/{relative}"
|
|
sensitive = is_sensitive_workspace_path(virtual_path)
|
|
except OSError:
|
|
return None
|
|
|
|
if sensitive:
|
|
return FileSnapshot(
|
|
path=virtual_path,
|
|
root=root.name,
|
|
size=size,
|
|
mtime_ns=mtime_ns,
|
|
sha256=None,
|
|
binary=False,
|
|
sensitive=True,
|
|
text=None,
|
|
content_unavailable_reason="sensitive",
|
|
)
|
|
|
|
try:
|
|
sample = host_file.read_bytes()[:SAMPLE_BYTES] if size <= SAMPLE_BYTES else _read_sample(host_file)
|
|
except OSError:
|
|
return None
|
|
|
|
binary = host_file.suffix.lower() in BINARY_EXTENSIONS or _looks_binary(sample)
|
|
sha256 = _sha256_file(host_file) if size <= limits.max_file_bytes_for_diff else None
|
|
text: str | None = None
|
|
text_path: str | None = None
|
|
reason: DiffUnavailableReason | None = None
|
|
|
|
should_include_text = include_text and (text_paths is None or virtual_path in text_paths)
|
|
|
|
if binary:
|
|
reason = "binary"
|
|
elif size > limits.max_file_bytes_for_diff:
|
|
reason = "large"
|
|
elif not should_include_text:
|
|
text = None
|
|
else:
|
|
try:
|
|
raw = host_file.read_bytes()
|
|
except OSError:
|
|
return None
|
|
decoded = _decode_text_bytes(raw)
|
|
if decoded is None:
|
|
binary = True
|
|
reason = "binary"
|
|
elif text_cache_dir is not None:
|
|
text_path = str(_cache_text_file(decoded, virtual_path, text_cache_dir))
|
|
else:
|
|
text = decoded
|
|
|
|
return FileSnapshot(
|
|
path=virtual_path,
|
|
root=root.name,
|
|
size=size,
|
|
mtime_ns=mtime_ns,
|
|
sha256=sha256,
|
|
binary=binary,
|
|
sensitive=sensitive,
|
|
text=text,
|
|
text_path=text_path,
|
|
content_unavailable_reason=reason,
|
|
)
|
|
|
|
|
|
def _snapshot_symlink(root: WorkspaceRoot, host_file: Path) -> FileSnapshot | None:
|
|
# Deliberately never follows the link (no read_bytes()/open() on the target):
|
|
# the target may point anywhere on the host, including outside the scanned
|
|
# root, so stat'ing or reading through it here would risk exposing arbitrary
|
|
# host file content/metadata as if it belonged to the workspace.
|
|
try:
|
|
stat = host_file.lstat()
|
|
size = stat.st_size
|
|
mtime_ns = stat.st_mtime_ns
|
|
relative = host_file.relative_to(root.host_path).as_posix()
|
|
virtual_path = f"{root.virtual_prefix}/{relative}"
|
|
sensitive = is_sensitive_workspace_path(virtual_path)
|
|
except OSError:
|
|
return None
|
|
|
|
try:
|
|
target = os.readlink(host_file)
|
|
except OSError:
|
|
target = None
|
|
|
|
return FileSnapshot(
|
|
path=virtual_path,
|
|
root=root.name,
|
|
size=size,
|
|
mtime_ns=mtime_ns,
|
|
sha256=None,
|
|
binary=False,
|
|
sensitive=sensitive,
|
|
text=None,
|
|
content_unavailable_reason="symlink",
|
|
symlink=True,
|
|
symlink_target=target,
|
|
)
|
|
|
|
|
|
def _cache_text_file(text: str, virtual_path: str, cache_dir: Path) -> Path:
|
|
cache_name = hashlib.sha256(virtual_path.encode("utf-8")).hexdigest()
|
|
target = cache_dir / cache_name
|
|
target.write_text(text, encoding="utf-8")
|
|
return target
|
|
|
|
|
|
def _read_sample(path: Path) -> bytes:
|
|
with path.open("rb") as file:
|
|
return file.read(SAMPLE_BYTES)
|
|
|
|
|
|
def _sha256_file(path: Path) -> str:
|
|
digest = hashlib.sha256()
|
|
with path.open("rb") as file:
|
|
for chunk in iter(lambda: file.read(1024 * 1024), b""):
|
|
digest.update(chunk)
|
|
return digest.hexdigest()
|
|
|
|
|
|
def _decode_text_bytes(data: bytes) -> str | None:
|
|
for encoding in ("utf-8-sig", "utf-8"):
|
|
try:
|
|
return data.decode(encoding)
|
|
except UnicodeDecodeError:
|
|
continue
|
|
|
|
if data.startswith(_UTF16_BOMS):
|
|
try:
|
|
return data.decode("utf-16")
|
|
except UnicodeDecodeError:
|
|
return None
|
|
|
|
return None
|
|
|
|
|
|
def _sample_decodes_as_text(sample: bytes, encoding: str) -> bool:
|
|
try:
|
|
decoder = getincrementaldecoder(encoding)()
|
|
decoder.decode(sample, final=False)
|
|
except UnicodeDecodeError:
|
|
return False
|
|
return True
|
|
|
|
|
|
def _looks_binary(sample: bytes) -> bool:
|
|
if sample.startswith(_UTF16_BOMS) and _sample_decodes_as_text(sample, "utf-16"):
|
|
return False
|
|
if b"\x00" in sample:
|
|
return True
|
|
if _sample_decodes_as_text(sample, "utf-8"):
|
|
return False
|
|
return True
|