Repository navigation
fix(sandbox): list the skills root when only category mounts exist - #5857
WillemJiang merged 5 commits into
Conversation
With the default (non-policy-scoped) LocalSandboxProvider layout, the four skill categories (/mnt/skills/public, custom, legacy, integrations) are mounted at separate host directories and there is no mapping for the aggregate /mnt/skills root itself. LocalSandbox.list_dir resolved the root to the unmapped host path, and the host list_dir raised FileNotFoundError before the virtual sub-directory overlay -- which exists exactly so the agent can discover the categories via "ls /mnt/skills" -- could run, so the ls tool reported "Directory not found: /mnt/skills" even with skills installed. Treat a FileNotFoundError from the host scan as an empty listing when mappings are mounted inside the requested container path, so the overlay can surface those children. Paths with nothing mounted inside them keep raising FileNotFoundError as before.
github-actions
Bot
added
area:sandbox
github-actions
Bot
added
size/M
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
Nice, focused fix. I verified the root cause against the head commit: the default (non-policy-scoped) provider layout really does mount only the four category paths under /mnt/skills with no aggregate root mapping, and _resolve_path_with_mapping leaves unmatched paths unchanged, so the host list_dir got the literal /mnt/skills and raised before the virtual-children overlay could run. The red->green regression test plus the two negative tests (nested-only mount, missing direct-child host dir) pin the contract well. Two minor notes below, neither blocking.
There was a problem hiding this comment.
The list_dir utility raises FileNotFoundError for two distinct cases — path missing and path exists but is not a directory (if not root_path.is_dir(): raise FileNotFoundError(path) in list_dir.py) — so this guard also swallows the not-a-directory case: if a mapping root ever resolved to a host file while a direct child mount exists, the call would now silently return the virtual children instead of surfacing the misconfiguration. Since this branch only executes on the error path, a cheap disambiguation would keep that error visible, e.g. if not virtual_children or os.path.exists(resolved_path): raise. The PR comment ("the resolved host directory is missing") slightly overstates what the exception guarantees, so either the code or the comment could be tightened.
There was a problem hiding this comment.
Nit: self._resolved_local_paths[mapping] already caches os.path.realpath(mapping.local_path) per mapping — the class comment above notes the cached views exist precisely to avoid repeated filesystem resolution on the agent's hot path. This pre-scan now runs on every list_dir call, so it could reuse the cache: os.path.isdir(self._resolved_local_paths[mapping]) (same value, no extra realpath syscall per child mount per call).
…ytedance#5857) * fix(sandbox): list the skills root when only category mounts exist With the default (non-policy-scoped) LocalSandboxProvider layout, the four skill categories (/mnt/skills/public, custom, legacy, integrations) are mounted at separate host directories and there is no mapping for the aggregate /mnt/skills root itself. LocalSandbox.list_dir resolved the root to the unmapped host path, and the host list_dir raised FileNotFoundError before the virtual sub-directory overlay -- which exists exactly so the agent can discover the categories via "ls /mnt/skills" -- could run, so the ls tool reported "Directory not found: /mnt/skills" even with skills installed. Treat a FileNotFoundError from the host scan as an empty listing when mappings are mounted inside the requested container path, so the overlay can surface those children. Paths with nothing mounted inside them keep raising FileNotFoundError as before. * fix(sandbox): preserve missing virtual parent errors * test(screening): align PII fixture with keyed placeholders * fix(sandbox): preserve non-directory errors for virtual parents --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Why
With the default (non-policy-scoped) skills projection, the agent's
ls /mnt/skillsreturnedError: Directory not found: /mnt/skillseven with skills installed and all four categories mounted.Root cause:
LocalSandboxProvidermounts the skills tree in one of two layouts (backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py:369-406). When the projection is policy-scoped (per-user isolated), an aggregatePathMappingmaps/mnt/skillsitself to a real host directory, so listing works. In the default layout there is no mapping for the/mnt/skillsroot — only the four category mounts (/mnt/skills/public,custom,legacy,integrations), each pointing at its own host directory.LocalSandbox.list_dirthen resolved/mnt/skillsto the literal, unmapped host path (local_sandbox.py:771-772—_resolve_path_with_mappingreturns an unmatched path unchanged), and the hostlist_dirraisedFileNotFoundError(list_dir.py:26-27) before the virtual sub-directory overlay — the block that exists exactly so the agent can discover the categories vials /mnt/skills(local_sandbox.py:795-800) — could run. Thelstool surfaced that as "Directory not found" (backend/packages/harness/deerflow/sandbox/tools.py:2326-2327).What changed
LocalSandbox.list_dirnow treats aFileNotFoundErrorfrom the host scan as an empty host listing when at least one mapping is mounted inside the requested container path, letting the existing virtual sub-directory overlay surface those children —ls /mnt/skillslists the four category directories. Any path with nothing mounted inside it keeps raisingFileNotFoundErrorexactly as before, so genuinely missing directories still error. No otherlist_dirbehavior changes.Surface area
frontend/backend/applanggraph.json, or prompt changedocker/or sandboxed executionskills/backend/pyproject.tomlorfrontend/package.json(say what it buys us)Screenshots / Recording
N/A — backend sandbox fix, no UI surface.
Bug fix verification
backend/tests/test_local_sandbox_virtual_path_contract.py::test_list_dir_on_skills_root_lists_category_mountsmainand green on this branch? yesRed run on the unpatched tree (
local_sandbox.pyatorigin/main, test kept):The test pins the scenario first — it asserts
/mnt/skillsitself is absent fromsbx.path_mappingswhile all four category mounts are present — so it reproduces the category-only layout rather than failing incidentally. Green on this branch:1 passed in 0.59s.Validation
PYTHONPATH=. PYTHONIOENCODING=utf-8 PYTHONUTF8=1 uv run pytest tests/test_local_sandbox_virtual_path_contract.py::test_list_dir_on_skills_root_lists_category_mounts -v→ 1 passed on this branch; 1 failed (FileNotFoundError: /mnt/skills) on the unpatched treePYTHONPATH=. PYTHONIOENCODING=utf-8 PYTHONUTF8=1 uv run pytest tests/test_local_sandbox_command_timeout.py tests/test_local_sandbox_encoding.py tests/test_local_sandbox_path_regex_cache.py tests/test_local_sandbox_provider_mounts.py tests/test_local_sandbox_virtual_path_contract.py tests/test_sandbox_search_contract.py tests/test_sandbox_tools_security.py -q→ 408 passed, 11 skippedmake lint→ ruff check All checks passed!, ruff format --check 1633 files already formattedPYTHONPATH=. PYTHONIOENCODING=utf-8 PYTHONUTF8=1 uv run pytest tests/test_local_sandbox_virtual_path_contract.py -q→ 25 passedAI assistance
Tool(s) used: Claude Code
How you used it: The agent traced the
ls /mnt/skillsfailure through the provider's two mount layouts and thelist_diroverlay, and drafted theFileNotFoundErrorguard plus the regression test; the diff was reviewed line by line and the red→green run, module suites, and lint above were executed and verified.