Skip to content

fix: avoid hardcoded sandbox skill root in prompt fallback #6983

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1zzxy1 wants to merge 1 commit into from
Closed

fix: avoid hardcoded sandbox skill root in prompt fallback #6983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions astrbot/core/skills/skill_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
SANDBOX_SKILLS_CACHE_FILENAME = "sandbox_skills_cache.json"
DEFAULT_SKILLS_CONFIG: dict[str, dict] = {"skills": {}}
SANDBOX_SKILLS_ROOT = "skills"
SANDBOX_WORKSPACE_ROOT = "/workspace"
_SANDBOX_SKILLS_CACHE_VERSION = 1

_SKILL_NAME_RE = re.compile(r"^[\w.-]+$")
Expand All @@ -36,7 +35,7 @@ def _normalize_skill_name(name: str | None) -> str:


def _default_sandbox_skill_path(name: str) -> str:
return f"{SANDBOX_WORKSPACE_ROOT}/{SANDBOX_SKILLS_ROOT}/{name}/SKILL.md"
return f"{SANDBOX_SKILLS_ROOT}/{name}/SKILL.md"


def _normalize_cached_sandbox_skill_path(name: str, path: str) -> str:
Expand Down Expand Up @@ -253,8 +252,8 @@ def build_skills_prompt(skills: list[SkillInfo]) -> str:
"explain why you chose not to.\n"
"3. **Mandatory grounding** — Before executing any skill you MUST "
"first read its `SKILL.md` by running a shell command compatible "
"with the current runtime shell and using the **absolute path** "
f"shown above (e.g. `{example_command}`). "
"with the current runtime shell and using the path shown above "
f"(e.g. `{example_command}`). "
"Never rely on memory or assumptions about a skill's content.\n"
"4. **Progressive disclosure** — Load only what is directly "
"referenced from `SKILL.md`:\n"
Expand Down
37 changes: 37 additions & 0 deletions tests/test_skill_manager_sandbox_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,40 @@ def test_sandbox_and_local_path_resolution_with_show_sandbox_path_false(
assert local_skill_path == skills_root / "custom-local" / "SKILL.md"
assert by_name["python-sandbox"].path == "/app/skills/python-sandbox/SKILL.md"


def test_sandbox_path_fallback_uses_relative_skills_root(
monkeypatch,
tmp_path: Path,
):
data_dir = tmp_path / "data"
temp_dir = tmp_path / "temp"
skills_root = tmp_path / "skills"
data_dir.mkdir(parents=True, exist_ok=True)
temp_dir.mkdir(parents=True, exist_ok=True)
skills_root.mkdir(parents=True, exist_ok=True)

monkeypatch.setattr(
"astrbot.core.skills.skill_manager.get_astrbot_data_path",
lambda: str(data_dir),
)
monkeypatch.setattr(
"astrbot.core.skills.skill_manager.get_astrbot_temp_path",
lambda: str(temp_dir),
)

mgr = SkillManager(skills_root=str(skills_root))
Comment on lines +163 to +179
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The setup logic for creating directories and monkeypatching path functions is repeated across multiple tests in this file. To improve maintainability and reduce code duplication, consider extracting this common setup into a pytest fixture.

This would make the tests cleaner and easier to manage. For example:

@pytest.fixture
def skill_manager(monkeypatch, tmp_path: Path) -> SkillManager:
    data_dir = tmp_path / "data"
    temp_dir = tmp_path / "temp"
    skills_root = tmp_path / "skills"
    data_dir.mkdir(parents=True, exist_ok=True)
    temp_dir.mkdir(parents=True, exist_ok=True)
    skills_root.mkdir(parents=True, exist_ok=True)

    monkeypatch.setattr(
        "astrbot.core.skills.skill_manager.get_astrbot_data_path",
        lambda: str(data_dir),
    )
    monkeypatch.setattr(
        "astrbot.core.skills.skill_manager.get_astrbot_temp_path",
        lambda: str(temp_dir),
    )

    return SkillManager(skills_root=str(skills_root))

Tests can then be simplified by using this fixture, for example:

def test_sandbox_path_fallback_uses_relative_skills_root(skill_manager: SkillManager):
    skill_manager.set_sandbox_skills_cache(...)
    skills = skill_manager.list_skills(runtime="sandbox")
    # ... assertions

mgr.set_sandbox_skills_cache(
[
{
"name": "docx",
"description": "sandbox skill",
"path": "/workspace/not-the-real-root/docx/readme.txt",
}
]
)

skills = mgr.list_skills(runtime="sandbox")

assert len(skills) == 1
assert skills[0].path == "skills/docx/SKILL.md"

17 changes: 17 additions & 0 deletions tests/test_skill_metadata_enrichment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,23 @@ def test_build_skills_prompt_keeps_placeholder_example_literal():
assert example_fragment == "cat <skills_root>/<skill_name>/SKILL.md"


def test_build_skills_prompt_allows_relative_sandbox_path_examples():
skills = [
SkillInfo(
name="docx",
description="sandbox skill",
path="skills/docx/SKILL.md",
active=True,
source_type="sandbox_only",
)
]

prompt = build_skills_prompt(skills)

assert "cat skills/docx/SKILL.md" in prompt
assert "**absolute path**" not in prompt


def test_build_skills_prompt_preserves_windows_absolute_path_in_example(monkeypatch):
monkeypatch.setattr("astrbot.core.skills.skill_manager.os.name", "nt")
skills = [
Expand Down
Loading