fix(skills): use actual sandbox path from cache instead of hardcoded workspace root (#6331)

* fix(skills): use actual sandbox path from cache instead of hardcoded workspace root

Fixes #6273

When using Shipyard booter, the sandbox workspace directory is
`/home/ship_{session_id}/workspace/` instead of the hardcoded `/workspace`.
This caused Agent to fail reading SKILL.md files with 'No such file or directory'.

Changes:
- In build_skills_prompt: prefer skill.path (from sandbox cache) over
  hardcoded SANDBOX_WORKSPACE_ROOT for sandbox_only skills
- In list_skills: always prefer sandbox_cached_paths over hardcoded path
  for sandbox_only skills

The actual path is resolved at sandbox scan time via Path.resolve() in
_build_scan_command, which returns the correct absolute path based on
the sandbox's actual working directory.

* docs: add comment explaining show_sandbox_path behavior for sandbox_only skills

Address Sourcery AI review comment:
- Clarify that show_sandbox_path is implicitly True for sandbox_only skills
- Explain why the flag is effectively ignored (no local path exists)

* refactor: simplify path_str fallback using or operator

Address review feedback: use single-line fallback instead of if-not pattern.

* style: format skill_manager.py with ruff

Fix ruff format-check failure

* fix(skills): sanitize cached sandbox skill paths

Normalize sandbox cache paths before reading or writing them so invalid,
empty, or mismatched entries fall back to a safe default SKILL.md path.

This avoids using malformed cached paths, keeps path rendering
consistent, and ensures sandbox skill listings always point to the
expected workspace location.

---------

Co-authored-by: ccsang <ccsang@users.noreply.github.com>
Co-authored-by: RC-CHN <1051989940@qq.com>
This commit is contained in:
qingyun
2026-03-21 10:39:52 +08:00
committed by GitHub
parent df4eb33582
commit e643bc94e5

View File

@@ -29,6 +29,28 @@ _SANDBOX_SKILLS_CACHE_VERSION = 1
_SKILL_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$")
def _default_sandbox_skill_path(name: str) -> str:
return f"{SANDBOX_WORKSPACE_ROOT}/{SANDBOX_SKILLS_ROOT}/{name}/SKILL.md"
def _normalize_cached_sandbox_skill_path(name: str, path: str) -> str:
normalized = str(path or "").strip().replace("\\", "/")
if not normalized:
return _default_sandbox_skill_path(name)
pure_path = PurePosixPath(normalized)
if ".." in pure_path.parts:
return _default_sandbox_skill_path(name)
if pure_path.name != "SKILL.md":
return _default_sandbox_skill_path(name)
if pure_path.parent.name != name:
return _default_sandbox_skill_path(name)
return str(pure_path)
def _is_ignored_zip_entry(name: str) -> bool:
parts = PurePosixPath(name).parts
if not parts:
@@ -162,10 +184,10 @@ def build_skills_prompt(skills: list[SkillInfo]) -> str:
description = "Read SKILL.md for details."
if skill.source_type == "sandbox_only":
rendered_path = (
f"{str(SANDBOX_WORKSPACE_ROOT)}/{str(SANDBOX_SKILLS_ROOT)}/"
f"{display_name}/SKILL.md"
)
# Prefer the actual path from sandbox cache if available
rendered_path = _sanitize_prompt_path_for_prompt(skill.path)
if not rendered_path:
rendered_path = _default_sandbox_skill_path(skill.name)
else:
rendered_path = _sanitize_prompt_path_for_prompt(skill.path)
if not rendered_path:
@@ -279,13 +301,13 @@ class SkillManager:
if not name or not _SKILL_NAME_RE.match(name):
continue
description = str(item.get("description", "") or "")
path = str(item.get("path", "") or "")
if not path:
path = f"{SANDBOX_WORKSPACE_ROOT}/{SANDBOX_SKILLS_ROOT}/{name}/SKILL.md"
path = _normalize_cached_sandbox_skill_path(
name, str(item.get("path", "") or "")
)
deduped[name] = {
"name": name,
"description": description,
"path": path.replace("\\", "/"),
"path": path,
}
cache = {
"version": _SANDBOX_SKILLS_CACHE_VERSION,
@@ -329,12 +351,13 @@ class SkillManager:
if not isinstance(item, dict):
continue
name = str(item.get("name", "") or "").strip()
path = str(item.get("path", "") or "").strip().replace("\\", "/")
path = _normalize_cached_sandbox_skill_path(
name, str(item.get("path", "") or "")
)
if not name or not _SKILL_NAME_RE.match(name):
continue
sandbox_cached_descriptions[name] = str(item.get("description", "") or "")
if path:
sandbox_cached_paths[name] = path
sandbox_cached_paths[name] = path
for entry in sorted(Path(self.skills_root).iterdir()):
if not entry.is_dir():
@@ -361,9 +384,9 @@ class SkillManager:
source_type = "both" if sandbox_exists else "local_only"
source_label = "synced" if sandbox_exists else "local"
if runtime == "sandbox" and show_sandbox_path:
path_str = sandbox_cached_paths.get(skill_name) or (
f"{SANDBOX_WORKSPACE_ROOT}/{SANDBOX_SKILLS_ROOT}/{skill_name}/SKILL.md"
)
path_str = sandbox_cached_paths.get(
skill_name
) or _default_sandbox_skill_path(skill_name)
else:
path_str = str(skill_md)
path_str = path_str.replace("\\", "/")
@@ -397,12 +420,12 @@ class SkillManager:
if active_only and not active:
continue
description = sandbox_cached_descriptions.get(skill_name, "")
if show_sandbox_path:
path_str = f"{SANDBOX_WORKSPACE_ROOT}/{SANDBOX_SKILLS_ROOT}/{skill_name}/SKILL.md"
else:
path_str = sandbox_cached_paths.get(skill_name, "")
if not path_str:
path_str = f"{SANDBOX_WORKSPACE_ROOT}/{SANDBOX_SKILLS_ROOT}/{skill_name}/SKILL.md"
# For sandbox_only skills, show_sandbox_path is implicitly True
# since there is no local path to show. Always prefer the
# actual path from sandbox cache.
path_str = sandbox_cached_paths.get(
skill_name
) or _default_sandbox_skill_path(skill_name)
skills_by_name[skill_name] = SkillInfo(
name=skill_name,
description=description,