fix(sandbox): make missing sandbox.mounts host_path a loud ERROR (#3244)#3250
Open
player0718 wants to merge 1 commit into
Open
fix(sandbox): make missing sandbox.mounts host_path a loud ERROR (#3244)#3250player0718 wants to merge 1 commit into
player0718 wants to merge 1 commit into
Conversation
…edance#3244) In Docker production deployments, LocalSandboxProvider runs inside the deer-flow-gateway container, so any `sandbox.mounts[].host_path` from config.yaml is resolved against the gateway container's filesystem — not the host machine. When the path isn't also bind-mounted into the gateway service, the mount was silently dropped with only a WARNING log, leaving agents reading an empty directory in production while the same config worked under `make dev`. Escalate the missing-host_path branch to logger.error with explicit guidance about Docker bind mounts and docker-compose, so the failure is hard to miss in default log configurations. Skip behaviour is preserved to avoid breaking existing deployments. Also clarify the misleading `VolumeMountConfig.host_path` field description so it documents reality for both providers: - LocalSandboxProvider checks host_path from inside the gateway process (host in `make dev`, container in `make up`). - AioSandboxProvider (DooD) passes host_path straight to `docker -v` for the sandbox container, where the host Docker daemon resolves it from the host machine's perspective. config.example.yaml's `sandbox.mounts` comment gets a Note: block pointing operators at the docker-compose bind-mount requirement so the Docker-mode gotcha is discoverable from the canonical template. Adds a regression test that: - confirms missing host_path is still skipped (no behaviour break); - asserts an ERROR record is emitted referencing the offending paths; - asserts the message contains actionable Docker/gateway/docker-compose keywords so future refactors can't quietly downgrade it. Refs: bytedance#3244
|
jingkang50 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes missing sandbox.mounts[].host_path entries more visible for LocalSandboxProvider, addressing Docker production-mode confusion where paths may not exist inside the gateway container.
Changes:
- Escalates missing custom mount paths from warning to actionable error logs.
- Clarifies
host_pathsemantics for Local vs AIO sandbox providers. - Adds example config guidance and regression coverage for the new logging behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py |
Logs missing custom mount paths as actionable errors while preserving skip behavior. |
backend/packages/harness/deerflow/config/sandbox_config.py |
Updates host_path field description to document provider-specific resolution semantics. |
config.example.yaml |
Adds Docker-mode guidance for custom sandbox mounts. |
backend/tests/test_local_sandbox_provider_mounts.py |
Adds regression coverage for missing-host-path error logging and preserved skip behavior. |
Collaborator
|
@player0718 Please update your GitHub profile with the git committer author email to address the complaint of the CLA assistant. |
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.
Summary
Refs: #3244
Under
make up(docker-compose),LocalSandboxProviderruns inside thedeer-flow-gatewaycontainer.sandbox.mounts[].host_pathfromconfig.yamlis resolved against the gateway container's filesystem,not the host machine — and the container only bind-mounts a fixed
allowlist (
config.yaml,extensions_config.json,../skills,${DEER_FLOW_HOME}, the docker socket,~/.claude,~/.codex).Any custom
host_pathfromconfig.yamlis therefore invisible insidethe container,
host_path.exists()returns False, and the mount issilently dropped with only a WARNING log. The same
config.yamlworksunder
make dev(the provider runs on the host directly) but breaksunder
make up, leaving agents reading an empty directory inproduction with no actionable signal to operators.
The reporter listed three remediations ("至少满足下面一条"). This PR
implements option 1: loud failure. Options 2 (
docker-awaremountauto-translation) and 3 (rename
host_path) are intentionally left forfollow-up — they involve breaking config schema or deployment
architecture changes that don't belong in a small DX-fix PR. Option 1
unblocks affected operators today and does not preclude either follow-up.
Changes
backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.pylogger.warning→logger.errorwith explicit Docker /docker-compose.yaml/services.gateway.volumesguidance. Skip behaviour preserved (no exception raised) to avoid breaking existing deployments.backend/packages/harness/deerflow/config/sandbox_config.py\"Path on the host machine\"description onVolumeMountConfig.host_pathso it documents reality for both providers:LocalSandboxProviderresolves from the gateway-process filesystem (host inmake dev, container inmake up);AioSandboxProvider(DooD) passes the value todocker -vand the host Docker daemon resolves it.config.example.yamlNote:block on thesandbox.mountstemplate pointing operators at the docker-compose bind-mount requirement and referencing #3244, so the Docker-mode gotcha is discoverable from the canonical template.backend/tests/test_local_sandbox_provider_mounts.pytest_setup_path_mappings_logs_actionable_error_for_missing_host_path: confirms missing host_path is still skipped (no behaviour break), asserts an ERROR record is emitted referencing the offending paths, asserts the message contains actionable docker / gateway / docker-compose keywords.Diff stats: 4 files, +86 / −5.
Test plan
cd backend && uv run pytest tests/test_local_sandbox_provider_mounts.py -v— 45 passed (including the new regression).AssertionError: expected an ERROR log when host_path is missing.cd backend && uvx ruff check .— All checks passed.cd backend && uv run pytest tests/(full suite) — 3652 passed / 16 skipped / 1 flake (test_runtime_lifecycle_e2e.py::test_cancel_interrupt_stops_running_background_run, statistically the same race condition onorigin/main: 3 fails / 20 runs vs. 1 fail / 20 runs on this branch — unrelated to this PR).make dev(real uvicorn + real HTTP upload triggering lazyget_sandbox_provider()): ERROR appears in uvicorn stderr exactly as designed; pre-fix only emits the buried WARNING.make up(fullfrontend + gateway + nginxproduction stack built from the actualbackend/Dockerfile): host directory exists on host but not inside the gateway container; upload triggersget_sandbox_provider(); the new ERROR appears indocker compose logs gatewaywith the actionable guidance referring toservices.gateway.volumesindocker/docker-compose.yaml— which I verified matches the real file (service namegatewayat L64, volumes block at L75).Compatibility & scope
host_pathis still skipped, just loudly._get_custom_mountsinbackend/packages/harness/deerflow/sandbox/tools.py(second-pass filter on the same config). The provider-side ERROR already surfaces the problem; adding a second logger there would mean introducing aloggingimport in a file that has none today.community/aio_sandbox/local_backend.py) was reviewed and is intentionally out of scope: it passeshost_pathdirectly todocker -v, so the docker daemon resolves it from the host's perspective. The newhost_pathdescription now covers this case explicitly so AIO users aren't misled.Happy to drop the explanatory block comment inside
_setup_path_mappings()or trim the docstring further if maintainers prefer a tighter footprint.